Linux I2C development
 help / color / mirror / Atom feed
* [PATCH v8 0/2] i2c: mux: Propagate firmware nodes to channel adapters
@ 2026-08-31 17:22 Ahmad Byagowi
  2026-08-31 17:22 ` [PATCH v8 1/2] i2c: mux: Factor out channel node lookup Ahmad Byagowi
  2026-08-31 17:22 ` [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
  0 siblings, 2 replies; 5+ messages in thread
From: Ahmad Byagowi @ 2026-08-31 17:22 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Peter Rosin, Andy Shevchenko, Jakub Kicinski, linux-i2c,
	linux-kernel, Ahmad Byagowi

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 v7:
  - Drop the channel-node leak fix now present in i2c-next.
  - Rebase onto the current i2c-next branch.
  - Remove the firmware-node type restriction so ACPI firmware nodes also
    use the generic lookup, as discussed with Peter Rosin and Andy
    Shevchenko.
  - Retain the existing acpi_preset_companion() path.
  - Keep a separate reference to the generic lookup result in case ACPI
    companion setup replaces the adapter's primary firmware node.
  - Return arbitrator and gate nodes directly from the lookup helper, as
    suggested by Peter.
  - Describe the conversion as firmware-node propagation.

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

v7: https://lore.kernel.org/r/cover.1787502619.git.ahmadexp@gmail.com/

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

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


base-commit: 1f3e66348d2527c31b9bedb0b4d9e28bdad9bc83
-- 
2.50.1 (Apple Git-155)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v8 1/2] i2c: mux: Factor out channel node lookup
  2026-08-31 17:22 [PATCH v8 0/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
@ 2026-08-31 17:22 ` Ahmad Byagowi
  2026-09-01  8:50   ` Andy Shevchenko
  2026-08-31 17:22 ` [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
  1 sibling, 1 reply; 5+ messages in thread
From: Ahmad Byagowi @ 2026-08-31 17:22 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Peter Rosin, Andy Shevchenko, Jakub Kicinski, linux-i2c,
	linux-kernel, 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.

Signed-off-by: Ahmad Byagowi <ahmadexp@gmail.com>
---
 drivers/i2c/i2c-mux.c | 82 +++++++++++++++++++++++--------------------
 1 file changed, 44 insertions(+), 38 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 68a4c34b5987..77921e5132bc 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -264,6 +264,49 @@ 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;
+	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 */
+		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 +370,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.
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters
  2026-08-31 17:22 [PATCH v8 0/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
  2026-08-31 17:22 ` [PATCH v8 1/2] i2c: mux: Factor out channel node lookup Ahmad Byagowi
@ 2026-08-31 17:22 ` Ahmad Byagowi
  2026-09-01  8:54   ` Andy Shevchenko
  1 sibling, 1 reply; 5+ messages in thread
From: Ahmad Byagowi @ 2026-08-31 17:22 UTC (permalink / raw)
  To: Andi Shyti
  Cc: Peter Rosin, Andy Shevchenko, Jakub Kicinski, linux-i2c,
	linux-kernel, 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 <ahmadexp@gmail.com>
---
 drivers/i2c/i2c-mux.c | 42 ++++++++++++++++++++----------------------
 1 file changed, 20 insertions(+), 22 deletions(-)

diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 77921e5132bc..dfe54b6d6567 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -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,11 +265,11 @@ 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;
+	struct fwnode_handle *dev_node = dev_fwnode(muxc->dev);
+	struct fwnode_handle *mux_node, *child;
 	u32 reg;
 	int ret;
 
@@ -276,34 +277,34 @@ i2c_mux_get_channel_node(struct i2c_mux_core *muxc, u32 chan_id)
 		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;
 }
 
@@ -366,11 +367,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.
@@ -414,7 +413,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;
 }
@@ -427,7 +426,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;
 
@@ -437,7 +435,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);
 	}
 }
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 1/2] i2c: mux: Factor out channel node lookup
  2026-08-31 17:22 ` [PATCH v8 1/2] i2c: mux: Factor out channel node lookup Ahmad Byagowi
@ 2026-09-01  8:50   ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-09-01  8:50 UTC (permalink / raw)
  To: Ahmad Byagowi
  Cc: Andi Shyti, Peter Rosin, Andy Shevchenko, Jakub Kicinski,
	linux-i2c, linux-kernel

On Mon, Aug 31, 2026 at 10:22:06AM -0700, Ahmad Byagowi wrote:
> 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.

...

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

It's better for maintenance to split assignment (due to validation below).

	struct device_node *dev_node;

> +	struct device_node *mux_node, *child;
> +	u32 reg;
> +	int ret;

// also use getter
	dev_node = dev_of_node(dev);

> +	if (!dev_node)
> +		return NULL;

Looking at the code, it's NULL-aware as far as I can see and this check is just
a shortcut.

> +	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;
> +}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters
  2026-08-31 17:22 ` [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
@ 2026-09-01  8:54   ` Andy Shevchenko
  0 siblings, 0 replies; 5+ messages in thread
From: Andy Shevchenko @ 2026-09-01  8:54 UTC (permalink / raw)
  To: Ahmad Byagowi
  Cc: Andi Shyti, Peter Rosin, Andy Shevchenko, Jakub Kicinski,
	linux-i2c, linux-kernel

On Mon, Aug 31, 2026 at 10:22:07AM -0700, Ahmad Byagowi wrote:
> 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.

...

>  	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);

Consider using __free(fwnode_handle) at some point. Perhaps as a patch
on top of this.

It also may help with the rest of the existing code elsewhere in the file.

>  			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;
>  }


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-01  8:54 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 17:22 [PATCH v8 0/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
2026-08-31 17:22 ` [PATCH v8 1/2] i2c: mux: Factor out channel node lookup Ahmad Byagowi
2026-09-01  8:50   ` Andy Shevchenko
2026-08-31 17:22 ` [PATCH v8 2/2] i2c: mux: Propagate firmware nodes to channel adapters Ahmad Byagowi
2026-09-01  8:54   ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox