1. Patchew
  2. linux
  3. View series

[PATCH v6] i2c: mux: Propagate software nodes to channel adapters

Ahmad Byagowi posted 1 patch 1 month, 1 week ago
There is a newer version of this series
drivers/i2c/i2c-mux.c | 61 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 2 deletions(-)
[PATCH v6] i2c: mux: Propagate software nodes to channel adapters
Posted by Ahmad Byagowi 1 month, 1 week ago 
Device Tree channel nodes are associated with the adapters created by
i2c-mux, but equivalent software-node descriptions are not.

When the mux device has a primary software node, find the child whose
reg value matches the channel and use it as the new adapter firmware node.
Keep the node reference through child-client removal and release it after
adapter deletion.

Also balance the Device Tree channel-node reference when adapter
registration fails.

Signed-off-by: Ahmad Byagowi <ahmadexp@gmail.com>
---
Changes since v5:
  - Send the I2C prerequisite as a standalone patch for the I2C tree.
  - Limit propagation to mux devices with a primary software node.
  - Avoid secondary-fwnode access and changes to generic adapter lookup.
  - Follow the i2c-atr reference-lifetime pattern for channel nodes.
  - Balance the Device Tree channel reference on adapter-add failure.
  - Rebase onto the I2C i2c-next branch.

v5: https://lore.kernel.org/r/cover.1786746757.git.ahmadexp@gmail.com/
---
 drivers/i2c/i2c-mux.c | 61 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 681a201c239ba..76fa89dc6a222 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>
 
@@ -33,6 +34,7 @@ struct i2c_mux_priv {
 	struct i2c_adapter adap;
 	struct i2c_algorithm algo;
 	struct i2c_mux_core *muxc;
+	struct fwnode_handle *swnode;
 	u32 chan_id;
 };
 
@@ -264,6 +266,49 @@ static const struct i2c_lock_operations i2c_parent_lock_ops = {
 	.unlock_bus =  i2c_parent_unlock_bus,
 };
 
+static struct fwnode_handle *
+i2c_mux_get_channel_swnode(struct i2c_mux_core *muxc, u32 chan_id)
+{
+	struct fwnode_handle *dev_node = dev_fwnode(muxc->dev);
+	struct fwnode_handle *mux_node, *child = NULL;
+	u32 reg;
+
+	if (!is_software_node(dev_node))
+		return NULL;
+
+	if (muxc->arbitrator)
+		mux_node = fwnode_get_named_child_node(dev_node, "i2c-arb");
+	else if (muxc->gate)
+		mux_node = fwnode_get_named_child_node(dev_node, "i2c-gate");
+	else
+		mux_node = fwnode_get_named_child_node(dev_node, "i2c-mux");
+
+	if (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 = fwnode_handle_get(dev_node);
+	else if (muxc->arbitrator || muxc->gate)
+		child = fwnode_handle_get(mux_node);
+
+	if (!child) {
+		fwnode_for_each_child_node(mux_node, child) {
+			if (fwnode_property_read_u32(child, "reg", &reg))
+				continue;
+			if (chan_id == reg)
+				break;
+		}
+	}
+
+	fwnode_handle_put(mux_node);
+	return child;
+}
+
 int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 			u32 force_nr, u32 chan_id)
 {
@@ -324,8 +369,8 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 		priv->adap.lock_ops = &i2c_parent_lock_ops;
 
 	/*
-	 * Try to populate the mux adapter's of_node, expands to
-	 * nothing if !CONFIG_OF.
+	 * Associate the mux adapter with its DT or software-node channel.
+	 * DT support expands to nothing if !CONFIG_OF.
 	 */
 	if (muxc->dev->of_node) {
 		struct device_node *dev_node = muxc->dev->of_node;
@@ -364,6 +409,10 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 
 		priv->adap.dev.of_node = child;
 		of_node_put(mux_node);
+	} else {
+		priv->swnode = i2c_mux_get_channel_swnode(muxc, chan_id);
+		if (priv->swnode)
+			device_set_node(&priv->adap.dev, priv->swnode);
 	}
 
 	/*
@@ -408,6 +457,8 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
 	return 0;
 
 err_free_priv:
+	fwnode_handle_put(priv->swnode);
+	of_node_put(priv->adap.dev.of_node);
 	kfree(priv);
 	return ret;
 }
@@ -429,7 +480,13 @@ void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
 		sysfs_remove_link(&muxc->dev->kobj, symlink_name);
 
 		sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
+		/*
+		 * Keep the software node through child removal. The adapter
+		 * device is cleared on deletion, so release the saved reference
+		 * afterwards.
+		 */
 		i2c_del_adapter(adap);
+		fwnode_handle_put(priv->swnode);
 		of_node_put(np);
 		kfree(priv);
 	}

base-commit: b0e590bb83a9e40dc1e77a2c82d7f3e670fb74b3
-- 
2.50.1 (Apple Git-155)
Re: [PATCH v6] i2c: mux: Propagate software nodes to channel adapters
Posted by Peter Rosin 1 month ago 
Hi Ahmad,

Thanks for your patch!

On 2026-08-20 19:41, Ahmad Byagowi wrote:
> Device Tree channel nodes are associated with the adapters created by
> i2c-mux, but equivalent software-node descriptions are not.
> 
> When the mux device has a primary software node, find the child whose
> reg value matches the channel and use it as the new adapter firmware node.
> Keep the node reference through child-client removal and release it after
> adapter deletion.
> 
> Also balance the Device Tree channel-node reference when adapter
> registration fails.
> 
> Signed-off-by: Ahmad Byagowi <ahmadexp@gmail.com>
> ---
> Changes since v5:
>   - Send the I2C prerequisite as a standalone patch for the I2C tree.
>   - Limit propagation to mux devices with a primary software node.
>   - Avoid secondary-fwnode access and changes to generic adapter lookup.
>   - Follow the i2c-atr reference-lifetime pattern for channel nodes.
>   - Balance the Device Tree channel reference on adapter-add failure.
>   - Rebase onto the I2C i2c-next branch.
> 
> v5: https://lore.kernel.org/r/cover.1786746757.git.ahmadexp@gmail.com/
> ---
>  drivers/i2c/i2c-mux.c | 61 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 59 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 681a201c239ba..76fa89dc6a222 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>
>  
> @@ -33,6 +34,7 @@ struct i2c_mux_priv {
>  	struct i2c_adapter adap;
>  	struct i2c_algorithm algo;
>  	struct i2c_mux_core *muxc;
> +	struct fwnode_handle *swnode;
>  	u32 chan_id;
>  };
>  
> @@ -264,6 +266,49 @@ static const struct i2c_lock_operations i2c_parent_lock_ops = {
>  	.unlock_bus =  i2c_parent_unlock_bus,
>  };
>  
> +static struct fwnode_handle *
> +i2c_mux_get_channel_swnode(struct i2c_mux_core *muxc, u32 chan_id)
> +{
> +	struct fwnode_handle *dev_node = dev_fwnode(muxc->dev);
> +	struct fwnode_handle *mux_node, *child = NULL;
> +	u32 reg;
> +
> +	if (!is_software_node(dev_node))
> +		return NULL;
> +
> +	if (muxc->arbitrator)
> +		mux_node = fwnode_get_named_child_node(dev_node, "i2c-arb");
> +	else if (muxc->gate)
> +		mux_node = fwnode_get_named_child_node(dev_node, "i2c-gate");
> +	else
> +		mux_node = fwnode_get_named_child_node(dev_node, "i2c-mux");
> +
> +	if (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 = fwnode_handle_get(dev_node);
> +	else if (muxc->arbitrator || muxc->gate)
> +		child = fwnode_handle_get(mux_node);
> +
> +	if (!child) {
> +		fwnode_for_each_child_node(mux_node, child) {
> +			if (fwnode_property_read_u32(child, "reg", &reg))
> +				continue;
> +			if (chan_id == reg)
> +				break;
> +		}
> +	}
> +
> +	fwnode_handle_put(mux_node);
> +	return child;
> +}
> +

I like how this is factored out from i2c_mux_add_adapter(), but
I do not like the duplication. There has got to be some nice way
to de-duplicate this since a fwnode_handle can represent an OF
node just fine.

I suggest a preparatory commit that only factors out the node
logic to a new helper i2c_mux_get_channel_node(). Initially it
should only handle the OF case (with a struct device_node arg),
but that is then changed to be generic with a struct fwnode_handle
arg in a followup commit.

>  int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  			u32 force_nr, u32 chan_id)
>  {
> @@ -324,8 +369,8 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  		priv->adap.lock_ops = &i2c_parent_lock_ops;
>  
>  	/*
> -	 * Try to populate the mux adapter's of_node, expands to
> -	 * nothing if !CONFIG_OF.
> +	 * Associate the mux adapter with its DT or software-node channel.
> +	 * DT support expands to nothing if !CONFIG_OF.
>  	 */
>  	if (muxc->dev->of_node) {
>  		struct device_node *dev_node = muxc->dev->of_node;
> @@ -364,6 +409,10 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  
>  		priv->adap.dev.of_node = child;
>  		of_node_put(mux_node);
> +	} else {
> +		priv->swnode = i2c_mux_get_channel_swnode(muxc, chan_id);
> +		if (priv->swnode)
> +			device_set_node(&priv->adap.dev, priv->swnode);
>  	}
Why do you need priv->swnode? device_set_node() stores the relevant
pointer as adap.dev.fwnode, and the fwnode can be put away based on
that pointer (similar to how the current code is making use of the
adap.dev.of_node pointer). Also, device_set_node() handles NULL just
fine.

I.e. this should be fine:

	device_set_node(&priv->adap.dev,
			i2c_mux_get_channel_swnode(muxc, chan_id));

if the below error handling...

>  
>  	/*
> @@ -408,6 +457,8 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  	return 0;
>  
>  err_free_priv:
> +	fwnode_handle_put(priv->swnode);

...is changed to
	fwnode_handle_put(dev_fwnode(&chan->adap.dev));

just like how it is done in i2c-atr.c.

You need to also adjust i2c_mux_del_adapters() to save a fwnode similar
to how it currently saves the of_node. Again, see i2c-atr.c.

> +	of_node_put(priv->adap.dev.of_node);

This one-liner fix should be a separate commit. Put this commit
first in the series please.

Cheers,
Peter

>  	kfree(priv);
>  	return ret;
>  }
> @@ -429,7 +480,13 @@ void i2c_mux_del_adapters(struct i2c_mux_core *muxc)
>  		sysfs_remove_link(&muxc->dev->kobj, symlink_name);
>  
>  		sysfs_remove_link(&priv->adap.dev.kobj, "mux_device");
> +		/*
> +		 * Keep the software node through child removal. The adapter
> +		 * device is cleared on deletion, so release the saved reference
> +		 * afterwards.
> +		 */
>  		i2c_del_adapter(adap);
> +		fwnode_handle_put(priv->swnode);
>  		of_node_put(np);
>  		kfree(priv);
>  	}
> 
> base-commit: b0e590bb83a9e40dc1e77a2c82d7f3e670fb74b3
Re: [PATCH v6] i2c: mux: Propagate software nodes to channel adapters
Posted by Peter Rosin 1 month ago 
On 2026-08-23 12:21, Peter Rosin wrote:
> ...is changed to
> 	fwnode_handle_put(dev_fwnode(&chan->adap.dev));

Bzzt, this is a copy-paste bug from the ATR code. The
ATR "chan" is "priv" here in the mux code.

The above should be:

	fwnode_handle_put(dev_fwnode(&priv->adap.dev));

Sorry for the noise.

Cheers,
Peter

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.