Thread (24 messages) 24 messages, 7 authors, 2025-06-02

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/


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help