From: Peter Rosin <peda@lysator.liu.se>
To: Ahmad Byagowi <ahmadexp@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>,
Andy Shevchenko <andy@kernel.org>,
Andi Shyti <andi.shyti@kernel.org>,
Jakub Kicinski <kuba@kernel.org>,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
Date: Mon, 31 Aug 2026 09:13:34 +0200 [thread overview]
Message-ID: <apUpnqAQQfWaflS8@gryt> (raw)
In-Reply-To: <CAOQiBWN5DgOcJ9Gg7s2GZmpQguJUh121Jn9yUzxxm=D530c8dA@mail.gmail.com>
Den Sat, Aug 29, 2026 at 10:10:40PM -0700, skrev Ahmad Byagowi:
> Peter, thanks for clarifying.
>
> The check is intentional. Without it, the generic traversal would also
> inspect ACPI firmware nodes and could match _DSD child nodes carrying
> a "reg" property, while the existing ACPI path below uses
> acpi_preset_companion() to associate the adapter by channel address.
Yes, I got that. The question is if the precaution is really needed.
Are "reg" properties common in ACPI nodes?
> I have not established that changing the ACPI lookup behavior is safe,
> and that change is not needed for software-node support. With the
> check retained, OF and software nodes use the generic lookup, while
> ACPI continues to use the existing acpi_preset_companion() path
> exclusively.
Yes, I figured as much, which is why I directed the question at Andy.
> That said, I will clarify this in the commit message for the next revision.
I'd rather tweak the check to exclude ACPI (instead of including OF and
SW) with a code comment about why ACPI is excluded. That is, if it can
be determined that ACPI actually needs to be excluded. If it is hard to
get an answer to that question, I'm inclined to just attempt to not
exclude ACPI, and then be prepared to exclude ACPI in a follow-up if
there is any fallout.
Cheers,
Peter
>
> Regards,
> Ahmad
>
>
> On Sat, Aug 29, 2026 at 9:55 PM Peter Rosin <peda@lysator.liu.se> wrote:
> >
> > Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> > > On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> > > > Hi Peter, Andy,
> > > >
> > > > Yes, software-node handling is needed for the ptp_ocp use case.
> > >
> > > That driver is a mess. I'm surprised nobody told to the authors of
> > > the respective changes to look at the auxiliary implementation.
> > >
> > > > ptp_ocp is a PCI driver. It creates its board-specific I2C topology at
> > > > runtime with software nodes: the mux, its channel nodes, sensors, and LED
> > > > controller. Firmware does not provide ACPI nodes for this topology.
> > > >
> > > > The existing acpi_preset_companion() path only associates a mux adapter
> > > > with an existing ACPI child. It does not associate the adapter with one of
> > > > these dynamically created software-node channel nodes. Without that
> > > > association, i2c_get_adapter_by_fwnode() cannot find the channel adapter by
> > > > the channel software node, so ptp_ocp cannot instantiate the downstream I2C
> > > > devices on the correct channel.
> > > >
> > > > Does this address the concern, or would you prefer a different way to
> > > > represent this dynamically created topology?
> > >
> > > Wouldn't it be possible to use some kind of DT overlay to have that?
> >
> > For me, the above is a bit unrelated to this patch series, which is
> > about converting i2c-mux from of-only properties to device properties.
> > That seems like a change that stands on its own.
> >
> > I probably wasn't clear enough with my original question, but what I
> > wondered about was what regression risk that conversion might have
> > for the ACPI case. Specifically, there might be ACPI properties that
> > match what the code is now looking for. It seems unlikely that such
> > properties are actually deployed, but I know next to nothing about
> > ACPI...
> >
> > TL;DR
> >
> > My original question should have been: Is it safe to simply remove
> > these lines from the patch:
> > > + if (!is_of_node(dev_node) && !is_software_node(dev_node))
> > > + return NULL;
> > and let the code trawl all kinds of device properties?
> >
> > Sorry for the confusion...
> >
> > Cheers,
> > Peter
> >
> > > > Thanks, Andi, for taking the first patch into i2c/i2c-fixes. I will drop it
> > > > from the next revision and rebase the remaining two patches once this
> > > > question is resolved.
> > >
> > > > On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@lysator.liu.se> wrote:
> > > > > Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:
> > > > > > Device Tree channel nodes are associated with the adapters created by
> > > > > > i2c-mux, but equivalent software-node descriptions are not.
> > > > > >
> > > > > > Use generic firmware-node operations for the existing channel lookup and
> > > > > > accept either an OF node or a software node. Associate the returned node
> > > > > > with the adapter so child I2C devices can be instantiated from
> > > > > > software-node properties.
> > > > > >
> > > > > > Save the adapter firmware node before adapter deletion and release the
> > > > > > reference afterwards, following the lifetime pattern in i2c-atr.
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> > >
> > >
>
>
>
> --
> 73
> With best wishes / Mit herzlichsten Grüßen
> Ahmad Byagowi, Ph.D., Dr. Techn., P.Eng.
> Phone: +1 (650) 924 6653
>
> Please consider the environment before printing this e-mail.
next prev parent reply other threads:[~2026-08-31 7:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-23 16:34 [PATCH v7 0/3] " Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
2026-08-25 5:49 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 2/3] i2c: mux: Factor out channel node lookup Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
[not found] ` <CAOQiBWNVvHpO7X8yWyZEa3-nV76YJfRbkzo+_4U-upEj4VNfow@mail.gmail.com>
2026-08-26 16:39 ` Ahmad Byagowi
2026-08-27 6:11 ` Andy Shevchenko
2026-08-30 4:55 ` Peter Rosin
2026-08-30 5:10 ` Ahmad Byagowi
2026-08-31 7:13 ` Peter Rosin [this message]
2026-08-31 7:28 ` Andy Shevchenko
2026-08-31 7:22 ` Andy Shevchenko
[not found] ` <CAOQiBWOZd+NVRqzRzJx9HP+B21JJXBUTy_OxAqDw8X7tvWeY+w@mail.gmail.com>
2026-08-31 7:25 ` Andy Shevchenko
2026-08-26 13:45 ` [PATCH v7 0/3] " Andi Shyti
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=apUpnqAQQfWaflS8@gryt \
--to=peda@lysator.liu.se \
--cc=ahmadexp@gmail.com \
--cc=andi.shyti@kernel.org \
--cc=andriy.shevchenko@intel.com \
--cc=andy@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/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
all inboxes | Powered by JetHome®