| Message ID | 20220929103043.1228235-4-biju.das.jz@bp.renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add RZ/G2L MTU3 PWM driver | expand |
On Thu, 29 Sep 2022, Biju Das wrote: > Document RZ/G2L MTU3 PWM support. It supports following pwm modes. > 1) PWM mode 1 > 2) PWM mode 2 > 3) Reset-synchronized PWM mode > 4) Complementary PWM mode 1 (transfer at crest) > 5) Complementary PWM mode 2 (transfer at trough) > 6) Complementary PWM mode 3 (transfer at crest and trough) Shouldn't all this go in the PWM driver binding? > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > index c4bcf28623d6..362fedf5bedb 100644 > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > @@ -223,6 +223,50 @@ patternProperties: > - compatible > - reg > > + "^pwm@([0-4]|[6-7])+$": > + type: object > + > + properties: > + compatible: > + const: renesas,rz-mtu3-pwm > + > + reg: > + description: Identify pwm channels. > + items: > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > + > + "#pwm-cells": > + const: 2 > + > + renesas,pwm-mode1: > + type: boolean > + description: Enable PWM mode 1. > + > + renesas,pwm-mode2: > + type: boolean > + description: Enable PWM mode 2. > + > + renesas,reset-synchronized-pwm-mode: > + type: boolean > + description: Enable Reset-synchronized PWM mode. > + > + renesas,complementary-pwm-mode1: > + type: boolean > + description: Complementary PWM mode 1 (transfer at crest). > + > + renesas,complementary-pwm-mode2: > + type: boolean > + description: Complementary PWM mode 2 (transfer at trough). > + > + renesas,complementary-pwm-mode3: > + type: boolean > + description: Complementary PWM mode 3 (transfer at crest and trough). > + > + required: > + - compatible > + - reg > + - "#pwm-cells" > + > required: > - compatible > - reg > @@ -305,6 +349,12 @@ examples: > compatible = "renesas,rzg2l-mtu3-counter"; > reg = <1>; > }; > + pwm@3 { > + compatible = "renesas,rz-mtu3-pwm"; > + reg = <3>; > + #pwm-cells = <2>; > + renesas,pwm-mode1; > + }; > }; > > ...
Hi Lee Jones, Thanks for the feedback. > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > On Thu, 29 Sep 2022, Biju Das wrote: > > > Document RZ/G2L MTU3 PWM support. It supports following pwm modes. > > 1) PWM mode 1 > > 2) PWM mode 2 > > 3) Reset-synchronized PWM mode > > 4) Complementary PWM mode 1 (transfer at crest) > > 5) Complementary PWM mode 2 (transfer at trough) > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > Shouldn't all this go in the PWM driver binding? Looks like at top level MTU3 IP provides similar HW functionality like below binding [1], where there is a core MFD driver and pwm, counter and timer as child devices. [1] https://elixir.bootlin.com/linux/v6.0-rc7/source/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml Cheers, Biju > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 > +++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > mtu3.yaml > > index c4bcf28623d6..362fedf5bedb 100644 > > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > @@ -223,6 +223,50 @@ patternProperties: > > - compatible > > - reg > > > > + "^pwm@([0-4]|[6-7])+$": > > + type: object > > + > > + properties: > > + compatible: > > + const: renesas,rz-mtu3-pwm > > + > > + reg: > > + description: Identify pwm channels. > > + items: > > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > > + > > + "#pwm-cells": > > + const: 2 > > + > > + renesas,pwm-mode1: > > + type: boolean > > + description: Enable PWM mode 1. > > + > > + renesas,pwm-mode2: > > + type: boolean > > + description: Enable PWM mode 2. > > + > > + renesas,reset-synchronized-pwm-mode: > > + type: boolean > > + description: Enable Reset-synchronized PWM mode. > > + > > + renesas,complementary-pwm-mode1: > > + type: boolean > > + description: Complementary PWM mode 1 (transfer at crest). > > + > > + renesas,complementary-pwm-mode2: > > + type: boolean > > + description: Complementary PWM mode 2 (transfer at trough). > > + > > + renesas,complementary-pwm-mode3: > > + type: boolean > > + description: Complementary PWM mode 3 (transfer at crest > and trough). > > + > > + required: > > + - compatible > > + - reg > > + - "#pwm-cells" > > + > > required: > > - compatible > > - reg > > @@ -305,6 +349,12 @@ examples: > > compatible = "renesas,rzg2l-mtu3-counter"; > > reg = <1>; > > }; > > + pwm@3 { > > + compatible = "renesas,rz-mtu3-pwm"; > > + reg = <3>; > > + #pwm-cells = <2>; > > + renesas,pwm-mode1; > > + }; > > }; > > > > ... > > -- > Lee Jones [李琼斯]
On Thu, 29 Sep 2022, Biju Das wrote: > Hi Lee Jones, > > Thanks for the feedback. > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > RZ/G2L MTU3 PWM > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > Document RZ/G2L MTU3 PWM support. It supports following pwm modes. > > > 1) PWM mode 1 > > > 2) PWM mode 2 > > > 3) Reset-synchronized PWM mode > > > 4) Complementary PWM mode 1 (transfer at crest) > > > 5) Complementary PWM mode 2 (transfer at trough) > > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > > > Shouldn't all this go in the PWM driver binding? > > Looks like at top level MTU3 IP provides similar HW functionality like below > binding [1], where there is a core MFD driver and pwm, counter and timer > as child devices. Previous mistakes are not good references for what should happen in the present and the future. =;) > [1] https://elixir.bootlin.com/linux/v6.0-rc7/source/Documentation/devicetree/bindings/mfd/st,stm32-timers.yaml > > Cheers, > Biju > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 > > +++++++++++++++++++ > > > 1 file changed, 50 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > > mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > > mtu3.yaml > > > index c4bcf28623d6..362fedf5bedb 100644 > > > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > > @@ -223,6 +223,50 @@ patternProperties: > > > - compatible > > > - reg > > > > > > + "^pwm@([0-4]|[6-7])+$": > > > + type: object > > > + > > > + properties: > > > + compatible: > > > + const: renesas,rz-mtu3-pwm > > > + > > > + reg: > > > + description: Identify pwm channels. > > > + items: > > > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > > > + > > > + "#pwm-cells": > > > + const: 2 > > > + > > > + renesas,pwm-mode1: > > > + type: boolean > > > + description: Enable PWM mode 1. > > > + > > > + renesas,pwm-mode2: > > > + type: boolean > > > + description: Enable PWM mode 2. > > > + > > > + renesas,reset-synchronized-pwm-mode: > > > + type: boolean > > > + description: Enable Reset-synchronized PWM mode. > > > + > > > + renesas,complementary-pwm-mode1: > > > + type: boolean > > > + description: Complementary PWM mode 1 (transfer at crest). > > > + > > > + renesas,complementary-pwm-mode2: > > > + type: boolean > > > + description: Complementary PWM mode 2 (transfer at trough). > > > + > > > + renesas,complementary-pwm-mode3: > > > + type: boolean > > > + description: Complementary PWM mode 3 (transfer at crest > > and trough). > > > + > > > + required: > > > + - compatible > > > + - reg > > > + - "#pwm-cells" > > > + > > > required: > > > - compatible > > > - reg > > > @@ -305,6 +349,12 @@ examples: > > > compatible = "renesas,rzg2l-mtu3-counter"; > > > reg = <1>; > > > }; > > > + pwm@3 { > > > + compatible = "renesas,rz-mtu3-pwm"; > > > + reg = <3>; > > > + #pwm-cells = <2>; > > > + renesas,pwm-mode1; > > > + }; > > > }; > > > > > > ... > >
On Thu, Sep 29, 2022 at 11:30:39AM +0100, Biju Das wrote: > Document RZ/G2L MTU3 PWM support. It supports following pwm modes. > 1) PWM mode 1 > 2) PWM mode 2 > 3) Reset-synchronized PWM mode > 4) Complementary PWM mode 1 (transfer at crest) > 5) Complementary PWM mode 2 (transfer at trough) > 6) Complementary PWM mode 3 (transfer at crest and trough) What does 'complementary' mean here? Mode 1, 2, 3 isn't very meaningful. Do other PWMs have similar modes? No way to tell without better descriptions. > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 +++++++++++++++++++ > 1 file changed, 50 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > index c4bcf28623d6..362fedf5bedb 100644 > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > @@ -223,6 +223,50 @@ patternProperties: > - compatible > - reg > > + "^pwm@([0-4]|[6-7])+$": > + type: object > + > + properties: > + compatible: > + const: renesas,rz-mtu3-pwm > + > + reg: > + description: Identify pwm channels. > + items: > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] At any given level in DT, there is only 1 address space. You've created 2 with pwms and counters. > + > + "#pwm-cells": > + const: 2 > + > + renesas,pwm-mode1: > + type: boolean > + description: Enable PWM mode 1. > + > + renesas,pwm-mode2: > + type: boolean > + description: Enable PWM mode 2. > + > + renesas,reset-synchronized-pwm-mode: > + type: boolean > + description: Enable Reset-synchronized PWM mode. > + > + renesas,complementary-pwm-mode1: > + type: boolean > + description: Complementary PWM mode 1 (transfer at crest). > + > + renesas,complementary-pwm-mode2: > + type: boolean > + description: Complementary PWM mode 2 (transfer at trough). > + > + renesas,complementary-pwm-mode3: > + type: boolean > + description: Complementary PWM mode 3 (transfer at crest and trough). These all look like client configuration and should be either runtime config or part of pwm cells args. > + > + required: > + - compatible > + - reg > + - "#pwm-cells" > + > required: > - compatible > - reg > @@ -305,6 +349,12 @@ examples: > compatible = "renesas,rzg2l-mtu3-counter"; > reg = <1>; > }; > + pwm@3 { > + compatible = "renesas,rz-mtu3-pwm"; > + reg = <3>; > + #pwm-cells = <2>; > + renesas,pwm-mode1; > + }; > }; > > ... > -- > 2.25.1 > >
Hi Rob, Thanks for the feedback. > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > On Thu, Sep 29, 2022 at 11:30:39AM +0100, Biju Das wrote: > > Document RZ/G2L MTU3 PWM support. It supports following pwm modes. > > 1) PWM mode 1 > > 2) PWM mode 2 > > 3) Reset-synchronized PWM mode > > 4) Complementary PWM mode 1 (transfer at crest) > > 5) Complementary PWM mode 2 (transfer at trough) > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > What does 'complementary' mean here? Through interlocked operation of MTU3/4 and MTU6/7, the positive and negative signals in six phases (12 phases in total) can be output in complementary PWM > > Mode 1, 2, 3 isn't very meaningful. Do other PWMs have similar modes? In complementary PWM mode, buffer registers are used to update the data in five compare registers for PWM duty and PWM cycle. The update data can be written to the buffer registers at any time. There is a temporary register between each of these registers and its buffer register. The temporary register value is transferred to the compare register at the data update timing set with Mode bits(MTU3.TMDR1.MD[3:0] (MTU6.TMDR1.MD[3:0]). Mode 1, 2, 3 corresponding to these modes. > No way to tell without better descriptions. OK will update description. > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 > +++++++++++++++++++ > > 1 file changed, 50 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > index c4bcf28623d6..362fedf5bedb 100644 > > --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > @@ -223,6 +223,50 @@ patternProperties: > > - compatible > > - reg > > > > + "^pwm@([0-4]|[6-7])+$": > > + type: object > > + > > + properties: > > + compatible: > > + const: renesas,rz-mtu3-pwm > > + > > + reg: > > + description: Identify pwm channels. > > + items: > > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > > At any given level in DT, there is only 1 address space. You've > created > 2 with pwms and counters. pwm and counters are mutually exclusive on the same channel which is taken care at higher level(eg: board level, we enable either pwm or counter, not both). Is it wrong to specify same channel for counter or pwm with same address? If needed, I could add logical addresses for these channels and on implementation side, will map these logical channels with actual hardware channels. Please let me know. > > > + > > + "#pwm-cells": > > + const: 2 > > + > > + renesas,pwm-mode1: > > + type: boolean > > + description: Enable PWM mode 1. > > + > > + renesas,pwm-mode2: > > + type: boolean > > + description: Enable PWM mode 2. > > + > > + renesas,reset-synchronized-pwm-mode: > > + type: boolean > > + description: Enable Reset-synchronized PWM mode. > > + > > + renesas,complementary-pwm-mode1: > > + type: boolean > > + description: Complementary PWM mode 1 (transfer at crest). > > + > > + renesas,complementary-pwm-mode2: > > + type: boolean > > + description: Complementary PWM mode 2 (transfer at trough). > > + > > + renesas,complementary-pwm-mode3: > > + type: boolean > > + description: Complementary PWM mode 3 (transfer at crest > and trough). > > These all look like client configuration and should be either runtime > config or part of pwm cells args. HW supports 6 modes, as mentioned above. How do we switch between modes?? Could it be a sysfs option?? If sysfs, do we need to document here? Cheers, Biju > > > + > > + required: > > + - compatible > > + - reg > > + - "#pwm-cells" > > + > > required: > > - compatible > > - reg > > @@ -305,6 +349,12 @@ examples: > > compatible = "renesas,rzg2l-mtu3-counter"; > > reg = <1>; > > }; > > + pwm@3 { > > + compatible = "renesas,rz-mtu3-pwm"; > > + reg = <3>; > > + #pwm-cells = <2>; > > + renesas,pwm-mode1; > > + }; > > }; > > > > ... > > -- > > 2.25.1 > > > >
Hi Lee Jones, > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > On Thu, 29 Sep 2022, Biju Das wrote: > > > Hi Lee Jones, > > > > Thanks for the feedback. > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > RZ/G2L MTU3 PWM > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > Document RZ/G2L MTU3 PWM support. It supports following pwm > modes. > > > > 1) PWM mode 1 > > > > 2) PWM mode 2 > > > > 3) Reset-synchronized PWM mode > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > > > > > Shouldn't all this go in the PWM driver binding? > > > > Looks like at top level MTU3 IP provides similar HW functionality > like > > below binding [1], where there is a core MFD driver and pwm, counter > > and timer as child devices. > > Previous mistakes are not good references for what should happen in > the present and the future. =;) Why do you think that reference is not a good one? I believe there should be some reason for it. Cheers, Biju > > > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > > --- > > > > .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 > > > +++++++++++++++++++ > > > > 1 file changed, 50 insertions(+) > > > > > > > > diff --git > a/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > > > mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > > > mtu3.yaml > > > > index c4bcf28623d6..362fedf5bedb 100644 > > > > --- > > > > a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml > > > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l- > mtu3.yam > > > > +++ l > > > > @@ -223,6 +223,50 @@ patternProperties: > > > > - compatible > > > > - reg > > > > > > > > + "^pwm@([0-4]|[6-7])+$": > > > > + type: object > > > > + > > > > + properties: > > > > + compatible: > > > > + const: renesas,rz-mtu3-pwm > > > > + > > > > + reg: > > > > + description: Identify pwm channels. > > > > + items: > > > > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > > > > + > > > > + "#pwm-cells": > > > > + const: 2 > > > > + > > > > + renesas,pwm-mode1: > > > > + type: boolean > > > > + description: Enable PWM mode 1. > > > > + > > > > + renesas,pwm-mode2: > > > > + type: boolean > > > > + description: Enable PWM mode 2. > > > > + > > > > + renesas,reset-synchronized-pwm-mode: > > > > + type: boolean > > > > + description: Enable Reset-synchronized PWM mode. > > > > + > > > > + renesas,complementary-pwm-mode1: > > > > + type: boolean > > > > + description: Complementary PWM mode 1 (transfer at > crest). > > > > + > > > > + renesas,complementary-pwm-mode2: > > > > + type: boolean > > > > + description: Complementary PWM mode 2 (transfer at > trough). > > > > + > > > > + renesas,complementary-pwm-mode3: > > > > + type: boolean > > > > + description: Complementary PWM mode 3 (transfer at > crest > > > and trough). > > > > + > > > > + required: > > > > + - compatible > > > > + - reg > > > > + - "#pwm-cells" > > > > + > > > > required: > > > > - compatible > > > > - reg > > > > @@ -305,6 +349,12 @@ examples: > > > > compatible = "renesas,rzg2l-mtu3-counter"; > > > > reg = <1>; > > > > }; > > > > + pwm@3 { > > > > + compatible = "renesas,rz-mtu3-pwm"; > > > > + reg = <3>; > > > > + #pwm-cells = <2>; > > > > + renesas,pwm-mode1; > > > > + }; > > > > }; > > > > > > > > ... > > > > > -- > Lee Jones [李琼斯]
On Sat, 01 Oct 2022, Biju Das wrote: > Hi Lee Jones, > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > RZ/G2L MTU3 PWM > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > Hi Lee Jones, > > > > > > Thanks for the feedback. > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > > RZ/G2L MTU3 PWM > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > Document RZ/G2L MTU3 PWM support. It supports following pwm > > modes. > > > > > 1) PWM mode 1 > > > > > 2) PWM mode 2 > > > > > 3) Reset-synchronized PWM mode > > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > > > > > > > Shouldn't all this go in the PWM driver binding? > > > > > > Looks like at top level MTU3 IP provides similar HW functionality > > like > > > below binding [1], where there is a core MFD driver and pwm, counter > > > and timer as child devices. > > > > Previous mistakes are not good references for what should happen in > > the present and the future. =;) > > Why do you think that reference is not a good one? I believe there > should be some reason for it. I didn't even look at it. What I "believe" is that documentation for each functionality belonging to a particular subsystem should live in subsystem's associated documentation directory and be reviewed/maintained by that subsystem's associated maintainer.
Hi Lee, > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > On Sat, 01 Oct 2022, Biju Das wrote: > > > Hi Lee Jones, > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > RZ/G2L MTU3 PWM > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > Hi Lee Jones, > > > > > > > > Thanks for the feedback. > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > Document > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > Document RZ/G2L MTU3 PWM support. It supports following pwm > > > modes. > > > > > > 1) PWM mode 1 > > > > > > 2) PWM mode 2 > > > > > > 3) Reset-synchronized PWM mode > > > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > > > > > > > > > Shouldn't all this go in the PWM driver binding? > > > > > > > > Looks like at top level MTU3 IP provides similar HW > functionality > > > like > > > > below binding [1], where there is a core MFD driver and pwm, > > > > counter and timer as child devices. > > > > > > Previous mistakes are not good references for what should happen > in > > > the present and the future. =;) > > > > Why do you think that reference is not a good one? I believe there > > should be some reason for it. > > I didn't even look at it. > > What I "believe" is that documentation for each functionality > belonging to a particular subsystem should live in subsystem's > associated documentation directory and be reviewed/maintained by that > subsystem's associated maintainer. If I am correct, MFD is subsystem for calling shared resources across subsystems. Here shared resources are channels which is shared by timer, counter and pwm They are child objects of MFD subsystems. That is the reason it is in MFDndings. Cheers, Biju
On Mon, 03 Oct 2022, Biju Das wrote: > Hi Lee, > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > RZ/G2L MTU3 PWM > > > > On Sat, 01 Oct 2022, Biju Das wrote: > > > > > Hi Lee Jones, > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > > RZ/G2L MTU3 PWM > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > Hi Lee Jones, > > > > > > > > > > Thanks for the feedback. > > > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > > Document > > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > > > Document RZ/G2L MTU3 PWM support. It supports following pwm > > > > modes. > > > > > > > 1) PWM mode 1 > > > > > > > 2) PWM mode 2 > > > > > > > 3) Reset-synchronized PWM mode > > > > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > > > > 6) Complementary PWM mode 3 (transfer at crest and trough) > > > > > > > > > > > > Shouldn't all this go in the PWM driver binding? > > > > > > > > > > Looks like at top level MTU3 IP provides similar HW > > functionality > > > > like > > > > > below binding [1], where there is a core MFD driver and pwm, > > > > > counter and timer as child devices. > > > > > > > > Previous mistakes are not good references for what should happen > > in > > > > the present and the future. =;) > > > > > > Why do you think that reference is not a good one? I believe there > > > should be some reason for it. > > > > I didn't even look at it. > > > > What I "believe" is that documentation for each functionality > > belonging to a particular subsystem should live in subsystem's > > associated documentation directory and be reviewed/maintained by that > > subsystem's associated maintainer. > > If I am correct, MFD is subsystem for calling shared resources > across subsystems. > > Here shared resources are channels which is shared by timer, counter and pwm Which API do the consumers use to obtain these shared resources? > They are child objects of MFD subsystems. That is the reason it is in MFDndings. If the properties belong to the child, they should be documented in the child's bindings. Shoving all functionality and by extension all documentation into the MFD driver and/or binding is incorrect behaviour. Looking at it from another perspective, I cannot/should not review PWM, Reset, Counter or Timer bindings, since I do not have the level of subject area knowledge as the assigned maintainers do. Please place all sub-system specific bindings in their correct (leaf) bindings and link to them from this one (run this): git grep \$ref -- Documentation/devicetree/bindings/mfd/
Hi Lee, > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > On Mon, 03 Oct 2022, Biju Das wrote: > > > Hi Lee, > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > RZ/G2L MTU3 PWM > > > > > > On Sat, 01 Oct 2022, Biju Das wrote: > > > > > > > Hi Lee Jones, > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > Document > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > Hi Lee Jones, > > > > > > > > > > > > Thanks for the feedback. > > > > > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > > > Document > > > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > > > > > Document RZ/G2L MTU3 PWM support. It supports following > > > > > > > > pwm > > > > > modes. > > > > > > > > 1) PWM mode 1 > > > > > > > > 2) PWM mode 2 > > > > > > > > 3) Reset-synchronized PWM mode > > > > > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > > > > > 6) Complementary PWM mode 3 (transfer at crest and > > > > > > > > trough) > > > > > > > > > > > > > > Shouldn't all this go in the PWM driver binding? > > > > > > > > > > > > Looks like at top level MTU3 IP provides similar HW > > > functionality > > > > > like > > > > > > below binding [1], where there is a core MFD driver and pwm, > > > > > > counter and timer as child devices. > > > > > > > > > > Previous mistakes are not good references for what should > happen > > > in > > > > > the present and the future. =;) > > > > > > > > Why do you think that reference is not a good one? I believe > there > > > > should be some reason for it. > > > > > > I didn't even look at it. > > > > > > What I "believe" is that documentation for each functionality > > > belonging to a particular subsystem should live in subsystem's > > > associated documentation directory and be reviewed/maintained by > > > that subsystem's associated maintainer. > > > > If I am correct, MFD is subsystem for calling shared resources > across > > subsystems. > > > > Here shared resources are channels which is shared by timer, counter > > and pwm > > Which API do the consumers use to obtain these shared resources? They need to use MFD driver API to get shared resources. > > > They are child objects of MFD subsystems. That is the reason it is > in MFDndings. > > If the properties belong to the child, they should be documented in > the child's bindings. Shoving all functionality and by extension all > documentation into the MFD driver and/or binding is incorrect > behaviour. Do you have an example, how will it look like, if the below binding to be part of pwm and linked against the parent MFD driver? + "^pwm@([0-4]|[6-7])+$": + type: object + + properties: + compatible: + const: renesas,rz-mtu3-pwm + + reg: + description: Identify pwm channels. + items: + enum: [ 0, 1, 2, 3, 4, 6, 7 ] + + "#pwm-cells": + const: 2 + + renesas,pwm-mode1: + type: boolean + description: Enable PWM mode 1. + + renesas,pwm-mode2: + type: boolean + description: Enable PWM mode 2. + + renesas,reset-synchronized-pwm-mode: + type: boolean + description: Enable Reset-synchronized PWM mode. + + renesas,complementary-pwm-mode1: + type: boolean + description: Complementary PWM mode 1 (transfer at crest). + + renesas,complementary-pwm-mode2: + type: boolean + description: Complementary PWM mode 2 (transfer at trough). + + renesas,complementary-pwm-mode3: + type: boolean + description: Complementary PWM mode 3 (transfer at crest and trough). + + required: + - compatible + - reg + - "#pwm-cells" + examples: + pwm@3 { + compatible = "renesas,rz-mtu3-pwm"; + reg = <3>; + #pwm-cells = <2>; + renesas,pwm-mode1; + }; }; Cheers, Biju > > Looking at it from another perspective, I cannot/should not review > PWM, Reset, Counter or Timer bindings, since I do not have the level > of subject area knowledge as the assigned maintainers do. > > Please place all sub-system specific bindings in their correct (leaf) > bindings and link to them from this one (run this): > > git grep \$ref -- Documentation/devicetree/bindings/mfd/ > > -- > Lee Jones [李琼斯]
Hi Lee Jpnes, Thanks for the feedback. > Subject: RE: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > RZ/G2L MTU3 PWM > > Hi Lee, > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > RZ/G2L MTU3 PWM > > > > On Mon, 03 Oct 2022, Biju Das wrote: > > > > > Hi Lee, > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document > > > > RZ/G2L MTU3 PWM > > > > > > > > On Sat, 01 Oct 2022, Biju Das wrote: > > > > > > > > > Hi Lee Jones, > > > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > > Document > > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > > > Hi Lee Jones, > > > > > > > > > > > > > > Thanks for the feedback. > > > > > > > > > > > > > > > Subject: Re: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: > > > > Document > > > > > > > > RZ/G2L MTU3 PWM > > > > > > > > > > > > > > > > On Thu, 29 Sep 2022, Biju Das wrote: > > > > > > > > > > > > > > > > > Document RZ/G2L MTU3 PWM support. It supports > following > > > > > > > > > pwm > > > > > > modes. > > > > > > > > > 1) PWM mode 1 > > > > > > > > > 2) PWM mode 2 > > > > > > > > > 3) Reset-synchronized PWM mode > > > > > > > > > 4) Complementary PWM mode 1 (transfer at crest) > > > > > > > > > 5) Complementary PWM mode 2 (transfer at trough) > > > > > > > > > 6) Complementary PWM mode 3 (transfer at crest and > > > > > > > > > trough) > > > > > > > > > > > > > > > > Shouldn't all this go in the PWM driver binding? > > > > > > > > > > > > > > Looks like at top level MTU3 IP provides similar HW > > > > functionality > > > > > > like > > > > > > > below binding [1], where there is a core MFD driver and > pwm, > > > > > > > counter and timer as child devices. > > > > > > > > > > > > Previous mistakes are not good references for what should > > happen > > > > in > > > > > > the present and the future. =;) > > > > > > > > > > Why do you think that reference is not a good one? I believe > > there > > > > > should be some reason for it. > > > > > > > > I didn't even look at it. > > > > > > > > What I "believe" is that documentation for each functionality > > > > belonging to a particular subsystem should live in subsystem's > > > > associated documentation directory and be reviewed/maintained by > > > > that subsystem's associated maintainer. > > > > > > If I am correct, MFD is subsystem for calling shared resources > > across > > > subsystems. > > > > > > Here shared resources are channels which is shared by timer, > counter > > > and pwm > > > > Which API do the consumers use to obtain these shared resources? > > They need to use MFD driver API to get shared resources. > > > > > > They are child objects of MFD subsystems. That is the reason it is > > in MFDndings. > > > > If the properties belong to the child, they should be documented in > > the child's bindings. Shoving all functionality and by extension > all > > documentation into the MFD driver and/or binding is incorrect > > behaviour. > > Do you have an example, how will it look like, if the below binding to > be part of pwm and linked against the parent MFD driver? > > > + "^pwm@([0-4]|[6-7])+$": > + type: object > + > + properties: > + compatible: > + const: renesas,rz-mtu3-pwm > + > + reg: > + description: Identify pwm channels. > + items: > + enum: [ 0, 1, 2, 3, 4, 6, 7 ] > + > + "#pwm-cells": > + const: 2 > + > + renesas,pwm-mode1: > + type: boolean > + description: Enable PWM mode 1. > + > + renesas,pwm-mode2: > + type: boolean > + description: Enable PWM mode 2. > + > + renesas,reset-synchronized-pwm-mode: > + type: boolean > + description: Enable Reset-synchronized PWM mode. > + > + renesas,complementary-pwm-mode1: > + type: boolean > + description: Complementary PWM mode 1 (transfer at crest). > + > + renesas,complementary-pwm-mode2: > + type: boolean > + description: Complementary PWM mode 2 (transfer at trough). > + > + renesas,complementary-pwm-mode3: > + type: boolean > + description: Complementary PWM mode 3 (transfer at crest and > trough). > + > + required: > + - compatible > + - reg > + - "#pwm-cells" > + > > examples: > + pwm@3 { > + compatible = "renesas,rz-mtu3-pwm"; > + reg = <3>; > + #pwm-cells = <2>; > + renesas,pwm-mode1; > + }; > }; > > > Cheers, > Biju > > > > > Looking at it from another perspective, I cannot/should not review > > PWM, Reset, Counter or Timer bindings, since I do not have the level > > of subject area knowledge as the assigned maintainers do. > > > > Please place all sub-system specific bindings in their correct > (leaf) > > bindings and link to them from this one (run this): > > > > git grep \$ref -- Documentation/devicetree/bindings/mfd/ Thanks for the pointer, I got references [1] and [2]. I can model like this, If everyone ok with it. [1] Documentation/devicetree/bindings/mfd/kontron,sl28cpld.yaml [2] Documentation/devicetree/bindings/pwm/kontron,sl28cpld-pwm.yaml Cheers, Biju
diff --git a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml index c4bcf28623d6..362fedf5bedb 100644 --- a/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml +++ b/Documentation/devicetree/bindings/mfd/renesas,rzg2l-mtu3.yaml @@ -223,6 +223,50 @@ patternProperties: - compatible - reg + "^pwm@([0-4]|[6-7])+$": + type: object + + properties: + compatible: + const: renesas,rz-mtu3-pwm + + reg: + description: Identify pwm channels. + items: + enum: [ 0, 1, 2, 3, 4, 6, 7 ] + + "#pwm-cells": + const: 2 + + renesas,pwm-mode1: + type: boolean + description: Enable PWM mode 1. + + renesas,pwm-mode2: + type: boolean + description: Enable PWM mode 2. + + renesas,reset-synchronized-pwm-mode: + type: boolean + description: Enable Reset-synchronized PWM mode. + + renesas,complementary-pwm-mode1: + type: boolean + description: Complementary PWM mode 1 (transfer at crest). + + renesas,complementary-pwm-mode2: + type: boolean + description: Complementary PWM mode 2 (transfer at trough). + + renesas,complementary-pwm-mode3: + type: boolean + description: Complementary PWM mode 3 (transfer at crest and trough). + + required: + - compatible + - reg + - "#pwm-cells" + required: - compatible - reg @@ -305,6 +349,12 @@ examples: compatible = "renesas,rzg2l-mtu3-counter"; reg = <1>; }; + pwm@3 { + compatible = "renesas,rz-mtu3-pwm"; + reg = <3>; + #pwm-cells = <2>; + renesas,pwm-mode1; + }; }; ...
Document RZ/G2L MTU3 PWM support. It supports following pwm modes. 1) PWM mode 1 2) PWM mode 2 3) Reset-synchronized PWM mode 4) Complementary PWM mode 1 (transfer at crest) 5) Complementary PWM mode 2 (transfer at trough) 6) Complementary PWM mode 3 (transfer at crest and trough) Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- .../bindings/mfd/renesas,rzg2l-mtu3.yaml | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+)