Hi Rob,
On Tue, Aug 18, 2020 at 1:32 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Fri, Aug 07, 2020 at 04:13:45PM +0200, Geert Uytterhoeven wrote:
> > Convert the Renesas Pin Function Controller (PFC) Device Tree binding
> > documentation to json-schema.
> >
> > Document missing properties.
> > Drop deprecated and obsolete #gpio-range-cells property.
> > Update the example to match reality.
> > Drop consumer examples, as they do not belong here.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > Still RFC, due to the FIXMEs near the enum descriptions.
> > If I enable the enums checks, I get e.g.:
> >
> > [[1800]] is not one of [1800, 3300]
> >
> > Note the double square brackets around 1800.
> > The usual error message doesn't have them, e.g.:
> >
> > 2000 is not one of [1800, 3300]
> >
> > So this looks like a bug in the tooling?
>
> Yes, we only recently started supporting schemas under
> 'additionalProperties', but failed to apply fixups.
>
> I have a fix I'm testing out. I'm bumping the version requirement in
> 5.10, so I'll make sure it is there.
Thanks, looking forward to it.
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,pfc.yaml
> > + interrupts-extended:
>
> Just use 'interrupts' here. 'interrupt-extended' is always magically
> supported.
Apparently not everywhere...
Documentation/devicetree/bindings/pinctrl/renesas,pfc.example.dt.yaml:
pin-controller@e6050000: 'interrupts' is a required property
Hence I kept it in both places, for consistency.
Cfr. https://lore.kernel.org/linux-gpio/CAMuHMdWSPDQBABXZVaPecETbSRsP2yyZXLHiL_+_R2n_-09jRg@xxxxxxxxxxxxxx/
> > +additionalProperties:
> > + anyOf:
> > + - type: object
> > + allOf:
> > + - $ref: pincfg-node.yaml#
> > + - $ref: pinmux-node.yaml#
> > +
> > + description:
> > + Pin controller client devices use pin configuration subnodes (children
> > + and grandchildren) for desired pin configuration.
> > + Client device subnodes use below standard properties.
> > +
> > + properties:
> > + phandle: true
>
> Once fixed, this won't be necessary.
OK.
> > + function: true
> > + groups: true
> > + pins: true
> > + bias-disable: true
> > + bias-pull-down: true
> > + bias-pull-up: true
> > + drive-strength:
> > + true # FIXME enum: [ 3, 6, 9, 12, 15, 18, 21, 24 ] # Superset of supported values
> > + # avb:pins_mdio:drive-strength: [[24]] is not one of [3, 6, 9, 12, 15, 18, 21, 24]
> > + power-source:
> > + true # FIXME enum: [ 1800, 3300 ]
> > + # sd0_uhs:power-source: [[1800]] is not one of [1800, 3300]
> > + gpio-hog: true
> > + gpios: true
> > + input: true
> > + output-high: true
> > + output-low: true
> > +
> > + additionalProperties: false
> > +
> > + - type: object
> > + properties:
> > + phandle: true
>
> For this one, you can just link it back to the first entry:
>
> - type: object
> additionalProperties:
> $ref: "#/additionalProperties/anyOf/0"
Thanks, cool!
> > +examples:
> > + - |
> > + pfc: pin-controller@e6050000 {
>
> Because we're been really consistent ( :( ):
>
> pinctrl@...
Oh, I had never noticed that was added in devicetree-specification-v0.2.pdf.
Yes, consistency... everything else is *-controller ;-)
Will fix.
>
> > + compatible = "renesas,pfc-sh73a0";
> > + reg = <0xe6050000 0x8000>, <0xe605801c 0x1c>;
> > + gpio-controller;
> > + #gpio-cells = <2>;
> > + gpio-ranges =
> > + <&pfc 0 0 119>, <&pfc 128 128 37>, <&pfc 192 192 91>,
> > + <&pfc 288 288 22>;
> > + interrupts-extended =
> > + <&irqpin0 0 0>, <&irqpin0 1 0>, <&irqpin0 2 0>, <&irqpin0 3 0>,
> > + <&irqpin0 4 0>, <&irqpin0 5 0>, <&irqpin0 6 0>, <&irqpin0 7 0>,
> > + <&irqpin1 0 0>, <&irqpin1 1 0>, <&irqpin1 2 0>, <&irqpin1 3 0>,
> > + <&irqpin1 4 0>, <&irqpin1 5 0>, <&irqpin1 6 0>, <&irqpin1 7 0>,
> > + <&irqpin2 0 0>, <&irqpin2 1 0>, <&irqpin2 2 0>, <&irqpin2 3 0>,
> > + <&irqpin2 4 0>, <&irqpin2 5 0>, <&irqpin2 6 0>, <&irqpin2 7 0>,
> > + <&irqpin3 0 0>, <&irqpin3 1 0>, <&irqpin3 2 0>, <&irqpin3 3 0>,
> > + <&irqpin3 4 0>, <&irqpin3 5 0>, <&irqpin3 6 0>, <&irqpin3 7 0>;
> > + power-domains = <&pd_c5>;
>
> child and grandchild nodes exercising the above schema and issues would
> be good here.
Good idea, will add. I'll probably have to replace the example completely,
as SH73A0 doesn't support drive-strength and power-source.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
