Skip to contentSkip to conversation

i2c: mux: Propagate firmware nodes to channel adapters

PATCH Superseded Ahmad Byagowi 3 participants 7 messages Sep 3 – Sep 5

Subsystems (3)
I2C subsystem host drivers I2C I2C muxes

v9 of 10 Superseded Latest is v10 →

lore
Share & tools
lore

Ahmad Byagowi

author
#0 lore raw

v9 cover i2c: mux: Propagate firmware nodes to channel adapters

Device Tree channel nodes are associated with the adapters created by i2c-mux, but equivalent firmware-node descriptions are not. Convert the existing lookup to generic firmware-node operations so software-node and ACPI descriptions can use the same property traversal.

This is the remaining I2C prerequisite split out of the Time Card series. The channel-node leak fix from v7 was applied to i2c-next as commit 385c7af4e3b9 ("i2c: mux: Fix channel node leak on adapter add failure"), so it is no longer included here. The first patch factors the Device Tree lookup into a helper without changing behavior. The second converts the helper and node lifetime handling to generic firmware-node operations.

The existing acpi_preset_companion() call remains in place after the generic lookup. A separate reference to the node returned by that lookup is retained because ACPI companion setup may replace the adapter's primary firmware node. The dependent ptp_ocp board profiles will be resent to net-next after the LED and I2C prerequisites land.

Changes since v8:
- Split the dev_node declaration from its assignment and use dev_of_node() in the Device Tree helper, as suggested by Andy Shevchenko.
- Preserve the split declaration and assignment when converting the helper to dev_fwnode().
- Leave broader __free(fwnode_handle) cleanup for a separate follow-up.

Validation:
- git diff --check
- strict checkpatch with an 80-column limit
- x86-64 GCC W=1 object build of drivers/i2c/i2c-mux.o
- mailbox replay onto the declared base with an identical resulting tree

v8: https://lore.kernel.org/r/[email protected]/

Ahmad Byagowi (2):
i2c: mux: Factor out channel node lookup
i2c: mux: Propagate firmware nodes to channel adapters

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

base-commit: 1f3e66348d2527c31b9bedb0b4d9e28bdad9bc83

Signature · 1 line
2.50.1 (Apple Git-155)
author
#1 lore raw

PATCH v9 1/2 i2c: mux: Factor out channel node lookup

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 | 83 +++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 38 deletions(-)
drivers/i2c/i2c-mux.c +45 -38
index 68a4c34b5987..37529c7627a6 100644
@@ -264,6 +264,50 @@ 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;
+	struct device_node *mux_node, *child;
+	u32 reg;
+	int ret;
+
+	dev_node = dev_of_node(muxc->dev);
+	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 */
+		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)
+		return mux_node;
+
+	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 +371,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)
#2 lore raw

Re: [PATCH v9 1/2] i2c: mux: Factor out channel node lookup

Quoted 4 lines · Ahmad Byagowi

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.

...

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

Quoted 5 lines
drivers/i2c/i2c-mux.c +5 -0
+static struct device_node *
+i2c_mux_get_channel_node(struct i2c_mux_core *muxc, u32 chan_id)
+{
+	struct device_node *dev_node;
+	struct device_node *mux_node, *child;

Now all three can be defined in a single line.

Quoted 38 lines
drivers/i2c/i2c-mux.c +38 -0
+	u32 reg;
+	int ret;
+
+	dev_node = dev_of_node(muxc->dev);
+	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 */
+		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)
+		return mux_node;
+
+	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;
+}
Signature · 2 lines
With Best Regards,
Andy Shevchenko
maintainer
#3 lore raw

Re: [PATCH v9 1/2] i2c: mux: Factor out channel node lookup

Den Thu, Sep 03, 2026 at 10:58:53PM +0300, skrev Andy Shevchenko:

Quoted 8 lines · Ahmad Byagowi
Quoted 4 lines

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.

...

Quoted 7 lines

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

Quoted 5 lines
drivers/i2c/i2c-mux.c +5 -0
+static struct device_node *
+i2c_mux_get_channel_node(struct i2c_mux_core *muxc, u32 chan_id)
+{
+	struct device_node *dev_node;
+	struct device_node *mux_node, *child;

Now all three can be defined in a single line.

They could, but three is a crowd. So, if you bother to send an update, please use one declaration per line instead to make future diffs clearer.

Cheers,
Peter

#4 lore raw

Re: [PATCH v9 1/2] i2c: mux: Factor out channel node lookup

Quoted 2 lines · Peter Rosin

Den Thu, Sep 03, 2026 at 10:58:53PM +0300, skrev Andy Shevchenko:

Quoted 1 line · Ahmad Byagowi

...

Quoted 8 lines
Quoted 4 lines

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

Quoted 2 lines
drivers/i2c/i2c-mux.c +2 -0
+	struct device_node *dev_node;
+	struct device_node *mux_node, *child;

Now all three can be defined in a single line.

They could, but three is a crowd. So, if you bother to send an update, please use one declaration per line instead to make future diffs clearer.

True, but it can reduce churn between these two patches if placed on a single line. Whatever, it's a bikeshedding, so go with what Peter prefers (as he is a maintainer IIRC of this code).

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#5 lore raw

Re: [PATCH v9 1/2] i2c: mux: Factor out channel node lookup

Peter, Andy,

Thanks for clarifying. I'll use one declaration per line for dev_node, mux_node, and child in both patches in the next revision.

Regards,
Ahmad

author
#6 lore raw

PATCH v9 2/2 i2c: mux: Propagate firmware nodes to channel adapters

In reply to Ahmad Byagowi

Device Tree channel nodes are associated with the adapters created by i2c-mux, but equivalent firmware-node descriptions are not.

Use generic firmware-node operations for the existing channel lookup and associate the returned node with the adapter. Do not restrict the lookup by firmware-node type, so Device Tree, software nodes, and ACPI descriptions all follow the same property traversal. The existing acpi_preset_companion() call remains in place for the standard ACPI channel association.

Keep a separate reference to the node returned by the generic lookup because acpi_preset_companion() may replace the device's primary firmware node. Release the saved reference after adapter deletion.

Signed-off-by Ahmad Byagowi <[email protected]>
---
 drivers/i2c/i2c-mux.c | 44 +++++++++++++++++++++----------------------
 1 file changed, 21 insertions(+), 23 deletions(-)
drivers/i2c/i2c-mux.c +21 -23
index 37529c7627a6..e0b0116d5b6d 100644
@@ -24,7 +24,7 @@
 #include <linux/i2c-mux.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/sysfs.h>
 

@@ -33,6 +33,7 @@ struct i2c_mux_priv {
 	struct i2c_adapter adap;
 	struct i2c_algorithm algo;
 	struct i2c_mux_core *muxc;
+	struct fwnode_handle *channel_node;
 	u32 chan_id;
 };
 

@@ -264,47 +265,47 @@ 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;
-	struct device_node *mux_node, *child;
+	struct fwnode_handle *dev_node;
+	struct fwnode_handle *mux_node, *child;
 	u32 reg;
 	int ret;
 
-	dev_node = dev_of_node(muxc->dev);
+	dev_node = dev_fwnode(muxc->dev);
 	if (!dev_node)
 		return NULL;
 
 	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)
 		return mux_node;
 
-	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)
 			break;
 	}
 
-	of_node_put(mux_node);
+	fwnode_handle_put(mux_node);
 	return child;
 }
 

@@ -367,11 +368,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 channel firmware node. */
+	priv->channel_node = i2c_mux_get_channel_node(muxc, chan_id);
+	device_set_node(&priv->adap.dev, priv->channel_node);
 
 	/*
 	 * Associate the mux channel with an ACPI node.

@@ -415,7 +414,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(priv->channel_node);
 	kfree(priv);
 	return ret;
 }

@@ -428,7 +427,6 @@ 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;
 
 		muxc->adapter[muxc->num_adapters] = NULL;
 

@@ -438,7 +436,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(priv->channel_node);
 		kfree(priv);
 	}
 }
Signature · 1 line
2.50.1 (Apple Git-155)