* simple gpio driver
@ 2009-08-13 5:49 Heiko Schocher
2009-08-17 21:18 ` Grant Likely
0 siblings, 1 reply; 5+ messages in thread
From: Heiko Schocher @ 2009-08-13 5:49 UTC (permalink / raw)
To: Anton Vorontsov; +Cc: linuxppc-dev
Hello Anton,
i am trying to use the arch/powerpc/sysdev/simple_gpio.c driver,
for accessing some gpios, and found, that u8_gpio_get()
returns not only a 1 or a 0, instead it returns the real bit
position from the gpio:
gpio return
base value
0 0/0x01
1 0/0x02
2 0/0x04
3 0/0x08
4 0/0x10
5 0/0x20
6 0/0x40
7 0/0x80
I also use the arch/powerpc/platforms/52xx/mpc52xx_gpio.c and
mpc52xx_gpt.c drivers, they all return for a gpio just a 1 or 0,
which seems correct to me, because a gpio can have only 1 or 0
as state ... what do you think?
I solved this issue (if it is) with the following patch:
diff --git a/arch/powerpc/sysdev/simple_gpio.c b/arch/powerpc/sysdev/simple_gpio.c
index 43c4569..bb0d79c 100644
--- a/arch/powerpc/sysdev/simple_gpio.c
+++ b/arch/powerpc/sysdev/simple_gpio.c
@@ -46,7 +46,7 @@ static int u8_gpio_get(struct gpio_chip *gc, unsigned int gpio)
{
struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
- return in_8(mm_gc->regs) & u8_pin2mask(gpio);
+ return (in_8(mm_gc->regs) & u8_pin2mask(gpio) ? 1 : 0);
}
static void u8_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: simple gpio driver
2009-08-13 5:49 simple gpio driver Heiko Schocher
@ 2009-08-17 21:18 ` Grant Likely
2009-08-17 23:38 ` Anton Vorontsov
2009-08-18 5:24 ` Heiko Schocher
0 siblings, 2 replies; 5+ messages in thread
From: Grant Likely @ 2009-08-17 21:18 UTC (permalink / raw)
To: hs; +Cc: linuxppc-dev
On Wed, Aug 12, 2009 at 11:49 PM, Heiko Schocher<hs@denx.de> wrote:
> Hello Anton,
>
> i am trying to use the arch/powerpc/sysdev/simple_gpio.c driver,
> for accessing some gpios, and found, that u8_gpio_get()
> returns not only a 1 or a 0, instead it returns the real bit
> position from the gpio:
>
> gpio =A0 =A0return
> base =A0 =A0value
> 0 =A0 =A0 =A0 0/0x01
> 1 =A0 =A0 =A0 0/0x02
> 2 =A0 =A0 =A0 0/0x04
> 3 =A0 =A0 =A0 0/0x08
> 4 =A0 =A0 =A0 0/0x10
> 5 =A0 =A0 =A0 0/0x20
> 6 =A0 =A0 =A0 0/0x40
> 7 =A0 =A0 =A0 0/0x80
>
> I also use the arch/powerpc/platforms/52xx/mpc52xx_gpio.c and
> mpc52xx_gpt.c drivers, they all return for a gpio just a 1 or 0,
> which seems correct to me, because a gpio can have only 1 or 0
> as state ... what do you think?
I think returning '1' is perhaps slightly 'better' (however you define
that), but I don't think the caller should make any assumptions beyond
zero/non-zero.
>
> I solved this issue (if it is) with the following patch:
>
> diff --git a/arch/powerpc/sysdev/simple_gpio.c b/arch/powerpc/sysdev/simp=
le_gpio.c
> index 43c4569..bb0d79c 100644
> --- a/arch/powerpc/sysdev/simple_gpio.c
> +++ b/arch/powerpc/sysdev/simple_gpio.c
> @@ -46,7 +46,7 @@ static int u8_gpio_get(struct gpio_chip *gc, unsigned i=
nt gpio)
> =A0{
> =A0 =A0 =A0 =A0struct of_mm_gpio_chip *mm_gc =3D to_of_mm_gpio_chip(gc);
>
> - =A0 =A0 =A0 return in_8(mm_gc->regs) & u8_pin2mask(gpio);
> + =A0 =A0 =A0 return (in_8(mm_gc->regs) & u8_pin2mask(gpio) ? 1 : 0);
For clarity, the brackets should be just around the & operands, and
"!=3D 0" instead of "? 1 : 0" might result in slightly smaller code.
return (in_8(mm_gc->regs) & u8_pin2mask(gpio)) !=3D 0;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: simple gpio driver
2009-08-17 21:18 ` Grant Likely
@ 2009-08-17 23:38 ` Anton Vorontsov
2009-08-18 5:25 ` Heiko Schocher
2009-08-18 5:24 ` Heiko Schocher
1 sibling, 1 reply; 5+ messages in thread
From: Anton Vorontsov @ 2009-08-17 23:38 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, hs
Oops, I missed that patch, sorry.
On Mon, Aug 17, 2009 at 03:18:37PM -0600, Grant Likely wrote:
> On Wed, Aug 12, 2009 at 11:49 PM, Heiko Schocher<hs@denx.de> wrote:
> > Hello Anton,
> >
> > i am trying to use the arch/powerpc/sysdev/simple_gpio.c driver,
> > for accessing some gpios, and found, that u8_gpio_get()
> > returns not only a 1 or a 0, instead it returns the real bit
> > position from the gpio:
> >
> > gpio return
> > base value
> > 0 0/0x01
> > 1 0/0x02
> > 2 0/0x04
> > 3 0/0x08
> > 4 0/0x10
> > 5 0/0x20
> > 6 0/0x40
> > 7 0/0x80
> >
> > I also use the arch/powerpc/platforms/52xx/mpc52xx_gpio.c and
> > mpc52xx_gpt.c drivers, they all return for a gpio just a 1 or 0,
There is also arch/powerpc/sysdev/qe_lib/gpio.c and
arch/powerpc/sysdev/mpc8xxx_gpio.c that don't do that.
> > which seems correct to me, because a gpio can have only 1 or 0
> > as state ... what do you think?
>
> I think returning '1' is perhaps slightly 'better' (however you define
> that), but I don't think the caller should make any assumptions beyond
> zero/non-zero.
Yep. So I don't think that the patch is needed.
Thanks,
--
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: simple gpio driver
2009-08-17 21:18 ` Grant Likely
2009-08-17 23:38 ` Anton Vorontsov
@ 2009-08-18 5:24 ` Heiko Schocher
1 sibling, 0 replies; 5+ messages in thread
From: Heiko Schocher @ 2009-08-18 5:24 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
Hello Grant,
Grant Likely wrote:
> On Wed, Aug 12, 2009 at 11:49 PM, Heiko Schocher<hs@denx.de> wrote:
>> Hello Anton,
>>
>> i am trying to use the arch/powerpc/sysdev/simple_gpio.c driver,
>> for accessing some gpios, and found, that u8_gpio_get()
>> returns not only a 1 or a 0, instead it returns the real bit
>> position from the gpio:
>>
>> gpio return
>> base value
>> 0 0/0x01
>> 1 0/0x02
>> 2 0/0x04
>> 3 0/0x08
>> 4 0/0x10
>> 5 0/0x20
>> 6 0/0x40
>> 7 0/0x80
>>
>> I also use the arch/powerpc/platforms/52xx/mpc52xx_gpio.c and
>> mpc52xx_gpt.c drivers, they all return for a gpio just a 1 or 0,
>> which seems correct to me, because a gpio can have only 1 or 0
>> as state ... what do you think?
>
> I think returning '1' is perhaps slightly 'better' (however you define
Yep.
> that), but I don't think the caller should make any assumptions beyond
> zero/non-zero.
Hmm... why? I think a gpio_pin can have as value only 0 or 1.
Ah, if you say zero versus non zero ... hmm... okay.
>> I solved this issue (if it is) with the following patch:
>>
>> diff --git a/arch/powerpc/sysdev/simple_gpio.c b/arch/powerpc/sysdev/simple_gpio.c
>> index 43c4569..bb0d79c 100644
>> --- a/arch/powerpc/sysdev/simple_gpio.c
>> +++ b/arch/powerpc/sysdev/simple_gpio.c
>> @@ -46,7 +46,7 @@ static int u8_gpio_get(struct gpio_chip *gc, unsigned int gpio)
>> {
>> struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
>>
>> - return in_8(mm_gc->regs) & u8_pin2mask(gpio);
>> + return (in_8(mm_gc->regs) & u8_pin2mask(gpio) ? 1 : 0);
>
> For clarity, the brackets should be just around the & operands, and
> "!= 0" instead of "? 1 : 0" might result in slightly smaller code.
>
> return (in_8(mm_gc->regs) & u8_pin2mask(gpio)) != 0;
Yep, you are right, thanks for the info.
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: simple gpio driver
2009-08-17 23:38 ` Anton Vorontsov
@ 2009-08-18 5:25 ` Heiko Schocher
0 siblings, 0 replies; 5+ messages in thread
From: Heiko Schocher @ 2009-08-18 5:25 UTC (permalink / raw)
To: avorontsov; +Cc: linuxppc-dev
Hello Anton,
Anton Vorontsov wrote:
> Oops, I missed that patch, sorry.
>
> On Mon, Aug 17, 2009 at 03:18:37PM -0600, Grant Likely wrote:
>> On Wed, Aug 12, 2009 at 11:49 PM, Heiko Schocher<hs@denx.de> wrote:
>>> Hello Anton,
>>>
>>> i am trying to use the arch/powerpc/sysdev/simple_gpio.c driver,
>>> for accessing some gpios, and found, that u8_gpio_get()
>>> returns not only a 1 or a 0, instead it returns the real bit
>>> position from the gpio:
>>>
>>> gpio return
>>> base value
>>> 0 0/0x01
>>> 1 0/0x02
>>> 2 0/0x04
>>> 3 0/0x08
>>> 4 0/0x10
>>> 5 0/0x20
>>> 6 0/0x40
>>> 7 0/0x80
>>>
>>> I also use the arch/powerpc/platforms/52xx/mpc52xx_gpio.c and
>>> mpc52xx_gpt.c drivers, they all return for a gpio just a 1 or 0,
>
> There is also arch/powerpc/sysdev/qe_lib/gpio.c and
> arch/powerpc/sysdev/mpc8xxx_gpio.c that don't do that.
Ah, okay.
>>> which seems correct to me, because a gpio can have only 1 or 0
>>> as state ... what do you think?
>> I think returning '1' is perhaps slightly 'better' (however you define
>> that), but I don't think the caller should make any assumptions beyond
>> zero/non-zero.
>
> Yep. So I don't think that the patch is needed.
Yes, if the gpio lib only differs in zero versus non zero.
Thanks for the info
bye
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-08-18 5:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-13 5:49 simple gpio driver Heiko Schocher
2009-08-17 21:18 ` Grant Likely
2009-08-17 23:38 ` Anton Vorontsov
2009-08-18 5:25 ` Heiko Schocher
2009-08-18 5:24 ` Heiko Schocher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).