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. In addition, we may change a output pin to input and then read the input status. So we also should update the ->data in "mpc8xxx_gpio_dir_in" function. 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? Best Regards, Liu Gang _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev