On Thu, Jul 2, 2015 at 7:33 PM, Clemens Gruber <[email protected]> wrote:
> This patch adds support for the NXP PCAL9555A GPIO expander's extra > features, called Agile I/O: > Input latching, interrupt masks and open-drain output stages can be > configured via 3 optional device tree properties. > > Cc: Linus Walleij <[email protected]> > Cc: Alexandre Courbot <[email protected]> > Signed-off-by: Clemens Gruber <[email protected]> (...) > +Optional properties for chips with Agile I/O: > +- nxp,input-latch: Latch input states by setting the pin's corresponding bits What does "latch" mean here. We need to know if this is something we will see a hundred times in the future or only this once. I am also very suspicious as to whether this should be in the GPIO controller, I think it should be on the consumer, because surely it is the user of the GPIO line that requires this, not the GPIO chip. > +- nxp,intr-mask: Enable interrupts by clearing the pin's corresponding mask > bits This is just wrong. Let the irqchip portions of the driver enable interrupts by masking them, don't try to outsmart operating system functions with hammering down things in device tree. > +- nxp,open-drain: Enable the open-drain output stage, one bit per port (bank) Again, this should be done on the consumer side. Any driver that reference a GPIO line can do this: foo-gpios = <&gpioN 12 GPIO_ACTIVE_LOW>; So why should it not be able to do this: foo-gpios = <&gpioN 12 GPIO_OPEN_DRAIN>; Add GPIO_OPEN_DRAIN to include/dt-bindings/gpio/gpio.h and implement handling in the GPIO core drivers/gpio/gpiolib-of.c instead of trying to duct-tape it with things like this. We already have GPIO_OPEN_DRAIN in include/linux/gpio/machine.h for the boardfile case, so implement similar handling. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
