Re: [PATCH 3/3] [POWERPC] QE: implement GPIO LIB API

[Anton Vorontsov]

On Sun, Jan 27, 2008 at 02:42:12PM +0100, Jochen Friedrich wrote:
> Hi Anton,
> 
> > +static void qe_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
> > +{
> > +   struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
> > +   struct port_regs *regs = mm_gc->regs;
> > +   u32 pin_mask;
> > +   u32 tmp_val;
> > +
> > +   /* calculate pin location */
> > +   pin_mask = (u32) (1 << (NUM_OF_PINS - 1 - gpio));
> > +
> > +   tmp_val = in_be32(&regs->cpdata);
> > +
> > +   if (val == 0)
> > +           out_be32(&regs->cpdata, ~pin_mask & tmp_val);
> > +   else
> > +           out_be32(&regs->cpdata, pin_mask | tmp_val);
> > +}
> 
> I see a possible problem with this (and in the corresponding call in CPM1, as 
> well):
> 
> if there is a pin configured as open drain, you might accidently switch this 
> pin to 0
> while switching a different pin, if an external device is pulling the pin to 
> 0.

Unfortunately I can't think out any workaround for this, except
implementing generic gpio_bank_{,un}lock(gpio_pin_on_the_bank), and
start using it in the drivers that might care about this issue. Though,
looking into i2c-gpio.c I don't clearly see were we can insert these
locks, there should be "start/end transaction" handlers or something,
but it seems that it's in the bitbanging code, not in the i2c-gpio
driver..

Actually, I see this as a hardware limitation. For example, on ARMs
PXA2xx, there are separate, per bank, read/set/clear GPIO registers,
not all-in-one data register.

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
backup email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev