Re: [PATCH 3/3] gpio: pcf857x: Add OF support

Re: [PATCH 3/3] gpio: pcf857x: Add OF support

[Linus Walleij]

On Mon, Jul 29, 2013 at 8:39 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:

> Add DT bindings for the pcf857x-compatible chips and parse the device
> tree node in the driver.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 ++++++++++++++++++++++

Do you think you can get one of the DT bindings mainatiners to ACK
this part?

I am told we need to get more strict on binding review.

It will be subject to timeout if it takes to long and you made a reasonable
effort to get their consent though.

> +  - interrupt-controller: Identifies the node as an interrupt controller.
> +  - #interrupt-cells: Number of cells to encode an interrupt source, shall be 2.
> +  - interrupt-parent: phandle of the parent interrupt controller.
> +  - interrupts: Interrupt specifier for the controllers interrupt.

So this is another one of of those GPIO controller that are
also interrupt controllers.

So it will be subject to the same dilemma as the OMAP
expander that has caused so much stir :-(

Can you provide your input in the discussion regarding my
patch with the subject:
"RFC: interrupt consistency check for OF GPIO IRQs"

This patch will affect this driver also, in a very direct way, by just
stealing and setting as input any GPIO line used for an IRQ.

> +Please refer to gpio.txt in this directory for details of the common GPIO
> +bindings used by client devices.
> +
> +Example: PCF8575 I/O expander node
> +
> +       pcf8575: gpio@20 {
> +               compatible = "nxp,pcf8575";
> +               reg = <0x20>;
> +               interrupt-parent = <&irqpin2>;
> +               interrupts = <3 0>;
> +               interrupt-controller;
> +               #interrupt-cells = <2>;
> +       };

Not gpio-controller?

Apart from that it looks good, please help out with the
gpio-controller vs interrupt-controller dilemma if you can
because it's getting really frustrating...

Yours,
Linus Walleij

Re: [PATCH 3/3] gpio: pcf857x: Add OF support

[Laurent Pinchart]

Hi Linus,

On Friday 16 August 2013 17:00:29 Linus Walleij wrote:
> On Mon, Jul 29, 2013 at 8:39 PM, Laurent Pinchart wrote:
> > Add DT bindings for the pcf857x-compatible chips and parse the device
> > tree node in the driver.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  .../devicetree/bindings/gpio/gpio-pcf857x.txt      | 69 +++++++++++++++++
>
> Do you think you can get one of the DT bindings mainatiners to ACK this
> part?

They're already CC'ed. I suppose vacations played a role in the lack of reply. 
I'd like to avoid having to switch to stalker mode ;-)

> I am told we need to get more strict on binding review.
> 
> It will be subject to timeout if it takes to long and you made a reasonable
> effort to get their consent though.
> 
> > +  - interrupt-controller: Identifies the node as an interrupt controller.
> > +  - #interrupt-cells: Number of cells to encode an interrupt source,
> > shall be 2.
> > +  - interrupt-parent: phandle of the parent interrupt controller.
> > +  - interrupts: Interrupt specifier for the controllers interrupt.
>
> So this is another one of of those GPIO controller that are also interrupt
> controllers.
> 
> So it will be subject to the same dilemma as the OMAP expander that has
> caused so much stir :-(
> 
> Can you provide your input in the discussion regarding my patch with the
> subject:
> "RFC: interrupt consistency check for OF GPIO IRQs"

Done.

> This patch will affect this driver also, in a very direct way, by just
> stealing and setting as input any GPIO line used for an IRQ.
> 
> > +Please refer to gpio.txt in this directory for details of the common GPIO
> > +bindings used by client devices.
> > +
> > +Example: PCF8575 I/O expander node
> > +
> > +       pcf8575: gpio@20 {
> > +               compatible = "nxp,pcf8575";
> > +               reg = <0x20>;
> > +               interrupt-parent = <&irqpin2>;
> > +               interrupts = <3 0>;
> > +               interrupt-controller;
> > +               #interrupt-cells = <2>;
> > +       };
> 
> Not gpio-controller?

I've fixed that in my tree already, it seems I forgot to repost. I will do so 
now.

> Apart from that it looks good, please help out with the gpio-controller vs
> interrupt-controller dilemma if you can because it's getting really
> frustrating...

-- 
Regards,

Laurent Pinchart