[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=Me-DXN9kx+5bqDb9doMG9MX2EiRJiC=_QqDc0q3gOz8wA@mail.gmail.com>
Date: Thu, 20 Nov 2025 10:12:39 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
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 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.
> > 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?
> > > > 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.
> > 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.
Look at all the find_device_by_fwnode() functions we have everywhere.
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.
Bart
Powered by blists - more mailing lists