diff mbox series

[40/62] gpio: gpio-siox: Use new GPIO_LINE_DIRECTION

Message ID 91a796dd2811b58f4be30875f5ef644f0e43f241.1572945896.git.matti.vaittinen@fi.rohmeurope.com
State New
Headers show
Series None | expand

Commit Message

Matti Vaittinen Nov. 5, 2019, 10:30 a.m. UTC
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 <matti.vaittinen@fi.rohmeurope.com>
---
 drivers/gpio/gpio-siox.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Thorsten Scherer Nov. 11, 2019, 7:27 a.m. UTC | #1
Hello,

On Tue, Nov 05, 2019 at 12:30:58PM +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 <matti.vaittinen@fi.rohmeurope.com>

for SIOX gpio:

Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>

> ---
>  drivers/gpio/gpio-siox.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c
> index 006a7e6a75f2..311f66757b92 100644
> --- a/drivers/gpio/gpio-siox.c
> +++ b/drivers/gpio/gpio-siox.c
> @@ -203,9 +203,9 @@ static int gpio_siox_direction_output(struct gpio_chip *chip,
>  static int gpio_siox_get_direction(struct gpio_chip *chip, unsigned int offset)
>  {
>  	if (offset < 12)
> -		return 1; /* input */
> +		return GPIO_LINE_DIRECTION_IN;
>  	else
> -		return 0; /* output */
> +		return GPIO_LINE_DIRECTION_OUT;
>  }
>  
>  static int gpio_siox_probe(struct siox_device *sdevice)
> -- 
> 2.21.0
> 

> Quoted from
> [PATCH 00/62] Add definition for GPIO direction
> <cover.1572875541.git.matti.vaittinen@fi.rohmeurope.com>
>
> Patches are compile-tested only. I have no HW to really test them.  Thus
> I'd appreciate carefull review. This work is mainly about converting
> zeros and ones to the new defines but it wouldn't be first time I
> get it wrong in one of the patches :)                                                   

Applied the patch(es) and tested them with SIOX device

Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>

> 
> -- 
> 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 =]
Matti Vaittinen Nov. 11, 2019, 7:43 a.m. UTC | #2
Hello Thorsten,

On Mon, 2019-11-11 at 08:27 +0100, Thorsten Scherer wrote:
> Hello,
> 
> On Tue, Nov 05, 2019 at 12:30:58PM +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 <matti.vaittinen@fi.rohmeurope.com>
> 
> for SIOX gpio:
> 
> Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> 
> > Patches are compile-tested only. I have no HW to really test
> > them.  Thus
> > I'd appreciate carefull review. This work is mainly about
> > converting
> > zeros and ones to the new defines but it wouldn't be first time I
> > get it wrong in one of the patches
> > :)                                                   
> 
> Applied the patch(es) and tested them with SIOX device
> 
> Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
> 

Big thanks! It's _really_ nice that someone takes the time to do the
testing! Highly appreciated! :]

Br,
	Matti Vaittinen
Uwe Kleine-König Nov. 11, 2019, 10:42 a.m. UTC | #3
Hello Matti,

On Mon, Nov 11, 2019 at 07:43:50AM +0000, Vaittinen, Matti wrote:
> On Mon, 2019-11-11 at 08:27 +0100, Thorsten Scherer wrote:
> > Hello,
> > 
> > On Tue, Nov 05, 2019 at 12:30:58PM +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 <matti.vaittinen@fi.rohmeurope.com>
> > 
> > for SIOX gpio:
> > 
> > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > 
> > > Patches are compile-tested only. I have no HW to really test
> > > them.  Thus
> > > I'd appreciate carefull review. This work is mainly about
> > > converting
> > > zeros and ones to the new defines but it wouldn't be first time I
> > > get it wrong in one of the patches
> > > :)                                                   
> > 
> > Applied the patch(es) and tested them with SIOX device
> > 
> > Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
> 
> Big thanks! It's _really_ nice that someone takes the time to do the
> testing! Highly appreciated! :]

without wanting to devalue Thorsten's testing, I think testing your
series can be trivially done without a runtime check as your patches
won't change the compiled result. So just compile once without the patch
and once with and compare the results. If they are bit-by-bit identical
everything is fine.

Best regards
Uwe
Matti Vaittinen Nov. 13, 2019, 8:46 a.m. UTC | #4
On Mon, Nov 11, 2019 at 11:42:52AM +0100, Uwe Kleine-König wrote:
> Hello Matti,
> 
> On Mon, Nov 11, 2019 at 07:43:50AM +0000, Vaittinen, Matti wrote:
> > On Mon, 2019-11-11 at 08:27 +0100, Thorsten Scherer wrote:
> > > Hello,
> > > 
> > > On Tue, Nov 05, 2019 at 12:30:58PM +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 <matti.vaittinen@fi.rohmeurope.com>
> > > 
> > > for SIOX gpio:
> > > 
> > > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > > 
> > > > Patches are compile-tested only. I have no HW to really test
> > > > them.  Thus
> > > > I'd appreciate carefull review. This work is mainly about
> > > > converting
> > > > zeros and ones to the new defines but it wouldn't be first time I
> > > > get it wrong in one of the patches
> > > > :)                                                   
> > > 
> > > Applied the patch(es) and tested them with SIOX device
> > > 
> > > Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > 
> > Big thanks! It's _really_ nice that someone takes the time to do the
> > testing! Highly appreciated! :]
> 
> without wanting to devalue Thorsten's testing, I think testing your
> series can be trivially done without a runtime check as your patches
> won't change the compiled result. So just compile once without the patch
> and once with and compare the results. If they are bit-by-bit identical
> everything is fine.

Right again Uwe. This is correct for most of the modules - assuming
there's no __LINE__ or time related macros used. Few of the modules did
get actual changes though.

Br,
	Matti
Uwe Kleine-König Nov. 13, 2019, 9:09 a.m. UTC | #5
On Wed, Nov 13, 2019 at 10:46:37AM +0200, Matti Vaittinen wrote:
> On Mon, Nov 11, 2019 at 11:42:52AM +0100, Uwe Kleine-König wrote:
> > Hello Matti,
> > 
> > On Mon, Nov 11, 2019 at 07:43:50AM +0000, Vaittinen, Matti wrote:
> > > On Mon, 2019-11-11 at 08:27 +0100, Thorsten Scherer wrote:
> > > > Hello,
> > > > 
> > > > On Tue, Nov 05, 2019 at 12:30:58PM +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 <matti.vaittinen@fi.rohmeurope.com>
> > > > 
> > > > for SIOX gpio:
> > > > 
> > > > Acked-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > > > 
> > > > > Patches are compile-tested only. I have no HW to really test
> > > > > them.  Thus
> > > > > I'd appreciate carefull review. This work is mainly about
> > > > > converting
> > > > > zeros and ones to the new defines but it wouldn't be first time I
> > > > > get it wrong in one of the patches
> > > > > :)                                                   
> > > > 
> > > > Applied the patch(es) and tested them with SIOX device
> > > > 
> > > > Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
> > > 
> > > Big thanks! It's _really_ nice that someone takes the time to do the
> > > testing! Highly appreciated! :]
> > 
> > without wanting to devalue Thorsten's testing, I think testing your
> > series can be trivially done without a runtime check as your patches
> > won't change the compiled result. So just compile once without the patch
> > and once with and compare the results. If they are bit-by-bit identical
> > everything is fine.
> 
> Right again Uwe. This is correct for most of the modules - assuming
> there's no __LINE__ or time related macros used. Few of the modules did
> get actual changes though.

So as you did this research, I think it's worth pointing this out in the
commit log. Either something like:

	There are no changes in the compile result.

or

	This results in changes to the compiled module because ...

(and probably ... is worth fixing).

Best regards
Uwe
diff mbox series

Patch

diff --git a/drivers/gpio/gpio-siox.c b/drivers/gpio/gpio-siox.c
index 006a7e6a75f2..311f66757b92 100644
--- a/drivers/gpio/gpio-siox.c
+++ b/drivers/gpio/gpio-siox.c
@@ -203,9 +203,9 @@  static int gpio_siox_direction_output(struct gpio_chip *chip,
 static int gpio_siox_get_direction(struct gpio_chip *chip, unsigned int offset)
 {
 	if (offset < 12)
-		return 1; /* input */
+		return GPIO_LINE_DIRECTION_IN;
 	else
-		return 0; /* output */
+		return GPIO_LINE_DIRECTION_OUT;
 }
 
 static int gpio_siox_probe(struct siox_device *sdevice)