[v2,1/7] dt-bindings: pwm: allwinner: Add H6 PWM description
diff mbox series

Message ID 20191103203334.10539-2-peron.clem@gmail.com
State New
Headers show
Series
  • Add support for H6 PWM
Related show

Commit Message

Clément Péron Nov. 3, 2019, 8:33 p.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>
Signed-off-by: Clément Péron <peron.clem@gmail.com>
---
 .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

Comments

Uwe Kleine-König Nov. 4, 2019, 8:03 a.m. UTC | #1
On Sun, Nov 03, 2019 at 09:33:28PM +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>
> Signed-off-by: Clément Péron <peron.clem@gmail.com>
> ---
>  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> index 0ac52f83a58c..bf36ea509f31 100644
> --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> @@ -30,13 +30,46 @@ properties:
>        - items:
>            - const: allwinner,sun50i-h5-pwm
>            - const: allwinner,sun5i-a13-pwm
> +      - const: allwinner,sun50i-h6-pwm
>  
>    reg:
>      maxItems: 1
>  
> -  clocks:
> +  # 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.
> +  clocks: true
> +  clock-names: true
> +  resets:
>      maxItems: 1
>  
> +  if:
> +    properties:
> +      compatible:
> +        contains:
> +          const: allwinner,sun50i-h6-pwm
> +
> +  then:
> +    properties:
> +      clocks:
> +        items:
> +          - description: Module Clock
> +          - description: Bus Clock
> +
> +      clock-names:
> +        items:
> +          - const: mod
> +          - const: bus
> +
> +    required:
> +      - clock-names
> +      - resets
> +
> +  else:
> +    properties:
> +      clocks:
> +        maxItems: 1
> +

I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
bus clock is required.", right?

I wonder if it is sensible to require a clock-names property in the else
branch, too. This would make it obvious if the clock there corresponds
to the "mod" or the "bus" clock on H6. (I guess it's "mod".)

Best regards
Uwe
Clément Péron Nov. 4, 2019, 5:49 p.m. UTC | #2
On Mon, 4 Nov 2019 at 09:04, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:
>
> On Sun, Nov 03, 2019 at 09:33:28PM +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>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
> >  1 file changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > index 0ac52f83a58c..bf36ea509f31 100644
> > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > @@ -30,13 +30,46 @@ properties:
> >        - items:
> >            - const: allwinner,sun50i-h5-pwm
> >            - const: allwinner,sun5i-a13-pwm
> > +      - const: allwinner,sun50i-h6-pwm
> >
> >    reg:
> >      maxItems: 1
> >
> > -  clocks:
> > +  # 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.
> > +  clocks: true
> > +  clock-names: true
> > +  resets:
> >      maxItems: 1
> >
> > +  if:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          const: allwinner,sun50i-h6-pwm
> > +
> > +  then:
> > +    properties:
> > +      clocks:
> > +        items:
> > +          - description: Module Clock
> > +          - description: Bus Clock
> > +
> > +      clock-names:
> > +        items:
> > +          - const: mod
> > +          - const: bus
> > +
> > +    required:
> > +      - clock-names
> > +      - resets
> > +
> > +  else:
> > +    properties:
> > +      clocks:
> > +        maxItems: 1
> > +
>
> I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
> bus clock is required.", right?

Correct.
>
>
> I wonder if it is sensible to require a clock-names property in the else
> branch, too. This would make it obvious if the clock there corresponds
> to the "mod" or the "bus" clock on H6. (I guess it's "mod".)

This will also require to change example and all the current allwinner
device-tree that have a PWM declared.

Regards,
Clément

>
> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Maxime Ripard Nov. 5, 2019, 11:11 a.m. UTC | #3
Hi Clement, Uwe,

On Mon, Nov 04, 2019 at 09:03:59AM +0100, Uwe Kleine-König wrote:
> On Sun, Nov 03, 2019 at 09:33:28PM +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>
> > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > ---
> >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
> >  1 file changed, 44 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > index 0ac52f83a58c..bf36ea509f31 100644
> > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > @@ -30,13 +30,46 @@ properties:
> >        - items:
> >            - const: allwinner,sun50i-h5-pwm
> >            - const: allwinner,sun5i-a13-pwm
> > +      - const: allwinner,sun50i-h6-pwm
> >
> >    reg:
> >      maxItems: 1
> >
> > -  clocks:
> > +  # 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.
> > +  clocks: true
> > +  clock-names: true
> > +  resets:
> >      maxItems: 1
> >
> > +  if:
> > +    properties:
> > +      compatible:
> > +        contains:
> > +          const: allwinner,sun50i-h6-pwm
> > +
> > +  then:
> > +    properties:
> > +      clocks:
> > +        items:
> > +          - description: Module Clock
> > +          - description: Bus Clock
> > +
> > +      clock-names:
> > +        items:
> > +          - const: mod
> > +          - const: bus
> > +
> > +    required:
> > +      - clock-names
> > +      - resets
> > +
> > +  else:
> > +    properties:
> > +      clocks:
> > +        maxItems: 1
> > +
>
> I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
> bus clock is required.", right?
>
> I wonder if it is sensible to require a clock-names property in the else
> branch, too. This would make it obvious if the clock there corresponds
> to the "mod" or the "bus" clock on H6. (I guess it's "mod".)

This can be done a bit differently and could address your concerns

Something like

properties:
  ...

  clocks:
    minItems: 1
    maxItems: 2
    items:
      - description: Bus Clock
      - description: Module Clock

required:
  - clocks

if:
  ...

then:
  properties:
    clocks:
      maxItems: 2

    clocks-names:
      items:
        - const: mod
        - const: bus

    required:
      - clock-names

else:
  properties:
    clocks:
      maxItems: 1

That way, the definition of the order and which clock is which is
pretty obvious in both cases, and we don't get any weird warnings.

Maxime
Clément Péron Nov. 5, 2019, 12:34 p.m. UTC | #4
Hi Maxime,

On Tue, 5 Nov 2019 at 12:11, Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi Clement, Uwe,
>
> On Mon, Nov 04, 2019 at 09:03:59AM +0100, Uwe Kleine-König wrote:
> > On Sun, Nov 03, 2019 at 09:33:28PM +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>
> > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > ---
> > >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
> > >  1 file changed, 44 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > index 0ac52f83a58c..bf36ea509f31 100644
> > > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > @@ -30,13 +30,46 @@ properties:
> > >        - items:
> > >            - const: allwinner,sun50i-h5-pwm
> > >            - const: allwinner,sun5i-a13-pwm
> > > +      - const: allwinner,sun50i-h6-pwm
> > >
> > >    reg:
> > >      maxItems: 1
> > >
> > > -  clocks:
> > > +  # 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.
> > > +  clocks: true
> > > +  clock-names: true
> > > +  resets:
> > >      maxItems: 1
> > >
> > > +  if:
> > > +    properties:
> > > +      compatible:
> > > +        contains:
> > > +          const: allwinner,sun50i-h6-pwm
> > > +
> > > +  then:
> > > +    properties:
> > > +      clocks:
> > > +        items:
> > > +          - description: Module Clock
> > > +          - description: Bus Clock
> > > +
> > > +      clock-names:
> > > +        items:
> > > +          - const: mod
> > > +          - const: bus
> > > +
> > > +    required:
> > > +      - clock-names
> > > +      - resets
> > > +
> > > +  else:
> > > +    properties:
> > > +      clocks:
> > > +        maxItems: 1
> > > +
> >
> > I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
> > bus clock is required.", right?
> >
> > I wonder if it is sensible to require a clock-names property in the else
> > branch, too. This would make it obvious if the clock there corresponds
> > to the "mod" or the "bus" clock on H6. (I guess it's "mod".)
>
> This can be done a bit differently and could address your concerns
>
> Something like
>
> properties:
>   ...
>
>   clocks:
>     minItems: 1
>     maxItems: 2
>     items:
>       - description: Bus Clock
>       - description: Module Clock
>
> required:
>   - clocks
>
> if:
>   ...
>
> then:
>   properties:
>     clocks:
>       maxItems: 2

Here we should set minItems to 2 right ?
so Max = Min = 2

Regards,
Clément

>
>     clocks-names:
>       items:
>         - const: mod
>         - const: bus
>
>     required:
>       - clock-names
>
> else:
>   properties:
>     clocks:
>       maxItems: 1
>
> That way, the definition of the order and which clock is which is
> pretty obvious in both cases, and we don't get any weird warnings.
>
> Maxime
Maxime Ripard Nov. 5, 2019, 5:32 p.m. UTC | #5
On Tue, Nov 05, 2019 at 01:34:37PM +0100, Clément Péron wrote:
> On Tue, 5 Nov 2019 at 12:11, Maxime Ripard <mripard@kernel.org> wrote:
> >
> > Hi Clement, Uwe,
> >
> > On Mon, Nov 04, 2019 at 09:03:59AM +0100, Uwe Kleine-König wrote:
> > > On Sun, Nov 03, 2019 at 09:33:28PM +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>
> > > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > > ---
> > > >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
> > > >  1 file changed, 44 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > index 0ac52f83a58c..bf36ea509f31 100644
> > > > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > @@ -30,13 +30,46 @@ properties:
> > > >        - items:
> > > >            - const: allwinner,sun50i-h5-pwm
> > > >            - const: allwinner,sun5i-a13-pwm
> > > > +      - const: allwinner,sun50i-h6-pwm
> > > >
> > > >    reg:
> > > >      maxItems: 1
> > > >
> > > > -  clocks:
> > > > +  # 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.
> > > > +  clocks: true
> > > > +  clock-names: true
> > > > +  resets:
> > > >      maxItems: 1
> > > >
> > > > +  if:
> > > > +    properties:
> > > > +      compatible:
> > > > +        contains:
> > > > +          const: allwinner,sun50i-h6-pwm
> > > > +
> > > > +  then:
> > > > +    properties:
> > > > +      clocks:
> > > > +        items:
> > > > +          - description: Module Clock
> > > > +          - description: Bus Clock
> > > > +
> > > > +      clock-names:
> > > > +        items:
> > > > +          - const: mod
> > > > +          - const: bus
> > > > +
> > > > +    required:
> > > > +      - clock-names
> > > > +      - resets
> > > > +
> > > > +  else:
> > > > +    properties:
> > > > +      clocks:
> > > > +        maxItems: 1
> > > > +
> > >
> > > I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
> > > bus clock is required.", right?
> > >
> > > I wonder if it is sensible to require a clock-names property in the else
> > > branch, too. This would make it obvious if the clock there corresponds
> > > to the "mod" or the "bus" clock on H6. (I guess it's "mod".)
> >
> > This can be done a bit differently and could address your concerns
> >
> > Something like
> >
> > properties:
> >   ...
> >
> >   clocks:
> >     minItems: 1
> >     maxItems: 2
> >     items:
> >       - description: Bus Clock
> >       - description: Module Clock
> >
> > required:
> >   - clocks
> >
> > if:
> >   ...
> >
> > then:
> >   properties:
> >     clocks:
> >       maxItems: 2
>
> Here we should set minItems to 2 right ?
> so Max = Min = 2

It's done automatically by the tooling when the other is missing.

Maxime
Clément Péron Nov. 6, 2019, 9:25 a.m. UTC | #6
Hi,

On Tue, 5 Nov 2019 at 18:32, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Tue, Nov 05, 2019 at 01:34:37PM +0100, Clément Péron wrote:
> > On Tue, 5 Nov 2019 at 12:11, Maxime Ripard <mripard@kernel.org> wrote:
> > >
> > > Hi Clement, Uwe,
> > >
> > > On Mon, Nov 04, 2019 at 09:03:59AM +0100, Uwe Kleine-König wrote:
> > > > On Sun, Nov 03, 2019 at 09:33:28PM +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>
> > > > > Signed-off-by: Clément Péron <peron.clem@gmail.com>
> > > > > ---
> > > > >  .../bindings/pwm/allwinner,sun4i-a10-pwm.yaml | 45 ++++++++++++++++++-
> > > > >  1 file changed, 44 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > > index 0ac52f83a58c..bf36ea509f31 100644
> > > > > --- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > > +++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
> > > > > @@ -30,13 +30,46 @@ properties:
> > > > >        - items:
> > > > >            - const: allwinner,sun50i-h5-pwm
> > > > >            - const: allwinner,sun5i-a13-pwm
> > > > > +      - const: allwinner,sun50i-h6-pwm
> > > > >
> > > > >    reg:
> > > > >      maxItems: 1
> > > > >
> > > > > -  clocks:
> > > > > +  # 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.
> > > > > +  clocks: true
> > > > > +  clock-names: true
> > > > > +  resets:
> > > > >      maxItems: 1
> > > > >
> > > > > +  if:
> > > > > +    properties:
> > > > > +      compatible:
> > > > > +        contains:
> > > > > +          const: allwinner,sun50i-h6-pwm
> > > > > +
> > > > > +  then:
> > > > > +    properties:
> > > > > +      clocks:
> > > > > +        items:
> > > > > +          - description: Module Clock
> > > > > +          - description: Bus Clock
> > > > > +
> > > > > +      clock-names:
> > > > > +        items:
> > > > > +          - const: mod
> > > > > +          - const: bus
> > > > > +
> > > > > +    required:
> > > > > +      - clock-names
> > > > > +      - resets
> > > > > +
> > > > > +  else:
> > > > > +    properties:
> > > > > +      clocks:
> > > > > +        maxItems: 1
> > > > > +
> > > >
> > > > I guess this hunk says "If this is a allwinner,sun50i-h6-pwm, a mod and
> > > > bus clock is required.", right?
> > > >
> > > > I wonder if it is sensible to require a clock-names property in the else
> > > > branch, too. This would make it obvious if the clock there corresponds
> > > > to the "mod" or the "bus" clock on H6. (I guess it's "mod".)
> > >
> > > This can be done a bit differently and could address your concerns
> > >
> > > Something like
> > >
> > > properties:
> > >   ...
> > >
> > >   clocks:
> > >     minItems: 1
> > >     maxItems: 2
> > >     items:
> > >       - description: Bus Clock
> > >       - description: Module Clock
> > >
> > > required:
> > >   - clocks
> > >
> > > if:
> > >   ...
> > >
> > > then:
> > >   properties:
> > >     clocks:
> > >       maxItems: 2
> >
> > Here we should set minItems to 2 right ?
> > so Max = Min = 2
>
> It's done automatically by the tooling when the other is missing.

Ok thanks,
I will update in v4.

Regards,
Clément

>
> Maxime

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
index 0ac52f83a58c..bf36ea509f31 100644
--- a/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
+++ b/Documentation/devicetree/bindings/pwm/allwinner,sun4i-a10-pwm.yaml
@@ -30,13 +30,46 @@  properties:
       - items:
           - const: allwinner,sun50i-h5-pwm
           - const: allwinner,sun5i-a13-pwm
+      - const: allwinner,sun50i-h6-pwm
 
   reg:
     maxItems: 1
 
-  clocks:
+  # 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.
+  clocks: true
+  clock-names: true
+  resets:
     maxItems: 1
 
+  if:
+    properties:
+      compatible:
+        contains:
+          const: allwinner,sun50i-h6-pwm
+
+  then:
+    properties:
+      clocks:
+        items:
+          - description: Module Clock
+          - description: Bus Clock
+
+      clock-names:
+        items:
+          - const: mod
+          - const: bus
+
+    required:
+      - clock-names
+      - resets
+
+  else:
+    properties:
+      clocks:
+        maxItems: 1
+
 required:
   - "#pwm-cells"
   - compatible
@@ -54,4 +87,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>;
+    };
+
 ...