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@xxxxxxxxxxxxxxx> 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