Re: [PATCH] gpio-pca953x: Support NXP PCAL9555A with Agile I/O

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Thu, Jul 2, 2015 at 7:33 PM, Clemens Gruber
<clemens.gruber@xxxxxxxxxxxx> 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 <linus.walleij@xxxxxxxxxx>
> Cc: Alexandre Courbot <gnurou@xxxxxxxxx>
> Signed-off-by: Clemens Gruber <clemens.gruber@xxxxxxxxxxxx>

(...)

> +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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html