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@xxxxxxxxxxxxxx>
> > ---
> > .../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
> >
> >
