From: philippe.langlais@stericsson•com (Philippe Langlais)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 3/6] U6715 gpio platform driver This driver is U6XXX platform generic
Date: Fri, 16 Jul 2010 15:09:55 +0200 [thread overview]
Message-ID: <4C405A23.7000009@stericsson.com> (raw)
In-Reply-To: <20100715111823.GB29322@n2100.arm.linux.org.uk>
Hi,
I take into account all comments and I'll send a new U6 GPIO patch
Regards,
Philippe
On 07/15/10 13:18, Russell King - ARM Linux wrote:
> Thanks, this is much better. Just a few remaining niggles.
>
> On Fri, Jul 09, 2010 at 05:21:50PM +0200, Philippe Langlais wrote:
>
>> diff --git a/arch/arm/mach-u67xx/board_u67xx_wavex.c b/arch/arm/mach-u67xx/board_u67xx_wavex.c
>> index 633989f..235e80d 100644
>> --- a/arch/arm/mach-u67xx/board_u67xx_wavex.c
>> +++ b/arch/arm/mach-u67xx/board_u67xx_wavex.c
>> @@ -23,6 +23,473 @@
>> #include<mach/hardware.h>
>> #include<mach/irqs.h>
>> #include<mach/timer.h>
>> +#include<mach/gpio.h>
>>
> Should be linux/gpio.h
>
>
>> diff --git a/arch/arm/mach-u67xx/devices.c b/arch/arm/mach-u67xx/devices.c
>> index 1d00b35..e88a106 100644
>> --- a/arch/arm/mach-u67xx/devices.c
>> +++ b/arch/arm/mach-u67xx/devices.c
>> @@ -11,10 +11,78 @@
>> #include<linux/kernel.h>
>> #include<linux/init.h>
>> #include<linux/device.h>
>> +#include<linux/ioport.h>
>> #include<linux/platform_device.h>
>> +#include<linux/fs.h>
>> +#include<linux/gpio.h>
>> +#include<mach/hardware.h>
>> +#include<mach/scon.h>
>> +#include<mach/gpio.h>
>>
> You're already including linux/gpio.h, so this shouldn't be required.
>
>
>> diff --git a/arch/arm/plat-u6xxx/gpio.c b/arch/arm/plat-u6xxx/gpio.c
>> new file mode 100644
>> index 0000000..40205d2
>> --- /dev/null
>> +++ b/arch/arm/plat-u6xxx/gpio.c
>> @@ -0,0 +1,672 @@
>> +/*
>> + * linux/arch/arm/plat-u6xxx/gpio.c
>> + *
>> + * Copyright (C) ST-Ericsson SA 2010
>> + * Author: Loic Pallardy<loic.pallardy@stericsson•com> for ST-Ericsson.
>> + * License terms: GNU General Public License (GPL), version 2
>> + * Support functions for GPIO
>> + */
>> +
>> +#include<linux/init.h>
>> +#include<linux/kernel.h>
>> +#include<linux/module.h>
>> +#include<linux/platform_device.h>
>> +#include<linux/sched.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/ptrace.h>
>> +#include<linux/sysdev.h>
>> +#include<linux/err.h>
>> +#include<linux/clk.h>
>> +#include<linux/io.h>
>> +
>> +#include<asm/irq.h>
>> +#include<asm/mach/irq.h>
>> +
>> +#include<mach/hardware.h>
>> +#include<mach/irqs.h>
>> +#include<mach/gpio.h>
>>
> linux/gpio.h again please.
>
>
>> +static struct platform_driver u6_gpio_driver = {
>> + .probe = u6_gpio_probe,
>> + .remove = NULL,
>> + .suspend = NULL,
>> + .resume = NULL,
>>
> NULL initializers aren't required.
>
>
>> + .driver = {
>> + .name = "u6-gpio",
>> + },
>>
> } should be aligned with the '.' of '.driver'.
>
> Lastly, I'm confused by this in a few places:
>
> local_irq_save(flags);
> writel(some value, register);
> local_irq_restore(flags);
>
> Is there a reason why this apparantly simple register write need IRQs to
> be disabled?
>
> I don't think this patch needs another review cycle, unless someone else
> wants to comment on it.
>
prev parent reply other threads:[~2010-07-16 13:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-09 15:21 The first 3 patches for U6715 after review Philippe Langlais
2010-07-09 15:21 ` [PATCH 1/6] U6/U6715 ARM architecture files Philippe Langlais
2010-07-15 11:17 ` Russell King - ARM Linux
2010-07-16 13:04 ` Philippe Langlais
2010-07-09 15:21 ` [PATCH 2/6] U6715 clocks gating management U6 clock generic driver & U6715 cgu clock specific Philippe Langlais
2010-07-15 11:27 ` Russell King - ARM Linux
2010-07-19 8:34 ` Vincent GUITTOT
2010-07-09 15:21 ` [PATCH 3/6] U6715 gpio platform driver This driver is U6XXX platform generic Philippe Langlais
2010-07-15 11:18 ` Russell King - ARM Linux
2010-07-16 13:09 ` Philippe Langlais [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4C405A23.7000009@stericsson.com \
--to=philippe.langlais@stericsson$(echo .)com \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox