Message-ID: <aR7luqqew3LRwFgn@smile.fi.intel.com>
Date: Thu, 20 Nov 2025 11:56:10 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Andy Shevchenko <andy.shevchenko@...il.com>,
	Charles Keepax <ckeepax@...nsource.cirrus.com>,
	David Rhodes <david.rhodes@...rus.com>,
	Richard Fitzgerald <rf@...nsource.cirrus.com>,
	Lee Jones <lee@...nel.org>, Mark Brown <broonie@...nel.org>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	Maciej Strozek <mstrozek@...nsource.cirrus.com>,
	Andy Shevchenko <andy@...nel.org>, linux-sound@...r.kernel.org,
	patches@...nsource.cirrus.com, linux-kernel@...r.kernel.org,
	linux-spi@...r.kernel.org,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH RFT/RFC] mfd: cs42l43: setup true links with software
 nodes

On Thu, Nov 20, 2025 at 10:12:39AM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 19, 2025 at 9:38 PM Andy Shevchenko
> <andriy.shevchenko@...ux.intel.com> wrote:
> > On Wed, Nov 19, 2025 at 03:30:46PM +0100, Bartosz Golaszewski wrote:
> > > On Wed, Nov 19, 2025 at 3:24 PM Andy Shevchenko
> > > <andy.shevchenko@...il.com> wrote:

...

> > > > > > My idea was to mark the fwnode with __private and fix the (ab)users,
> > > > > > should not be so many. Can somebody mock up a coccinelle script to
> > > > > > find all dereferences of fwnode from struct device?
> > > > >
> > > > > I think you're underestimating the level of complexity. What about the
> > > > > concept of dev_fwnode()? It literally makes no sense if we switch to a
> > > > > list of fwnodes.
> > > >
> > > > Why not? It will return the pointer to the primary node. You can look
> > > > for example how the list of the DMA descriptors is done in
> > > > drivers/dma/dw/core.c. Not the best solution, but gives you an idea of
> > > > how it may look.
> > >
> > > What even is a primary node though? You can have auxiliary devices
> > > without an ACPI or OF node.
> >
> > The one which gives the main set of the properties for the device.
> > The devices that don't have it, simply have a list of fwnodes empty.
> 
> Well, what *is* the *main* set of properties? Who declares that? I
> agree, it's easy for device-tree - the OF node is the main one, but
> then what if a sub-device inherits the OF node of the parent while we
> also add a software node? It's not that straightforward and we'll run
> into issues for sure just like what we're seeing now with secondary
> fwnodes.

First rule is that swnode NEVER is the "main" property provider.
But it could be a single property provider.
The rest becomes easier after this.

> > > Only with software nodes. Which one is the
> > > primary? The first one we add?
> >
> > Yes. The problem here might be if we add the SW node before the actual FW node
> > appear (it can be if we created some devices before the actual FW based
> > enumeration happens. It would probably need to have some kind of weight
> > (or priority value) and list should be sorted based on that number.
> 
> Ugh, please no. Firmware node priority?? With magic numbers?

Why not? Many tools and things rely on weights.
But again, first problem first, the design. The additional property bundles
(as swnodes or OF overlays on ACPI-based systems) can come later on.

> > > > > For it to make sense we'd have to have a kind of "dynamic" firmware
> > > > > node attached to a device which we'd fill with an aggregation of all
> > > > > properties from firmware nodes in the list.
> > > >
> > > > "Dynamic" is just a node in the list. The only potential problem here
> > > > is prioritisation. Should we add to the head, tail or insert? But
> > > > converting current approach will be straightforward.
> > >
> > > What I have in my mind is not another firmware node in the list in
> > > struct device but rather a new firmware node implementation, that
> > > would be assigned to the device via a dedicated pointer and would be
> > > filled with a logical OR of properties from other firmware nodes added
> > > to the list.
> >
> > Oh no, this won't work in corner cases. What if we actually need to "fix"
> > an existing primary node (there were discussions long time ago about inverting
> > primary/secondary in some corner cases, but it didn't appear so far as
> > it most likely will give tons of issues in the _current_ design)?
> 
> What do you mean by "fix"? Like repair? Or fix in place? I'm not following.

When the primary already has a property that needs to be rewritten
(same name, different value). Note, that there are use cases already
for this AFAIR, but probably they were worked around in not too hackish
way or abandoned for now.

> > > Then dev_fwnode() would return this rather than any one
> > > of the firmware nodes from the list. Think of it as the "master
> > > fwnode" of a struct device.
> >
> > fwnode should not be in any relations to the device, I mean when we do fwnodes,
> > we should not assume that it's backing the device. In the idea you shared it
> > won't be possible ("dedicated pointer") in mine it is (just a list of something
> > that may belong to the device, or to another object, doesn't matter).
> 
> That's not true, we expect fwnodes to back real devices all the time.

No. Even OF doesn't do that. OF node can be a separate thing without struct
device being associated with it.

> Look at all the find_device_by_fwnode() functions we have everywhere.

What do you mean, please?

$ git grep -n -w find_device_by_fwnode | wc -l
0

Even if you refer to *_find_device_by_fwnode(), still it's not everywhere,
just in a dozen of modules.

> The crux of the problem Charles identified is the fact that the
> secondary fwnode is a field of struct fwnode_handle and not of a
> struct device. This really doesn't make sense as we see where multiple
> devices use a single "real" fwnode but want to have different
> secondary software nodes.
> 
> Moving the secondary fwnode to struct device would already help a lot
> but if we're doing that then we may as well switch to a list of
> fwnodes.

They all should be an independent entity that can be part of the struct device
but it's not obligatory relation ship.

Again, device may have a backing fwnode, while fwnode might not have a struct
device consumer.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists