* [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters
@ 2026-08-23 16:34 Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-23 16:34 UTC (permalink / raw)
To: Andi Shyti
Cc: Peter Rosin, Jakub Kicinski, linux-i2c, linux-kernel,
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 channel lookup so child I2C devices can be
instantiated from software-node properties.
This is the I2C prerequisite split out of the Time Card series. The first
patch fixes the existing Device Tree channel-node leak on adapter-add
failure. The second factors the Device Tree lookup into a helper without
changing behavior. The third converts that helper and the node lifetime
handling to generic firmware-node operations.
The dependent ptp_ocp board profiles will be resent to net-next after the
LED and I2C prerequisites land.
---
Changes since v6:
- Split the existing Device Tree leak fix into its own patch.
- Add a preparatory patch that only factors out the OF lookup.
- Convert the helper to generic firmware-node operations in the final
patch.
- Drop the separate private software-node pointer and use the adapter's
firmware node for cleanup.
- Save the firmware node before adapter deletion, then release it
afterwards following the i2c-atr lifetime pattern.
- Rebase onto the current I2C i2c-next branch.
Validation:
- git diff --check
- strict checkpatch
- x86-64 Clang syntax check
v6: https://lore.kernel.org/r/eb4a8ab1eedd8a01ca34c6a7e0685639d0498423.1787247467.git.ahmadexp@gmail.com/
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
drivers/i2c/i2c-mux.c | 96 +++++++++++++++++++++++--------------------
1 file changed, 52 insertions(+), 44 deletions(-)
base-commit: cd5fec79af3fa04e88e5900cdfa56d5525f5740b
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure
2026-08-23 16:34 [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
@ 2026-08-23 16:34 ` Ahmad Byagowi
2026-08-25 5:49 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 2/3] i2c: mux: Factor out channel node lookup Ahmad Byagowi
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-23 16:34 UTC (permalink / raw)
To: Andi Shyti
Cc: Peter Rosin, Jakub Kicinski, linux-i2c, linux-kernel,
Ahmad Byagowi
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 <ahmadexp@gmail.com>
---
drivers/i2c/i2c-mux.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 681a201c239b..68a4c34b5987 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -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;
}
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v7 2/3] i2c: mux: Factor out channel node lookup
2026-08-23 16:34 [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
@ 2026-08-23 16:34 ` Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-26 13:45 ` [PATCH v7 0/3] " Andi Shyti
3 siblings, 1 reply; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-23 16:34 UTC (permalink / raw)
To: Andi Shyti
Cc: Peter Rosin, 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 | 84 +++++++++++++++++++++++--------------------
1 file changed, 46 insertions(+), 38 deletions(-)
diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
index 68a4c34b5987..a8b94b97a725 100644
--- a/drivers/i2c/i2c-mux.c
+++ b/drivers/i2c/i2c-mux.c
@@ -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 */
+ if (!of_property_read_u32(mux_node, "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", ®);
+ 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", ®)) {
- 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", ®);
- 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] 16+ messages in thread
* [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-23 16:34 [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 2/3] i2c: mux: Factor out channel node lookup Ahmad Byagowi
@ 2026-08-23 16:34 ` Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
2026-08-26 13:45 ` [PATCH v7 0/3] " Andi Shyti
3 siblings, 1 reply; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-23 16:34 UTC (permalink / raw)
To: Andi Shyti
Cc: Peter Rosin, Jakub Kicinski, linux-i2c, linux-kernel,
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@gmail.com>
---
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;
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)
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
@ 2026-08-25 5:49 ` Peter Rosin
0 siblings, 0 replies; 16+ messages in thread
From: Peter Rosin @ 2026-08-25 5:49 UTC (permalink / raw)
To: Ahmad Byagowi; +Cc: Andi Shyti, Jakub Kicinski, linux-i2c, linux-kernel
Den Sun, Aug 23, 2026 at 09:34:36AM -0700, skrev Ahmad Byagowi:
> 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 <ahmadexp@gmail.com>
Acked-by: Peter Rosin <peda@lysator.liu.se>
Cheers,
Peter
> ---
> drivers/i2c/i2c-mux.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 681a201c239b..68a4c34b5987 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -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;
> }
> --
> 2.50.1 (Apple Git-155)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 2/3] i2c: mux: Factor out channel node lookup
2026-08-23 16:34 ` [PATCH v7 2/3] i2c: mux: Factor out channel node lookup Ahmad Byagowi
@ 2026-08-25 5:50 ` Peter Rosin
0 siblings, 0 replies; 16+ messages in thread
From: Peter Rosin @ 2026-08-25 5:50 UTC (permalink / raw)
To: Ahmad Byagowi; +Cc: Andi Shyti, Jakub Kicinski, linux-i2c, linux-kernel
Hi!
Den Sun, Aug 23, 2026 at 09:34:37AM -0700, skrev 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 | 84 +++++++++++++++++++++++--------------------
> 1 file changed, 46 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 68a4c34b5987..a8b94b97a725 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -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 */
> + if (!of_property_read_u32(mux_node, "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);
You can return early here with "return mux_node;" (which also
obviates the need for the above " = NULL"-initializer and the
below "if (!child)"-test).
Cheers,
Peter
> +
> + if (!child) {
> + for_each_child_of_node(mux_node, child) {
> + ret = of_property_read_u32(child, "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", ®)) {
> - 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", ®);
> - 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 [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-23 16:34 ` [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
@ 2026-08-25 5:50 ` Peter Rosin
[not found] ` <CAOQiBWNVvHpO7X8yWyZEa3-nV76YJfRbkzo+_4U-upEj4VNfow@mail.gmail.com>
0 siblings, 1 reply; 16+ messages in thread
From: Peter Rosin @ 2026-08-25 5:50 UTC (permalink / raw)
To: Ahmad Byagowi, Andy Shevchenko
Cc: Andi Shyti, Jakub Kicinski, linux-i2c, linux-kernel
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@gmail.com>
> ---
> 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)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-23 16:34 [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
` (2 preceding siblings ...)
2026-08-23 16:34 ` [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
@ 2026-08-26 13:45 ` Andi Shyti
3 siblings, 0 replies; 16+ messages in thread
From: Andi Shyti @ 2026-08-26 13:45 UTC (permalink / raw)
To: Ahmad Byagowi; +Cc: Peter Rosin, Jakub Kicinski, linux-i2c, linux-kernel
Hi Ahmad,
> Ahmad Byagowi (3):
> i2c: mux: Fix channel node leak on adapter add failure
I merged this first patch into i2c/i2c-fixes, while...
> i2c: mux: Factor out channel node lookup
> i2c: mux: Propagate software nodes to channel adapters
... I left these out because there are some pending comments.
Thanks,
Andi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
[not found] ` <CAOQiBWNVvHpO7X8yWyZEa3-nV76YJfRbkzo+_4U-upEj4VNfow@mail.gmail.com>
@ 2026-08-26 16:39 ` Ahmad Byagowi
2026-08-27 6:11 ` Andy Shevchenko
1 sibling, 0 replies; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-26 16:39 UTC (permalink / raw)
To: Peter Rosin
Cc: Andy Shevchenko, Andi Shyti, Jakub Kicinski, linux-i2c,
linux-kernel
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@gmail.com> 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@lysator.liu.se> 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@gmail.com>
>> > ---
>> > 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
[not found] ` <CAOQiBWNVvHpO7X8yWyZEa3-nV76YJfRbkzo+_4U-upEj4VNfow@mail.gmail.com>
2026-08-26 16:39 ` Ahmad Byagowi
@ 2026-08-27 6:11 ` Andy Shevchenko
2026-08-30 4:55 ` Peter Rosin
1 sibling, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2026-08-27 6:11 UTC (permalink / raw)
To: Ahmad Byagowi
Cc: Peter Rosin, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> 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.
> 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?
Wouldn't it be possible to use some kind of DT overlay to have that?
> 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.
> On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@lysator.liu.se> wrote:
> > 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.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-27 6:11 ` Andy Shevchenko
@ 2026-08-30 4:55 ` Peter Rosin
2026-08-30 5:10 ` Ahmad Byagowi
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Peter Rosin @ 2026-08-30 4:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Ahmad Byagowi, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> > 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.
>
> > 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?
>
> 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:
> + 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
> > 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.
>
> > On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@lysator.liu.se> wrote:
> > > 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.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-30 4:55 ` Peter Rosin
@ 2026-08-30 5:10 ` Ahmad Byagowi
2026-08-31 7:13 ` Peter Rosin
2026-08-31 7:22 ` Andy Shevchenko
[not found] ` <CAOQiBWOZd+NVRqzRzJx9HP+B21JJXBUTy_OxAqDw8X7tvWeY+w@mail.gmail.com>
2 siblings, 1 reply; 16+ messages in thread
From: Ahmad Byagowi @ 2026-08-30 5:10 UTC (permalink / raw)
To: Peter Rosin
Cc: Andy Shevchenko, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
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
On Sat, Aug 29, 2026 at 9:55 PM Peter Rosin <peda@lysator.liu.se> wrote:
>
> Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> > On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> > > 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.
> >
> > > 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?
> >
> > 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:
> > + 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
>
> > > 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.
> >
> > > On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@lysator.liu.se> wrote:
> > > > 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.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
--
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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-30 5:10 ` Ahmad Byagowi
@ 2026-08-31 7:13 ` Peter Rosin
2026-08-31 7:28 ` Andy Shevchenko
0 siblings, 1 reply; 16+ messages in thread
From: Peter Rosin @ 2026-08-31 7:13 UTC (permalink / raw)
To: Ahmad Byagowi
Cc: Andy Shevchenko, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
Den Sat, Aug 29, 2026 at 10:10:40PM -0700, skrev Ahmad Byagowi:
> 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?
> 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.
> 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.
Cheers,
Peter
>
> Regards,
> Ahmad
>
>
> On Sat, Aug 29, 2026 at 9:55 PM Peter Rosin <peda@lysator.liu.se> wrote:
> >
> > Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> > > On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> > > > 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.
> > >
> > > > 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?
> > >
> > > 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:
> > > + 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
> >
> > > > 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.
> > >
> > > > On Mon, Aug 24, 2026 at 10:50 PM Peter Rosin <peda@lysator.liu.se> wrote:
> > > > > 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.
> > >
> > > --
> > > With Best Regards,
> > > Andy Shevchenko
> > >
> > >
>
>
>
> --
> 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.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-30 4:55 ` Peter Rosin
2026-08-30 5:10 ` Ahmad Byagowi
@ 2026-08-31 7:22 ` Andy Shevchenko
[not found] ` <CAOQiBWOZd+NVRqzRzJx9HP+B21JJXBUTy_OxAqDw8X7tvWeY+w@mail.gmail.com>
2 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-08-31 7:22 UTC (permalink / raw)
To: Peter Rosin
Cc: Ahmad Byagowi, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
On Sun, Aug 30, 2026 at 06:55:51AM +0200, Peter Rosin wrote:
> Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> > On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
> > > 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.
> >
> > > 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?
> >
> > 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:
> > + if (!is_of_node(dev_node) && !is_software_node(dev_node))
> > + return NULL;
> and let the code trawl all kinds of device properties?
Technically it's safe. Administratively, it might be some "creative" ACPI
firmware author (who presumably hasn't seen anything than OF in their life
before) may assume that it's fine to use OF approach (while ACPI has even
documented "binding" for I²C muxes). So my answer "yes and no" :-) It all
depends on the human factor. If we trust people enough to read documentation
and follow, we are fine without these lines.
> Sorry for the confusion...
NP, I'm glad to help with ACPI.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
[not found] ` <CAOQiBWOZd+NVRqzRzJx9HP+B21JJXBUTy_OxAqDw8X7tvWeY+w@mail.gmail.com>
@ 2026-08-31 7:25 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-08-31 7:25 UTC (permalink / raw)
To: Ahmad Byagowi
Cc: Peter Rosin, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
On Sat, Aug 29, 2026 at 10:04:13PM -0700, Ahmad Byagowi wrote:
Please, stop top-posting!
> 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.
Also use proper check then if (!is_acpi_node(...)) or more granular one
(ACPI device or data node in case you we are talking about certain type
of it).
> On Sat, Aug 29, 2026 at 9:55 PM Peter Rosin <peda@lysator.liu.se> wrote:
>
> > Den Thu, Aug 27, 2026 at 09:11:31AM +0300, skrev Andy Shevchenko:
> > > On Wed, Aug 26, 2026 at 08:29:47AM -0700, Ahmad Byagowi wrote:
...
> > > 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:
> > > + if (!is_of_node(dev_node) && !is_software_node(dev_node))
> > > + return NULL;
> > and let the code trawl all kinds of device properties?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters
2026-08-31 7:13 ` Peter Rosin
@ 2026-08-31 7:28 ` Andy Shevchenko
0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2026-08-31 7:28 UTC (permalink / raw)
To: Peter Rosin
Cc: Ahmad Byagowi, Andy Shevchenko, Andi Shyti, Jakub Kicinski,
linux-i2c, linux-kernel
On Mon, Aug 31, 2026 at 09:13:34AM +0200, Peter Rosin wrote:
> Den Sat, Aug 29, 2026 at 10:10:40PM -0700, skrev Ahmad Byagowi:
> > 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?
Nope, and we have documented how ACPI DSDT is supposed to be constructed for
the I²C muxes, but as I answered there, it relies on a human (factor).
> > 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.
>
> > 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.
Agree (and also answered in the similar way earlier).
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-08-31 7:28 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-23 16:34 [PATCH v7 0/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-23 16:34 ` [PATCH v7 1/3] i2c: mux: Fix channel node leak on adapter add failure Ahmad Byagowi
2026-08-25 5:49 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 2/3] i2c: mux: Factor out channel node lookup Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
2026-08-23 16:34 ` [PATCH v7 3/3] i2c: mux: Propagate software nodes to channel adapters Ahmad Byagowi
2026-08-25 5:50 ` Peter Rosin
[not found] ` <CAOQiBWNVvHpO7X8yWyZEa3-nV76YJfRbkzo+_4U-upEj4VNfow@mail.gmail.com>
2026-08-26 16:39 ` Ahmad Byagowi
2026-08-27 6:11 ` Andy Shevchenko
2026-08-30 4:55 ` Peter Rosin
2026-08-30 5:10 ` Ahmad Byagowi
2026-08-31 7:13 ` Peter Rosin
2026-08-31 7:28 ` Andy Shevchenko
2026-08-31 7:22 ` Andy Shevchenko
[not found] ` <CAOQiBWOZd+NVRqzRzJx9HP+B21JJXBUTy_OxAqDw8X7tvWeY+w@mail.gmail.com>
2026-08-31 7:25 ` Andy Shevchenko
2026-08-26 13:45 ` [PATCH v7 0/3] " Andi Shyti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox