Re: [PATCH 5/7] dt-bindings: arm: tegra: pmc: Restructure pad configuration node schema

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 9483 bytes --]

On Fri, Jul 07, 2023 at 04:35:03PM -0600, Rob Herring wrote:
> On Fri, Jul 07, 2023 at 03:17:09PM +0200, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The pad configuration node schema in its current form can accidentally
> > match other properties as well. Restructure the schema to better match
> > how the device trees are using these.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  .../arm/tegra/nvidia,tegra20-pmc.yaml         | 181 ++++++++++++------
> >  1 file changed, 120 insertions(+), 61 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.yaml b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.yaml
> > index 82070d47ac7c..271aa8f80a65 100644
> > --- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.yaml
> > +++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.yaml
> > @@ -245,69 +245,82 @@ properties:
> >            - resets
> >            - '#power-domain-cells'
> >  
> > -patternProperties:
> > -  "^[a-f0-9]+-[a-f0-9]+$":
> > +  pinmux:
> >      type: object
> > -    description:
> > -      This is a Pad configuration node. On Tegra SOCs a pad is a set of
> > -      pins which are configured as a group. The pin grouping is a fixed
> > -      attribute of the hardware. The PMC can be used to set pad power state
> > -      and signaling voltage. A pad can be either in active or power down mode.
> > -      The support for power state and signaling voltage configuration varies
> > -      depending on the pad in question. 3.3V and 1.8V signaling voltages
> > -      are supported on pins where software controllable signaling voltage
> > -      switching is available.
> > -
> > -      The pad configuration state nodes are placed under the pmc node and they
> > -      are referred to by the pinctrl client properties. For more information
> > -      see Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt.
> > -      The pad name should be used as the value of the pins property in pin
> > -      configuration nodes.
> > -
> > -      The following pads are present on Tegra124 and Tegra132
> > -      audio, bb, cam, comp, csia, csb, cse, dsi, dsib, dsic, dsid, hdmi, hsic,
> > -      hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2, pex-cntrl,
> > -      sdmmc1, sdmmc3, sdmmc4, sys_ddc, uart, usb0, usb1, usb2, usb_bias.
> > -
> > -      The following pads are present on Tegra210
> > -      audio, audio-hv, cam, csia, csib, csic, csid, csie, csif, dbg,
> > -      debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2, gpio, hdmi,
> > -      hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2, pex-cntrl, sdmmc1,
> > -      sdmmc3, spi, spi-hv, uart, usb0, usb1, usb2, usb3, usb-bias.
> > -
> >      properties:
> > -      pins:
> > -        $ref: /schemas/types.yaml#/definitions/string
> > -        description: Must contain name of the pad(s) to be configured.
> > -
> > -      low-power-enable:
> > -        $ref: /schemas/types.yaml#/definitions/flag
> > -        description: Configure the pad into power down mode.
> > -
> > -      low-power-disable:
> > -        $ref: /schemas/types.yaml#/definitions/flag
> > -        description: Configure the pad into active mode.
> > -
> > -      power-source:
> > -        $ref: /schemas/types.yaml#/definitions/uint32
> > -        description:
> > -          Must contain either TEGRA_IO_PAD_VOLTAGE_1V8 or
> > -          TEGRA_IO_PAD_VOLTAGE_3V3 to select between signaling voltages.
> > -          The values are defined in
> > -          include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h.
> > -          Power state can be configured on all Tegra124 and Tegra132
> > -          pads. None of the Tegra124 or Tegra132 pads support signaling
> > -          voltage switching.
> > -          All of the listed Tegra210 pads except pex-cntrl support power
> > -          state configuration. Signaling voltage switching is supported
> > -          on below Tegra210 pads.
> > -          audio, audio-hv, cam, dbg, dmic, gpio, pex-cntrl, sdmmc1,
> > -          sdmmc3, spi, spi-hv, and uart.
> > -
> > -    required:
> > -      - pins
> > -
> > -    additionalProperties: false
> > +      status: true
> 
> If you need this, that's a bug in dtschema.

Looks like I don't need it here...

> 
> > +
> > +    additionalProperties:
> > +      type: object
> > +      description: |
> > +        This is a pad configuration node. On Tegra SoCs a pad is a set of pins
> > +        which are configured as a group. The pin grouping is a fixed attribute
> > +        of the hardware. The PMC can be used to set pad power state and
> > +        signaling voltage. A pad can be either in active or power down mode.
> > +        The support for power state and signaling voltage configuration varies
> > +        depending on the pad in question. 3.3V and 1.8V signaling voltages are
> > +        supported on pins where software controllable signaling voltage
> > +        switching is available.
> > +
> > +        The pad configuration state nodes are placed under the pmc node and
> > +        they are referred to by the pinctrl client properties. For more
> > +        information see:
> > +
> > +          Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > +
> > +        The pad name should be used as the value of the pins property in pin
> > +        configuration nodes.
> > +
> > +        The following pads are present on Tegra124 and Tegra132:
> > +
> > +          audio, bb, cam, comp, csia, csb, cse, dsi, dsib, dsic, dsid, hdmi,
> > +          hsic, hv, lvds, mipi-bias, nand, pex-bias, pex-clk1, pex-clk2,
> > +          pex-cntrl, sdmmc1, sdmmc3, sdmmc4, sys_ddc, uart, usb0, usb1, usb2,
> > +          usb_bias
> > +
> > +        The following pads are present on Tegra210:
> > +
> > +          audio, audio-hv, cam, csia, csib, csic, csid, csie, csif, dbg,
> > +          debug-nonao, dmic, dp, dsi, dsib, dsic, dsid, emmc, emmc2, gpio,
> > +          hdmi, hsic, lvds, mipi-bias, pex-bias, pex-clk1, pex-clk2, pex-cntrl,
> > +          sdmmc1, sdmmc3, spi, spi-hv, uart, usb0, usb1, usb2, usb3, usb-bias
> > +      additionalProperties: false
> > +      properties:
> > +        pins:
> > +          $ref: /schemas/types.yaml#/definitions/string-array
> > +          description: Must contain name of the pad(s) to be configured.
> > +
> > +        low-power-enable:
> > +          $ref: /schemas/types.yaml#/definitions/flag
> > +          description: Configure the pad into power down mode.
> > +
> > +        low-power-disable:
> > +          $ref: /schemas/types.yaml#/definitions/flag
> > +          description: Configure the pad into active mode.
> > +
> > +        power-source:
> > +          $ref: /schemas/types.yaml#/definitions/uint32
> > +          description: |
> > +            Must contain either TEGRA_IO_PAD_VOLTAGE_1V8 or
> > +            TEGRA_IO_PAD_VOLTAGE_3V3 to select between signaling voltages. The
> > +            values are defined in:
> > +
> > +              include/dt-bindings/pinctrl/pinctrl-tegra-io-pad.h
> > +
> > +            Power state can be configured on all Tegra124 and Tegra132 pads.
> > +            None of the Tegra124 or Tegra132 pads support signaling voltage
> > +            switching. All of the listed Tegra210 pads except pex-cntrl support
> > +            power state configuration. Signaling voltage switching is supported
> > +            on the following Tegra210 pads:
> > +
> > +              audio, audio-hv, cam, dbg, dmic, gpio, pex-cntrl, sdmmc1, sdmmc3,
> > +              spi, spi-hv, uart
> > +
> > +        phandle:
> > +          $ref: /schemas/types.yaml#/definitions/uint32
> 
> ditto

... and not this either. Might be some leftovers from testing with
earlier versions of dtschema. I've dropped them now.

> 
> > +
> > +      required:
> > +        - pins
> >  
> >  required:
> >    - compatible
> > @@ -316,6 +329,52 @@ required:
> >    - clocks
> >    - '#clock-cells'
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: nvidia,tegra124-pmc
> > +    then:
> > +      properties:
> > +        pinmux:
> > +          properties:
> > +            status: true

However I do get complaints if I drop this one:

	Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml: allOf:0:then:properties:pinmux: 'anyOf' conditional failed, one must be fixed:
		'type' is a required property
		'properties' is a required property
		'patternProperties' is a required property
		hint: 'additionalProperties' depends on 'properties' or 'patternProperties'
		from schema $id: http://devicetree.org/meta-schemas/core.yaml#
	Documentation/devicetree/bindings/soc/tegra/nvidia,tegra20-pmc.yaml: allOf:1:then:properties:pinmux: 'anyOf' conditional failed, one must be fixed:
		'type' is a required property
		'properties' is a required property
		'patternProperties' is a required property
		hint: 'additionalProperties' depends on 'properties' or 'patternProperties'
		from schema $id: http://devicetree.org/meta-schemas/core.yaml#

Would you consider that a bug in dtschema as well that I should try to
fix or is that expected?

Thanks,
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]