* staging: iio: ak8975: make gpio platdata mandatory
@ 2011-03-04 5:34 Naveen Krishna Ch
2011-03-04 5:51 ` Kyungmin Park
2011-03-04 11:50 ` Sergei Shtylyov
0 siblings, 2 replies; 6+ messages in thread
From: Naveen Krishna Ch @ 2011-03-04 5:34 UTC (permalink / raw)
To: linux-arm-kernel
Issue:
For some architectures CONFIG_GENERIC_GPIO is defined,
leaving irq_to_gpio undefined. Causing build break.
Solution:
1. Some architechtures define irq_to_gpio in machine specific code.
2. Make GPIO in platdata mandatory.
Signed-off-by: Naveen Krishna Ch <ch.naveen@samsung.com>
---
drivers/staging/iio/magnetometer/ak8975.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/iio/magnetometer/ak8975.c
b/drivers/staging/iio/magnetometer/ak8975.c
index 80c0f41..8456d1f 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -98,7 +98,6 @@ struct ak8975_data {
unsigned long mode;
u8 reg_cache[AK8975_MAX_REGS];
int eoc_gpio;
- int eoc_irq;
};
/*
@@ -453,12 +452,13 @@ static int ak8975_probe(struct i2c_client *client,
mutex_init(&data->lock);
/* Grab and set up the supplied GPIO. */
- data->eoc_irq = client->irq;
pdata = client->dev.platform_data;
if (pdata)
data->eoc_gpio = pdata->gpio;
- else
- data->eoc_gpio = irq_to_gpio(client->irq);
+ else {
+ else {
+ dev_err(&client->dev, "failed, no platform GPIO specified\n");
+ goto exit;
+ }
if (!data->eoc_gpio) {
dev_err(&client->dev, "failed, no valid GPIO\n");
--
1.7.0.4
--
Shine bright,
(: Nav :)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* staging: iio: ak8975: make gpio platdata mandatory
2011-03-04 5:34 staging: iio: ak8975: make gpio platdata mandatory Naveen Krishna Ch
@ 2011-03-04 5:51 ` Kyungmin Park
2011-03-06 0:31 ` Naveen Krishna Ch
2011-03-04 11:50 ` Sergei Shtylyov
1 sibling, 1 reply; 6+ messages in thread
From: Kyungmin Park @ 2011-03-04 5:51 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Fri, Mar 4, 2011 at 2:34 PM, Naveen Krishna Ch
<naveenkrishna.ch@gmail.com> wrote:
> Issue:
> For some architectures CONFIG_GENERIC_GPIO is defined,
> leaving irq_to_gpio undefined. ?Causing build break.
It's better to implement the irq_to_gpio. Since gpio_to_irq is already
implemented. it's just vice versa.
Thank you,
Kyungmin Park
>
> Solution:
> 1. Some architechtures define irq_to_gpio in machine specific code.
> 2. Make GPIO in platdata mandatory.
>
> Signed-off-by: Naveen Krishna Ch <ch.naveen@samsung.com>
> ---
>
> ?drivers/staging/iio/magnetometer/ak8975.c | ? ?8 ++++----
> ?1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/ak8975.c
> b/drivers/staging/iio/magnetometer/ak8975.c
> index 80c0f41..8456d1f 100644
> --- a/drivers/staging/iio/magnetometer/ak8975.c
> +++ b/drivers/staging/iio/magnetometer/ak8975.c
> @@ -98,7 +98,6 @@ struct ak8975_data {
> ? ? ? ?unsigned long ? ? ? ? ? mode;
> ? ? ? ?u8 ? ? ? ? ? ? ? ? ? ? ?reg_cache[AK8975_MAX_REGS];
> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? eoc_gpio;
> - ? ? ? int ? ? ? ? ? ? ? ? ? ? eoc_irq;
> ?};
>
> ?/*
> @@ -453,12 +452,13 @@ static int ak8975_probe(struct i2c_client *client,
> ? ? ? ?mutex_init(&data->lock);
>
> ? ? ? ?/* Grab and set up the supplied GPIO. */
> - ? ? ? data->eoc_irq = client->irq;
> ? ? ? ?pdata = client->dev.platform_data;
> ? ? ? ?if (pdata)
> ? ? ? ? ? ? ? ?data->eoc_gpio = pdata->gpio;
> - ? ? ? else
> - ? ? ? ? ? ? ? data->eoc_gpio = irq_to_gpio(client->irq);
> + ? ? ? else {
> + ? ? ? else {
> + ? ? ? ? ? ? ? dev_err(&client->dev, "failed, no platform GPIO specified\n");
> + ? ? ? ? ? ? ? goto exit;
> + ? ? ? }
>
> ? ? ? ?if (!data->eoc_gpio) {
> ? ? ? ? ? ? ? ?dev_err(&client->dev, "failed, no valid GPIO\n");
> --
> 1.7.0.4
>
> --
> Shine bright,
> (: Nav :)
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* staging: iio: ak8975: make gpio platdata mandatory
2011-03-04 5:34 staging: iio: ak8975: make gpio platdata mandatory Naveen Krishna Ch
2011-03-04 5:51 ` Kyungmin Park
@ 2011-03-04 11:50 ` Sergei Shtylyov
1 sibling, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2011-03-04 11:50 UTC (permalink / raw)
To: linux-arm-kernel
Hello.
On 04-03-2011 8:34, Naveen Krishna Ch wrote:
> Issue:
> For some architectures CONFIG_GENERIC_GPIO is defined,
> leaving irq_to_gpio undefined. Causing build break.
> Solution:
> 1. Some architechtures define irq_to_gpio in machine specific code.
> 2. Make GPIO in platdata mandatory.
> Signed-off-by: Naveen Krishna Ch<ch.naveen@samsung.com>
[...]
> diff --git a/drivers/staging/iio/magnetometer/ak8975.c
> b/drivers/staging/iio/magnetometer/ak8975.c
> index 80c0f41..8456d1f 100644
> --- a/drivers/staging/iio/magnetometer/ak8975.c
> +++ b/drivers/staging/iio/magnetometer/ak8975.c
> @@ -98,7 +98,6 @@ struct ak8975_data {
> unsigned long mode;
> u8 reg_cache[AK8975_MAX_REGS];
> int eoc_gpio;
> - int eoc_irq;
Your patch seems to be whitespace damaged, i.e. all tabs replaced by spaces.
> };
>
> /*
> @@ -453,12 +452,13 @@ static int ak8975_probe(struct i2c_client *client,
> mutex_init(&data->lock);
>
> /* Grab and set up the supplied GPIO. */
> - data->eoc_irq = client->irq;
> pdata = client->dev.platform_data;
> if (pdata)
> data->eoc_gpio = pdata->gpio;
> - else
> - data->eoc_gpio = irq_to_gpio(client->irq);
> + else {
> + else {
Have you tried to compile this?
WBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread
* staging: iio: ak8975: make gpio platdata mandatory
2011-03-04 5:51 ` Kyungmin Park
@ 2011-03-06 0:31 ` Naveen Krishna Ch
2011-03-06 0:58 ` Russell King - ARM Linux
0 siblings, 1 reply; 6+ messages in thread
From: Naveen Krishna Ch @ 2011-03-06 0:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Kyungmin Park,
On 4 March 2011 14:51, Kyungmin Park <kmpark@infradead.org> wrote:
> Hi,
>
> On Fri, Mar 4, 2011 at 2:34 PM, Naveen Krishna Ch
> <naveenkrishna.ch@gmail.com> wrote:
>> Issue:
>> For some architectures CONFIG_GENERIC_GPIO is defined,
>> leaving irq_to_gpio undefined. ?Causing build break.
>
> It's better to implement the irq_to_gpio. Since gpio_to_irq is already
> implemented. it's just vice versa.
Implementing such macros in machine header files leads to inclusion of
mach/gpio.h or irqs.h
Russel King, seems against to the same...
Any suggestions, please
>
> Thank you,
> Kyungmin Park
>>
>> Solution:
>> 1. Some architechtures define irq_to_gpio in machine specific code.
>> 2. Make GPIO in platdata mandatory.
>>
>> Signed-off-by: Naveen Krishna Ch <ch.naveen@samsung.com>
>> ---
>>
>> ?drivers/staging/iio/magnetometer/ak8975.c | ? ?8 ++++----
>> ?1 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/iio/magnetometer/ak8975.c
>> b/drivers/staging/iio/magnetometer/ak8975.c
>> index 80c0f41..8456d1f 100644
>> --- a/drivers/staging/iio/magnetometer/ak8975.c
>> +++ b/drivers/staging/iio/magnetometer/ak8975.c
>> @@ -98,7 +98,6 @@ struct ak8975_data {
>> ? ? ? ?unsigned long ? ? ? ? ? mode;
>> ? ? ? ?u8 ? ? ? ? ? ? ? ? ? ? ?reg_cache[AK8975_MAX_REGS];
>> ? ? ? ?int ? ? ? ? ? ? ? ? ? ? eoc_gpio;
>> - ? ? ? int ? ? ? ? ? ? ? ? ? ? eoc_irq;
>> ?};
>>
>> ?/*
>> @@ -453,12 +452,13 @@ static int ak8975_probe(struct i2c_client *client,
>> ? ? ? ?mutex_init(&data->lock);
>>
>> ? ? ? ?/* Grab and set up the supplied GPIO. */
>> - ? ? ? data->eoc_irq = client->irq;
>> ? ? ? ?pdata = client->dev.platform_data;
>> ? ? ? ?if (pdata)
>> ? ? ? ? ? ? ? ?data->eoc_gpio = pdata->gpio;
>> - ? ? ? else
>> - ? ? ? ? ? ? ? data->eoc_gpio = irq_to_gpio(client->irq);
>> + ? ? ? else {
>> + ? ? ? else {
>> + ? ? ? ? ? ? ? dev_err(&client->dev, "failed, no platform GPIO specified\n");
>> + ? ? ? ? ? ? ? goto exit;
>> + ? ? ? }
>>
>> ? ? ? ?if (!data->eoc_gpio) {
>> ? ? ? ? ? ? ? ?dev_err(&client->dev, "failed, no valid GPIO\n");
>> --
>> 1.7.0.4
>>
>> --
>> Shine bright,
>> (: Nav :)
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
--
Shine bright,
(: Nav :)
^ permalink raw reply [flat|nested] 6+ messages in thread
* staging: iio: ak8975: make gpio platdata mandatory
2011-03-06 0:31 ` Naveen Krishna Ch
@ 2011-03-06 0:58 ` Russell King - ARM Linux
2011-03-06 1:40 ` Naveen Krishna Ch
0 siblings, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2011-03-06 0:58 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Mar 06, 2011 at 09:31:41AM +0900, Naveen Krishna Ch wrote:
> Hi Kyungmin Park,
>
> On 4 March 2011 14:51, Kyungmin Park <kmpark@infradead.org> wrote:
> > Hi,
> >
> > On Fri, Mar 4, 2011 at 2:34 PM, Naveen Krishna Ch
> > <naveenkrishna.ch@gmail.com> wrote:
> >> Issue:
> >> For some architectures CONFIG_GENERIC_GPIO is defined,
> >> leaving irq_to_gpio undefined. ?Causing build break.
> >
> > It's better to implement the irq_to_gpio. Since gpio_to_irq is already
> > implemented. it's just vice versa.
> Implementing such macros in machine header files leads to inclusion of
> mach/gpio.h or irqs.h
> Russel King, seems against to the same...
Four reasons:
1. including mach/gpio.h means that the code is non-portable to other
architectures.
2. linux/gpio.h defines helpers for the !GENERIC_GPIO case.
3. it's good practice to always use linux/ includes rather than asm/ or
mach/ where-ever possible.
4. over time, standard helpers tend to get split out into the common
architecture-independent header files. Who would like to volunteer
to fix up all the mach/gpio.h->linux/gpio.h when that happens.
However:
> >> For some architectures CONFIG_GENERIC_GPIO is defined,
> >> leaving irq_to_gpio undefined. ?Causing build break.
I read this as saying: "There are architectures which set
CONFIG_GENERIC_GPIO but do not provide a definition for irq_to_gpio()".
If that's true, then that's a problem for those architectures to solve.
However, first I'd like you to clear up some confusion: what do you mean
by architecture? ARM/Sparc/x86? Or S3C2410 vs OMAP4430 vs Orion vs
Tegra? The former are architectures. The latter are SoCs or families
of SoCs/machines/platforms.
Last point: do _not_ cc: mailing list managers for normal discussion
email - I'm talking about the majordomo at vger.kernel.org which was in the
CC in this thread. Mailing list managers take specially formatted email
messages in order to take automated action. They do not recognise plain
language, and you'll probably get a verbose error reply.
^ permalink raw reply [flat|nested] 6+ messages in thread
* staging: iio: ak8975: make gpio platdata mandatory
2011-03-06 0:58 ` Russell King - ARM Linux
@ 2011-03-06 1:40 ` Naveen Krishna Ch
0 siblings, 0 replies; 6+ messages in thread
From: Naveen Krishna Ch @ 2011-03-06 1:40 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell King,
On 6 March 2011 09:58, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote:
> On Sun, Mar 06, 2011 at 09:31:41AM +0900, Naveen Krishna Ch wrote:
>> Hi Kyungmin Park,
>>
>> On 4 March 2011 14:51, Kyungmin Park <kmpark@infradead.org> wrote:
>> > Hi,
>> >
>> > On Fri, Mar 4, 2011 at 2:34 PM, Naveen Krishna Ch
>> > <naveenkrishna.ch@gmail.com> wrote:
>> >> Issue:
>> >> For some architectures CONFIG_GENERIC_GPIO is defined,
>> >> leaving irq_to_gpio undefined. ?Causing build break.
>> >
>> > It's better to implement the irq_to_gpio. Since gpio_to_irq is already
>> > implemented. it's just vice versa.
>> Implementing such macros in machine header files leads to inclusion of
>> mach/gpio.h or irqs.h
>> Russel King, seems against to the same...
>
> Four reasons:
>
> 1. including mach/gpio.h means that the code is non-portable to other
> ? architectures.
> 2. linux/gpio.h defines helpers for the !GENERIC_GPIO case.
> 3. it's good practice to always use linux/ includes rather than asm/ or
> ? mach/ where-ever possible.
> 4. over time, standard helpers tend to get split out into the common
> ? architecture-independent header files. ?Who would like to volunteer
> ? to fix up all the mach/gpio.h->linux/gpio.h when that happens.
I understand, the complication created by using machine specific code.
>
> However:
>> >> For some architectures CONFIG_GENERIC_GPIO is defined,
>> >> leaving irq_to_gpio undefined. ?Causing build break.
>
> I read this as saying: "There are architectures which set
> CONFIG_GENERIC_GPIO but do not provide a definition for irq_to_gpio()".
> If that's true, then that's a problem for those architectures to solve.
I will start looking into fixing the problem, instead of this issue.
>
> However, first I'd like you to clear up some confusion: what do you mean
> by architecture? ?ARM/Sparc/x86? ?Or S3C2410 vs OMAP4430 vs Orion vs
> Tegra? ?The former are architectures. ?The latter are SoCs or families
> of SoCs/machines/platforms.
I mean, SoCs or families.
>
> Last point: do _not_ cc: mailing list managers for normal discussion
> email - I'm talking about the majordomo at vger.kernel.org which was in the
> CC in this thread.
Sorry my bad, Will correct it here on.
> Mailing list managers take specially formatted email
> messages in order to take automated action. ?They do not recognise plain
> language, and you'll probably get a verbose error reply.
Yes, I Did..
>
Thanks, for your prompt reply and the explanation.
--
Shine bright,
(: Nav :)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-03-06 1:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-04 5:34 staging: iio: ak8975: make gpio platdata mandatory Naveen Krishna Ch
2011-03-04 5:51 ` Kyungmin Park
2011-03-06 0:31 ` Naveen Krishna Ch
2011-03-06 0:58 ` Russell King - ARM Linux
2011-03-06 1:40 ` Naveen Krishna Ch
2011-03-04 11:50 ` Sergei Shtylyov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.