diff mbox series

[v6,1/8] dt-bindings: pwm: allwinner: Add H6 PWM description

Message ID 20191118110034.19444-2-peron.clem@gmail.com
State Superseded
Headers show
Series Add support for H6 PWM | expand

Commit Message

Clément Péron Nov. 18, 2019, 11 a.m. UTC
From: Jernej Skrabec <jernej.skrabec@siol.net>

H6 PWM block is basically the same as A20 PWM, except that it also has
bus clock and reset line which needs to be handled accordingly.

Expand Allwinner PWM binding with H6 PWM specifics.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Maxime Ripard Nov. 18, 2019, 11:06 a.m. UTC | #1
Hi,

On Mon, Nov 18, 2019 at 12:00:27PM +0100, Clément Péron wrote:
> From: Jernej Skrabec <jernej.skrabec@siol.net>
>
> H6 PWM block is basically the same as A20 PWM, except that it also has
> bus clock and reset line which needs to be handled accordingly.
>
> Expand Allwinner PWM binding with H6 PWM specifics.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 48 +++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> index 0ac52f83a58c..1bae446febbb 100644
> --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> @@ -30,13 +30,51 @@ properties:
>        - items:
>            - const: allwinner,sun50i-h5-pwm
>            - const: allwinner,sun5i-a13-pwm
> +      - const: allwinner,sun50i-h6-pwm
>
>    reg:
>      maxItems: 1
>
>    clocks:
> +    minItems: 1
> +    maxItems: 2
> +    items:
> +      - description: Module Clock
> +      - description: Bus Clock
> +
> +  # Even though it only applies to subschemas under the conditionals,
> +  # not listing them here will trigger a warning because of the
> +  # additionalsProperties set to false.
> +  clock-names: true
> +
> +  resets:
>      maxItems: 1
>
> +  if:
> +    properties:
> +      compatible:
> +        contains:
> +          const: allwinner,sun50i-h6-pwm
> +
> +  then:
> +    properties:
> +      clocks:
> +        maxItems: 2
> +
> +      clock-names:
> +        items:
> +          - const: mod
> +          - const: bus
> +
> +    required:
> +      - clock-names
> +      - resets
> +
> +  else:
> +    properties:
> +      clocks:
> +        maxItems: 1
> +

Sorry for not noticing this earlier, but this should be at the topmost
level

Maxime
Clément Péron Nov. 18, 2019, 12:42 p.m. UTC | #2
Hi Maxime

On Mon, 18 Nov 2019 at 12:06, Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi,
>
> On Mon, Nov 18, 2019 at 12:00:27PM +0100, Clément Péron wrote:
> > From: Jernej Skrabec <jernej.skrabec@siol.net>
> >
> > H6 PWM block is basically the same as A20 PWM, except that it also has
> > bus clock and reset line which needs to be handled accordingly.
> >
> > Expand Allwinner PWM binding with H6 PWM specifics.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 48 +++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > index 0ac52f83a58c..1bae446febbb 100644
> > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > @@ -30,13 +30,51 @@ properties:
> >        - items:
> >            - const: allwinner,sun50i-h5-pwm
> >            - const: allwinner,sun5i-a13-pwm
> > +      - const: allwinner,sun50i-h6-pwm
> >
> >    reg:
> >      maxItems: 1
> >
> >    clocks:
> > +    minItems: 1
> > +    maxItems: 2
> > +    items:
> > +      - description: Module Clock
> > +      - description: Bus Clock
> > +
> > +  # Even though it only applies to subschemas under the conditionals,
> > +  # not listing them here will trigger a warning because of the
> > +  # additionalsProperties set to false.
> > +  clock-names: true
> > +
> > +  resets:
> >      maxItems: 1
> >
> > +  if:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          const: allwinner,sun50i-h6-pwm
> > +
> > +  then:
> > +    properties:
> > +      clocks:
> > +        maxItems: 2
> > +
> > +      clock-names:
> > +        items:
> > +          - const: mod
> > +          - const: bus
> > +
> > +    required:
> > +      - clock-names
> > +      - resets
> > +
> > +  else:
> > +    properties:
> > +      clocks:
> > +        maxItems: 1
> > +
>
> Sorry for not noticing this earlier, but this should be at the topmost
> level

No problem, but I don't get what you want, (yaml format is new for me).
Do you mean I should put the if condition before the "resets" ?

Regards,
Clément

>
> Maxime
Maxime Ripard Nov. 18, 2019, 12:57 p.m. UTC | #3
On Mon, Nov 18, 2019 at 01:42:48PM +0100, Clément Péron wrote:
> Hi Maxime
>
> On Mon, 18 Nov 2019 at 12:06, Maxime Ripard <mripard@kernel.org> wrote:
> >
> > Hi,
> >
> > On Mon, Nov 18, 2019 at 12:00:27PM +0100, Clément Péron wrote:
> > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > >
> > > H6 PWM block is basically the same as A20 PWM, except that it also has
> > > bus clock and reset line which needs to be handled accordingly.
> > >
> > > Expand Allwinner PWM binding with H6 PWM specifics.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > ---
> > >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 48 +++++++++++++++++++
> > >  1 file changed, 48 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > index 0ac52f83a58c..1bae446febbb 100644
> > > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > @@ -30,13 +30,51 @@ properties:
> > >        - items:
> > >            - const: allwinner,sun50i-h5-pwm
> > >            - const: allwinner,sun5i-a13-pwm
> > > +      - const: allwinner,sun50i-h6-pwm
> > >
> > >    reg:
> > >      maxItems: 1
> > >
> > >    clocks:
> > > +    minItems: 1
> > > +    maxItems: 2
> > > +    items:
> > > +      - description: Module Clock
> > > +      - description: Bus Clock
> > > +
> > > +  # Even though it only applies to subschemas under the conditionals,
> > > +  # not listing them here will trigger a warning because of the
> > > +  # additionalsProperties set to false.
> > > +  clock-names: true
> > > +
> > > +  resets:
> > >      maxItems: 1
> > >
> > > +  if:
> > > +    properties:
> > > +      compatible:
> > > +        contains:
> > > +          const: allwinner,sun50i-h6-pwm
> > > +
> > > +  then:
> > > +    properties:
> > > +      clocks:
> > > +        maxItems: 2
> > > +
> > > +      clock-names:
> > > +        items:
> > > +          - const: mod
> > > +          - const: bus
> > > +
> > > +    required:
> > > +      - clock-names
> > > +      - resets
> > > +
> > > +  else:
> > > +    properties:
> > > +      clocks:
> > > +        maxItems: 1
> > > +
> >
> > Sorry for not noticing this earlier, but this should be at the topmost
> > level
>
> No problem, but I don't get what you want, (yaml format is new for me).
> Do you mean I should put the if condition before the "resets" ?

No, here if we condense a bit the file, we have something like:

title: PWM

properties:
  compatible:
    ...

  ...

  resets:
    ...

  if:
    properties:
      ...

  then:
    properties:
      ...

which means that you expect that the node may contain a compatible
property, a resets one, and then two properties "if" and "then", which
in turn contain properties (ie, two nodes).

This is obviously not what you want, what you want instead is:

properties:
  compatible:
    ...

  ...

  resets:
    ...

if:
  properties:
    ...

then:
  properties:
    ...

Which then describes that there's two properties, compatible and
resets, and if the schema under 'if' is valid against the node we try
to validate, the schema under 'then' is used to validate the node as
well.

I hope it's clearer,
Maxime
Clément Péron Nov. 18, 2019, 1:23 p.m. UTC | #4
Hi,

On Mon, 18 Nov 2019 at 13:57, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Mon, Nov 18, 2019 at 01:42:48PM +0100, Clément Péron wrote:
> > Hi Maxime
> >
> > On Mon, 18 Nov 2019 at 12:06, Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > Hi,
> > >
> > > On Mon, Nov 18, 2019 at 12:00:27PM +0100, Clément Péron wrote:
> > > > From: Jernej Skrabec <jernej.skrabec@siol.net>
> > > >
> > > > H6 PWM block is basically the same as A20 PWM, except that it also has
> > > > bus clock and reset line which needs to be handled accordingly.
> > > >
> > > > Expand Allwinner PWM binding with H6 PWM specifics.
> > > >
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > > ---
> > > >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 48 +++++++++++++++++++
> > > >  1 file changed, 48 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > index 0ac52f83a58c..1bae446febbb 100644
> > > > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > @@ -30,13 +30,51 @@ properties:
> > > >        - items:
> > > >            - const: allwinner,sun50i-h5-pwm
> > > >            - const: allwinner,sun5i-a13-pwm
> > > > +      - const: allwinner,sun50i-h6-pwm
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > >
> > > >    clocks:
> > > > +    minItems: 1
> > > > +    maxItems: 2
> > > > +    items:
> > > > +      - description: Module Clock
> > > > +      - description: Bus Clock
> > > > +
> > > > +  # Even though it only applies to subschemas under the conditionals,
> > > > +  # not listing them here will trigger a warning because of the
> > > > +  # additionalsProperties set to false.
> > > > +  clock-names: true
> > > > +
> > > > +  resets:
> > > >      maxItems: 1
> > > >
> > > > +  if:
> > > > +    properties:
> > > > +      compatible:
> > > > +        contains:
> > > > +          const: allwinner,sun50i-h6-pwm
> > > > +
> > > > +  then:
> > > > +    properties:
> > > > +      clocks:
> > > > +        maxItems: 2
> > > > +
> > > > +      clock-names:
> > > > +        items:
> > > > +          - const: mod
> > > > +          - const: bus
> > > > +
> > > > +    required:
> > > > +      - clock-names
> > > > +      - resets
> > > > +
> > > > +  else:
> > > > +    properties:
> > > > +      clocks:
> > > > +        maxItems: 1
> > > > +
> > >
> > > Sorry for not noticing this earlier, but this should be at the topmost
> > > level
> >
> > No problem, but I don't get what you want, (yaml format is new for me).
> > Do you mean I should put the if condition before the "resets" ?
>
> No, here if we condense a bit the file, we have something like:
>
> title: PWM
>
> properties:
>   compatible:
>     ...
>
>   ...
>
>   resets:
>     ...
>
>   if:
>     properties:
>       ...
>
>   then:
>     properties:
>       ...
>
> which means that you expect that the node may contain a compatible
> property, a resets one, and then two properties "if" and "then", which
> in turn contain properties (ie, two nodes).
>
> This is obviously not what you want, what you want instead is:
>
> properties:
>   compatible:
>     ...
>
>   ...
>
>   resets:
>     ...
>
> if:
>   properties:
>     ...
>
> then:
>   properties:
>     ...
>
> Which then describes that there's two properties, compatible and
> resets, and if the schema under 'if' is valid against the node we try
> to validate, the schema under 'then' is used to validate the node as
> well.
>
> I hope it's clearer,

Yes it's totally clear didn't see this bad indentation,
Thanks for the catch and for the explanation.

Regards,
Clément

> Maxime
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
index 0ac52f83a58c..1bae446febbb 100644
--- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
@@ -30,13 +30,51 @@  properties:
       - items:
           - const: allwinner,sun50i-h5-pwm
           - const: allwinner,sun5i-a13-pwm
+      - const: allwinner,sun50i-h6-pwm
 
   reg:
     maxItems: 1
 
   clocks:
+    minItems: 1
+    maxItems: 2
+    items:
+      - description: Module Clock
+      - description: Bus Clock
+
+  # Even though it only applies to subschemas under the conditionals,
+  # not listing them here will trigger a warning because of the
+  # additionalsProperties set to false.
+  clock-names: true
+
+  resets:
     maxItems: 1
 
+  if:
+    properties:
+      compatible:
+        contains:
+          const: allwinner,sun50i-h6-pwm
+
+  then:
+    properties:
+      clocks:
+        maxItems: 2
+
+      clock-names:
+        items:
+          - const: mod
+          - const: bus
+
+    required:
+      - clock-names
+      - resets
+
+  else:
+    properties:
+      clocks:
+        maxItems: 1
+
 required:
   - "#pwm-cells"
   - compatible
@@ -54,4 +92,14 @@  examples:
         #pwm-cells = <3>;
     };
 
+  - |
+    pwm@300a000 {
+      compatible = "allwinner,sun50i-h6-pwm";
+      reg = <0x0300a000 0x400>;
+      clocks = <&osc24M>, <&ccu CLK_BUS_PWM>;
+      clock-names = "mod", "bus";
+      resets = <&ccu RST_BUS_PWM>;
+      #pwm-cells = <3>;
+    };
+
 ...