RE: [PATCH RFC 3/7] dt-bindings: mfd: rz-mtu3: Document RZ/G2L MTU3 PWM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux