Thread (45 messages) 45 messages, 7 authors, 2026-01-15

Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node

From: Lee Jones <lee@kernel.org>
Date: 2026-01-09 10:31:11
Also in: lkml

On Tue, 16 Dec 2025, Vladimir Oltean wrote:
On Tue, Dec 16, 2025 at 09:18:31AM +0000, Lee Jones wrote:
quoted
Unless you add/convert more child devices that are outside of net/ and
drivers/net AND move the core MFD usage to drivers/mfd/, then we can't
conclude that [ this device is suitable for MFD ].
To me, the argument that child devices can't all be under drivers/net/
is superficial. An mii_bus is very different in purpose from a phylink_pcs
and from a net_device, yet all 3 live in drivers/net/.
Understood.
Furthermore, I am looking at schemas such as /devicetree/bindings/mfd/adi,max77541.yaml:
"MAX77540 is a Power Management IC with 2 buck regulators."
and I don't understand how it possibly passed this criterion. It is one
chip with two devices of the same kind, and nothing else.
The MAX77541 has Regulators and an Analog to Digital Converter.

2 makes it Multi and passes criterion.

The ADC is 'hidden' from DT by MFD.
If moving the core MFD usage to drivers/mfd/ is another hard requirement,
this is also attacking form rather than substance. You as the MFD
maintainer can make an appeal to authority and NACK aesthetics you don't
like, but I just want everyone to be on the same page about this.
My plan, when and if I manage to find a few spare cycles, is to remove
MFD use from outside drivers/mfd.  That's been my rule since forever.
Having this in place ensures that the other rules are kept and (mild)
chaos doesn't ensue.  The MFD API is trivial to abuse.  You wouldn't
believe some of things I've seen over the years.  Each value I have is
there for a historical reason.
quoted
quoted
quoted
There does appear to be at least some level of misunderstanding
between us.  I'm not for one moment suggesting that a switch
can't be an MFD. If it contains probe-able components that need
to be split-up across multiple different subsystems, then by all
means, move the core driver into drivers/mfd/ and register child
devices 'till your heart's content.
Are you still speaking generically here, or have you actually
looked at any "nxp,sja1105q" or "nxp,sja1110a" device trees to see
what it would mean for these compatible strings to be probed by a
driver in drivers/mfd?
It's not my role to go digging into existing implementations and
previous submissions to prove whether a particular submission is
suitable for inclusion into MFD.

Please put in front of me, in a concise way (please), why you think
this is fit for inclusion.
No new information, I think the devices are fit for MFD because of
their memory map which was shown in the previous reply.
And Andrew's opinion reflects that, so I'm inclined to agree in general
terms.
quoted
I've explained what is usually required, but I'll (over-)simplify
again for clarity:

 - The mfd_* API call-sites must only exist in drivers/mfd/ -
 Consumers usually spit out non-system specific logic into a 'core'
 - MFDs need to have more than one child - This is where the 'Multi'
 comes in - Children should straddle different sub-systems -
 drivers/net is not enough [0] - If all of your sub-devices are in
 'net' use the platform_* API - <other stipulations less relevant to
 this stipulation> ...

There will always be exceptions, but previous mistakes are not good
justifications for future ones.

[0]

  .../bindings/net/dsa/nxp,sja1105.yaml         |  28 +
  .../bindings/net/pcs/snps,dw-xpcs.yaml        |   8 + MAINTAINERS
  |   2 + drivers/mfd/mfd-core.c                        |  11 +-
  drivers/net/dsa/sja1105/Kconfig               |   2 +
  drivers/net/dsa/sja1105/Makefile              |   2 +-
  drivers/net/dsa/sja1105/sja1105.h             |  42 +-
  drivers/net/dsa/sja1105/sja1105_main.c        | 169 +++---
  drivers/net/dsa/sja1105/sja1105_mdio.c        | 507
  ------------------ drivers/net/dsa/sja1105/sja1105_mfd.c         |
  293 ++++++++++ drivers/net/dsa/sja1105/sja1105_mfd.h         |  11
  + drivers/net/dsa/sja1105/sja1105_spi.c         | 113 +++-
  drivers/net/mdio/Kconfig                      |  21 +-
  drivers/net/mdio/Makefile                     |   2 +
  drivers/net/mdio/mdio-regmap-simple.c         |  77 +++
  drivers/net/mdio/mdio-regmap.c                |   7 +-
  drivers/net/mdio/mdio-sja1110-cbt1.c          | 173 ++++++
  drivers/net/pcs/pcs-xpcs-plat.c               | 146 +++--
  drivers/net/pcs/pcs-xpcs.c                    |  12 +
  drivers/net/phy/phylink.c                     |  75 ++-
  include/linux/mdio/mdio-regmap.h              |   2 +
  include/linux/mfd/core.h                      |   7 +
  include/linux/pcs/pcs-xpcs.h                  |   1 +
  include/linux/phylink.h                       |   5 + 24 files
  changed, 1033 insertions(+), 683 deletions(-) delete mode 100644
  drivers/net/dsa/sja1105/sja1105_mdio.c create mode 100644
  drivers/net/dsa/sja1105/sja1105_mfd.c create mode 100644
  drivers/net/dsa/sja1105/sja1105_mfd.h create mode 100644
  drivers/net/mdio/mdio-regmap-simple.c create mode 100644
  drivers/net/mdio/mdio-sja1110-cbt1.c
quoted
What OF node would remain for the DSA switch (child) device
driver? The same? Or are you suggesting that the entire
drivers/net/dsa/sja1105/ would move to drivers/mfd/? Or?
See bullet 1.1 above.

[...]
quoted
quoted
I don't recall those discussions from 3 years ago, but the
Ocelot platform, whatever it may be, seems to have quite a lot
more cross-subsystem device support requirements going on than I
see here:

drivers/i2c/busses/i2c-designware-platdrv.c
drivers/irqchip/irq-mscc-ocelot.c drivers/mfd/ocelot-*
drivers/net/dsa/ocelot/* drivers/net/ethernet/mscc/ocelot*
drivers/net/mdio/mdio-mscc-miim.c
drivers/phy/mscc/phy-ocelot-serdes.c
drivers/pinctrl/pinctrl-microchip-sgpio.c
drivers/pinctrl/pinctrl-ocelot.c
drivers/power/reset/ocelot-reset.c drivers/spi/spi-dw-mmio.c
net/dsa/tag_ocelot_8021q.c
This is a natural effect of Ocelot being "whatever it may be". It
is a family of networking SoCs, of which VSC7514 has a MIPS CPU
and Linux port, where the above drivers are used. The VSC7512 is
then a simplified variant with the MIPS CPU removed, and the
internal components controlled externally over SPI. Hence MFD to
reuse the same drivers as Linux on MIPS (using MMIO) did. This is
all that matters, not the quantity.
From what I can see, Ocelot ticks all of the boxes for MFD API
usage, whereas this submission does not.  The fact that the
overarching device provides a similar function is neither here nor
there.

These are the results from my searches of your device:

  git grep -i SJA1110 | grep -v 'net\|arch\|include' <no results>

[...]
quoted
quoted
My point is, you don't seem to have have any of that here.
What do you want to see exactly which is not here?

I have converted three classes of sub-devices on the NXP SJA1110
to MFD children in this patch set. Two MDIO buses and an Ethernet
PCS for SGMII.

In the SJA1110 memory map, the important resources look something
like this:

Name         Description
Start      End SWITCH       Ethernet Switch Subsystem
0x000000   0x3ffffc 100BASE-T1   Internal MDIO bus for 100BASE-T1
PHY (port 5 - 10)  0x704000   0x704ffc SGMII1       SGMII Port 1
0x705000   0x705ffc SGMII2       SGMII Port 2
0x706000   0x706ffc SGMII3       SGMII Port 3
0x707000   0x707ffc SGMII4       SGMII Port 4
0x708000   0x708ffc 100BASE-TX   Internal MDIO bus for 100BASE-TX
PHY                0x709000   0x709ffc
All in drivers/net.
quoted
ACU          Auxiliary Control Unit
0x711000   0x711ffc GPIO         General Purpose Input/Output
0x712000   0x712ffc
Where are these drivers?
For the GPIO I have no driver yet.

For the ACU, there is a reusable group of 4 registers for which I
wrote a cascaded interrupt controller driver in 2022. This register
group is instantiated multiple times in the SJA1110, which justified a
reusable driver.

Upstreaming it was blocked by the inability to instantiate it from the
main DSA driver using backwards-compatible DT bindings.

In any case, on the older generation SJA1105 (common driver with
SJA1110), the GPIO and interrupt controller blocks are missing. There
is an ACU block, but it handles just pinmux and pad configuration, and
the DSA driver programs it directly rather than going through the
pinmux subsystem.

This highlights a key requirement I have from the API for
instantiating sub-devices: that it is sufficiently flexible to split
them out of the main device when that starts making sense (we identify
a reusable block, or we need to configure it in the device tree, etc).
Otherwise, chopping up the switch address space upfront is a huge
overhead that may have no practical gains.
I certainly understand the challenges.

However, from my PoV, if you are instantiating one driver, even if it
does a bunch of different things which _could_ be split-up into all
sorts of far reaching subsystems, it's still one driver and therefore
does not meet the criteria for inclusion into MFD.

I had to go and remind myself of your DT:

        ethernet-switch@0 {
                compatible = "nxp,sja1110a";

                mdios {
                        mdio@0 {
                                compatible = "nxp,sja1110-base-t1-mdio";
                        };

                        mdio@1 {
                                compatible = "nxp,sja1110-base-tx-mdio";
                        };
                };
        };

To my untrained eye, this looks like two instances of a MDIO device.

Are they truly different enough to be classified for "Multi"?
quoted
quoted
I need to remind you that my purpose here is not to add drivers in
breadth for all SJA1110 sub-devices now.
You'll see from my discussions with Colin, sub-drivers (if they are
to be used for MFD justification (point 3 above), then they must be
added as part of the first submission.  Perhaps this isn't an MFD,
"yet"?

[...]
IMHO, the concept of being or not being MFD "yet" is silly. Based on
the register map, you are, or are not.
Perhaps I didn't explain this very well.  What I'm alluding to here is
that perhaps this a collection of different devices that may well fit
comfortably with the remit of MFD.  However, I haven't seen any
compelling evidence of that in this current submission.

As an example, when contributors submit an MFD core driver with only one
device, let's say a few Regulators but promise that the device is
actually capable of operating as a Watchdog, a Real-Time Clock and a
Power-on Key, only they haven't authored the drivers for those yet.  The
driver get NACKed until at least one other piece of functionality is
available.  Else the "Multi" box isn't ticked and therefore does not
qualify for inclusion.

The "yet" part was alluding to the fact that this may be the case here.
quoted
quoted
The SGMII blocks are highly reusable IPs licensed from Synopsys,
and Linux already has DT bindings and a corresponding platform
driver for the case where their registers are viewed using MMIO.
This is a good reason for dividing them up into subordinate platform
devices.  However, it is not a good use-case of MFD.  In it's
current guise, your best bet is to use the platform_* API directly.

This is a well trodden path and it not challenging:

  % git grep platform_device_add -- arch drivers sound | wc -l 398

[...]
quoted
In my opinion I do not need to add handling for any other
sub-device, for the support to be more "cross-system" like for
Ocelot. What is here is enough for you to decide if this is
adequate for MFD or not.
Currently ... it's not.

[...]

Hopefully that helps to clarify my expectations a little.

TL;DR, this looks like a good candidate for direct platform_* usage.
I do have a local branch with platform devices created manually, and
yet, I considered the mfd_add_devices() form looked cleaner when
submitting.

I expect the desire to split up reusable register regions into
platform sub-devices to pop up again, so the logic should be available
as library code at some level (possibly DSA).

Unless you have something against the idea, I'm thinking a good name
for this library code would be "nmfd", for "Not MFD". It is like MFD,
except: - the parent can simultaneously handle the main function of
the device while delegating other regions to sub-devices - the
sub-devices can all have drivers in the same subsystem (debatable
whether MFD follows this - just to avoid discussions) - their OF nodes
don't have to be direct children of the parent.
Well, we already have Simple MFD which works for some basic use-cases.

When I've thought about replacing the existing occurrences of the MFD
API being used outside of drivers/mfd, I have often thought of a
platform_add_device_simple() call which I believe would do what most
people of these use-cases actually want.

-- 
Lee Jones [李琼斯]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help