| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAMRc=Md=r7GaO3A_7de+EqzboyA2cqNSTZx7+64VSMvRBb9gpw@mail.gmail.com>
Date: Mon, 3 Nov 2025 11:36:36 +0100
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Linus Walleij <linus.walleij@...aro.org>, Daniel Scally <djrscally@...il.com>,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
Danilo Krummrich <dakr@...nel.org>, Philipp Zabel <p.zabel@...gutronix.de>,
Krzysztof Kozlowski <krzk@...nel.org>, linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-acpi@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v4 03/10] software node: allow referencing firmware nodes
On Mon, Nov 3, 2025 at 10:49 AM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> On Mon, Nov 03, 2025 at 10:35:23AM +0100, Bartosz Golaszewski wrote:
> >
> > At the moment software nodes can only reference other software nodes.
> > This is a limitation for devices created, for instance, on the auxiliary
> > bus with a dynamic software node attached which cannot reference devices
> > the firmware node of which is "real" (as an OF node or otherwise).
> >
> > Make it possible for a software node to reference all firmware nodes in
> > addition to static software nodes. To that end: add a second pointer to
> > struct software_node_ref_args of type struct fwnode_handle. The core
> > swnode code will first check the swnode pointer and if it's NULL, it
> > will assume the fwnode pointer should be set. Rework the helper macros
> > and deprecate the existing ones whose names don't indicate the reference
> > type.
> >
> > Software node graphs remain the same, as in: the remote endpoints still
> > have to be software nodes.
>
> ...
>
> > + /*
> > + * A software node can reference other software nodes or firmware
> > + * nodes (which are the abstraction layer sitting on top of them).
> > + * This is done to ensure we can create references to static software
> > + * nodes before they're registered with the firmware node framework.
> > + * At the time the reference is being resolved, we expect the swnodes
> > + * in question to already have been registered and to be backed by
> > + * a firmware node. This is why we use the fwnode API below to read the
>
> A nit-pick (since anyway it requires a new version): move 'the' to the next
> line to make them more equal in the length.
>
> > + * relevant properties and bump the reference count.
> > + */
>
> ...
>
> > -#define SOFTWARE_NODE_REFERENCE(_ref_, ...) \
> > +#define __SOFTWARE_NODE_REF(_ref, ...) \
>
> No, NAK. The renaming of the parameters is not related to this change _at all_.
> Why do you change established style here? Did I miss your answer to my question
> in the previous rounds?
>
Ah, my brain just filtered out the trailing '_'.
> > (const struct software_node_ref_args) { \
> > - .node = _ref_, \
> > + .swnode = _Generic(_ref, \
> > + const struct software_node *: _ref, \
> > + default: NULL), \
> > + .fwnode = _Generic(_ref, \
> > + struct fwnode_handle *: _ref, \
> > + default: NULL), \
> > .nargs = COUNT_ARGS(__VA_ARGS__), \
> > .args = { __VA_ARGS__ }, \
> > }
>
> ...
>
> > +#define SOFTWARE_NODE_REF_SWNODE(_ref, ...) \
> > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > +
> > +#define SOFTWARE_NODE_REF_FWNODE(_ref, ...) \
> > + __SOFTWARE_NODE_REF(_ref, __VA_ARGS__)
> > +
> > +/* DEPRECATED, use SOFTWARE_NODE_REF_SWNODE() instead. */
> > +#define SOFTWARE_NODE_REFERENCE(_ref, ...) \
> > + SOFTWARE_NODE_REF_SWNODE(_ref, __VA_ARGS__)
>
> Now, useless.
>
No, why? With these changes, SOFTWARE_NODE_REFERENCE()'s name is a bit
misleading or incomplete, so I'm proposing to start replacing it with
SOFTWARE_NODE_REF_SWNODE() which is compatible with the former but has
a better name.
> ...
>
>
> > -#define PROPERTY_ENTRY_REF(_name_, _ref_, ...) \
> > +#define __PROPERTY_ENTRY_REF(_type, _name, _ref, ...) \
> > (struct property_entry) { \
> > - .name = _name_, \
> > + .name = _name, \
> > .length = sizeof(struct software_node_ref_args), \
> > .type = DEV_PROP_REF, \
> > - { .pointer = &SOFTWARE_NODE_REFERENCE(_ref_, ##__VA_ARGS__), }, \
> > + { .pointer = &_type(_ref, ##__VA_ARGS__), }, \
> > }
>
> Do we need this now? I assume that _Generic() takes case of this.
>
Ah, right, it should be done here as well.
> ...
>
> > +#define PROPERTY_ENTRY_REF_SWNODE(_name, _ref, ...) \
> > + __PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_SWNODE, \
> > + _name, _ref, __VA_ARGS__)
> > +
> > +#define PROPERTY_ENTRY_REF_FWNODE(_name, _ref, ...) \
> > + __PROPERTY_ENTRY_REF(SOFTWARE_NODE_REF_FWNODE, \
> > + _name, _ref, __VA_ARGS__)
> > +
> > +/* DEPRECATED, use PROPERTY_ENTRY_REF_SWNODE() instead. */
> > +#define PROPERTY_ENTRY_REF(_name, _ref, ...) \
> > + PROPERTY_ENTRY_REF_SWNODE(_name, _ref, __VA_ARGS__)
>
> Seems like useless churn.
>
This is the same argument as with SOFTWARE_NODE_REF_SWNODE(). It's not
clear from the name what PROPERTY_ENTRY_REF() is really referencing.
Bart
Powered by blists - more mailing lists