All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
@ 2013-08-02 21:24 Graeme Smecher
  2013-08-04 12:35 ` Haojian Zhuang
  0 siblings, 1 reply; 10+ messages in thread
From: Graeme Smecher @ 2013-08-02 21:24 UTC (permalink / raw)
  To: linux-kernel; +Cc: Graeme Smecher, Grant Likely, Haojian Zhuang

The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
level and direction in the pca957x_probe function. This seems like the wrong
thing to do, since it can cause hardware bit twiddles during warm reboots when
the chip state and reset values don't match.

This kind of initialization is best left upstream (in a bootloader) or
downstream (in userspace). It's also an inconsistency across devices supported
by this driver.

Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
---
 drivers/gpio/gpio-pca953x.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index 426c51d..47c08d8 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -683,17 +683,6 @@ static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 	int ret;
 	u8 val[MAX_BANK];
 
-	/* Let every port in proper state, that could save power */
-	memset(val, 0, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_PUPD, val);
-	memset(val, 0xFF, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_CFG, val);
-	memset(val, 0, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_OUT, val);
-
-	ret = pca953x_read_regs(chip, PCA957X_IN, val);
-	if (ret)
-		goto out;
 	ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
 	if (ret)
 		goto out;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-08-02 21:24 [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them Graeme Smecher
@ 2013-08-04 12:35 ` Haojian Zhuang
  2013-09-09 18:14   ` Graeme Smecher
  0 siblings, 1 reply; 10+ messages in thread
From: Haojian Zhuang @ 2013-08-04 12:35 UTC (permalink / raw)
  To: Graeme Smecher; +Cc: linux-kernel@vger.kernel.org, Grant Likely, Haojian Zhuang

On Sat, Aug 3, 2013 at 5:24 AM, Graeme Smecher
<gsmecher@threespeedlogic.com> wrote:
> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
> level and direction in the pca957x_probe function. This seems like the wrong
> thing to do, since it can cause hardware bit twiddles during warm reboots when
> the chip state and reset values don't match.
>
> This kind of initialization is best left upstream (in a bootloader) or
> downstream (in userspace). It's also an inconsistency across devices supported
> by this driver.
>
> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Haojian Zhuang <haojian.zhuang@marvell.com>
> ---
>  drivers/gpio/gpio-pca953x.c |   11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index 426c51d..47c08d8 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -683,17 +683,6 @@ static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
>         int ret;
>         u8 val[MAX_BANK];
>
> -       /* Let every port in proper state, that could save power */
> -       memset(val, 0, NBANK(chip));
> -       pca953x_write_regs(chip, PCA957X_PUPD, val);
> -       memset(val, 0xFF, NBANK(chip));
> -       pca953x_write_regs(chip, PCA957X_CFG, val);
> -       memset(val, 0, NBANK(chip));
> -       pca953x_write_regs(chip, PCA957X_OUT, val);
> -
> -       ret = pca953x_read_regs(chip, PCA957X_IN, val);
> -       if (ret)
> -               goto out;
>         ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
>         if (ret)
>                 goto out;
> --
> 1.7.10.4
>

Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-08-04 12:35 ` Haojian Zhuang
@ 2013-09-09 18:14   ` Graeme Smecher
  0 siblings, 0 replies; 10+ messages in thread
From: Graeme Smecher @ 2013-09-09 18:14 UTC (permalink / raw)
  To: Grant Likely; +Cc: Haojian Zhuang, linux-kernel@vger.kernel.org

Hi Grant,

On 04/08/13 05:35 AM, Haojian Zhuang wrote:
> On Sat, Aug 3, 2013 at 5:24 AM, Graeme Smecher
> <gsmecher@threespeedlogic.com>  wrote:
>> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
>> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
>> level and direction in the pca957x_probe function. This seems like the wrong
>> thing to do, since it can cause hardware bit twiddles during warm reboots when
>> the chip state and reset values don't match.
>>
>> This kind of initialization is best left upstream (in a bootloader) or
>> downstream (in userspace). It's also an inconsistency across devices supported
>> by this driver.
>>
>> Signed-off-by: Graeme Smecher<gsmecher@threespeedlogic.com>
>> Cc: Grant Likely<grant.likely@linaro.org>
>> Cc: Haojian Zhuang<haojian.zhuang@marvell.com>
>> ---
>>   drivers/gpio/gpio-pca953x.c |   11 -----------
>>   1 file changed, 11 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
>> index 426c51d..47c08d8 100644
>> --- a/drivers/gpio/gpio-pca953x.c
>> +++ b/drivers/gpio/gpio-pca953x.c
>> @@ -683,17 +683,6 @@ static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
>>          int ret;
>>          u8 val[MAX_BANK];
>>
>> -       /* Let every port in proper state, that could save power */
>> -       memset(val, 0, NBANK(chip));
>> -       pca953x_write_regs(chip, PCA957X_PUPD, val);
>> -       memset(val, 0xFF, NBANK(chip));
>> -       pca953x_write_regs(chip, PCA957X_CFG, val);
>> -       memset(val, 0, NBANK(chip));
>> -       pca953x_write_regs(chip, PCA957X_OUT, val);
>> -
>> -       ret = pca953x_read_regs(chip, PCA957X_IN, val);
>> -       if (ret)
>> -               goto out;
>>          ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
>>          if (ret)
>>                  goto out;
>> --
>> 1.7.10.4
>>
> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>

Just a quick ping: I think you're the responsible maintainer for this 
patch, and I haven't seen it show up on git.secretlab.ca. Do you want me 
to resubmit with Haojian's e-mail corrected?

I should also note that this patch is NOT boot-tested. My hardware 
containing this part is only supported by the vendor's aging 2.6.37 
branch. (It's a custom ARM board using the TI AM3874.) The pca953x 
driver has evolved enough that the patches for 2.6.37 and mainline 
aren't identical, but they still rhyme.

thanks,
Graeme

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
@ 2013-09-13 21:41 Graeme Smecher
  2013-09-20 20:24 ` Linus Walleij
  2013-09-23 10:57 ` Linus Walleij
  0 siblings, 2 replies; 10+ messages in thread
From: Graeme Smecher @ 2013-09-13 21:41 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Haojian Zhuang, linux-gpio, Graeme Smecher

The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
level and direction in the pca957x_probe function. This seems like the wrong
thing to do, since it can cause hardware bit twiddles during warm reboots when
the chip state and reset values don't match.

This kind of initialization is best left upstream (in a bootloader) or
downstream (in userspace). It's also an inconsistency across devices supported
by this driver.

This patch is NOT boot-tested: the SoC I'm  using is stuck on 2.6.37, and the
patch doesn't apply trivially.

Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/gpio/gpio-pca953x.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
index cdd1aa1..6e48c07 100644
--- a/drivers/gpio/gpio-pca953x.c
+++ b/drivers/gpio/gpio-pca953x.c
@@ -683,17 +683,6 @@ static int device_pca957x_init(struct pca953x_chip *chip, u32 invert)
 	int ret;
 	u8 val[MAX_BANK];
 
-	/* Let every port in proper state, that could save power */
-	memset(val, 0, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_PUPD, val);
-	memset(val, 0xFF, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_CFG, val);
-	memset(val, 0, NBANK(chip));
-	pca953x_write_regs(chip, PCA957X_OUT, val);
-
-	ret = pca953x_read_regs(chip, PCA957X_IN, val);
-	if (ret)
-		goto out;
 	ret = pca953x_read_regs(chip, PCA957X_OUT, chip->reg_output);
 	if (ret)
 		goto out;
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-13 21:41 Graeme Smecher
@ 2013-09-20 20:24 ` Linus Walleij
  2013-09-20 20:37   ` Gregory CLEMENT
  2013-09-23 10:57 ` Linus Walleij
  1 sibling, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2013-09-20 20:24 UTC (permalink / raw)
  To: Graeme Smecher, Gregory CLEMENT
  Cc: Haojian Zhuang, linux-gpio@vger.kernel.org

On Fri, Sep 13, 2013 at 11:41 PM, Graeme Smecher
<gsmecher@threespeedlogic.com> wrote:

> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
> level and direction in the pca957x_probe function. This seems like the wrong
> thing to do, since it can cause hardware bit twiddles during warm reboots when
> the chip state and reset values don't match.
>
> This kind of initialization is best left upstream (in a bootloader) or
> downstream (in userspace). It's also an inconsistency across devices supported
> by this driver.
>
> This patch is NOT boot-tested: the SoC I'm  using is stuck on 2.6.37, and the
> patch doesn't apply trivially.
>
> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Looks like you're deleting code from Gregory CLEMENT,
Gregory what do you say about this?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-20 20:24 ` Linus Walleij
@ 2013-09-20 20:37   ` Gregory CLEMENT
  2013-09-20 21:06     ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Gregory CLEMENT @ 2013-09-20 20:37 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Graeme Smecher, Haojian Zhuang, linux-gpio@vger.kernel.org

Hi Linus,

On 20/09/2013 22:24, Linus Walleij wrote:
> On Fri, Sep 13, 2013 at 11:41 PM, Graeme Smecher
> <gsmecher@threespeedlogic.com> wrote:
> 
>> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
>> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
>> level and direction in the pca957x_probe function. This seems like the wrong
>> thing to do, since it can cause hardware bit twiddles during warm reboots when
>> the chip state and reset values don't match.
>>
>> This kind of initialization is best left upstream (in a bootloader) or
>> downstream (in userspace). It's also an inconsistency across devices supported
>> by this driver.
>>
>> This patch is NOT boot-tested: the SoC I'm  using is stuck on 2.6.37, and the
>> patch doesn't apply trivially.
>>
>> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
>> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>
> 
> Looks like you're deleting code from Gregory CLEMENT,
> Gregory what do you say about this?

Thanks to point me this patch, I didn't have noticed it.
The code being removed is mine because I change it to take care of the
multiple bankd. I didn't change the functionality. The original code
was from Haojian Zhuang so as he gave his acked-by it should be fine.

However could you wait for until the end of this week end, to let me test
it on a real hardware?

Thanks,

Gregory

-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-20 20:37   ` Gregory CLEMENT
@ 2013-09-20 21:06     ` Linus Walleij
  2013-09-23  9:51       ` Gregory CLEMENT
  0 siblings, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2013-09-20 21:06 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Graeme Smecher, Haojian Zhuang, linux-gpio@vger.kernel.org

On Fri, Sep 20, 2013 at 10:37 PM, Gregory CLEMENT
<gregory.clement@free-electrons.com> wrote:

> However could you wait for until the end of this week end, to let me test
> it on a real hardware?

Sure, no hurry.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-20 21:06     ` Linus Walleij
@ 2013-09-23  9:51       ` Gregory CLEMENT
  0 siblings, 0 replies; 10+ messages in thread
From: Gregory CLEMENT @ 2013-09-23  9:51 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Graeme Smecher, Haojian Zhuang, linux-gpio@vger.kernel.org

Hi,

On 20/09/2013 23:06, Linus Walleij wrote:
> On Fri, Sep 20, 2013 at 10:37 PM, Gregory CLEMENT
> <gregory.clement@free-electrons.com> wrote:
> 
>> However could you wait for until the end of this week end, to let me test
>> it on a real hardware?
> 
> Sure, no hurry.
> 

I tested the patch using the GPIO box connected to the Mirabox and I
didn't see any regression. So you can add my:

Tested-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

Thanks,

> Yours,
> Linus Walleij
> 


-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-13 21:41 Graeme Smecher
  2013-09-20 20:24 ` Linus Walleij
@ 2013-09-23 10:57 ` Linus Walleij
  2013-09-23 13:36   ` Graeme Smecher
  1 sibling, 1 reply; 10+ messages in thread
From: Linus Walleij @ 2013-09-23 10:57 UTC (permalink / raw)
  To: Graeme Smecher; +Cc: Haojian Zhuang, linux-gpio@vger.kernel.org

On Fri, Sep 13, 2013 at 11:41 PM, Graeme Smecher
<gsmecher@threespeedlogic.com> wrote:

> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
> level and direction in the pca957x_probe function. This seems like the wrong
> thing to do, since it can cause hardware bit twiddles during warm reboots when
> the chip state and reset values don't match.
>
> This kind of initialization is best left upstream (in a bootloader) or
> downstream (in userspace). It's also an inconsistency across devices supported
> by this driver.
>
> This patch is NOT boot-tested: the SoC I'm  using is stuck on 2.6.37, and the
> patch doesn't apply trivially.
>
> Signed-off-by: Graeme Smecher <gsmecher@threespeedlogic.com>
> Acked-by: Haojian Zhuang <haojian.zhuang@gmail.com>

Patch applied with Gregory's Tested-by tag!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them.
  2013-09-23 10:57 ` Linus Walleij
@ 2013-09-23 13:36   ` Graeme Smecher
  0 siblings, 0 replies; 10+ messages in thread
From: Graeme Smecher @ 2013-09-23 13:36 UTC (permalink / raw)
  To: Linus Walleij; +Cc: Haojian Zhuang, linux-gpio@vger.kernel.org, gregory.clement

Hi,

On 23/09/13 06:57 AM, Linus Walleij wrote:
> On Fri, Sep 13, 2013 at 11:41 PM, Graeme Smecher
> <gsmecher@threespeedlogic.com>  wrote:
>
>> The pca957x driver supports a handful of I2C GPIO expanders from NXP, Maxim,
>> and TI. For the PCA9574 and PCA9575 devices only, the driver resets the GPIO
>> level and direction in the pca957x_probe function. This seems like the wrong
>> thing to do, since it can cause hardware bit twiddles during warm reboots when
>> the chip state and reset values don't match.
>>
>> This kind of initialization is best left upstream (in a bootloader) or
>> downstream (in userspace). It's also an inconsistency across devices supported
>> by this driver.
>>
>> This patch is NOT boot-tested: the SoC I'm  using is stuck on 2.6.37, and the
>> patch doesn't apply trivially.
>>
>> Signed-off-by: Graeme Smecher<gsmecher@threespeedlogic.com>
>> Acked-by: Haojian Zhuang<haojian.zhuang@gmail.com>
> Patch applied with Gregory's Tested-by tag!

Thanks all!

cheers,
Graeme

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-09-23 13:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-02 21:24 [PATCH] gpio: pca953x: Don't flip bits on PCA957x GPIO expanders when probing them Graeme Smecher
2013-08-04 12:35 ` Haojian Zhuang
2013-09-09 18:14   ` Graeme Smecher
  -- strict thread matches above, loose matches on Subject: below --
2013-09-13 21:41 Graeme Smecher
2013-09-20 20:24 ` Linus Walleij
2013-09-20 20:37   ` Gregory CLEMENT
2013-09-20 21:06     ` Linus Walleij
2013-09-23  9:51       ` Gregory CLEMENT
2013-09-23 10:57 ` Linus Walleij
2013-09-23 13:36   ` Graeme Smecher

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.