On Thu, Nov 03, 2022 at 10:14:04AM -0400, Krzysztof Kozlowski wrote:
> On 03/11/2022 08:01, Thierry Reding wrote:
> > From: Thierry Reding <treding@xxxxxxxxxx>
> >
> > Convert the Tegra PWFM bindings from the free-form text format to
> > json-schema.
> >
> > Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> > ---
> > .../bindings/pwm/nvidia,tegra20-pwm.txt | 77 ----------
> > .../bindings/pwm/nvidia,tegra20-pwm.yaml | 144 ++++++++++++++++++
> > 2 files changed, 144 insertions(+), 77 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > create mode 100644 Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > deleted file mode 100644
> > index 74c41e34c3b6..000000000000
> > --- a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.txt
> > +++ /dev/null
> > @@ -1,77 +0,0 @@
> > -Tegra SoC PWFM controller
> > -
> > -Required properties:
> > -- compatible: Must be:
> > - - "nvidia,tegra20-pwm": for Tegra20
> > - - "nvidia,tegra30-pwm", "nvidia,tegra20-pwm": for Tegra30
> > - - "nvidia,tegra114-pwm", "nvidia,tegra20-pwm": for Tegra114
> > - - "nvidia,tegra124-pwm", "nvidia,tegra20-pwm": for Tegra124
> > - - "nvidia,tegra132-pwm", "nvidia,tegra20-pwm": for Tegra132
> > - - "nvidia,tegra210-pwm", "nvidia,tegra20-pwm": for Tegra210
> > - - "nvidia,tegra186-pwm": for Tegra186
> > - - "nvidia,tegra194-pwm": for Tegra194
> > -- reg: physical base address and length of the controller's registers
> > -- #pwm-cells: should be 2. See pwm.yaml in this directory for a description of
> > - the cells format.
> > -- clocks: Must contain one entry, for the module clock.
> > - See ../clocks/clock-bindings.txt for details.
> > -- resets: Must contain an entry for each entry in reset-names.
> > - See ../reset/reset.txt for details.
> > -- reset-names: Must include the following entries:
> > - - pwm
> > -
> > -Optional properties:
> > -============================
> > -In some of the interface like PWM based regulator device, it is required
> > -to configure the pins differently in different states, especially in suspend
> > -state of the system. The configuration of pin is provided via the pinctrl
> > -DT node as detailed in the pinctrl DT binding document
> > - Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> > -
> > -The PWM node will have following optional properties.
> > -pinctrl-names: Pin state names. Must be "default" and "sleep".
> > -pinctrl-0: phandle for the default/active state of pin configurations.
> > -pinctrl-1: phandle for the sleep state of pin configurations.
> > -
> > -Example:
> > -
> > - pwm: pwm@7000a000 {
> > - compatible = "nvidia,tegra20-pwm";
> > - reg = <0x7000a000 0x100>;
> > - #pwm-cells = <2>;
> > - clocks = <&tegra_car 17>;
> > - resets = <&tegra_car 17>;
> > - reset-names = "pwm";
> > - };
> > -
> > -
> > -Example with the pin configuration for suspend and resume:
> > -=========================================================
> > -Suppose pin PE7 (On Tegra210) interfaced with the regulator device and
> > -it requires PWM output to be tristated when system enters suspend.
> > -Following will be DT binding to achieve this:
> > -
> > -#include <dt-bindings/pinctrl/pinctrl-tegra.h>
> > -
> > - pinmux@700008d4 {
> > - pwm_active_state: pwm_active_state {
> > - pe7 {
> > - nvidia,pins = "pe7";
> > - nvidia,tristate = <TEGRA_PIN_DISABLE>;
> > - };
> > - };
> > -
> > - pwm_sleep_state: pwm_sleep_state {
> > - pe7 {
> > - nvidia,pins = "pe7";
> > - nvidia,tristate = <TEGRA_PIN_ENABLE>;
> > - };
> > - };
> > - };
> > -
> > - pwm@7000a000 {
> > - /* Mandatory PWM properties */
> > - pinctrl-names = "default", "sleep";
> > - pinctrl-0 = <&pwm_active_state>;
> > - pinctrl-1 = <&pwm_sleep_state>;
> > - };
> > diff --git a/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml
> > new file mode 100644
> > index 000000000000..9c73e78ff434
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pwm/nvidia,tegra20-pwm.yaml
> > @@ -0,0 +1,144 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pwm/nvidia,tegra20-pwm.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra PWFM controller
> > +
> > +maintainers:
> > + - Thierry Reding <thierry.reding@xxxxxxxxx>
> > + - Jon Hunter <jonathanh@xxxxxxxxxx>
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - const: nvidia,tegra20-pwm
> > +
> > + - items:
> > + - enum:
> > + - nvidia,tegra30-pwm
> > + - nvidia,tegra114-pwm
> > + - nvidia,tegra124-pwm
> > + - nvidia,tegra132-pwm
> > + - nvidia,tegra210-pwm
> > + - enum:
> > + - nvidia,tegra20-pwm
> > +
> > + - items:
> > + - const: nvidia,tegra186-pwm
>
> I guess you wanted to keep some order between nvidia,tegra20-pwm and
> nvidia,tegra186-pwm, but this creates impression you will have here more
> items, which of course cannot happen. So either keep this one with
> tegra20 as one enum or drop "items".
Done.
>
> > +
> > + - items:
> > + - const: nvidia,tegra194-pwm
> > + - const: nvidia,tegra186-pwm
> > +
> > + - items:
> > + - const: nvidia,tegra234-pwm
> > + - const: nvidia,tegra194-pwm
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: module clock
>
> Just maxItems: 1, because description is not really helping.
Okay, seems fine.
> > +
> > + clock-names:
> > + items:
> > + - const: pwm
>
> This wasn't in original binding and does not look needed. Mention
> changes from pure conversion.
At some point (looks like with the switch to 64-bit ARM) we started
adding these for consistency because we were noticing that sometimes
either we were missing clock entries or newer SoC generations gained
additional clocks. Whenever that happened it would become somewhat
cumbersome to describe this in device tree bindings and/or driver
code, so consistently adding a clock-names property preventively
even if only a single clock was used in the first iteration seemed a
prudent thing to do.
So these are not technically necessary, but many device tree files will
have these entries, so this is here for those to pass validation.
Note that the property doesn't show up along the "clocks" property in
"required:" below.
>
> > +
> > + resets:
> > + items:
> > + - description: module reset
> > +
> > + reset-names:
> > + items:
> > + - const: pwm
> > +
> > + "#pwm-cells":
> > + const: 2
> > +
> > + pinctrl-names:
> > + items:
> > + - const: default
> > + - const: sleep
> > +
> > + pinctrl-0:
> > + description: configuration for the default/active state
> > +
> > + pinctrl-1:
> > + description: configuration for the sleep state
> > +
> > + operating-points-v2:
> > + $ref: "/schemas/types.yaml#/definitions/phandle"
>
> Drop quotes. We should actually define it in some common schema.
Yeah, good idea. I'll drop the quotes for now and see if I can find a
good place to add this in dt-schema.
> > +
> > + power-domains:
> > + items:
> > + - description: phandle to the core power domain
> > +
> > +allOf:
> > + - $ref: pwm.yaml
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - resets
> > + - reset-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/clock/tegra20-car.h>
> > +
> > + pwm: pwm@7000a000 {
> > + compatible = "nvidia,tegra20-pwm";
> > + reg = <0x7000a000 0x100>;
> > + #pwm-cells = <2>;
> > + clocks = <&tegra_car TEGRA20_CLK_PWM>;
> > + resets = <&tegra_car 17>;
> > + reset-names = "pwm";
> > + };
> > +
> > + # Example with the pin configuration for suspend and resume:
> > + # ==========================================================
> > + # Suppose pin PE7 (On Tegra210) interfaced with the regulator device and it requires PWM output
> > + # to be tristated when system enters suspend.
> > + - |
> > + #include <dt-bindings/clock/tegra210-car.h>
> > + #include <dt-bindings/pinctrl/pinctrl-tegra.h>
> > +
> > + pinmux@700008d4 {
> > + compatible = "nvidia,tegra210-pinmux";
> > + reg = <0x700008d4 0x29c>, /* Pad control registers */
> > + <0x70003000 0x294>; /* Mux registers */
> > +
> > + pwm_active_state: pwm_active_state {
>
> No underscores in node names.
I've dropped the entire example, but I may want to put it back once the
pinmux conversion is done, in which case I'll make sure to replace the
underscores.
Thanks,
Thierry
Attachment:
signature.asc
Description: PGP signature
