i2c: mux: Propagate firmware nodes to channel adapters
Subsystems (3)
v10 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 v9:
- Declare dev_node, mux_node, and child on separate lines in both patches, as requested by Peter Rosin and agreed by Andy Shevchenko.
The broader __free(fwnode_handle) cleanup remains 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
v9: 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 | 98 +++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 46 deletions(-)
base-commit: 1f3e66348d2527c31b9bedb0b4d9e28bdad9bc83
Signature · 1 line
2.50.1 (Apple Git-155)
PATCH v10 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.
--- drivers/i2c/i2c-mux.c | 84 +++++++++++++++++++++++-------------------- 1 file changed, 46 insertions(+), 38 deletions(-)
drivers/i2c/i2c-mux.c +46 -38
@@ -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; +struct device_node *mux_node; +struct device_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" , ®)) { +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" , ®); +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.
Signature · 1 line
2.50.1 (Apple Git-155)
Re: [PATCH v10 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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v10 1/2] i2c: mux: Factor out channel node lookup
In reply to Ahmad ByagowiQuoted 6 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.
Signed-off-by Ahmad Byagowi <[email protected]>
Cheers,
Peter
PATCH v10 2/2 i2c: mux: Propagate firmware nodes to channel adapters
In reply to Ahmad ByagowiDevice 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.
--- drivers/i2c/i2c-mux.c | 46 +++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 24 deletions(-)
drivers/i2c/i2c-mux.c +22 -24
@@ -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,48 +265,48 @@ 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; -struct device_node *child; +struct fwnode_handle *dev_node; +struct fwnode_handle *mux_node; +struct fwnode_handle *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" , ®)) { -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)return mux_node; -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)break ; } -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 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.
@@ -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 (priv->channel_node);kfree (priv);return ret; }
@@ -429,7 +428,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 ;
@@ -439,7 +437,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)
Quoted 10 lines · 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.
Quoted 3 lines
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.
...
drivers/i2c/i2c-mux.c static const struct i2c_lock_operations i2c_parent_lock_ops = { · line 273
Quoted 6 lines
drivers/i2c/i2c-mux.c +3
-3-struct device_node *dev_node; -struct device_node *mux_node; -struct device_node *child; +struct fwnode_handle *dev_node; +struct fwnode_handle *mux_node; +struct fwnode_handle *child;
When the local variable changes the type (usually this is related to the pointers) the best practice is to rename. In such a case it will avoid the cases when some API accepts void * and the change would not have been noticed until run-time crash or subtle issues.
I hope here it's just a mechanical stuff and we don't have side effects as per above.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Andy,
Quoted 2 lines
I hope here it's just a mechanical stuff and we don't have side effects as per above.
Thanks for reviewing both patches and for the advice about renaming variables when their types change.
I checked all uses of dev_node, mux_node, and child in the converted helper. They use firmware-node APIs throughout, with no remaining OF calls or void * consumers for those pointers. I did not find a type-mismatch issue in this conversion.
Regards,
Ahmad
Quoted 10 lines · 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.
Quoted 5 lines
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]>
Cheers,
Peter
Peter Rosin
maintainerQuoted 30 lines · 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 dependentptp_ocpboard profiles will be resent to net-next after the LED and I2C prerequisites land.Changes since v9:
- Declaredev_node,mux_node, and child on separate lines in both patches, as requested by Peter Rosin and agreed by Andy Shevchenko.The broader
__free(fwnode_handle)cleanup remains a separate follow-up.Validation:
- git diff --check
- strict checkpatch with an 80-column limit
- x86-64 GCC W=1 object build ofdrivers/i2c/i2c-mux.o
- mailbox replay onto the declared base with an identical resulting tree
Quoted 8 lines
v9: https://lore.kernel.org/r/[email protected]/
Ahmad Byagowi (2):
i2c: mux: Factor out channel node lookup
i2c: mux: Propagate firmware nodes to channel adaptersdrivers/i2c/i2c-mux.c | 98 +++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 46 deletions(-)
Thanks!
Andi, you can pick this series now, thanks.
Cheers,
Peter
Hi Ahmad,
Quoted 3 lines
Ahmad Byagowi (2):
i2c: mux: Factor out channel node lookup
i2c: mux: Propagate firmware nodes to channel adapters
pushed to i2c/i2c.
Thanks,
Andi