Re: [PATCH] mfd: core: Preserve OF node when ACPI handle is present
From: Brian Mak
Date: Thu Feb 26 2026 - 14:40:50 EST
On Feb 25, 2026, at 11:39 PM, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> 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