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

[Liu Gang <Gang.Liu@freescale.com>]

On Tue, 2013-11-19 at 16:32 +0100, Anatolij Gustschin wrote:
> On Fri, 15 Nov 2013 15:16:29 +0800
> Liu Gang <Gang.Liu@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 if the input pins are
> > asserted high, they will always read high due to the shadow
> > data, even if the pins are set to low.
> 
> Could you please add a better description of the problem?
> I'm having some difficulties to understand the last sentence
> above. Does the issue appear if some pins were configured as
> inputs and were asserted high before booting the kernel, and
> therefore the shadow data has been initialized with these pin
> values?
> 
> Or does the issue appear if some pin has been configured as output
> first and has been set to the high value, then reconfigured as
> input? Now reading the pin state will always return high even
> if the actual pin state is low?
> 
> It seems the issue will appear in both cases. If so, please add
> this information to the commit message.
> 
Yes, you are right.
I'll updated the description more clear.

> >  	val = in_be32(mm->regs + GPIO_DAT) & ~in_be32(mm->regs + GPIO_DIR);
> > +	mpc8xxx_gc->data &= in_be32(mm->regs + GPIO_DIR);
> 
> we can reduce one in_be32() call here, i.e.
> 
> 	u32 out_mask;
> 	...
> 	out_mask = in_be32(mm->regs + GPIO_DIR);
> 	val = in_be32(mm->regs + GPIO_DAT) & ~out_mask;
> 	mpc8xxx_gc->data &= out_mask;
> 
> >  	return (val | mpc8xxx_gc->data) & mpc8xxx_gpio2mask(gpio);
> >  }
> 
> Thanks,
> 
> Anatolij
> 
Granted, it will be better to reduce one in_be32() call.
I'll improve the method based on your and Scott's comments.

Thanks
Liu Gang