Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
From: Ahmad Byagowi
Date: Wed Aug 26 2026 - 13:06:01 EST
Resending in plain text because my previous reply was rejected by the
mailing lists.
Hi Peter, Andy,
Yes, software-node handling is needed for the ptp_ocp use case.
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?
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.
Thanks,
Ahmad
On Wed, Aug 26, 2026 at 8:29 AM Ahmad Byagowi <ahmadexp@xxxxxxxxx> wrote:
>
> Hi Peter, Andy,
>
> Yes, software-node handling is needed for the ptp_ocp use case.
>
> 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?
>
> 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.
>
> Thanks,
> Ahmad
>
> On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@xxxxxxxxxxxxxx> wrote:
>>
>> Hi!
>>
>> [Adding Andy]
>>
>> 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.
>> >
>> > Signed-off-by: Ahmad Byagowi <ahmadexp@xxxxxxxxx>
>> > ---
>> > drivers/i2c/i2c-mux.c | 45 +++++++++++++++++++++----------------------
>> > 1 file changed, 22 insertions(+), 23 deletions(-)
>> >
>> > diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
>> > index a8b94b97a725..b37d36d3b121 100644
>> > --- a/drivers/i2c/i2c-mux.c
>> > +++ b/drivers/i2c/i2c-mux.c
>> > @@ -25,6 +25,7 @@
>> > #include <linux/kernel.h>
>> > #include <linux/module.h>
>> > #include <linux/of.h>
>> > +#include <linux/property.h>
>> > #include <linux/slab.h>
>> > #include <linux/sysfs.h>
>> >
>> > @@ -264,40 +265,40 @@ static const struct i2c_lock_operations i2c_parent_lock_ops = {
>> > .unlock_bus = i2c_parent_unlock_bus,
>> > };
>> >
>> > -static struct device_node *
>> > +static struct fwnode_handle *
>> > i2c_mux_get_channel_node(struct i2c_mux_core *muxc, u32 chan_id)
>> > {
>> > - struct device_node *dev_node = muxc->dev->of_node;
>> > - struct device_node *mux_node, *child = NULL;
>> > + struct fwnode_handle *dev_node = dev_fwnode(muxc->dev);
>> > + struct fwnode_handle *mux_node, *child = NULL;
>> > u32 reg;
>> > int ret;
>> >
>> > - if (!dev_node)
>> > + if (!is_of_node(dev_node) && !is_software_node(dev_node))
>> > return NULL;
>>
>> I wonder if this is needed? Andy, I seem to recall that you were
>> involved with adding ACPI support? Do you have anything to add?
>>
>> Cheers,
>> Peter
>>
>> >
>> > if (muxc->arbitrator)
>> > - mux_node = of_get_child_by_name(dev_node, "i2c-arb");
>> > + mux_node = fwnode_get_named_child_node(dev_node, "i2c-arb");
>> > else if (muxc->gate)
>> > - mux_node = of_get_child_by_name(dev_node, "i2c-gate");
>> > + mux_node = fwnode_get_named_child_node(dev_node, "i2c-gate");
>> > else
>> > - mux_node = of_get_child_by_name(dev_node, "i2c-mux");
>> > + mux_node = fwnode_get_named_child_node(dev_node, "i2c-mux");
>> >
>> > if (mux_node) {
>> > - /* A "reg" property indicates an old-style DT entry */
>> > - if (!of_property_read_u32(mux_node, "reg", ®)) {
>> > - of_node_put(mux_node);
>> > + /* A "reg" property indicates an old-style firmware entry. */
>> > + if (!fwnode_property_read_u32(mux_node, "reg", ®)) {
>> > + fwnode_handle_put(mux_node);
>> > mux_node = NULL;
>> > }
>> > }
>> >
>> > if (!mux_node)
>> > - mux_node = of_node_get(dev_node);
>> > + mux_node = fwnode_handle_get(dev_node);
>> > else if (muxc->arbitrator || muxc->gate)
>> > - child = of_node_get(mux_node);
>> > + child = fwnode_handle_get(mux_node);
>> >
>> > if (!child) {
>> > - for_each_child_of_node(mux_node, child) {
>> > - ret = of_property_read_u32(child, "reg", ®);
>> > + fwnode_for_each_child_node(mux_node, child) {
>> > + ret = fwnode_property_read_u32(child, "reg", ®);
>> > if (ret)
>> > continue;
>> > if (chan_id == reg)
>> > @@ -305,7 +306,7 @@ i2c_mux_get_channel_node(struct i2c_mux_core *muxc, u32 chan_id)
>> > }
>> > }
>> >
>> > - of_node_put(mux_node);
>> > + fwnode_handle_put(mux_node);
>> > return child;
>> > }
>> >
>> > @@ -368,11 +369,9 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>> > else
>> > priv->adap.lock_ops = &i2c_parent_lock_ops;
>> >
>> > - /*
>> > - * Try to populate the mux adapter's of_node, expands to
>> > - * nothing if !CONFIG_OF.
>> > - */
>> > - priv->adap.dev.of_node = i2c_mux_get_channel_node(muxc, chan_id);
>> > + /* Associate the mux adapter with its OF or software-node channel. */
>> > + device_set_node(&priv->adap.dev,
>> > + i2c_mux_get_channel_node(muxc, chan_id));
>> >
>> > /*
>> > * Associate the mux channel with an ACPI node.
>> > @@ -416,7 +415,7 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>> > return 0;
>> >
>> > err_free_priv:
>> > - of_node_put(priv->adap.dev.of_node);
>> > + fwnode_handle_put(dev_fwnode(&priv->adap.dev));
>> > kfree(priv);
>> > return ret;
>> > }
>> > @@ -429,7 +428,7 @@ void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
>> > while (muxc->num_adapters) {
>> > struct i2c_adapter *adap = muxc->adapter[--muxc->num_adapters];
>> > struct i2c_mux_priv *priv = adap->algo_data;
>> > - struct device_node *np = adap->dev.of_node;
>> > + struct fwnode_handle *fwnode = dev_fwnode(&adap->dev);
>> >
>> > muxc->adapter[muxc->num_adapters] = NULL;
>> >
>> > @@ -439,7 +438,7 @@ void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
>> >
>> > sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
>> > i2c_del_adapter(adap);
>> > - of_node_put(np);
>> > + fwnode_handle_put(fwnode);
>> > kfree(priv);
>> > }
>> > }
>> > --
>> > 2.50.1 (Apple Git-155)
>> >
>
>
>
> --
> 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.
--
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.