mfd: core: Preserve OF node when ACPI handle is present

PATCH Superseded Brian Mak 2 participants 6 messages Feb 25 – Feb 26

Subsystems (2)

Stable branch Multifunction devices (MFD)

lore

Share & tools

lore

Brian Mak

author

2026-02-25 23:32 UTC #0 lore raw

PATCH 1/1 mfd: core: Preserve OF node when ACPI handle is present

Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI fwnode does not overwrite the of_node with NULL.

This allows MFD children with both OF nodes and ACPI handles to have OF nodes again.

Fixes 51e3b257099d ("mfd: core: Make use of device_set_node()")

Cc [email protected]

Signed-off-by Brian Mak <[email protected]>

---
 drivers/mfd/mfd-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/mfd/mfd-core.c +1 -1

index 6be58eb5a746..3c8b06d52d19 100644
@@ -88,7 +88,7 @@ static void mfd_acpi_add_device(const struct mfd_cell *cell,
 		}
 	}
 
-	device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+	ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
 }
 #else
 static inline void mfd_acpi_add_device(const struct mfd_cell *cell,

base-commit: d9d32e5bd5a4e57675f2b70ddf73c3dc5cf44fc2

Signature · 1 line

2.25.1

Andy Shevchenko

2026-02-26 07:27 UTC #1 lore raw

Quoted 1 line · Brian Mak

Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI

device_set_node()
ACPI_COMPANION_SET() // but see below.

Quoted 4 lines

fwnode does not overwrite the of_node with NULL.
This allows MFD children with both OF nodes and ACPI handles to have OF nodes again.

Do you have a real use case? Can you elaborate more (platform, drivers being involved, et cetera)?

...

drivers/mfd/mfd-core.c static void mfd_acpi_add_device(const struct mfd_cell *cell, · line 91

Quoted 2 lines

drivers/mfd/mfd-core.c +1 -1

-	device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+	ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);

As a quick fix this may be fine, but it needs a big FIXME explaining that this is actually a design limitation of fwnode that doesn't allow proper sharing and stacking.

Bouncing back to ACPI_COMPANION_SET() also doesn't feel right as it hides the real thing here, and real thing is the primary/secondary fwnode types that we need to care of. Just call set_primary_fwnode() directly. It helps also to get rid of ACPI_COMPANION_SET() calls where it may be replaced with simple device_set_node().

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Andy Shevchenko

2026-02-26 07:39 UTC #2 lore raw

Quoted 6 lines · Andy Shevchenko

Quoted 1 line · Brian Mak
Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
device_set_node()
ACPI_COMPANION_SET() // but see below.

Quoted 7 lines

Quoted 4 lines
fwnode does not overwrite the of_node with NULL.
This allows MFD children with both OF nodes and ACPI handles to have OF nodes again.
Do you have a real use case? Can you elaborate more (platform, drivers being involved, et cetera)?

Even more thinking on this it looks like a violation of the levels of the fwnodes. The current design was not expecting the ACPI *and* OF node to appear in the list. They both are considered "primary" from the design point of view.

In your case it seems like an OF node comes from likely DT overlay and messes up this logic (although it can be considered 'secondary' semantically). But the proposed patch is not a solution, it's a band-aid.

TL;DR: if there is a use case that worked before (please elaborate), the patch as a quick fix looks okay, but with FIXME added and open coded logic:

	/*
	 * ...description of use case...
	 *
	 * FIXME: The fwnode design doesn't allow proper sharing and stacking.
	 * Eventually this should become some device fwnode API call that will
	 * prepend the list of fwnodes (making ACPI one "primary" in that sense).
	 */
	if (adev)
		set_primary_fwnode(&pdev->dev, acpi_fwnode_handle(adev));
	else if (parent)
		set_primary_fwnode(&pdev->dev, acpi_fwnode_handle(parent));

...

Quoted 12 lines

drivers/mfd/mfd-core.c static void mfd_acpi_add_device(const struct mfd_cell *cell, · line 91
Quoted 2 lines
drivers/mfd/mfd-core.c +1 -1
-	device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+	ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
As a quick fix this may be fine, but it needs a big FIXME explaining that this is actually a design limitation of fwnode that doesn't allow proper sharing and stacking.
Bouncing back to ACPI_COMPANION_SET() also doesn't feel right as it hides the real thing here, and real thing is the primary/secondary fwnode types that we need to care of. Just call set_primary_fwnode() directly. It helps also to get rid of ACPI_COMPANION_SET() calls where it may be replaced with simple device_set_node().

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Brian Mak

author

2026-02-26 19:40 UTC #3 lore raw

Quoted 7 lines · Andy Shevchenko

Quoted 6 lines · Andy Shevchenko
Quoted 1 line · Brian Mak
Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
device_set_node()
ACPI_COMPANION_SET() // but see below.

Quoted 7 lines

Quoted 7 lines
Quoted 4 lines
fwnode does not overwrite the of_node with NULL.
This allows MFD children with both OF nodes and ACPI handles to have OF nodes again.
Do you have a real use case? Can you elaborate more (platform, drivers being involved, et cetera)?

Yes, at HPE Juniper, we have some MFD drivers for some PCIe devices on our x86 platforms that need to read properties from a device tree. These also have ACPI nodes attached to them, which do not have adequate descriptions for the HW.

Quoted 4 lines

Even more thinking on this it looks like a violation of the levels of the fwnodes. The current design was not expecting the ACPI *and* OF node to appear in the list. They both are considered "primary" from the design point of view.

For my reference, is there anything documented/implied that indicates that fwnodes were not designed to be used in such a way. To me, it seems that secondary fwnodes are designed to allow drivers to pull properties when the primary fwnode does not have the property, which is exactly how we're using it.

Quoted 7 lines

Quoted 7 lines
drivers/mfd/mfd-core.c static void mfd_acpi_add_device(const struct mfd_cell *cell, · line 91
Quoted 2 lines
drivers/mfd/mfd-core.c +1 -1
-   device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+   ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
As a quick fix this may be fine, but it needs a big FIXME explaining that this is actually a design limitation of fwnode that doesn't allow proper sharing and stacking.

Quoted 5 lines

Quoted 5 lines
Bouncing back to ACPI_COMPANION_SET() also doesn't feel right as it hides the real thing here, and real thing is the primary/secondary fwnode types that we need to care of. Just call set_primary_fwnode() directly. It helps also to get rid of ACPI_COMPANION_SET() calls where it may be replaced with simple device_set_node().

Sure, I can call set_primary_fwnode directly in v2. My only concern here is with the FIXME comment. To me, it seems like the fwnode API has already allowed for such a case, simply by allowing there to be a secondary fwnode. We have no need for more than a primary and secondary here. Before I add the FIXME, can you elaborate on why you believe we need more than that?

Thanks,
Brian

Andy Shevchenko

2026-02-26 20:22 UTC #4 lore raw

Quoted 14 lines · Brian Mak

Quoted 13 lines · Andy Shevchenko
Quoted 12 lines · Andy Shevchenko
Quoted 1 line · Brian Mak
Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
device_set_node()
ACPI_COMPANION_SET() // but see below.
Quoted 4 lines
fwnode does not overwrite the of_node with NULL.
This allows MFD children with both OF nodes and ACPI handles to have OF nodes again.

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
Do you have a real use case? Can you elaborate more (platform, drivers being involved, et cetera)?
Yes, at HPE Juniper, we have some MFD drivers for some PCIe devices on our x86 platforms that need to read properties from a device tree. These also have ACPI nodes attached to them, which do not have adequate descriptions for the HW.

Yes, that's what I was thinking of.

Quoted 10 lines

Quoted 4 lines
Even more thinking on this it looks like a violation of the levels of the fwnodes. The current design was not expecting the ACPI *and* OF node to appear in the list. They both are considered "primary" from the design point of view.
For my reference, is there anything documented/implied that indicates that fwnodes were not designed to be used in such a way. To me, it seems that secondary fwnodes are designed to allow drivers to pull properties when the primary fwnode does not have the property, which is exactly how we're using it.

OF by definition is _firmware_ node. Secondary (when it was introduced) was only about device properties (today is _software_ node). The concept of using DT overlays on ACPI platforms not new, but was implemented much later after the initial fwnode / unified device property approach. Basically you are (mis)using it due to the design limitations / flaws and historical evolution of the concept of device properties.

Quoted 13 lines

Quoted 7 lines
Quoted 7 lines
drivers/mfd/mfd-core.c static void mfd_acpi_add_device(const struct mfd_cell *cell, · line 91
Quoted 2 lines
drivers/mfd/mfd-core.c +1 -1
-   device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+   ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
As a quick fix this may be fine, but it needs a big FIXME explaining that this is actually a design limitation of fwnode that doesn't allow proper sharing and stacking.
Quoted 5 lines
Quoted 5 lines
Bouncing back to ACPI_COMPANION_SET() also doesn't feel right as it hides the real thing here, and real thing is the primary/secondary fwnode types that we need to care of. Just call set_primary_fwnode() directly. It helps also to get rid of ACPI_COMPANION_SET() calls where it may be replaced with simple device_set_node().

Quoted 5 lines

Sure, I can call set_primary_fwnode directly in v2. My only concern here is with the FIXME comment. To me, it seems like the fwnode API has already allowed for such a case, simply by allowing there to be a secondary fwnode. We have no need for more than a primary and secondary here.

Again, it's allowed technically, but not by design or definition. primary == real firmware node (OF/ACPI)
secondary == (kernel built-in) device properties (software node)

Your case is primary + primary.
Or let's say not-so-primary, but definitely not built-in (a.k.a. secondary).

Quoted 2 lines

Before I add the FIXME, can you elaborate on why you believe we need more than that?

Because tomorrow it might be an ACPI device that uses driver that already has a software node for something and you will want to add DT overlay to it.

Or even more realistic case is the complex device where we have one firmware node and several children which want to share same firmware node with different software nodes (this is the case that may not be realised with the current design).

It's just matter of time when we face the issue in full and some poor guy will have to address that somehow.

What I think of is having a reference to parent and child without limitations of the length of the lists and keeping secondary as a sibling pointer.

This hierarchy will allow to have a tree of fwnodes where one may be present in different lists as a parent and/or sibling. With that we probably may have a tree-like structure with many possible combinations and relationships.

TL;DR: It is not a problem in MFD, it's problem in fwnode current design. FIXME is just to make sure we won't forget this.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Brian Mak

author

2026-02-26 21:15 UTC #5 lore raw

Quoted 61 lines · Andy Shevchenko

Quoted 10 lines
Quoted 4 lines
Even more thinking on this it looks like a violation of the levels of the fwnodes. The current design was not expecting the ACPI *and* OF node to appear in the list. They both are considered "primary" from the design point of view.
For my reference, is there anything documented/implied that indicates that fwnodes were not designed to be used in such a way. To me, it seems that secondary fwnodes are designed to allow drivers to pull properties when the primary fwnode does not have the property, which is exactly how we're using it.
OF by definition is _firmware_ node. Secondary (when it was introduced) was only about device properties (today is _software_ node). The concept of using DT overlays on ACPI platforms not new, but was implemented much later after the initial fwnode / unified device property approach. Basically you are (mis)using it due to the design limitations / flaws and historical evolution of the concept of device properties.
Quoted 13 lines
Quoted 7 lines
Quoted 7 lines
drivers/mfd/mfd-core.c static void mfd_acpi_add_device(const struct mfd_cell *cell, · line 91
Quoted 2 lines
drivers/mfd/mfd-core.c +1 -1
-   device_set_node(&pdev->dev, acpi_fwnode_handle(adev ?: parent));
+   ACPI_COMPANION_SET(&pdev->dev, adev ?: parent);
As a quick fix this may be fine, but it needs a big FIXME explaining that this is actually a design limitation of fwnode that doesn't allow proper sharing and stacking.
Quoted 5 lines
Quoted 5 lines
Bouncing back to ACPI_COMPANION_SET() also doesn't feel right as it hides the real thing here, and real thing is the primary/secondary fwnode types that we need to care of. Just call set_primary_fwnode() directly. It helps also to get rid of ACPI_COMPANION_SET() calls where it may be replaced with simple device_set_node().
Quoted 5 lines
Sure, I can call set_primary_fwnode directly in v2. My only concern here is with the FIXME comment. To me, it seems like the fwnode API has already allowed for such a case, simply by allowing there to be a secondary fwnode. We have no need for more than a primary and secondary here.
Again, it's allowed technically, but not by design or definition. primary == real firmware node (OF/ACPI)
secondary == (kernel built-in) device properties (software node)
Your case is primary + primary.
Or let's say not-so-primary, but definitely not built-in (a.k.a. secondary).
Quoted 2 lines
Before I add the FIXME, can you elaborate on why you believe we need more than that?
Because tomorrow it might be an ACPI device that uses driver that already has a software node for something and you will want to add DT overlay to it.
Or even more realistic case is the complex device where we have one firmware node and several children which want to share same firmware node with different software nodes (this is the case that may not be realised with the current design).
It's just matter of time when we face the issue in full and some poor guy will have to address that somehow.
What I think of is having a reference to parent and child without limitations of the length of the lists and keeping secondary as a sibling pointer.

Quoted 6 lines

This hierarchy will allow to have a tree of fwnodes where one may be present in different lists as a parent and/or sibling. With that we probably may have a tree-like structure with many possible combinations and relationships.
TL;DR: It is not a problem in MFD, it's problem in fwnode current design. FIXME is just to make sure we won't forget this.

Your explanation makes sense. Thanks for clarifying! I was missing the historical context and software node implications. I will raise a v2 with your requested changes.

Best,
Brian