| Message ID | 20220816183911.2517173-1-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] dt-bindings: pinctrl: brcm: Ensure all child node properties are documented | expand |
On Tue, 16 Aug 2022 at 20:39, Rob Herring <robh@kernel.org> wrote: > > The Broadcom pinctrl bindings are incomplete for child nodes as they are > missing 'unevaluatedProperties: false' to prevent unknown properties. > Fixing this reveals many warnings including having grandchild nodes in some > cases. > > Signed-off-by: Rob Herring <robh@kernel.org> > --- > This recursive schema requires a fix not yet committed in dtschema. > > I'm looking for feedback on whether group->pins or group->groups is the > right fix here. There's more warnings with this change in the gpio-sysctl > bindings. The answer is "yes", though pins is probably the closest for most. bcm6318 has multiple field-per-pin registers, where each pin is controlled separately, with more fields than available GPIOs, and the pins outside the GPIO range controlling other functions, like switching the second USB port between host and client mode. bcm6328/6362/6368/63268 have two registers. The first one enables an alternative function for the first 32 GPIOs, with a 1:1 mapping of bits to GPIO. The second one enables a function for whole, arbitrary groups. These groups can overlap, and may also target pins outside the first 32 GPIOs. The actual pins in use are not documented, and can sometimes be guessed/inferred by the function names (e.g. "GPIO35"), sometimes not ("NAND", "UTOPIA"). bcm6358 has only the groups register, which also includes non GPIO related functions, like inversing the MII clocks for the integrated macs. Not supported is bcm6348, which would be the only one where groups would definitely make more sense: there are 5 groups of 8 GPIOs, where each group can be set to a certain function (but not all functions are valid for all groups). E.g. for PCI support, you would need to set the fields to PCI for groups 0, 1 and 3 (and 2 and 5 could be set to a different function). You can ignore this though, as bcm6348 is ancient (doesn't even support ADSL2+). Hope that helps a bit. Best regards, Jonas
On Thu, Aug 25, 2022 at 10:23 AM Jonas Gorski <jonas.gorski@gmail.com> wrote: > > On Tue, 16 Aug 2022 at 20:39, Rob Herring <robh@kernel.org> wrote: > > > > The Broadcom pinctrl bindings are incomplete for child nodes as they are > > missing 'unevaluatedProperties: false' to prevent unknown properties. > > Fixing this reveals many warnings including having grandchild nodes in some > > cases. > > > > Signed-off-by: Rob Herring <robh@kernel.org> > > --- > > This recursive schema requires a fix not yet committed in dtschema. > > > > I'm looking for feedback on whether group->pins or group->groups is the > > right fix here. There's more warnings with this change in the gpio-sysctl > > bindings. > > The answer is "yes", though pins is probably the closest for most. > > bcm6318 has multiple field-per-pin registers, where each pin is > controlled separately, with more fields than available GPIOs, and the > pins outside the GPIO range controlling other functions, like > switching the second USB port between host and client mode. > > bcm6328/6362/6368/63268 have two registers. The first one enables an > alternative function for the first 32 GPIOs, with a 1:1 mapping of > bits to GPIO. The second one enables a function for whole, arbitrary > groups. These groups can overlap, and may also target pins outside the > first 32 GPIOs. The actual pins in use are not documented, and can > sometimes be guessed/inferred by the function names (e.g. "GPIO35"), > sometimes not ("NAND", "UTOPIA"). > > bcm6358 has only the groups register, which also includes non GPIO > related functions, like inversing the MII clocks for the integrated > macs. > > Not supported is bcm6348, which would be the only one where groups > would definitely make more sense: there are 5 groups of 8 GPIOs, where > each group can be set to a certain function (but not all functions are > valid for all groups). E.g. for PCI support, you would need to set the > fields to PCI for groups 0, 1 and 3 (and 2 and 5 could be set to a > different function). You can ignore this though, as bcm6348 is ancient > (doesn't even support ADSL2+). > > Hope that helps a bit. Only in determining that someone else needs to fix this. Please send me patches. Rob
diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml index ab019a1998e8..1792d07e497d 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6318-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + additionalProperties: false properties: function: @@ -37,6 +38,10 @@ patternProperties: enum: [ gpio0, gpio1, gpio2, gpio3, gpio4, gpio5, gpio6, gpio7, gpio8, gpio9, gpio10, gpio11, gpio12, gpio13, gpio40 ] + patternProperties: + '-pins$': + $ref: '#/patternProperties/-pins$' + allOf: - $ref: "pinctrl.yaml#" diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml index 8c9d4668c8c4..7aa1b6738ebf 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm63268-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + unevaluatedProperties: false properties: function: @@ -41,6 +42,10 @@ patternProperties: vdsl_phy_override_1_grp, vdsl_phy_override_2_grp, vdsl_phy_override_3_grp, dsl_gpio8, dsl_gpio9 ] + patternProperties: + '-pins$': + $ref: '#/patternProperties/-pins$' + allOf: - $ref: "pinctrl.yaml#" @@ -122,46 +127,46 @@ examples: pinctrl_nand: nand-pins { function = "nand"; - group = "nand_grp"; + pins = "nand_grp"; }; pinctrl_gpio35_alt: gpio35_alt-pins { function = "gpio35_alt"; - pin = "gpio35"; + pins = "gpio35"; }; pinctrl_dectpd: dectpd-pins { function = "dectpd"; - group = "dectpd_grp"; + pins = "dectpd_grp"; }; pinctrl_vdsl_phy_override_0: vdsl_phy_override_0-pins { function = "vdsl_phy_override_0"; - group = "vdsl_phy_override_0_grp"; + pins = "vdsl_phy_override_0_grp"; }; pinctrl_vdsl_phy_override_1: vdsl_phy_override_1-pins { function = "vdsl_phy_override_1"; - group = "vdsl_phy_override_1_grp"; + pins = "vdsl_phy_override_1_grp"; }; pinctrl_vdsl_phy_override_2: vdsl_phy_override_2-pins { function = "vdsl_phy_override_2"; - group = "vdsl_phy_override_2_grp"; + pins = "vdsl_phy_override_2_grp"; }; pinctrl_vdsl_phy_override_3: vdsl_phy_override_3-pins { function = "vdsl_phy_override_3"; - group = "vdsl_phy_override_3_grp"; + pins = "vdsl_phy_override_3_grp"; }; pinctrl_dsl_gpio8: dsl_gpio8-pins { function = "dsl_gpio8"; - group = "dsl_gpio8"; + pins = "dsl_gpio8"; }; pinctrl_dsl_gpio9: dsl_gpio9-pins { function = "dsl_gpio9"; - group = "dsl_gpio9"; + pins = "dsl_gpio9"; }; }; diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml index a8e22ec02215..e42071abf282 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6328-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + unevaluatedProperties: false properties: function: @@ -36,6 +37,10 @@ patternProperties: gpio20, gpio25, gpio26, gpio27, gpio28, hsspi_cs1, usb_port1 ] + patternProperties: + '-pins$': + $ref: '#/patternProperties/-pins$' + allOf: - $ref: "pinctrl.yaml#" diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml index 35867355a47a..20dbb1a3c19b 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6358-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + unevaluatedProperties: false properties: function: diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml index b584d4b27223..ac0e775383b9 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6362-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + unevaluatedProperties: false properties: function: @@ -41,6 +42,10 @@ patternProperties: gpio15, gpio16, gpio17, gpio18, gpio19, gpio20, gpio21, gpio22, gpio23, gpio24, gpio25, gpio26, gpio27, nand_grp ] + patternProperties: + '-pins$': + $ref: '#/patternProperties/-pins$' + allOf: - $ref: "pinctrl.yaml#" @@ -204,6 +209,6 @@ examples: pinctrl_nand: nand-pins { function = "nand"; - group = "nand_grp"; + pins = "nand_grp"; }; }; diff --git a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml index 229323d9237d..91f0ec45c871 100644 --- a/Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml +++ b/Documentation/devicetree/bindings/pinctrl/brcm,bcm6368-pinctrl.yaml @@ -24,6 +24,7 @@ patternProperties: '-pins$': type: object $ref: pinmux-node.yaml# + unevaluatedProperties: false properties: function: @@ -42,6 +43,10 @@ patternProperties: gpio24, gpio25, gpio26, gpio27, gpio28, gpio29, gpio30, gpio31, uart1_grp ] + patternProperties: + '-pins$': + $ref: '#/patternProperties/-pins$' + allOf: - $ref: "pinctrl.yaml#" @@ -215,6 +220,6 @@ examples: pinctrl_uart1: uart1-pins { function = "uart1"; - group = "uart1_grp"; + pins = "uart1_grp"; }; };
The Broadcom pinctrl bindings are incomplete for child nodes as they are missing 'unevaluatedProperties: false' to prevent unknown properties. Fixing this reveals many warnings including having grandchild nodes in some cases. Signed-off-by: Rob Herring <robh@kernel.org> --- This recursive schema requires a fix not yet committed in dtschema. I'm looking for feedback on whether group->pins or group->groups is the right fix here. There's more warnings with this change in the gpio-sysctl bindings. --- .../pinctrl/brcm,bcm6318-pinctrl.yaml | 5 ++++ .../pinctrl/brcm,bcm63268-pinctrl.yaml | 23 +++++++++++-------- .../pinctrl/brcm,bcm6328-pinctrl.yaml | 5 ++++ .../pinctrl/brcm,bcm6358-pinctrl.yaml | 1 + .../pinctrl/brcm,bcm6362-pinctrl.yaml | 7 +++++- .../pinctrl/brcm,bcm6368-pinctrl.yaml | 7 +++++- 6 files changed, 37 insertions(+), 11 deletions(-)