It's hard for occasional GPIO code reader/writer to know if values 0/1
equal to IN or OUT. Use defined GPIO_LINE_DIRECTION_IN and
GPIO_LINE_DIRECTION_OUT to help them out.
Signed-off-by: Matti Vaittinen <[email protected]>
---
drivers/gpio/gpio-f7188x.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
index fdc639f856f1..cadd02993539 100644
--- a/drivers/gpio/gpio-f7188x.c
+++ b/drivers/gpio/gpio-f7188x.c
@@ -250,7 +250,10 @@ static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
superio_exit(sio->addr);
- return !(dir & 1 << offset);
+ if (dir & 1 << offset)
+ return GPIO_LINE_DIRECTION_OUT;
+
+ return GPIO_LINE_DIRECTION_IN;
}
static int f7188x_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
--
2.21.0
--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND
~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]
On Tue, Nov 05, 2019 at 12:16:03PM +0200, Matti Vaittinen wrote:
> It's hard for occasional GPIO code reader/writer to know if values 0/1
> equal to IN or OUT. Use defined GPIO_LINE_DIRECTION_IN and
> GPIO_LINE_DIRECTION_OUT to help them out.
>
> Signed-off-by: Matti Vaittinen <[email protected]>
> ---
> drivers/gpio/gpio-f7188x.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-f7188x.c
> index fdc639f856f1..cadd02993539 100644
> --- a/drivers/gpio/gpio-f7188x.c
> +++ b/drivers/gpio/gpio-f7188x.c
> @@ -250,7 +250,10 @@ static int f7188x_gpio_get_direction(struct gpio_chip *chip, unsigned offset)
>
> superio_exit(sio->addr);
>
> - return !(dir & 1 << offset);
> + if (dir & 1 << offset)
> + return GPIO_LINE_DIRECTION_OUT;
> +
> + return GPIO_LINE_DIRECTION_IN
Hi Matti,
I am probably missing something but I can't find GPIO_LINE_DIRECTION_IN
and GPIO_LINE_DIRECTION_OUT defined anywhere.
Besides I am an occasional code reader/writer and I find the original
code easy to understand.
Simon
Morning Simon,
On Wed, 2019-11-06 at 06:34 +0100, Simon Guinot wrote:
> On Tue, Nov 05, 2019 at 12:16:03PM +0200, Matti Vaittinen wrote:
> > It's hard for occasional GPIO code reader/writer to know if values
> > 0/1
> > equal to IN or OUT. Use defined GPIO_LINE_DIRECTION_IN and
> > GPIO_LINE_DIRECTION_OUT to help them out.
> >
> > Signed-off-by: Matti Vaittinen <[email protected]>
> > ---
> > drivers/gpio/gpio-f7188x.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-
> > f7188x.c
> > index fdc639f856f1..cadd02993539 100644
> > --- a/drivers/gpio/gpio-f7188x.c
> > +++ b/drivers/gpio/gpio-f7188x.c
> > @@ -250,7 +250,10 @@ static int f7188x_gpio_get_direction(struct
> > gpio_chip *chip, unsigned offset)
> >
> > superio_exit(sio->addr);
> >
> > - return !(dir & 1 << offset);
> > + if (dir & 1 << offset)
> > + return GPIO_LINE_DIRECTION_OUT;
> > +
> > + return GPIO_LINE_DIRECTION_IN
>
> Hi Matti,
>
> I am probably missing something but I can't find
> GPIO_LINE_DIRECTION_IN
> and GPIO_LINE_DIRECTION_OUT defined anywhere.
Sorry. I accidentally sent the patch 01/62 to limited audience - and
also messed up the message-ID from the series so threading messages is
probably not working :( I did resend the patch adding defines to all
reviewers yesterday - title should be "[RESEND PATCH 01/62] gpio: Add
definition for GPIO direction".
> Besides I am an occasional code reader/writer and I find the original
> code easy to understand.
Glad to hear that. When I read code:
return !(dir & 1 << offset);
It's impossible for me to tell if dir having bit at offset 'offset' set
means IN or OUT - I know the meaning of code, it checks this bit for
in/out - but which dir value is IN and which is OUT?
When this is written as:
if (dir & 1 << offset)
return GPIO_LINE_DIRECTION_OUT;
return GPIO_LINE_DIRECTION_IN
it get's quite obvious even for me that having the matching bit set
means direction to be OUT.
Best Regards
Matti Vaittinen
Hi Matti,
On Wed, Nov 6, 2019 at 7:45 AM Vaittinen, Matti
<[email protected]> wrote:
> On Wed, 2019-11-06 at 06:34 +0100, Simon Guinot wrote:
> > On Tue, Nov 05, 2019 at 12:16:03PM +0200, Matti Vaittinen wrote:
> > > It's hard for occasional GPIO code reader/writer to know if values
> > > 0/1
> > > equal to IN or OUT. Use defined GPIO_LINE_DIRECTION_IN and
> > > GPIO_LINE_DIRECTION_OUT to help them out.
> > >
> > > Signed-off-by: Matti Vaittinen <[email protected]>
> > > ---
> > > drivers/gpio/gpio-f7188x.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-
> > > f7188x.c
> > > index fdc639f856f1..cadd02993539 100644
> > > --- a/drivers/gpio/gpio-f7188x.c
> > > +++ b/drivers/gpio/gpio-f7188x.c
> > > @@ -250,7 +250,10 @@ static int f7188x_gpio_get_direction(struct
> > > gpio_chip *chip, unsigned offset)
> > >
> > > superio_exit(sio->addr);
> > >
> > > - return !(dir & 1 << offset);
> > > + if (dir & 1 << offset)
> > > + return GPIO_LINE_DIRECTION_OUT;
> > > +
> > > + return GPIO_LINE_DIRECTION_IN
> >
> > Hi Matti,
> >
> > I am probably missing something but I can't find
> > GPIO_LINE_DIRECTION_IN
> > and GPIO_LINE_DIRECTION_OUT defined anywhere.
>
> Sorry. I accidentally sent the patch 01/62 to limited audience - and
> also messed up the message-ID from the series so threading messages is
> probably not working :( I did resend the patch adding defines to all
> reviewers yesterday - title should be "[RESEND PATCH 01/62] gpio: Add
> definition for GPIO direction".
>
> > Besides I am an occasional code reader/writer and I find the original
> > code easy to understand.
>
> Glad to hear that. When I read code:
>
> return !(dir & 1 << offset);
>
> It's impossible for me to tell if dir having bit at offset 'offset' set
> means IN or OUT - I know the meaning of code, it checks this bit for
> in/out - but which dir value is IN and which is OUT?
>
> When this is written as:
>
> if (dir & 1 << offset)
> return GPIO_LINE_DIRECTION_OUT;
>
> return GPIO_LINE_DIRECTION_IN
>
> it get's quite obvious even for me that having the matching bit set
> means direction to be OUT.
"suggest parentheses around... " warning?
if (dir & BIT(offset))
...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hello Geert,
On Wed, 2019-11-06 at 13:38 +0100, Geert Uytterhoeven wrote:
> Hi Matti,
>
> On Wed, Nov 6, 2019 at 7:45 AM Vaittinen, Matti
> <[email protected]> wrote:
> > On Wed, 2019-11-06 at 06:34 +0100, Simon Guinot wrote:
> > > On Tue, Nov 05, 2019 at 12:16:03PM +0200, Matti Vaittinen wrote:
> > > > It's hard for occasional GPIO code reader/writer to know if
> > > > values
> > > > 0/1
> > > > equal to IN or OUT. Use defined GPIO_LINE_DIRECTION_IN and
> > > > GPIO_LINE_DIRECTION_OUT to help them out.
> > > >
> > > > Signed-off-by: Matti Vaittinen <
> > > > [email protected]>
> > > > ---
> > > > drivers/gpio/gpio-f7188x.c | 5 ++++-
> > > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpio/gpio-f7188x.c b/drivers/gpio/gpio-
> > > > f7188x.c
> > > > index fdc639f856f1..cadd02993539 100644
> > > > --- a/drivers/gpio/gpio-f7188x.c
> > > > +++ b/drivers/gpio/gpio-f7188x.c
> > > > @@ -250,7 +250,10 @@ static int
> > > > f7188x_gpio_get_direction(struct
> > > > gpio_chip *chip, unsigned offset)
> > > >
> > > > superio_exit(sio->addr);
> > > >
> > > > - return !(dir & 1 << offset);
> > > > + if (dir & 1 << offset)
> > > > + return GPIO_LINE_DIRECTION_OUT;
> > > > +
> > > > + return GPIO_LINE_DIRECTION_IN
> > >
> > > Hi Matti,
> > >
> > > I am probably missing something but I can't find
> > > GPIO_LINE_DIRECTION_IN
> > > and GPIO_LINE_DIRECTION_OUT defined anywhere.
> >
> > Sorry. I accidentally sent the patch 01/62 to limited audience -
> > and
> > also messed up the message-ID from the series so threading messages
> > is
> > probably not working :( I did resend the patch adding defines to
> > all
> > reviewers yesterday - title should be "[RESEND PATCH 01/62] gpio:
> > Add
> > definition for GPIO direction".
> >
> > > Besides I am an occasional code reader/writer and I find the
> > > original
> > > code easy to understand.
> >
> > Glad to hear that. When I read code:
> >
> > return !(dir & 1 << offset);
> >
> > It's impossible for me to tell if dir having bit at offset 'offset'
> > set
> > means IN or OUT - I know the meaning of code, it checks this bit
> > for
> > in/out - but which dir value is IN and which is OUT?
> >
> > When this is written as:
> >
> > if (dir & 1 << offset)
> > return GPIO_LINE_DIRECTION_OUT;
> >
> > return GPIO_LINE_DIRECTION_IN
> >
> > it get's quite obvious even for me that having the matching bit set
> > means direction to be OUT.
>
> "suggest parentheses around... " warning?
I don't think I saw that. Also, simple test:
#include <stdio.h>
int main(int argc, char *argv[])
{
int offset = argc;
if (4 & 1 << offset)
printf("foo\n");
printf("%d", !(4 & 1 << offset));
return !(4 & 1 << offset);
}
and building with gcc -Wall does not show any warnings.
nor did compilation for the driver:
[mvaittin@localhost linux]$ make ARCH=arm CROSS_COMPILE=${CC}
LOADADDR=0x80008000
CALL scripts/checksyscalls.sh
CALL scripts/atomic/check-atomics.sh
CHK include/generated/compile.h
CC [M] drivers/gpio/gpio-f7188x.o
Kernel: arch/arm/boot/Image is ready
Kernel: arch/arm/boot/zImage is ready
Building modules, stage 2.
MODPOST 1282 modules
LD [M] drivers/gpio/gpio-f7188x.ko
[mvaittin@localhost linux]$
In any case, my intention was to keep the logic exactly same - except
for the documented change for cases where I changed bit-position
specific return value to 1.
Br,
Matti Vaittinen