[PATCH] mfd: core: Preserve OF node when ACPI handle is present

[PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Brian Mak]

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: stable@vger.kernel.org
Signed-off-by: Brian Mak <makb@juniper.net>
---
 drivers/mfd/mfd-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 6be58eb5a746..3c8b06d52d19 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -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
-- 
2.25.1

Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Andy Shevchenko]

On Wed, Feb 25, 2026 at 03:21:05PM -0800, Brian Mak wrote:
> Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI

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

> 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)?

...

> -	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().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Andy Shevchenko]

On Thu, Feb 26, 2026 at 09:27:50AM +0200, Andy Shevchenko wrote:
> On Wed, Feb 25, 2026 at 03:21:05PM -0800, Brian Mak wrote:
> > Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
> 
> device_set_node()
> ACPI_COMPANION_SET() // but see below.
> 
> > 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));

...

> > -	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().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Brian Mak]

On Feb 25, 2026, at 11:39 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Feb 26, 2026 at 09:27:50AM +0200, Andy Shevchenko wrote:
>> On Wed, Feb 25, 2026 at 03:21:05PM -0800, Brian Mak wrote:
>>> Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
>> 
>> device_set_node()
>> ACPI_COMPANION_SET() // but see below.
>> 
>>> 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.

> 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.

>>> -   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().

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

Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Andy Shevchenko]

On Thu, Feb 26, 2026 at 07:40:30PM +0000, Brian Mak wrote:
> On Feb 25, 2026, at 11:39 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Feb 26, 2026 at 09:27:50AM +0200, Andy Shevchenko wrote:
> >> On Wed, Feb 25, 2026 at 03:21:05PM -0800, Brian Mak wrote:

> >>> Switch device_set_node back to ACPI_COMPANION_SET, so that the ACPI
> >> 
> >> device_set_node()
> >> ACPI_COMPANION_SET() // but see below.
> >> 
> >>> 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.

Yes, that's what I was thinking of.

> > 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.

> >>> -   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().
> 
> 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).

> 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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present

[Brian Mak]

On Feb 26, 2026, at 12:22 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

>>> 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.
> 
>>>>> -   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().
>> 
>> 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).
> 
>> 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.

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