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
Attachment:
signature.asc
Description: PGP signature
