public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
From: "Jiawen Wu" <jiawenwu@trustnetic.com>
To: "'Bartosz Golaszewski'" <brgl@kernel.org>
Cc: "'Bartosz Golaszewski'" <bartosz.golaszewski@linaro.org>,
	<andriy.shevchenko@linux.intel.com>, <andy@kernel.org>,
	<broonie@kernel.org>, <ckeepax@opensource.cirrus.com>,
	<dakr@kernel.org>, <david.rhodes@cirrus.com>,
	<djrscally@gmail.com>, <gregkh@linuxfoundation.org>,
	<heikki.krogerus@linux.intel.com>, <krzk@kernel.org>,
	<linus.walleij@linaro.org>, <linux-acpi@vger.kernel.org>,
	<linux-gpio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-sound@vger.kernel.org>, <linux-spi@vger.kernel.org>,
	<mstrozek@opensource.cirrus.com>, <p.zabel@pengutronix.de>,
	<patches@opensource.cirrus.com>, <rafael@kernel.org>,
	<rf@opensource.cirrus.com>, <sakari.ailus@linux.intel.com>
Subject: RE: [PATCH v7 3/9] software node: allow referencing firmware nodes
Date: Tue, 23 Dec 2025 17:48:22 +0800	[thread overview]
Message-ID: <030101dc73f1$46a62b40$d3f281c0$@trustnetic.com> (raw)
In-Reply-To: <CAMRc=Meugd9tEDefPnYHidDMTdCP+8fptVXNvqjSi1tjXPuVRA@mail.gmail.com>

On Tue, Dec 23, 2025 5:37 PM, Bartosz Golaszewski wrote:
> On Tue, Dec 23, 2025 at 9:44 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:
> >
> > On Tue, Dec 23, 2025 4:09 PM, Bartosz Golaszewski wrote:
> > > On Tue, Dec 23, 2025 at 8:39 AM Jiawen Wu <jiawenwu@trustnetic.com> wrote:
> > > >
> > > > >
> > > > >  #define SOFTWARE_NODE_REFERENCE(_ref_, ...)                  \
> > > > >  (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__ },                                \
> > > > >  }
> > > >
> > > > This change seems incompatible with my driver txgbe, since the software nodes
> > > > are registered in " struct software_node * " but not " const struct software_node * ".
> > > >
> > > > So when I pulled the net-next-6.19-rc1 that merged this patch, to probe my driver.
> > > > The error logs shows:
> > > >
> > > > [    5.243396] txgbe 0000:10:00.0 (unnamed net_device) (uninitialized): unable to attach SFP bus: -EINVAL
> > > > [    5.243399] txgbe 0000:10:00.0: failed to init phylink
> > > > [    5.576008] txgbe 0000:10:00.0: probe with driver txgbe failed with error -22
> > > > [    6.109548] txgbe 0000:10:00.1 (unnamed net_device) (uninitialized): unable to attach SFP bus: -EINVAL
> > > > [    6.109551] txgbe 0000:10:00.1: failed to init phylink
> > > > [    6.442044] txgbe 0000:10:00.1: probe with driver txgbe failed with error -22
> > > >
> > >
> > > This shouldn't have changed anything for existing software nodes - the
> > > pointer in struct software_node_ref_args has always been const. This
> > > would have failed at build-time if this was an issue. Have you
> > > bisected it to this very commit? What line is the -EINVAL assigned and
> > > for what reason?
> >
> > The -EINVAL is assigned software_node_get_reference_args():
> >
> >         if (ref->swnode)
> >                 refnode = software_node_fwnode(ref->swnode);
> >         else if (ref->fwnode)
> >                 refnode = ref->fwnode;
> >         else
> >                 return -EINVAL;
> >
> > I think the reason is that _Generic selection is restrictive, it only accept
> > exactly const struct software_node for software node references. So the
> > macro SOFTWARE_NODE_REFERENCE fall back to the default to set .swnode =  NULL.
> >
> > And I temporarily added this line to fix it:
> >
> > diff --git a/include/linux/property.h b/include/linux/property.h
> > index 272bfbdea7bf..e30ef23a9af3 100644
> > --- a/include/linux/property.h
> > +++ b/include/linux/property.h
> > @@ -371,6 +371,7 @@ struct software_node_ref_args {
> >  (const struct software_node_ref_args) {                                \
> >         .swnode = _Generic(_ref_,                               \
> >                            const struct software_node *: _ref_, \
> > +                          struct software_node *: _ref_,       \
> >                            default: NULL),                      \
> >         .fwnode = _Generic(_ref_,                               \
> >                            struct fwnode_handle *: _ref_,       \
> >
> 
> Ah I see, we'd assign struct software_node * to const struct
> software_node * and it used to work but with _Generic() we need the
> exact type. I agree with this approach, do you want to send a proper
> patch?

It might be more appropriate for you to send the patch, and could also
check if there are any other missed details, like for fwnode...
I'm not very proficient in this field. :)



  reply	other threads:[~2025-12-23  9:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-23  7:39 [PATCH v7 3/9] software node: allow referencing firmware nodes Jiawen Wu
2025-12-23  8:09 ` Bartosz Golaszewski
2025-12-23  8:44   ` Jiawen Wu
2025-12-23  9:36     ` Bartosz Golaszewski
2025-12-23  9:48       ` Jiawen Wu [this message]
2025-12-23 10:41         ` Danilo Krummrich
2025-12-24  1:59           ` Jiawen Wu
  -- strict thread matches above, loose matches on Subject: below --
2025-11-20 13:23 [PATCH v7 0/9] reset: rework reset-gpios handling Bartosz Golaszewski
2025-11-20 13:23 ` [PATCH v7 3/9] software node: allow referencing firmware nodes Bartosz Golaszewski
2025-11-20 13:27   ` Charles Keepax

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='030101dc73f1$46a62b40$d3f281c0$@trustnetic.com' \
    --to=jiawenwu@trustnetic.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=andy@kernel.org \
    --cc=bartosz.golaszewski@linaro.org \
    --cc=brgl@kernel.org \
    --cc=broonie@kernel.org \
    --cc=ckeepax@opensource.cirrus.com \
    --cc=dakr@kernel.org \
    --cc=david.rhodes@cirrus.com \
    --cc=djrscally@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=krzk@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mstrozek@opensource.cirrus.com \
    --cc=p.zabel@pengutronix.de \
    --cc=patches@opensource.cirrus.com \
    --cc=rafael@kernel.org \
    --cc=rf@opensource.cirrus.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox