| Message ID | 20221010145222.1047748-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add RZ/G2L MTU3a MFD, Counter and pwm driver | expand |
On 10/10/2022 10:52, Biju Das wrote: > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer > channels and one 32-bit timer channel. It supports the following > functions > - Counter > - Timer > - PWM > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v3->v4: > * Dropped counter and pwm compatibeles as they don't have any resources. > * Made rz-mtu3 as pwm provider. > * Updated the example and description. > v2->v3: > * Dropped counter bindings and integrated with mfd as it has only one property. > * Removed "#address-cells" and "#size-cells" as it do not have children with > unit addresses. > * Removed quotes from counter and pwm. > * Provided full path for pwm bindings. > * Updated the example. > v1->v2: > * Modelled counter and pwm as a single device that handles > multiple channels. > * Moved counter and pwm bindings to respective subsystems > * Dropped 'bindings' from MFD binding title. > * Updated the example > * Changed the compatible names. > --- > .../bindings/mfd/renesas,rz-mtu3.yaml | 305 ++++++++++++++++++ > 1 file changed, 305 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml This should not be in MFD. Just because some device has few features, does not mean it should go to MFD... Choose either timer or pwm. > > diff --git a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > new file mode 100644 > index 000000000000..1b0be9f5cd18 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > @@ -0,0 +1,305 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/renesas,rz-mtu3.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a) > + > +maintainers: > + - Biju Das <biju.das.jz@bp.renesas.com> > + > +description: | > + This hardware block pconsisting of eight 16-bit timer channels and one "This hardware block consists of..." > + 32- bit timer channel. It supports the following specifications: > + - Pulse input/output: 28 lines max. > + - Pulse input 3 lines > + - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks > + for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination > + (when LWA = 1)) > + - Operating frequency Up to 100 MHz Best regards, Krzysztof
Hi Krzysztof Kozlowski, > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > bindings > > On 10/10/2022 10:52, Biju Das wrote: > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in > > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer > > channels and one 32-bit timer channel. It supports the following > > functions > > - Counter > > - Timer > > - PWM > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v3->v4: > > * Dropped counter and pwm compatibeles as they don't have any > resources. > > * Made rz-mtu3 as pwm provider. > > * Updated the example and description. > > v2->v3: > > * Dropped counter bindings and integrated with mfd as it has only > one property. > > * Removed "#address-cells" and "#size-cells" as it do not have > children with > > unit addresses. > > * Removed quotes from counter and pwm. > > * Provided full path for pwm bindings. > > * Updated the example. > > v1->v2: > > * Modelled counter and pwm as a single device that handles > > multiple channels. > > * Moved counter and pwm bindings to respective subsystems > > * Dropped 'bindings' from MFD binding title. > > * Updated the example > > * Changed the compatible names. > > --- > > .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > ++++++++++++++++++ > > 1 file changed, 305 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > This should not be in MFD. Just because some device has few features, > does not mean it should go to MFD... Choose either timer or pwm. MFD is for multifunction device. This IP supports multiple functions like timer, pwm, clock source/events. That is the reason I have added here. MFD is core which provides register access for client devices. For me moving it to pwm or counter is not a big problem. Why do you think it cannot be MFD? Cheers, Biju > > > > > > diff --git > > a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > new file mode 100644 > > index 000000000000..1b0be9f5cd18 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > @@ -0,0 +1,305 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > > > +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a) > > + > > +maintainers: > > + - Biju Das <biju.das.jz@bp.renesas.com> > > + > > +description: | > > + This hardware block pconsisting of eight 16-bit timer channels > and > > +one > > "This hardware block consists of..." OK, will fix this in next version. Cheers, Biju
On 11/10/2022 10:55, Biju Das wrote: > >>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 >> ++++++++++++++++++ >>> 1 file changed, 305 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml >> >> This should not be in MFD. Just because some device has few features, >> does not mean it should go to MFD... Choose either timer or pwm. > > MFD is for multifunction device. This IP supports multiple functions > like timer, pwm, clock source/events. That is the reason I have added > here. MFD is core which provides register access for client devices. > > For me moving it to pwm or counter is not a big problem. > Why do you think it cannot be MFD? Because it makes MFD a dump for everything where author did not want to think about real device aspects, but instead represented driver design (MFD driver). MFDs are pretty often combining unrelated features, e.g. PMICs which have wakeup and system power control, regulator, 32 kHz clocks, RTC and some USB connector. Just because you will have clocksource driver, PWM driver and timer driver does not make it a MFD. Best regards, Krzysztof
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > bindings > > On 11/10/2022 10:55, Biju Das wrote: > > > >>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > >> ++++++++++++++++++ > >>> 1 file changed, 305 insertions(+) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > >> > >> This should not be in MFD. Just because some device has few > features, > >> does not mean it should go to MFD... Choose either timer or pwm. > > > > MFD is for multifunction device. This IP supports multiple functions > > like timer, pwm, clock source/events. That is the reason I have > added > > here. MFD is core which provides register access for client devices. > > > > For me moving it to pwm or counter is not a big problem. > > Why do you think it cannot be MFD? > > > Because it makes MFD a dump for everything where author did not want > to think about real device aspects, but instead represented driver > design (MFD driver). Core driver is MFD, just provides resources to child devices and is not aware of any real device aspects. > > MFDs are pretty often combining unrelated features, e.g. PMICs which > have wakeup and system power control, regulator, 32 kHz clocks, RTC > and some USB connector. Here also same right? pwm, counter and clock are 3 unrelated features. That is the reason we have separate subsystems for these features. > > Just because you will have clocksource driver, PWM driver and timer > driver does not make it a MFD. MFD is multi function device. So are are you agreeing Clock source, PWM and timer are different functionalities or not? If not, why do we have 3 subsystems, if it is same? Where do keep these bindings as there is only single "rz_mtu" bindings for these 3 different functionalities? pwm or counter or mfd? Please let me know. Cheers, Biju
On 11/10/2022 15:23, Biju Das wrote: >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a >> bindings >> >> On 11/10/2022 10:55, Biju Das wrote: >>> >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 >>>> ++++++++++++++++++ >>>>> 1 file changed, 305 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml >>>> >>>> This should not be in MFD. Just because some device has few >> features, >>>> does not mean it should go to MFD... Choose either timer or pwm. >>> >>> MFD is for multifunction device. This IP supports multiple functions >>> like timer, pwm, clock source/events. That is the reason I have >> added >>> here. MFD is core which provides register access for client devices. >>> >>> For me moving it to pwm or counter is not a big problem. >>> Why do you think it cannot be MFD? >> >> >> Because it makes MFD a dump for everything where author did not want >> to think about real device aspects, but instead represented driver >> design (MFD driver). > > Core driver is MFD, just provides resources to child devices > and is not aware of any real device aspects. > >> >> MFDs are pretty often combining unrelated features, e.g. PMICs which >> have wakeup and system power control, regulator, 32 kHz clocks, RTC >> and some USB connector. > > Here also same right? pwm, counter and clock are 3 unrelated features. > That is the reason we have separate subsystems for these features. These are quite similar features of a similar piece of hardware. Sometimes called timer. > >> >> Just because you will have clocksource driver, PWM driver and timer >> driver does not make it a MFD. > > MFD is multi function device. No. MFD is a Linux subsystem name. Not a device type. The bindings are located in respective type. > So are are you agreeing Clock source, PWM and > timer are different functionalities or not? If not, why do we have 3 subsystems, > if it is same? Linux subsystems? We can have millions of them and it is not related to bindings. > Where do keep these bindings as there is only single "rz_mtu" bindings for these 3 different functionalities? Again, focus on hardware not on Linux drivers. Hardware is called MTU - Multi-Function TIMER Unit. Timer. > pwm or counter or mfd? Not MFD. I already proposed where to put it. Other Timer/PWM/Counter units are also in timer. Renesas is not special to get some exceptions. Best regards, Krzysztof
> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > bindings > > On 11/10/2022 15:23, Biju Das wrote: > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > >> bindings > >> > >> On 11/10/2022 10:55, Biju Das wrote: > >>> > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > >>>> ++++++++++++++++++ > >>>>> 1 file changed, 305 insertions(+) create mode 100644 > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > >>>> > >>>> This should not be in MFD. Just because some device has few > >> features, > >>>> does not mean it should go to MFD... Choose either timer or pwm. > >>> > >>> MFD is for multifunction device. This IP supports multiple > functions > >>> like timer, pwm, clock source/events. That is the reason I have > >> added > >>> here. MFD is core which provides register access for client > devices. > >>> > >>> For me moving it to pwm or counter is not a big problem. > >>> Why do you think it cannot be MFD? > >> > >> > >> Because it makes MFD a dump for everything where author did not > want > >> to think about real device aspects, but instead represented driver > >> design (MFD driver). > > > > Core driver is MFD, just provides resources to child devices and is > > not aware of any real device aspects. > > > >> > >> MFDs are pretty often combining unrelated features, e.g. PMICs > which > >> have wakeup and system power control, regulator, 32 kHz clocks, RTC > >> and some USB connector. > > > > Here also same right? pwm, counter and clock are 3 unrelated > features. > > That is the reason we have separate subsystems for these features. > > These are quite similar features of a similar piece of hardware. > Sometimes called timer. > > > > >> > >> Just because you will have clocksource driver, PWM driver and timer > >> driver does not make it a MFD. > > > > MFD is multi function device. > > No. MFD is a Linux subsystem name. Not a device type. The bindings are > located in respective type. > > > So are are you agreeing Clock source, PWM and timer are different > > functionalities or not? If not, why do we have 3 subsystems, if it > is > > same? > > Linux subsystems? We can have millions of them and it is not related > to bindings. OK. > > > > Where do keep these bindings as there is only single "rz_mtu" > bindings for these 3 different functionalities? > > Again, focus on hardware not on Linux drivers. Hardware is called MTU > - Multi-Function TIMER Unit. Timer. OK > > > pwm or counter or mfd? > > Not MFD. I already proposed where to put it. Other Timer/PWM/Counter > units are also in timer. > I guess for counter/pwm maintainers, it is ok to model MTU3 as a single binding "rz-mtu3" in timer that binds against counter and pwm functionalities as well?? Cheers, Biju
On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote: > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > > bindings > > > > On 11/10/2022 15:23, Biju Das wrote: > > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > > >> bindings > > >> > > >> On 11/10/2022 10:55, Biju Das wrote: > > >>> > > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > > >>>> ++++++++++++++++++ > > >>>>> 1 file changed, 305 insertions(+) create mode 100644 > > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > >>>> > > >>>> This should not be in MFD. Just because some device has few > > >> features, > > >>>> does not mean it should go to MFD... Choose either timer or pwm. > > >>> > > >>> MFD is for multifunction device. This IP supports multiple > > functions > > >>> like timer, pwm, clock source/events. That is the reason I have > > >> added > > >>> here. MFD is core which provides register access for client > > devices. > > >>> > > >>> For me moving it to pwm or counter is not a big problem. > > >>> Why do you think it cannot be MFD? > > >> > > >> > > >> Because it makes MFD a dump for everything where author did not > > want > > >> to think about real device aspects, but instead represented driver > > >> design (MFD driver). > > > > > > Core driver is MFD, just provides resources to child devices and is > > > not aware of any real device aspects. > > > > > >> > > >> MFDs are pretty often combining unrelated features, e.g. PMICs > > which > > >> have wakeup and system power control, regulator, 32 kHz clocks, RTC > > >> and some USB connector. > > > > > > Here also same right? pwm, counter and clock are 3 unrelated > > features. > > > That is the reason we have separate subsystems for these features. > > > > These are quite similar features of a similar piece of hardware. > > Sometimes called timer. > > > > > > > >> > > >> Just because you will have clocksource driver, PWM driver and timer > > >> driver does not make it a MFD. > > > > > > MFD is multi function device. > > > > No. MFD is a Linux subsystem name. Not a device type. The bindings are > > located in respective type. > > > > > So are are you agreeing Clock source, PWM and timer are different > > > functionalities or not? If not, why do we have 3 subsystems, if it > > is > > > same? > > > > Linux subsystems? We can have millions of them and it is not related > > to bindings. > > OK. > > > > > > > > Where do keep these bindings as there is only single "rz_mtu" > > bindings for these 3 different functionalities? > > > > Again, focus on hardware not on Linux drivers. Hardware is called MTU > > - Multi-Function TIMER Unit. Timer. > > OK > > > > > pwm or counter or mfd? > > > > Not MFD. I already proposed where to put it. Other Timer/PWM/Counter > > units are also in timer. > > > > I guess for counter/pwm maintainers, it is ok to model MTU3 as a single > binding "rz-mtu3" in timer that binds against counter and pwm > functionalities as well?? > > Cheers, > Biju I'm okay with putting the MTU3 binding under timer; we already have Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so adding a new renesas,mtu3.yaml next to it seems reasonable. Just to reiterate Krzysztof's point, the subsystems in Linux serve as a way to group drivers together that utilize the same ABIs, whereas the devicetree is a structure for organizing physical hardware. The structure of physical hardware types don't necessarily match the organization of the ABIs we use to support them. This is why you may end up with differing heirarchies between the devicetree and driver subsystems. To illustrate the point, take for example a hypothetical digital-to-analog (DAC) device with a few GPIO lines. Those GPIO input signals could be tied to buttons used to indicate to the system that a user wants to reset or adjust the DAC output, while the GPIO output signals could be status lights or triggers indicating that the DAC is operating. The respective driver for this device may utilize the IIO subsystem to support the DAC and the GPIO subsystem to support those GPIO lines, but it would be incorrect to put this under MFD because the purpose of the GPIO lines is to assist in the operation of the DAC; in other words, this is primarily a DAC device with some auxiliary convenience functionalities, not a MFD with distinct unrelated separate components. William Breathitt Gray
Hi William Breathitt Gray, > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > bindings > > On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote: > > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L > MTU3a > > > bindings > > > > > > On 11/10/2022 15:23, Biju Das wrote: > > > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L > > > >> MTU3a bindings > > > >> > > > >> On 11/10/2022 10:55, Biju Das wrote: > > > >>> > > > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > > > >>>> ++++++++++++++++++ > > > >>>>> 1 file changed, 305 insertions(+) create mode 100644 > > > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > > >>>> > > > >>>> This should not be in MFD. Just because some device has few > > > >> features, > > > >>>> does not mean it should go to MFD... Choose either timer or > pwm. > > > >>> > > > >>> MFD is for multifunction device. This IP supports multiple > > > functions > > > >>> like timer, pwm, clock source/events. That is the reason I > have > > > >> added > > > >>> here. MFD is core which provides register access for client > > > devices. > > > >>> > > > >>> For me moving it to pwm or counter is not a big problem. > > > >>> Why do you think it cannot be MFD? > > > >> > > > >> > > > >> Because it makes MFD a dump for everything where author did not > > > want > > > >> to think about real device aspects, but instead represented > > > >> driver design (MFD driver). > > > > > > > > Core driver is MFD, just provides resources to child devices and > > > > is not aware of any real device aspects. > > > > > > > >> > > > >> MFDs are pretty often combining unrelated features, e.g. PMICs > > > which > > > >> have wakeup and system power control, regulator, 32 kHz clocks, > > > >> RTC and some USB connector. > > > > > > > > Here also same right? pwm, counter and clock are 3 unrelated > > > features. > > > > That is the reason we have separate subsystems for these > features. > > > > > > These are quite similar features of a similar piece of hardware. > > > Sometimes called timer. > > > > > > > > > > >> > > > >> Just because you will have clocksource driver, PWM driver and > > > >> timer driver does not make it a MFD. > > > > > > > > MFD is multi function device. > > > > > > No. MFD is a Linux subsystem name. Not a device type. The bindings > > > are located in respective type. > > > > > > > So are are you agreeing Clock source, PWM and timer are > different > > > > functionalities or not? If not, why do we have 3 subsystems, if > it > > > is > > > > same? > > > > > > Linux subsystems? We can have millions of them and it is not > related > > > to bindings. > > > > OK. > > > > > > > > > > > > Where do keep these bindings as there is only single "rz_mtu" > > > bindings for these 3 different functionalities? > > > > > > Again, focus on hardware not on Linux drivers. Hardware is called > > > MTU > > > - Multi-Function TIMER Unit. Timer. > > > > OK > > > > > > > pwm or counter or mfd? > > > > > > Not MFD. I already proposed where to put it. Other > Timer/PWM/Counter > > > units are also in timer. > > > > > > > I guess for counter/pwm maintainers, it is ok to model MTU3 as a > > single binding "rz-mtu3" in timer that binds against counter and pwm > > functionalities as well?? > > > > Cheers, > > Biju > > I'm okay with putting the MTU3 binding under timer; we already have > Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so > adding a new renesas,mtu3.yaml next to it seems reasonable. OK. > > Just to reiterate Krzysztof's point, the subsystems in Linux serve as > a way to group drivers together that utilize the same ABIs, whereas > the devicetree is a structure for organizing physical hardware. The > structure of physical hardware types don't necessarily match the > organization of the ABIs we use to support them. This is why you may > end up with differing heirarchies between the devicetree and driver > subsystems. > > To illustrate the point, take for example a hypothetical digital-to- > analog (DAC) device with a few GPIO lines. Those GPIO input signals > could be tied to buttons used to indicate to the system that a user > wants to reset or adjust the DAC output, while the GPIO output signals > could be status lights or triggers indicating that the DAC is > operating. The respective driver for this device may utilize the IIO > subsystem to support the DAC and the GPIO subsystem to support those > GPIO lines, but it would be incorrect to put this under MFD because > the purpose of the GPIO lines is to assist in the operation of the > DAC; in other words, this is primarily a DAC device with some > auxiliary convenience functionalities, not a MFD with distinct > unrelated separate components. OK agreed. Thanks for the explanation. Cheers, Biju
On Sat, Oct 15, 2022 at 10:28:53AM -0400, William Breathitt Gray wrote: > On Tue, Oct 11, 2022 at 08:31:48PM +0000, Biju Das wrote: > > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > > > bindings > > > > > > On 11/10/2022 15:23, Biju Das wrote: > > > >> Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > > > >> bindings > > > >> > > > >> On 11/10/2022 10:55, Biju Das wrote: > > > >>> > > > >>>>> .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > > > >>>> ++++++++++++++++++ > > > >>>>> 1 file changed, 305 insertions(+) create mode 100644 > > > >>>>> Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > > >>>> > > > >>>> This should not be in MFD. Just because some device has few > > > >> features, > > > >>>> does not mean it should go to MFD... Choose either timer or pwm. > > > >>> > > > >>> MFD is for multifunction device. This IP supports multiple > > > functions > > > >>> like timer, pwm, clock source/events. That is the reason I have > > > >> added > > > >>> here. MFD is core which provides register access for client > > > devices. > > > >>> > > > >>> For me moving it to pwm or counter is not a big problem. > > > >>> Why do you think it cannot be MFD? > > > >> > > > >> > > > >> Because it makes MFD a dump for everything where author did not > > > want > > > >> to think about real device aspects, but instead represented driver > > > >> design (MFD driver). > > > > > > > > Core driver is MFD, just provides resources to child devices and is > > > > not aware of any real device aspects. > > > > > > > >> > > > >> MFDs are pretty often combining unrelated features, e.g. PMICs > > > which > > > >> have wakeup and system power control, regulator, 32 kHz clocks, RTC > > > >> and some USB connector. > > > > > > > > Here also same right? pwm, counter and clock are 3 unrelated > > > features. > > > > That is the reason we have separate subsystems for these features. > > > > > > These are quite similar features of a similar piece of hardware. > > > Sometimes called timer. > > > > > > > > > > >> > > > >> Just because you will have clocksource driver, PWM driver and timer > > > >> driver does not make it a MFD. > > > > > > > > MFD is multi function device. > > > > > > No. MFD is a Linux subsystem name. Not a device type. The bindings are > > > located in respective type. > > > > > > > So are are you agreeing Clock source, PWM and timer are different > > > > functionalities or not? If not, why do we have 3 subsystems, if it > > > is > > > > same? > > > > > > Linux subsystems? We can have millions of them and it is not related > > > to bindings. > > > > OK. > > > > > > > > > > > > Where do keep these bindings as there is only single "rz_mtu" > > > bindings for these 3 different functionalities? > > > > > > Again, focus on hardware not on Linux drivers. Hardware is called MTU > > > - Multi-Function TIMER Unit. Timer. > > > > OK > > > > > > > pwm or counter or mfd? > > > > > > Not MFD. I already proposed where to put it. Other Timer/PWM/Counter > > > units are also in timer. > > > > > > > I guess for counter/pwm maintainers, it is ok to model MTU3 as a single > > binding "rz-mtu3" in timer that binds against counter and pwm > > functionalities as well?? > > > > Cheers, > > Biju > > I'm okay with putting the MTU3 binding under timer; we already have > Documentation/devicetree/bindings/timer/renesas,mtu2.yaml there so > adding a new renesas,mtu3.yaml next to it seems reasonable. > > Just to reiterate Krzysztof's point, the subsystems in Linux serve as a > way to group drivers together that utilize the same ABIs, whereas the > devicetree is a structure for organizing physical hardware. The > structure of physical hardware types don't necessarily match the > organization of the ABIs we use to support them. This is why you may end > up with differing heirarchies between the devicetree and driver > subsystems. > > To illustrate the point, take for example a hypothetical > digital-to-analog (DAC) device with a few GPIO lines. Those GPIO > input signals could be tied to buttons used to indicate to the system > that a user wants to reset or adjust the DAC output, while the GPIO > output signals could be status lights or triggers indicating that the > DAC is operating. The respective driver for this device may utilize the > IIO subsystem to support the DAC and the GPIO subsystem to support those > GPIO lines, but it would be incorrect to put this under MFD because the > purpose of the GPIO lines is to assist in the operation of the DAC; in > other words, this is primarily a DAC device with some auxiliary > convenience functionalities, not a MFD with distinct unrelated separate > components. Exactly. In addition to the DT aspect, another misconception is that a driver for a multi-function device needs to be somehow split up to match the Linux subsystem structure. In most cases that's not true. Pick the subsystem that most closely fits the main function of a device and implement the driver. If you can expose further functions using other subsystems, that's absolutely fine. Drivers can register with multiple subsystems at the same time. Yes, that's not quite as neat as having individual drivers for each subsystem, but it's a much better approximation of the hardware and therefore will lead to more compact and stable drivers. Trying to split things up arbitrarily often makes for wonky constructs. We've gained powerful tools over the years to easily deal with cross- subsystem drivers, so there's seldom a reason to strictly split things across subsystem boundaries anymore. Thierry
On Tue, 11 Oct 2022, Biju Das wrote: > > Hi Krzysztof Kozlowski, > > > Subject: Re: [PATCH v4 1/4] dt-bindings: mfd: Document RZ/G2L MTU3a > > bindings > > > > On 10/10/2022 10:52, Biju Das wrote: > > > The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in > > > the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer > > > channels and one 32-bit timer channel. It supports the following > > > functions > > > - Counter > > > - Timer > > > - PWM > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v3->v4: > > > * Dropped counter and pwm compatibeles as they don't have any > > resources. > > > * Made rz-mtu3 as pwm provider. > > > * Updated the example and description. > > > v2->v3: > > > * Dropped counter bindings and integrated with mfd as it has only > > one property. > > > * Removed "#address-cells" and "#size-cells" as it do not have > > children with > > > unit addresses. > > > * Removed quotes from counter and pwm. > > > * Provided full path for pwm bindings. > > > * Updated the example. > > > v1->v2: > > > * Modelled counter and pwm as a single device that handles > > > multiple channels. > > > * Moved counter and pwm bindings to respective subsystems > > > * Dropped 'bindings' from MFD binding title. > > > * Updated the example > > > * Changed the compatible names. > > > --- > > > .../bindings/mfd/renesas,rz-mtu3.yaml | 305 > > ++++++++++++++++++ > > > 1 file changed, 305 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml > > > > This should not be in MFD. Just because some device has few features, > > does not mean it should go to MFD... Choose either timer or pwm. > > MFD is for multifunction device. This IP supports multiple functions > like timer, pwm, clock source/events. That is the reason I have added > here. MFD is core which provides register access for client devices. > > For me moving it to pwm or counter is not a big problem. > Why do you think it cannot be MFD? Sorry for jumping in late here. I see this has been resolved. The TL;DR is: if you're not using the MFD Core (and including mfd/core.h), it's not an MFD. You *could* split this up into its component parts, place them into their own subsystems and use an MFD core driver to register them all, but as Thierry says, this is not a hard requirement either.
diff --git a/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml new file mode 100644 index 000000000000..1b0be9f5cd18 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml @@ -0,0 +1,305 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/renesas,rz-mtu3.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Renesas RZ/G2L Multi-Function Timer Pulse Unit 3 (MTU3a) + +maintainers: + - Biju Das <biju.das.jz@bp.renesas.com> + +description: | + This hardware block pconsisting of eight 16-bit timer channels and one + 32- bit timer channel. It supports the following specifications: + - Pulse input/output: 28 lines max. + - Pulse input 3 lines + - Count clock 11 clocks for each channel (14 clocks for MTU0, 12 clocks + for MTU2, and 10 clocks for MTU5, four clocks for MTU1-MTU2 combination + (when LWA = 1)) + - Operating frequency Up to 100 MHz + - Available operations [MTU0 to MTU4, MTU6, MTU7, and MTU8] + - Waveform output on compare match + - Input capture function (noise filter setting available) + - Counter-clearing operation + - Simultaneous writing to multiple timer counters (TCNT) + (excluding MTU8). + - Simultaneous clearing on compare match or input capture + (excluding MTU8). + - Simultaneous input and output to registers in synchronization with + counter operations (excluding MTU8). + - Up to 12-phase PWM output in combination with synchronous operation + (excluding MTU8) + - [MTU0 MTU3, MTU4, MTU6, MTU7, and MTU8] + - Buffer operation specifiable + - [MTU1, MTU2] + - Phase counting mode can be specified independently + - 32-bit phase counting mode can be specified for interlocked operation + of MTU1 and MTU2 (when TMDR3.LWA = 1) + - Cascade connection operation available + - [MTU3, MTU4, MTU6, and MTU7] + - 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 and reset-synchronized PWM operation. + - In complementary PWM mode, values can be transferred from buffer + registers to temporary registers at crests and troughs of the timer- + counter values or when the buffer registers (TGRD registers in MTU4 + and MTU7) are written to. + - Double-buffering selectable in complementary PWM mode. + - [MTU3 and MTU4] + - Through interlocking with MTU0, a mode for driving AC synchronous + motors (brushless DC motors) by using complementary PWM output and + reset-synchronized PWM output is settable and allows the selection + of two types of waveform output (chopping or level). + - [MTU5] + - Capable of operation as a dead-time compensation counter. + - [MTU0/MTU5, MTU1, MTU2, and MTU8] + - 32-bit phase counting mode specifiable by combining MTU1 and MTU2 and + through interlocked operation with MTU0/MTU5 and MTU8. + - Interrupt-skipping function + - In complementary PWM mode, interrupts on crests and troughs of counter + values and triggers to start conversion by the A/D converter can be + skipped. + - Interrupt sources: 43 sources. + - Buffer operation: + - Automatic transfer of register data (transfer from the buffer + register to the timer register). + - Trigger generation + - A/D converter start triggers can be generated + - A/D converter start request delaying function enables A/D converter + to be started with any desired timing and to be synchronized with + PWM output. + - Low power consumption function + - The MTU3a can be placed in the module-stop state. + + There are two phase counting modes. 16-bit phase counting mode in which + MTU1 and MTU2 operate independently, and cascade connection 32-bit phase + counting mode in which MTU1 and MTU2 are cascaded. + + In phase counting mode, the phase difference between two external input + clocks is detected and the corresponding TCNT is incremented or + decremented. + The below counters are supported + count0 - MTU1 16-bit phase counting + count1 - MTU2 16-bit phase counting + count2 - MTU1+ MTU2 32-bit phase counting + + The module supports PWM mode{1,2}, Reset-synchronized PWM mode and + complementary PWM mode{1,2,3}. + + In complementary PWM mode, six positive-phase and six negative-phase PWM + waveforms (12 phases in total) with dead time can be output by + combining MTU{3,4} and MTU{6,7}. + + The below pwm channels are supported in pwm mode 1. + pwm0 - MTU0.MTIOC0A PWM mode 1 + pwm1 - MTU0.MTIOC0C PWM mode 1 + pwm2 - MTU1.MTIOC1A PWM mode 1 + pwm3 - MTU2.MTIOC2A PWM mode 1 + pwm4 - MTU3.MTIOC3A PWM mode 1 + pwm5 - MTU3.MTIOC3C PWM mode 1 + pwm6 - MTU4.MTIOC4A PWM mode 1 + pwm7 - MTU4.MTIOC4C PWM mode 1 + pwm8 - MTU6.MTIOC6A PWM mode 1 + pwm9 - MTU6.MTIOC6C PWM mode 1 + pwm10 - MTU7.MTIOC7A PWM mode 1 + pwm11 - MTU7.MTIOC7C PWM mode 1 + +properties: + compatible: + items: + - enum: + - renesas,r9a07g044-mtu3 # RZ/G2{L,LC} + - renesas,r9a07g054-mtu3 # RZ/V2L + - const: renesas,rz-mtu3 + + reg: + maxItems: 1 + + interrupts: + items: + - description: MTU0.TGRA input capture/compare match + - description: MTU0.TGRB input capture/compare match + - description: MTU0.TGRC input capture/compare match + - description: MTU0.TGRD input capture/compare match + - description: MTU0.TCNT overflow + - description: MTU0.TGRE compare match + - description: MTU0.TGRF compare match + - description: MTU1.TGRA input capture/compare match + - description: MTU1.TGRB input capture/compare match + - description: MTU1.TCNT overflow + - description: MTU1.TCNT underflow + - description: MTU2.TGRA input capture/compare match + - description: MTU2.TGRB input capture/compare match + - description: MTU2.TCNT overflow + - description: MTU2.TCNT underflow + - description: MTU3.TGRA input capture/compare match + - description: MTU3.TGRB input capture/compare match + - description: MTU3.TGRC input capture/compare match + - description: MTU3.TGRD input capture/compare match + - description: MTU3.TCNT overflow + - description: MTU4.TGRA input capture/compare match + - description: MTU4.TGRB input capture/compare match + - description: MTU4.TGRC input capture/compare match + - description: MTU4.TGRD input capture/compare match + - description: MTU4.TCNT overflow/underflow + - description: MTU5.TGRU input capture/compare match + - description: MTU5.TGRV input capture/compare match + - description: MTU5.TGRW input capture/compare match + - description: MTU6.TGRA input capture/compare match + - description: MTU6.TGRB input capture/compare match + - description: MTU6.TGRC input capture/compare match + - description: MTU6.TGRD input capture/compare match + - description: MTU6.TCNT overflow + - description: MTU7.TGRA input capture/compare match + - description: MTU7.TGRB input capture/compare match + - description: MTU7.TGRC input capture/compare match + - description: MTU7.TGRD input capture/compare match + - description: MTU7.TCNT overflow/underflow + - description: MTU8.TGRA input capture/compare match + - description: MTU8.TGRB input capture/compare match + - description: MTU8.TGRC input capture/compare match + - description: MTU8.TGRD input capture/compare match + - description: MTU8.TCNT overflow + - description: MTU8.TCNT underflow + + interrupt-names: + items: + - const: tgia0 + - const: tgib0 + - const: tgic0 + - const: tgid0 + - const: tgiv0 + - const: tgie0 + - const: tgif0 + - const: tgia1 + - const: tgib1 + - const: tgiv1 + - const: tgiu1 + - const: tgia2 + - const: tgib2 + - const: tgiv2 + - const: tgiu2 + - const: tgia3 + - const: tgib3 + - const: tgic3 + - const: tgid3 + - const: tgiv3 + - const: tgia4 + - const: tgib4 + - const: tgic4 + - const: tgid4 + - const: tgiv4 + - const: tgiu5 + - const: tgiv5 + - const: tgiw5 + - const: tgia6 + - const: tgib6 + - const: tgic6 + - const: tgid6 + - const: tgiv6 + - const: tgia7 + - const: tgib7 + - const: tgic7 + - const: tgid7 + - const: tgiv7 + - const: tgia8 + - const: tgib8 + - const: tgic8 + - const: tgid8 + - const: tgiv8 + - const: tgiu8 + + clocks: + maxItems: 1 + + power-domains: + maxItems: 1 + + resets: + maxItems: 1 + + "#pwm-cells": + const: 2 + +required: + - compatible + - reg + - interrupts + - interrupt-names + - clocks + - power-domains + - resets + +additionalProperties: false + +examples: + - | + #include <dt-bindings/clock/r9a07g044-cpg.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + + mtu3: timer@10001200 { + compatible = "renesas,r9a07g044-mtu3", + "renesas,rz-mtu3"; + reg = <0x10001200 0xb00>; + interrupts = <GIC_SPI 170 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 171 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 172 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 173 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 174 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 175 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 176 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 177 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 178 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 179 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 180 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 181 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 182 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 183 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 184 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 185 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 186 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 187 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 188 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 189 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 190 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 191 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 192 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 193 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 194 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 195 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 196 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 197 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 198 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 199 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 200 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 201 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 202 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 203 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 204 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 206 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 207 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 208 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 209 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 210 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 211 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 212 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 213 IRQ_TYPE_EDGE_RISING>; + interrupt-names = "tgia0", "tgib0", "tgic0", "tgid0", "tgiv0", "tgie0", + "tgif0", + "tgia1", "tgib1", "tgiv1", "tgiu1", + "tgia2", "tgib2", "tgiv2", "tgiu2", + "tgia3", "tgib3", "tgic3", "tgid3", "tgiv3", + "tgia4", "tgib4", "tgic4", "tgid4", "tgiv4", + "tgiu5", "tgiv5", "tgiw5", + "tgia6", "tgib6", "tgic6", "tgid6", "tgiv6", + "tgia7", "tgib7", "tgic7", "tgid7", "tgiv7", + "tgia8", "tgib8", "tgic8", "tgid8", "tgiv8", "tgiu8"; + clocks = <&cpg CPG_MOD R9A07G044_MTU_X_MCK_MTU3>; + power-domains = <&cpg>; + resets = <&cpg R9A07G044_MTU_X_PRESET_MTU3>; + #pwm-cells = <2>; + }; + +...
The RZ/G2L multi-function timer pulse unit 3 (MTU3a) is embedded in the Renesas RZ/G2L family SoC's. It consists of eight 16-bit timer channels and one 32-bit timer channel. It supports the following functions - Counter - Timer - PWM Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v3->v4: * Dropped counter and pwm compatibeles as they don't have any resources. * Made rz-mtu3 as pwm provider. * Updated the example and description. v2->v3: * Dropped counter bindings and integrated with mfd as it has only one property. * Removed "#address-cells" and "#size-cells" as it do not have children with unit addresses. * Removed quotes from counter and pwm. * Provided full path for pwm bindings. * Updated the example. v1->v2: * Modelled counter and pwm as a single device that handles multiple channels. * Moved counter and pwm bindings to respective subsystems * Dropped 'bindings' from MFD binding title. * Updated the example * Changed the compatible names. --- .../bindings/mfd/renesas,rz-mtu3.yaml | 305 ++++++++++++++++++ 1 file changed, 305 insertions(+) create mode 100644 Documentation/devicetree/bindings/mfd/renesas,rz-mtu3.yaml