Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Rob Herring]

On Fri, Mar 23, 2018 at 12:00:20PM +0100, Boris Brezillon wrote:
> Document the Cadence I3C gpio expander bindings.
> 
> Signed-off-by: Boris Brezillon 
> ---
>  .../devicetree/bindings/gpio/gpio-cdns-i3c.txt | 38 
> ++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt 
> b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> new file mode 100644
> index ..634b1f268215
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> @@ -0,0 +1,38 @@
> +* Cadence I3C GPIO expander
> +
> +The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
> +This GPIOs can be configured in output or input mode and if they are in input
> +mode they can generate IBIs (In Band Interrupts).
> +
> +Required properties for GPIO node:
> +- reg : 3 cells encoding the I3C static address (none in our case) and the 
> I3C
> + Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt for
> + more details.
> + Should be <0x0 0x392 0x0>.
> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two. The first cell is the pin number and
> +  the second cell is used to specify the gpio polarity:
> +  0 = active high
> +  1 = active low
> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
> +  The second cell bits[3:0] is used to specify trigger type and level flags:
> +  1 = low-to-high edge triggered.
> +  2 = high-to-low edge triggered.
> +  3 = triggered on both edges.
> +  4 = active high level-sensitive.
> +  8 = active low level-sensitive.
> +
> +Example:
> +
> + i3c-master@xxx {
> + ...
> + i3c_gpio_expander: gpio@0,1c9,0 {

The unit address is wrong here.

> + reg = <0 0x392 0x0>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> + ...
> + };
> -- 
> 2.14.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Geert Uytterhoeven]

Hi Boris,

On Mon, Mar 26, 2018 at 1:25 PM, Boris Brezillon
 wrote:
> On Mon, 26 Mar 2018 12:12:54 +0200
> Geert Uytterhoeven  wrote:
>> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
>>  wrote:
>> > Document the Cadence I3C gpio expander bindings.
>> >
>> > Signed-off-by: Boris Brezillon 
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
>>
>> > +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
>> > +  The second cell bits[3:0] is used to specify trigger type and level 
>> > flags:
>> > +  1 = low-to-high edge triggered.
>> > +  2 = high-to-low edge triggered.
>> > +  3 = triggered on both edges.
>> > +  4 = active high level-sensitive.
>> > +  8 = active low level-sensitive.
>>
>> These are identical to the values in 
>> .
>> Perhaps you can refer to those definitions?
>
> Well, I'm not sure this is allowed since DT bindings docs are
> supposed to be OS-agnostic and macros defined in
>  are, AFAIK, only available to
> Linux.

If they're under include/dt-bindings/, they're part of the DT bindings.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Boris Brezillon]

On Mon, 26 Mar 2018 12:12:54 +0200
Geert Uytterhoeven  wrote:

> Hi Boris,
> 
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
>  wrote:
> > Document the Cadence I3C gpio expander bindings.
> >
> > Signed-off-by: Boris Brezillon   
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt  
> 
> > +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
> > +  The second cell bits[3:0] is used to specify trigger type and level 
> > flags:
> > +  1 = low-to-high edge triggered.
> > +  2 = high-to-low edge triggered.
> > +  3 = triggered on both edges.
> > +  4 = active high level-sensitive.
> > +  8 = active low level-sensitive.  
> 
> These are identical to the values in .
> Perhaps you can refer to those definitions?

Well, I'm not sure this is allowed since DT bindings docs are
supposed to be OS-agnostic and macros defined in
 are, AFAIK, only available to
Linux.

Note that I copied these definitions from another binding ;-).

Rob, any opinion?


-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Boris Brezillon]

Hi Geert,

On Mon, 26 Mar 2018 12:17:26 +0200
Geert Uytterhoeven  wrote:

> Hi Boris,
> 
> On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
>  wrote:
> > Document the Cadence I3C gpio expander bindings.
> >
> > Signed-off-by: Boris Brezillon   
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> > @@ -0,0 +1,38 @@
> > +* Cadence I3C GPIO expander
> > +
> > +The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
> > +This GPIOs can be configured in output or input mode and if they are in 
> > input
> > +mode they can generate IBIs (In Band Interrupts).
> > +
> > +Required properties for GPIO node:
> > +- reg : 3 cells encoding the I3C static address (none in our case) and the 
> > I3C
> > +   Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt 
> > for
> > +   more details.
> > +   Should be <0x0 0x392 0x0>.  
> 
> No compatible value?

See my other reply.

> 
> > +- gpio-controller : Marks the device node as a gpio controller.
> > +- #gpio-cells : Should be two. The first cell is the pin number and
> > +  the second cell is used to specify the gpio polarity:
> > +  0 = active high
> > +  1 = active low
> > +- interrupt-controller: Marks the device node as an interrupt controller.
> > +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
> > +  The second cell bits[3:0] is used to specify trigger type and level 
> > flags:
> > +  1 = low-to-high edge triggered.
> > +  2 = high-to-low edge triggered.
> > +  3 = triggered on both edges.
> > +  4 = active high level-sensitive.
> > +  8 = active low level-sensitive.
> > +
> > +Example:
> > +
> > +   i3c-master@xxx {
> > +   ...
> > +   i3c_gpio_expander: gpio@0,1c9,0 {  
> 
> gpio@0,392,0?

Actually, if I follow the DT bindings, it should be gpio@0,392

Thanks,

Boris

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Geert Uytterhoeven]

Hi Boris,

On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
 wrote:
> Document the Cadence I3C gpio expander bindings.
>
> Signed-off-by: Boris Brezillon 

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt
> @@ -0,0 +1,38 @@
> +* Cadence I3C GPIO expander
> +
> +The Cadence I3C GPIO expander provides 8 GPIOs controllable over I3C.
> +This GPIOs can be configured in output or input mode and if they are in input
> +mode they can generate IBIs (In Band Interrupts).
> +
> +Required properties for GPIO node:
> +- reg : 3 cells encoding the I3C static address (none in our case) and the 
> I3C
> +   Provisional ID. See Documentation/devicetree/bindings/i3c/i3c.txt for
> +   more details.
> +   Should be <0x0 0x392 0x0>.

No compatible value?

> +- gpio-controller : Marks the device node as a gpio controller.
> +- #gpio-cells : Should be two. The first cell is the pin number and
> +  the second cell is used to specify the gpio polarity:
> +  0 = active high
> +  1 = active low
> +- interrupt-controller: Marks the device node as an interrupt controller.
> +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
> +  The second cell bits[3:0] is used to specify trigger type and level flags:
> +  1 = low-to-high edge triggered.
> +  2 = high-to-low edge triggered.
> +  3 = triggered on both edges.
> +  4 = active high level-sensitive.
> +  8 = active low level-sensitive.
> +
> +Example:
> +
> +   i3c-master@xxx {
> +   ...
> +   i3c_gpio_expander: gpio@0,1c9,0 {

gpio@0,392,0?

> +   reg = <0 0x392 0x0>;
> +   gpio-controller;
> +   #gpio-cells = <2>;
> +   interrupt-controller;
> +   #interrupt-cells = <2>;
> +   };
> +   ...
> +   };

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 11/11] dt-bindings: gpio: Add bindings for Cadence I3C gpio expander

[Geert Uytterhoeven]

Hi Boris,

On Fri, Mar 23, 2018 at 12:00 PM, Boris Brezillon
 wrote:
> Document the Cadence I3C gpio expander bindings.
>
> Signed-off-by: Boris Brezillon 

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-cdns-i3c.txt

> +- #interrupt-cells : Should be 2.  The first cell is the GPIO number.
> +  The second cell bits[3:0] is used to specify trigger type and level flags:
> +  1 = low-to-high edge triggered.
> +  2 = high-to-low edge triggered.
> +  3 = triggered on both edges.
> +  4 = active high level-sensitive.
> +  8 = active low level-sensitive.

These are identical to the values in .
Perhaps you can refer to those definitions?
I don't think we want to see the hardcoded numbers in DTS files.

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html