[PATCH v2] powerpc/gpio: Fix the wrong GPIO input data on MPC8572/MPC8536

[Kumar Gala]

On Nov 22, 2013, at 2:12 AM, Liu Gang <Gang.Liu at freescale.com> wrote:

> For MPC8572/MPC8536, the status of GPIOs defined as output
> cannot be determined by reading GPDAT register, so the code
> use shadow data register instead. But the code may give the
> wrong status of GPIOs defined as input under some scenarios:
> 
> 1. If some pins were configured as inputs and were asserted
> high before booting the kernel, the shadow data has been
> initialized with those pin values.
> 2. Some pins have been configured as output first and have
> been set to the high value, then reconfigured as input.
> 
> The above cases will make the shadow data for those input
> pins to be set to high. Then reading the pin status will
> always return high even if the actual pin status is low.
> 
> The code should eliminate the effects of the shadow data to
> the input pins, and the status of those pins should be
> read directly from GPDAT.
> 
> Signed-off-by: Liu Gang <Gang.Liu at freescale.com>
> ---
> changes in v2:
> - Added more description of the problem.
> - Reduced one in_be32() call.
> - Do not modify the shadow data.
> 
> drivers/gpio/gpio-mpc8xxx.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
> index 914e859..d7d6d72 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -70,10 +70,14 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> 	u32 val;
> 	struct of_mm_gpio_chip *mm = to_of_mm_gpio_chip(gc);
> 	struct mpc8xxx_gpio_chip *mpc8xxx_gc = to_mpc8xxx_gpio_chip(mm);
> +	u32 out_mask, out_shadow;
> 
> -	val = in_be32(mm->regs + GPIO_DAT) & ~in_be32(mm->regs + GPIO_DIR);
> +	out_mask = in_be32(mm->regs + GPIO_DIR);
> 
> -	return (val | mpc8xxx_gc->data) & mpc8xxx_gpio2mask(gpio);
> +	val = in_be32(mm->regs + GPIO_DAT) & ~out_mask;
> +	out_shadow = mpc8xxx_gc->data & out_mask;
> +
> +	return (val | out_shadow) & mpc8xxx_gpio2mask(gpio);
> }

I really think a comment is worth adding here about why we do this.

- k

> 
> static int mpc8xxx_gpio_get(struct gpio_chip *gc, unsigned int gpio)
> -- 
> 1.8.4.1
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev