Re: [PATCH 4/7] soc: rockchip: add mfpwm driver
From: Heiko Stübner <heiko@sntech.de>
Date: 2025-06-02 13:14:37
Also in:
linux-arm-kernel, linux-gpio, linux-iio, linux-pwm, linux-rockchip, lkml
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:quoted
Am Dienstag, 8. April 2025, 14:32:16 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
quoted
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 [off-list ref] (maintainer:AUXILIARY BUS DRIVER) Dave Ertman [off-list ref] (reviewer:AUXILIARY BUS DRIVER) Ira Weiny [off-list ref] (reviewer:AUXILIARY BUS DRIVER) Leon Romanovsky [off-list ref] (reviewer:AUXILIARY BUS DRIVER) - and for MFD it's of course Lee: Lee Jones [off-list ref] (maintainer:MULTIFUNCTION DEVICES (MFD)) Heiko
[1]: https://lwn.net/Articles/840416/