Re: [PATCH 4/7] soc: rockchip: add mfpwm driver

["Heiko Stübner" <heiko@sntech.de>]

Am Montag, 2. Juni 2025, 14:15:45 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> On Saturday, 31 May 2025 23:48:29 Central European Summer Time Heiko Stübner wrote:
> > Am Dienstag, 8. April 2025, 14:32:16 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:

> > On a more general note, what is the differentiation to an MFD here?
> > 
> > Like you can already bind dt-nodes to MFD subdevices, and can implement
> > the exclusivity API thing on top of a general mfd device, to make sure only
> > one mfd-cell gets activated at one time.
> > 
> > Other than that, this looks like it reimplements MFDs?
> 
> What initially made me not make this an MFD was Uwe Kleine-König expressing
> some doubts, which lead me to alternatives like the auxiliary bus. Reading the
> auxiliary bus docs I found:
> 
>   A key requirement for utilizing the auxiliary bus is that there is no
>   dependency on a physical bus, device, register accesses or regmap support.
>   These individual devices split from the core cannot live on the platform
>   bus as they are not physical devices that are controlled by DT/ACPI. The
>   same argument applies for not using MFD in this scenario as MFD relies on
>   individual function devices being physical devices.

Interestingly the 5 year old LWN article seems to have been overtaken by
real-world usage ;-) .

I see pinctrl/pinctrl-ep93xx.c using regmaps (and thus registers), similarly
in gpu/drm/bridge/ti-sn65dsi86.c and a number more.


> Additionally, LWN[1] about the auxiliary bus, which I've read up on during my
> ill-fated journey into that version of the driver, also goes further into why
> MFD is sometimes a bad fit:

[...] LWN excerpt [...]

> The individual function devices may be all pointing at the same physical
> device here, but they're not distinct parts of the device. However, there
> still *is* a physical device, which convinced me that auxiliary bus wasn't
> the right one either, and the idea for just using the platform bus came
> during a work meeting. If someone with experience on aux bus vs platform bus
> (what this uses) vs MFD, then feel free to chime in. Unfortunately, as is the
> norm, I can't seem to find much in terms of MFD documentation. Needing to know
> what type of exclusion they guarantee and what type of abstractions they bring
> with them that would make them more useful than my solution would need some
> justification in more than just an auto-generated header listing.

I think MFD itself does not provide any exclusivity - aka allowing definitions
that combinations of sub-devices cannot be used at the same time.

But as I see it right now, you have sort of a mfd-device in there, creating
all the sub-devices and then the aquire/release logic on top making sure
only one device is ever active at the same time.

Right now I really don't see (prone to code-blindness though) why the
aquire/release logic could not live in a mfd-device.


> I am very inclined to start pretending things that aren't documented do
> not actually exist in the kernel, because it's very annoying to have to
> constantly deal with this.

Sadly the "ostrich method" won't work ;-)

So as a way forward, I'd suggest you posting your v2, so that all the
current review comments get addressed and amending the
cover-letter with the aux-bux / mfd discussion thing (ideally in a
somewhat highlighed block so that people skimming along will notice)
and include the relevant people:

- for aux-bux get_maintainer.pl says:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> (maintainer:AUXILIARY BUS DRIVER)
Dave Ertman <david.m.ertman@intel.com> (reviewer:AUXILIARY BUS DRIVER)
Ira Weiny <ira.weiny@intel.com> (reviewer:AUXILIARY BUS DRIVER)
Leon Romanovsky <leon@kernel.org> (reviewer:AUXILIARY BUS DRIVER)

- and for MFD it's of course Lee:
Lee Jones <lee@kernel.org> (maintainer:MULTIFUNCTION DEVICES (MFD))


Heiko


> [1]: https://lwn.net/Articles/840416/