On Wed, Jul 06, 2022 at 10:28:34PM +0200, Thierry Reding wrote:
> On Wed, Jul 06, 2022 at 09:38:22AM -0600, Rob Herring wrote:
> > On Mon, Jul 04, 2022 at 06:57:59PM +0200, Thierry Reding wrote:
> > > From: Thierry Reding <treding@xxxxxxxxxx>
> > >
> > > Convert the NVIDIA Tegra pinmux controller bindings from the free-form
> > > text format to json-schema.
> > >
> > > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > > ---
> > > Changes in v2:
> > > - wrap lines at 80 characters instead of the standard 100 characters in Linux
> > > - use GPL-2.0-only instead of GPL-2.0+ license for DT bindings
> > > - reorder additionalProperties for better readability
> > > - move common definitions into a shared schema
> > > - remove consumer snippets from examples
> > >
> > > .../bindings/clock/nvidia,tegra124-dfll.yaml | 2 +-
> > > .../pinctrl/nvidia,tegra-pinmux-common.yaml | 195 ++++++++++++++++++
> > > .../pinctrl/nvidia,tegra114-pinmux.txt | 131 ------------
> > > .../pinctrl/nvidia,tegra114-pinmux.yaml | 162 +++++++++++++++
> > > .../pinctrl/nvidia,tegra124-pinmux.txt | 153 --------------
> > > .../pinctrl/nvidia,tegra124-pinmux.yaml | 182 ++++++++++++++++
> > > .../pinctrl/nvidia,tegra194-pinmux.txt | 107 ----------
> > > .../pinctrl/nvidia,tegra194-pinmux.yaml | 88 ++++++++
> > > .../pinctrl/nvidia,tegra20-pinmux.txt | 143 -------------
> > > .../pinctrl/nvidia,tegra20-pinmux.yaml | 107 ++++++++++
> > > .../pinctrl/nvidia,tegra210-pinmux.txt | 166 ---------------
> > > .../pinctrl/nvidia,tegra210-pinmux.yaml | 146 +++++++++++++
> > > .../pinctrl/nvidia,tegra30-pinmux.txt | 144 -------------
> > > .../pinctrl/nvidia,tegra30-pinmux.yaml | 176 ++++++++++++++++
> > > 14 files changed, 1057 insertions(+), 845 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra-pinmux-common.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra114-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra114-pinmux.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra124-pinmux.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra194-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra194-pinmux.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra20-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra20-pinmux.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.yaml
> > > delete mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra30-pinmux.txt
> > > create mode 100644 Documentation/devicetree/bindings/pinctrl/nvidia,tegra30-pinmux.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.yaml b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.yaml
> > > index 85234a48b590..96c54c215f77 100644
> > > --- a/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra124-dfll.yaml
> > > @@ -219,7 +219,7 @@ examples:
> > >
> > > /*
> > > * pinmux nodes added for completeness. Binding doc can be found in:
> > > - * Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.txt
> > > + * Documentation/devicetree/bindings/pinctrl/nvidia,tegra210-pinmux.yaml
> > > */
> > >
> > > pinmux: pinmux@700008d4 {
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-pinmux-common.yaml b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-pinmux-common.yaml
> > > new file mode 100644
> > > index 000000000000..cb6b722b89af
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/nvidia,tegra-pinmux-common.yaml
> > > @@ -0,0 +1,195 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pinctrl/nvidia,tegra-pinmux-common.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: NVIDIA Tegra Pinmux Controller
> > > +
> > > +maintainers:
> > > + - Thierry Reding <thierry.reding@xxxxxxxxx>
> > > + - Jonathan Hunter <jonathanh@xxxxxxxxxx>
> > > +
> > > +definitions:
> >
> > This likely doesn't work because 'definitions' doesn't get fixups
> > applied. '$defs' would, but in general I prefer avoiding this pattern
> > except when really needed. I don't think it helps here. More below.
> >
> > > + pinconf-node:
> > > + type: object
> > > + description: |
> > > + Please refer to pinctrl-bindings.txt in this directory for details of the
> > > + common pinctrl bindings used by client devices, including the meaning of
> > > + the phrase "pin configuration node".
> > > +
> > > + Tegra's pin configuration nodes act as a container for an arbitrary number
> > > + of subnodes. Each of these subnodes represents some desired configuration
> > > + for a pin, a group, or a list of pins or groups. This configuration can
> > > + include the mux function to select on those pin(s)/ group(s), and various
> > > + pin configuration parameters, such as pull-up, tristate, drive strength,
> > > + etc.
> > > +
> > > + The name of each subnode is not important; all subnodes should be
> > > + enumerated and processed purely based on their content.
> > > +
> > > + Each subnode only affects those parameters that are explicitly listed. In
> > > + other words, a subnode that lists a mux function but no pin configuration
> > > + parameters implies no information about any pin configuration parameters.
> > > +
> > > + Similarly, a pin subnode that describes a pullup parameter implies no
> > > + information about e.g. the mux function or tristate parameter. For this
> > > + reason, even seemingly boolean values are actually tristates in this
> > > + binding: unspecified, off, or on. Unspecified is represented as an absent
> > > + property, and off/on are represented as integer values 0 and 1.
> > > +
> > > + Note that many of these properties are only valid for certain specific pins
> > > + or groups. See the Tegra TRM and various pinmux spreadsheets for complete
> > > + details regarding which groups support which functionality. The Linux
> > > + pinctrl driver may also be a useful reference, since it consolidates,
> > > + disambiguates, and corrects data from all those sources.
> > > +
> > > + patternProperties:
> > > + "^pinmux(-[a-z0-9-_]+)?$":
> >
> > Drop this and make all the below be at the top level (i.e. no indent).
>
> The intention was to include the general structure as well as the
> description in each of the derived schemas. Some of the description
> could perhaps be dropped, but especially the bit about the tristate
> nature of some of these properties should stay in because it has caused
> confusion in the past.
>
> Is there any other way that the description can be shared with your
> proposal (other than duplicating it in each binding document).
The 'description' above? You can move that to top-level too.
> > > + type: object
> > > + properties:
> > > + nvidia,pins:
> > > + $ref: /schemas/types.yaml#/definitions/string-array
> > > + description: An array of strings. Each string contains the name of
> > > + a pin or group. Valid values for these names are listed below.
> > > +
> > > + nvidia,function:
> > > + $ref: /schemas/types.yaml#/definitions/string
> > > + description: A string containing the name of the function to mux to
> > > + the pin or group. Valid values for function names are listed
> > > + below. See the Tegra TRM to determine which are valid for each
> > > + pin or group.
> > > +
> > > + nvidia,pull:
> > > + description: Pull-down/up setting to apply to the pin.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: none
> > > + const: 0
> > > + - description: down
> > > + const: 1
> > > + - description: up
> > > + const: 2
> > > +
> > > + nvidia,tristate:
> > > + description: Tristate setting to apply to the pin.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: drive
> > > + const: 0
> > > + - description: tristate
> > > + const: 1
> > > +
> > > + nvidia,schmitt:
> > > + description: Enable Schmitt trigger on the input.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: disable Schmitt trigger on the input
> > > + const: 0
> > > + - description: enable Schmitt trigger on the input
> > > + const: 1
> > > +
> > > + nvidia,pull-down-strength:
> > > + description: Controls drive strength. 0 is weakest. The range of
> > > + valid values depends on the pingroup. See "CAL_DRVDN" in the
> > > + Tegra TRM.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > + nvidia,pull-up-strength:
> > > + description: Controls drive strength. 0 is weakest. The range of
> > > + valid values depends on the pingroup. See "CAL_DRVUP" in the
> > > + Tegra TRM.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > + high-speed-mode:
> >
> > Why do all these need to be definitions? They all have the same name and
> > apply to the pinmux nodes. Just move them there with the right name.
>
> All of the ones that are separate definitions are properties that appear
> for only a subset of the IP generations. So the idea was to create a
> definition and then derived bindings could cherry-pick these as
> necessary to complement the pinconf-node definition.
>
> If I move these definitions to the top level, then they'll end up
> validating properly on devices where they shouldn't exist. Alternatively
> I'd have to duplicate all of these across various derivative bindings
> which gets us mostly back to the previous version of the patch (i.e. we
> don't save very much by only factoring out the truly common properties).
Not if you use 'additionalProperties: false'. Since it doesn't see into
$ref's like unevaluatedProperties does, you have to list out which
properties you are using in the derived binding:
$ref: list-of-common-props.yaml#
properties:
high-speed-mode: true
additionalProperties: false
list-of-common-props.yaml could have a gazillion properties in
it, but anything not listed locally will be disallowed.
> > > + description: Enable high speed mode the pins.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: normal speed mode
> > > + const: 0
> > > + - description: high speed mode
> > > + const: 1
> > > +
> > > + low-power-mode:
> > > + description: Controls the drive power or current. Valid values are
> > > + from 0 through 3, where 0 specifies the least power and 3 specifies
> > > + the most power. See "Low Power Mode" or "LPMD1" and "LPMD0" in the
> > > + Tegra TRM.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [ 0, 1, 2, 3 ]
> > > +
> > > + enable-input:
> > > + description: Enable the pin's input path.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: disable input (i.e. output only)
> > > + const: 0
> > > + - description: enable input
> > > + const: 1
> > > +
> > > + open-drain:
> > > + description: Open-drain configuration for the pin.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: disable open-drain
> > > + const: 0
> > > + - description: enable open-drain
> > > + const: 1
> > > +
> > > + lock:
> > > + description: Lock the pin configuration against further changes until
> > > + reset.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: disable pin configuration lock
> > > + const: 0
> > > + - description: enable pin configuration lock
> > > + const: 1
> > > +
> > > + io-reset:
> > > + description: reset the I/O path
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [ 0, 1 ]
> > > +
> > > + rcv-sel:
> > > + description: select VIL/VIH receivers
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: normal receivers
> > > + const: 0
> > > + - description: high-voltage receivers
> > > + const: 1
> > > +
> > > + drive-type:
> > > + description: Drive type to configure for the pin.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [ 0, 1, 2, 3 ]
> > > +
> > > + io-hv:
> > > + description: Select high-voltage receivers.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + oneOf:
> > > + - description: Use normal receivers.
> > > + const: 0
> > > + - description: Use high-voltage receivers.
> > > + const: 1
> > > +
> > > + slew-rate-rising:
> > > + description: Controls rising signal slew rate. 0 is fastest. The
> > > + range of valid values depends on the pingroup. See "DRVDN_SLWR" in
> > > + the Tegra TRM.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > + slew-rate-falling:
> > > + description: Controls falling signal slew rate. 0 is fastest. The
> > > + range of valid values depends on the pingroup. See "DRVUP_SLWF" in
> > > + the Tegra TRM.
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +properties:
> > > + reg:
> > > + minItems: 1
> > > + maxItems: 4
> >
> > This doesn't get used anywhere.
>
> I think I needed the properties keyword here because the tooling would
> otherwise consider this schema invalid. Perhaps if I restructure this
> that error will go away as well.
Probably so. All part of keeping people within the lines. :)
Rob
