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

Vladimir Oltean posted 15 patches 1 week, 6 days ago
[PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Vladimir Oltean 1 week, 6 days ago
I would like the "nxp,sja1110a" driver, in the configuration below, to
be able to probe the drivers for "nxp,sja1110-base-t1-mdio" and for
"nxp,sja1110-base-tx-mdio" via mfd_add_devices():

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

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

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

This isn't currently possible, because mfd assumes that the parent
OF node ("mdios") == OF node of the parent ("ethernet-switch@0"), which
in this case isn't true, and as it searches through the children of
"ethernet-switch@0", it finds no MDIO bus to probe.

Cc: Lee Jones <lee@kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/mfd/mfd-core.c   | 11 +++++++++--
 include/linux/mfd/core.h |  7 +++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 7d14a1e7631e..e0b7f93a2654 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -181,8 +181,14 @@ static int mfd_add_device(struct device *parent, int id,
 	if (ret < 0)
 		goto fail_res;
 
-	if (IS_ENABLED(CONFIG_OF) && parent->of_node && cell->of_compatible) {
-		for_each_child_of_node(parent->of_node, np) {
+	if (IS_ENABLED(CONFIG_OF)) {
+		const struct device_node *parent_of_node;
+
+		parent_of_node = cell->parent_of_node ?: parent->of_node;
+		if (!parent_of_node || !cell->of_compatible)
+			goto skip_of;
+
+		for_each_child_of_node(parent_of_node, np) {
 			if (of_device_is_compatible(np, cell->of_compatible)) {
 				/* Skip 'disabled' devices */
 				if (!of_device_is_available(np)) {
@@ -213,6 +219,7 @@ static int mfd_add_device(struct device *parent, int id,
 				cell->name, platform_id);
 	}
 
+skip_of:
 	mfd_acpi_add_device(cell, pdev);
 
 	if (cell->pdata_size) {
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index faeea7abd688..2e94ea376125 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -81,6 +81,13 @@ struct mfd_cell {
 	/* Software node for the device. */
 	const struct software_node *swnode;
 
+	/*
+	 * Parent OF node of the device, if different from the OF node
+	 * of the MFD parent (e.g. there is at least one more hierarchical
+	 * level between them)
+	 */
+	const struct device_node *parent_of_node;
+
 	/*
 	 * Device Tree compatible string
 	 * See: Documentation/devicetree/usage-model.rst Chapter 2.2 for details
-- 
2.34.1
Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Lee Jones 1 week, 4 days ago
On Tue, 18 Nov 2025, Vladimir Oltean wrote:

> I would like the "nxp,sja1110a" driver, in the configuration below, to
> be able to probe the drivers for "nxp,sja1110-base-t1-mdio" and for
> "nxp,sja1110-base-tx-mdio" via mfd_add_devices():
> 
> 	ethernet-switch@0 {
> 		compatible = "nxp,sja1110a";
> 
> 		mdios {
> 			mdio@0 {
> 				compatible = "nxp,sja1110-base-t1-mdio";
> 			};
> 
> 			mdio@1 {
> 				compatible = "nxp,sja1110-base-tx-mdio";
> 			};
> 		};
> 	};

This device is not an MFD.

Please find a different way to instantiate these network drivers.

> This isn't currently possible, because mfd assumes that the parent
> OF node ("mdios") == OF node of the parent ("ethernet-switch@0"), which
> in this case isn't true, and as it searches through the children of
> "ethernet-switch@0", it finds no MDIO bus to probe.
> 
> Cc: Lee Jones <lee@kernel.org>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/mfd/mfd-core.c   | 11 +++++++++--
>  include/linux/mfd/core.h |  7 +++++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 7d14a1e7631e..e0b7f93a2654 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -181,8 +181,14 @@ static int mfd_add_device(struct device *parent, int id,
>  	if (ret < 0)
>  		goto fail_res;
>  
> -	if (IS_ENABLED(CONFIG_OF) && parent->of_node && cell->of_compatible) {
> -		for_each_child_of_node(parent->of_node, np) {
> +	if (IS_ENABLED(CONFIG_OF)) {
> +		const struct device_node *parent_of_node;
> +
> +		parent_of_node = cell->parent_of_node ?: parent->of_node;
> +		if (!parent_of_node || !cell->of_compatible)
> +			goto skip_of;
> +
> +		for_each_child_of_node(parent_of_node, np) {
>  			if (of_device_is_compatible(np, cell->of_compatible)) {
>  				/* Skip 'disabled' devices */
>  				if (!of_device_is_available(np)) {
> @@ -213,6 +219,7 @@ static int mfd_add_device(struct device *parent, int id,
>  				cell->name, platform_id);
>  	}
>  
> +skip_of:
>  	mfd_acpi_add_device(cell, pdev);
>  
>  	if (cell->pdata_size) {
> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> index faeea7abd688..2e94ea376125 100644
> --- a/include/linux/mfd/core.h
> +++ b/include/linux/mfd/core.h
> @@ -81,6 +81,13 @@ struct mfd_cell {
>  	/* Software node for the device. */
>  	const struct software_node *swnode;
>  
> +	/*
> +	 * Parent OF node of the device, if different from the OF node
> +	 * of the MFD parent (e.g. there is at least one more hierarchical
> +	 * level between them)
> +	 */
> +	const struct device_node *parent_of_node;
> +
>  	/*
>  	 * Device Tree compatible string
>  	 * See: Documentation/devicetree/usage-model.rst Chapter 2.2 for details
> -- 
> 2.34.1
> 

-- 
Lee Jones [李琼斯]
Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Vladimir Oltean 1 week, 4 days ago
On Thu, Nov 20, 2025 at 02:41:36PM +0000, Lee Jones wrote:
> On Tue, 18 Nov 2025, Vladimir Oltean wrote:
> 
> > I would like the "nxp,sja1110a" driver, in the configuration below, to
> > be able to probe the drivers for "nxp,sja1110-base-t1-mdio" and for
> > "nxp,sja1110-base-tx-mdio" via mfd_add_devices():
> > 
> > 	ethernet-switch@0 {
> > 		compatible = "nxp,sja1110a";
> > 
> > 		mdios {
> > 			mdio@0 {
> > 				compatible = "nxp,sja1110-base-t1-mdio";
> > 			};
> > 
> > 			mdio@1 {
> > 				compatible = "nxp,sja1110-base-tx-mdio";
> > 			};
> > 		};
> > 	};
> 
> This device is not an MFD.
> 
> Please find a different way to instantiate these network drivers.

Ok.. but what is an MFD? I'm seriously interested in a definition.

One data point: the VSC7512 (driver in drivers/mfd/ocelot-spi.c,
bindings in Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml) is
almost the same class of hardware (except the embedded Cortex-M7 in
SJA1110 can't run Linux, and the CPU in VSC7512 can). It instantiates
MDIO bus children, like this patch proposes too, except it works with a
different device tree hierarchy which I need to adapt to, without breaking.
Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Lee Jones 1 week, 3 days ago
On Thu, 20 Nov 2025, Vladimir Oltean wrote:

> On Thu, Nov 20, 2025 at 02:41:36PM +0000, Lee Jones wrote:
> > On Tue, 18 Nov 2025, Vladimir Oltean wrote:
> > 
> > > I would like the "nxp,sja1110a" driver, in the configuration below, to
> > > be able to probe the drivers for "nxp,sja1110-base-t1-mdio" and for
> > > "nxp,sja1110-base-tx-mdio" via mfd_add_devices():
> > > 
> > > 	ethernet-switch@0 {
> > > 		compatible = "nxp,sja1110a";
> > > 
> > > 		mdios {
> > > 			mdio@0 {
> > > 				compatible = "nxp,sja1110-base-t1-mdio";
> > > 			};
> > > 
> > > 			mdio@1 {
> > > 				compatible = "nxp,sja1110-base-tx-mdio";
> > > 			};
> > > 		};
> > > 	};
> > 
> > This device is not an MFD.
> > 
> > Please find a different way to instantiate these network drivers.
> 
> Ok.. but what is an MFD? I'm seriously interested in a definition.
> 
> One data point: the VSC7512 (driver in drivers/mfd/ocelot-spi.c,
> bindings in Documentation/devicetree/bindings/mfd/mscc,ocelot.yaml) is
> almost the same class of hardware (except the embedded Cortex-M7 in
> SJA1110 can't run Linux, and the CPU in VSC7512 can). It instantiates
> MDIO bus children, like this patch proposes too, except it works with a
> different device tree hierarchy which I need to adapt to, without breaking.

The devices should be different types i.e. be located in different
subsystems.  If you're simply instantiating Watchdog timers, the code
should live solely in drivers/watchdog.  If the devices all pertain to
Networking, the code should live in the Networking subsystem, etc.

MFD is Linuxisum, simply used to split devices up such that each
component can be located it their own applicable subsystem and be
reviewed and maintained by the subject matter experts of those domains.

TL;DR: if your device only deals with Networking, that's where it should
live.  And from there, it should handle its own device registration and
instantiation without reaching into other, non-related subsystems.

-- 
Lee Jones [李琼斯]
Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Vladimir Oltean 1 week, 3 days ago
On Fri, Nov 21, 2025 at 12:06:46PM +0000, Lee Jones wrote:
> MFD is Linuxisum, simply used to split devices up such that each
> component can be located it their own applicable subsystem and be
> reviewed and maintained by the subject matter experts of those domains.

Perfect, so the SJA1110 fits the MFD bill perfectly.

I'm getting the impression that the more I write to explain, the fewer
chances I have for you to read. I'll try to keep things as concise as I
can, but please remember that:
- We've had the exact same discussions with Colin Foster's VSC7512
  work, which you ended up accepting
- This email sent to you in 2022 and again in the other reply on patch 8:
  https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/
  already explains what is the kind of hardware I'm dealing with

> TL;DR: if your device only deals with Networking, that's where it should
> live.  And from there, it should handle its own device registration and
> instantiation without reaching into other, non-related subsystems.

Ok, you make a vague reference which I think I understand the point of.

I need to merge the discussion with the one from patch 8:
https://lore.kernel.org/netdev/20251121120037.GA1117685@google.com/
where you say:

| Another more recent avenue you may explore is the Auxiliary Bus.

Excerpt from documentation here:
https://docs.kernel.org/driver-api/auxiliary_bus.html

  When Should the Auxiliary Bus Be Used

  (...)
  The emphasis here is on a common generic interface that keeps subsystem
  customization out of the bus infrastructure.
  (...)
  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.

The thesis I need to defend is that the SJA1110 usage is 100% fit for
MFD and 0% auxiliary bus. In order to explain it I have to give a bit of
history on DSA.

DSA is a Linuxism for managing Ethernet switches. Key thing is they are
a hardware IP with registers to configure them. There are many ways
to integrate an Ethernet switch hardware IP in a chip that you sell.
You can (a) sell the IP itself for SoC vendors to put in their address
space and access using MMIO, or you can (b) sell them an entire chip
with the switch IP in it, that they access over a bus like PCIe, SPI,
I2C, MDIO, whatever, and integrate with their existing Linux SoC.

DSA has started from a place where it didn't really understand that its
core domain of expertise was the Ethernet switching IP itself. The first
devices it supported were all of the (b) kind, discrete chips on buses.
Thus, many drivers were written where DSA takes charge of the struct
spi_device, mdio_device, i2c_client etc.

These early drivers are simplistic, they configure the switch to pass
traffic, and the PHYs through the internal MDIO bus to establish a link,
and voila! They pass traffic, they're good to go.

Then you start to want to develop these further. You want to avoid
polling PHYs for link status every second.. well, you find there's an
interrupt controller in that chip too, that you should be using with
irqchip. You want to read the chip's temperature to prevent it from
overheating - you find temperature sensors too, for which you register
with hwmon. You find reset blocks, clock generation blocks, power
management blocks, GPIO controllers, what have you.

See, the more you look at the datasheet, the more you start to notice
an entire universe of hardware IPs, and then.. you notice a microcontroller!
Those hardware IPs are all also memory-mapped in the address space of
that microcontroller, and when you from Linux are accessing them, you're
just going through a SPI-to-AHB bridge.

Things become really shitty when the DSA chip that you want to drive
from drivers/net/dsa has a full-blown microprocessor capable of running
Linux instead of that microcontroller! Then you have to support driving
the same switch from the small Linux, using MMIO, or from the big Linux,
over SPI.

Out of a lack of expressivity that we as engineers have, we call both
the SoC at large "a switch", and the switching IP "a switch". Hell,
we even call the rack-mounted pizza box computer with many ports "a switch",
no wonder nobody understands anything! We just name things after the
most important thing that's in them.

So unwind 100 steps from the rabbit hole and ask: what does DSA concern
itself with?

Ideally, the answer is "the Ethernet switch IP". This is always mapped
in somebody's address space from address X to Y.

Practically, legacy makes it that DSA concerns itself with the entire
address space of SPI devices, MDIO devices, I2C devices etc. If you
imagine a microprocessor in these discrete chips (which is architecturally
almost always possible), the device tree of that would only describe a
single region with the Ethernet switching IP, and Linux would probe a
platform device for that. The rest is.. other stuff (of various degrees
of functional relatedness, but nonetheless, other stuff) with *other*
platform devices.

So, DSA is in a process of trying to define a conversion model that is
coherent, compatible with the past, minimal in its description,
applicable to other devices, and not a pain in the butt.

Fact of the matter is, we will always clash with the MFD maintainer in
this process, and it simply doesn't scale for us to keep repeating the
same stuff over and over. It is just too much friction. We went through
this once, with Colin Foster who added the Microchip VSC7512 as MFD
through your tree, and that marked the first time when a DSA driver over
a SPI device concerned itself with just the switching IP, using MFD as
the abstraction layer.

The NXP SJA1110 is just another step in that journey, but this one is
harder because it has legacy device tree bindings to maintain. However,
if we are to accept that Colin Foster's work was not an architectural
mistake, then the SJA1110 is not the end of the road either, and you
have to be prepared for more devices to come and do the same thing.

So practically speaking, the fact that DSA has these particular needs
is just a fact. Treat the above description as a "global prompt", if you
will :)

So why not the auxiliary bus? That creates auxiliary_device structures,
which are fake things that some core device wants to keep out to make
things leaner. But what we want is a platform_device, because that is
the common denominator between what kind of drivers the "small Linux"
and the "big Linux" would use for the same hardware IPs. MFD gives us
exactly that, and regmap provides the abstraction between MMIO and SPI.

================================================================

The above was the "global prompt" that you need to have in your context,
now let's return to the patch at hand.

SJA1110 is *not* capable of running Linux inside. This allows us to get
away with partial conversions, where the DSA driver still remains in
charge of the entire SPI device, but delegates the other stuff to MFD.

The existing bindings cannot be broken. Hindsight is 20/20, but whatever
stupid decisions I made in the past with this "mdios" container node are
there to stay.
Re: [PATCH net-next 07/15] mfd: core: add ability for cells to probe on a custom parent OF node
Posted by Lee Jones 5 days, 16 hours ago
I acknowledge receipt of your mail, but I need to get my list of reviews
down to a sensible number before fully processing it.  Please stand-by.

/ Lee

> On Fri, Nov 21, 2025 at 12:06:46PM +0000, Lee Jones wrote:
> > MFD is Linuxisum, simply used to split devices up such that each
> > component can be located it their own applicable subsystem and be
> > reviewed and maintained by the subject matter experts of those domains.
> 
> Perfect, so the SJA1110 fits the MFD bill perfectly.
> 
> I'm getting the impression that the more I write to explain, the fewer
> chances I have for you to read. I'll try to keep things as concise as I
> can, but please remember that:
> - We've had the exact same discussions with Colin Foster's VSC7512
>   work, which you ended up accepting
> - This email sent to you in 2022 and again in the other reply on patch 8:
>   https://lore.kernel.org/lkml/20221222134844.lbzyx5hz7z5n763n@skbuf/
>   already explains what is the kind of hardware I'm dealing with
> 
> > TL;DR: if your device only deals with Networking, that's where it should
> > live.  And from there, it should handle its own device registration and
> > instantiation without reaching into other, non-related subsystems.
> 
> Ok, you make a vague reference which I think I understand the point of.
> 
> I need to merge the discussion with the one from patch 8:
> https://lore.kernel.org/netdev/20251121120037.GA1117685@google.com/
> where you say:
> 
> | Another more recent avenue you may explore is the Auxiliary Bus.
> 
> Excerpt from documentation here:
> https://docs.kernel.org/driver-api/auxiliary_bus.html
> 
>   When Should the Auxiliary Bus Be Used
> 
>   (...)
>   The emphasis here is on a common generic interface that keeps subsystem
>   customization out of the bus infrastructure.
>   (...)
>   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.
> 
> The thesis I need to defend is that the SJA1110 usage is 100% fit for
> MFD and 0% auxiliary bus. In order to explain it I have to give a bit of
> history on DSA.
> 
> DSA is a Linuxism for managing Ethernet switches. Key thing is they are
> a hardware IP with registers to configure them. There are many ways
> to integrate an Ethernet switch hardware IP in a chip that you sell.
> You can (a) sell the IP itself for SoC vendors to put in their address
> space and access using MMIO, or you can (b) sell them an entire chip
> with the switch IP in it, that they access over a bus like PCIe, SPI,
> I2C, MDIO, whatever, and integrate with their existing Linux SoC.
> 
> DSA has started from a place where it didn't really understand that its
> core domain of expertise was the Ethernet switching IP itself. The first
> devices it supported were all of the (b) kind, discrete chips on buses.
> Thus, many drivers were written where DSA takes charge of the struct
> spi_device, mdio_device, i2c_client etc.
> 
> These early drivers are simplistic, they configure the switch to pass
> traffic, and the PHYs through the internal MDIO bus to establish a link,
> and voila! They pass traffic, they're good to go.
> 
> Then you start to want to develop these further. You want to avoid
> polling PHYs for link status every second.. well, you find there's an
> interrupt controller in that chip too, that you should be using with
> irqchip. You want to read the chip's temperature to prevent it from
> overheating - you find temperature sensors too, for which you register
> with hwmon. You find reset blocks, clock generation blocks, power
> management blocks, GPIO controllers, what have you.
> 
> See, the more you look at the datasheet, the more you start to notice
> an entire universe of hardware IPs, and then.. you notice a microcontroller!
> Those hardware IPs are all also memory-mapped in the address space of
> that microcontroller, and when you from Linux are accessing them, you're
> just going through a SPI-to-AHB bridge.
> 
> Things become really shitty when the DSA chip that you want to drive
> from drivers/net/dsa has a full-blown microprocessor capable of running
> Linux instead of that microcontroller! Then you have to support driving
> the same switch from the small Linux, using MMIO, or from the big Linux,
> over SPI.
> 
> Out of a lack of expressivity that we as engineers have, we call both
> the SoC at large "a switch", and the switching IP "a switch". Hell,
> we even call the rack-mounted pizza box computer with many ports "a switch",
> no wonder nobody understands anything! We just name things after the
> most important thing that's in them.
> 
> So unwind 100 steps from the rabbit hole and ask: what does DSA concern
> itself with?
> 
> Ideally, the answer is "the Ethernet switch IP". This is always mapped
> in somebody's address space from address X to Y.
> 
> Practically, legacy makes it that DSA concerns itself with the entire
> address space of SPI devices, MDIO devices, I2C devices etc. If you
> imagine a microprocessor in these discrete chips (which is architecturally
> almost always possible), the device tree of that would only describe a
> single region with the Ethernet switching IP, and Linux would probe a
> platform device for that. The rest is.. other stuff (of various degrees
> of functional relatedness, but nonetheless, other stuff) with *other*
> platform devices.
> 
> So, DSA is in a process of trying to define a conversion model that is
> coherent, compatible with the past, minimal in its description,
> applicable to other devices, and not a pain in the butt.
> 
> Fact of the matter is, we will always clash with the MFD maintainer in
> this process, and it simply doesn't scale for us to keep repeating the
> same stuff over and over. It is just too much friction. We went through
> this once, with Colin Foster who added the Microchip VSC7512 as MFD
> through your tree, and that marked the first time when a DSA driver over
> a SPI device concerned itself with just the switching IP, using MFD as
> the abstraction layer.
> 
> The NXP SJA1110 is just another step in that journey, but this one is
> harder because it has legacy device tree bindings to maintain. However,
> if we are to accept that Colin Foster's work was not an architectural
> mistake, then the SJA1110 is not the end of the road either, and you
> have to be prepared for more devices to come and do the same thing.
> 
> So practically speaking, the fact that DSA has these particular needs
> is just a fact. Treat the above description as a "global prompt", if you
> will :)
> 
> So why not the auxiliary bus? That creates auxiliary_device structures,
> which are fake things that some core device wants to keep out to make
> things leaner. But what we want is a platform_device, because that is
> the common denominator between what kind of drivers the "small Linux"
> and the "big Linux" would use for the same hardware IPs. MFD gives us
> exactly that, and regmap provides the abstraction between MMIO and SPI.
> 
> ================================================================
> 
> The above was the "global prompt" that you need to have in your context,
> now let's return to the patch at hand.
> 
> SJA1110 is *not* capable of running Linux inside. This allows us to get
> away with partial conversions, where the DSA driver still remains in
> charge of the entire SPI device, but delegates the other stuff to MFD.
> 
> The existing bindings cannot be broken. Hindsight is 20/20, but whatever
> stupid decisions I made in the past with this "mdios" container node are
> there to stay.

-- 
Lee Jones [李琼斯]