i2c: mux: Propagate software nodes to channel adapters

i2c_mux_add_adapter() takes a reference to the Device Tree channel node before registering the new adapter. If adapter registration fails, the error path frees the private data without dropping that reference.

Release the channel node before freeing the private data.

Fixes bc45449b1444 ("i2c/of: Automatically populate i2c mux busses from device tree data.")

Signed-off-by Ahmad Byagowi <[email protected]>

---
 drivers/i2c/i2c-mux.c | 1 +
 1 file changed, 1 insertion(+)

drivers/i2c/i2c-mux.c +1 -0

index 681a201c239b..68a4c34b5987 100644
@@ -408,6 +408,7 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	return 0;
 
 err_free_priv:
+	of_node_put(priv->adap.dev.of_node);
 	kfree(priv);
 	return ret;
 }

Signature · 1 line

2.50.1 (Apple Git-155)

Move the existing Device Tree channel-node lookup into a helper in preparation for using generic firmware-node operations.

This is a pure refactoring with no functional change.

Signed-off-by Ahmad Byagowi <[email protected]>

---
 drivers/i2c/i2c-mux.c | 84 +++++++++++++++++++++++--------------------
 1 file changed, 46 insertions(+), 38 deletions(-)

drivers/i2c/i2c-mux.c +20 -0

index 68a4c34b5987..a8b94b97a725 100644
@@ -264,6 +264,51 @@ static const struct i2c_lock_operations i2c_parent_lock_ops = {
 	.unlock_bus =  i2c_parent_unlock_bus,
 };
 
+static struct device_node *
+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;
+	u32 reg;
+	int ret;
+
+	if (!dev_node)
+		return NULL;
+
+	if (muxc->arbitrator)
+		mux_node = of_get_child_by_name(dev_node, "i2c-arb");
+	else if (muxc->gate)
+		mux_node = of_get_child_by_name(dev_node, "i2c-gate");
+	else
+		mux_node = of_get_child_by_name(dev_node, "i2c-mux");
+
+	if (mux_node) {
+		/* A "reg" property indicates an old-style DT entry */

drivers/i2c/i2c-mux.c +16 -38

+
+	if (!child) {
+		for_each_child_of_node(mux_node, child) {
+			ret = of_property_read_u32(child, "reg", &reg);
+			if (ret)
+				continue;
+			if (chan_id == reg)
+				break;
+		}
+	}
+
+	of_node_put(mux_node);
+	return child;
+}
+
 int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 			u32 force_nr, u32 chan_id)
 {

@@ -327,44 +372,7 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	 * Try to populate the mux adapter's of_node, expands to
 	 * nothing if !CONFIG_OF.
 	 */
-	if (muxc->dev->of_node) {
-		struct device_node *dev_node = muxc->dev->of_node;
-		struct device_node *mux_node, *child = NULL;
-		u32 reg;
-
-		if (muxc->arbitrator)
-			mux_node = of_get_child_by_name(dev_node, "i2c-arb");
-		else if (muxc->gate)
-			mux_node = of_get_child_by_name(dev_node, "i2c-gate");
-		else
-			mux_node = of_get_child_by_name(dev_node, "i2c-mux");
-
-		if (mux_node) {
-			/* A "reg" property indicates an old-style DT entry */
-			if (!of_property_read_u32(mux_node, "reg", &reg)) {
-				of_node_put(mux_node);
-				mux_node = NULL;
-			}
-		}
-
-		if (!mux_node)
-			mux_node = of_node_get(dev_node);
-		else if (muxc->arbitrator || muxc->gate)
-			child = of_node_get(mux_node);
-
-		if (!child) {
-			for_each_child_of_node(mux_node, child) {
-				ret = of_property_read_u32(child, "reg", &reg);
-				if (ret)
-					continue;
-				if (chan_id == reg)
-					break;
-			}
-		}
-
-		priv->adap.dev.of_node = child;
-		of_node_put(mux_node);
-	}
+	priv->adap.dev.of_node = i2c_mux_get_channel_node(muxc, chan_id);
 
 	/*
 	 * Associate the mux channel with an ACPI node.

Signature · 1 line

2.50.1 (Apple Git-155)

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 <[email protected]>

---
 drivers/i2c/i2c-mux.c | 45 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

drivers/i2c/i2c-mux.c +2 -1

index a8b94b97a725..b37d36d3b121 100644
@@ -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)
 {

drivers/i2c/i2c-mux.c +17 -19

 	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", &reg)) {
-			of_node_put(mux_node);
+		/* A "reg" property indicates an old-style firmware entry. */
+		if (!fwnode_property_read_u32(mux_node, "reg", &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", &reg);
+		fwnode_for_each_child_node(mux_node, child) {
+			ret = fwnode_property_read_u32(child, "reg", &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);
 	}
 }

Signature · 1 line

2.50.1 (Apple Git-155)

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

Quoted 140 lines · Peter Rosin

Hi!

[Adding Andy]

Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:

Quoted 36 lines

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 <[email protected]>

---
 drivers/i2c/i2c-mux.c | 45 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

drivers/i2c/i2c-mux.c +2 -1

index a8b94b97a725..b37d36d3b121 100644
@@ -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)
 {

drivers/i2c/i2c-mux.c static const struct i2c_lock_operations i2c_parent_lock_ops = { · line 277

Quoted 10 lines

drivers/i2c/i2c-mux.c +3 -3

-     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

Quoted 81 lines

drivers/i2c/i2c-mux.c +16 -18

      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", &reg)) {
-                     of_node_put(mux_node);
+             /* A "reg" property indicates an old-style firmware entry. */
+             if (!fwnode_property_read_u32(mux_node, "reg", &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", &reg);
+             fwnode_for_each_child_node(mux_node, child) {
+                     ret = fwnode_property_read_u32(child, "reg", &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)
 

Quoted 10 lines

Quoted 10 lines

drivers/i2c/i2c-mux.c +1 -2

              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.

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.

Quoted 11 lines · Peter Rosin

Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:

Quoted 10 lines

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.

Quoted 3 lines

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.

Quoted 11 lines

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.

Quoted 2 lines

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?

Quoted 16 lines

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.

Quoted 11 lines · Peter Rosin

Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:

Quoted 10 lines

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.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:

Quoted 19 lines · Ahmad Byagowi

Quoted 3 lines

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.

Quoted 11 lines

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.

Quoted 4 lines

Quoted 2 lines

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:

Review

Quoted 2 lines

(quoted diff) +2 -0

+     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

Quoted 22 lines

Quoted 4 lines

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.

Quoted 12 lines

Quoted 11 lines · Peter Rosin

Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:

Quoted 10 lines

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

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.

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.

That said, I will clarify this in the commit message for the next revision.

Regards,
Ahmad

Quoted 72 lines · Peter Rosin

Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:

Quoted 19 lines · Ahmad Byagowi

Quoted 3 lines

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.

Quoted 11 lines

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.

Quoted 4 lines

Quoted 2 lines

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:

Review

Quoted 2 lines

(quoted diff) +2 -0

+     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

Quoted 22 lines

Quoted 4 lines

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.

Quoted 12 lines

Quoted 11 lines · Peter Rosin

Den Sun, Aug 23, 2026 at 09:34:38AM -0700, skrev Ahmad Byagowi:

Quoted 10 lines

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

Signature · 6 lines

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.

Den Sat, Aug 29, 2026 at 10:10:40PM -0700, skrev Ahmad Byagowi:

Quoted 6 lines

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?

Quoted 5 lines

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.

Quoted 1 line

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.

Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:

Quoted 18 lines · Ahmad Byagowi

Quoted 2 lines

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.

Quoted 11 lines

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.

Quoted 4 lines

Quoted 2 lines

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:

Review

Quoted 2 lines

(quoted diff) +2 -0

+	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...

Peter, thanks for clarifying.
The check is intentional as 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. 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.
That being said, I will clarify that in the commit message for the next revision.

Quoted 3 lines

Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:

Quoted 1 line · Ahmad Byagowi

Quoted 20 lines

Quoted 1 line

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:

Review

Quoted 2 lines

(quoted diff) +2 -0

+     if (!is_of_node(dev_node) && !is_software_node(dev_node))
+             return NULL;

and let the code trawl all kinds of device properties?

Ahmad Byagowi (3):
i2c: mux: Fix channel node leak on adapter add failure

i2c: mux: Factor out channel node lookup
i2c: mux: Propagate software nodes to channel adapters