On Wed, 2013-11-20 at 10:54 +0800, Liu Gang wrote: > On Tue, 2013-11-19 at 16:51 -0600, Scott Wood wrote: > > > @@ -71,6 +71,7 @@ static int mpc8572_gpio_get(struct gpio_chip *gc, > > > unsigned int gpio) > > > struct mpc8xxx_gpio_chip *mpc8xxx_gc = to_mpc8xxx_gpio_chip(mm); > > > > > > val = in_be32(mm->regs + GPIO_DAT) & ~in_be32(mm->regs + GPIO_DIR); > > > + mpc8xxx_gc->data &= in_be32(mm->regs + GPIO_DIR); > > > > > > return (val | mpc8xxx_gc->data) & mpc8xxx_gpio2mask(gpio); > > > } > > > > It seems odd to update ->data in a function that's supposed to be > > reading things... Perhaps it would be better to keep ->data in a good > > state from the beginning. > > > > -Scott > > Yes, keeping the ->data in a good state from the beginning will be > better. But this will need more code in different functions to cover > all the scenarios. > First, we should check the direct of the pin in the function > "mpc8xxx_gpio_set", and clean the input bit in ->data after setting > operation.
For userspace value setting, it looks like gpiolib blocks the write if the pin if FLAG_IS_OUT is set. This suggests that this is an error condition for other uses as well. Though, I notice that mpc8xxx_gpio_dir_out() calls gpio_set() before actually changing the direction. So it may be useful to avoid races where the wrong value is output briefly after the direction is changed (especially in open drain situations, where the signal could have a meaningful default even before we begin outputting). But that raises the question of how you'd do that from userspace, and it also renders the to-be-output value as write-only data (until the direction is actually changed), since a readback would get the input value instead. > So maybe it's better to eliminate the effects of the ->data to the > input pins when reading the status, regardless of the possible changes > of the pins and the data. > Do you think so? Perhaps, but that doesn't require you to modify ->data in the get() function. -Scott _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
