Re: [RFC PATCH 1/3] i2c: core: Follow i2c-parent when retrieving an adapter from node

Wolfram Sang
2025-04-03T09:03:35Z

[-- Attachment #1: Type: text/plain, Size: 3116 bytes --]

> Extend i2c_find_adapter_by_fwnode() to perform the walking from the
> given fwnode through i2c-parent references up to the adapter.

Even with the review of the schema going on, here are some comments
already.

>
> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
> drivers/i2c/i2c-core-base.c | 43 ++++++++++++++++++++++++++++++++++++-
> 1 file changed, 42 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 5546184df05f..cb7adc5c1285 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1827,14 +1827,55 @@ static int i2c_dev_or_parent_fwnode_match(struct device *dev, const void *data)
> */
> struct i2c_adapter *i2c_find_adapter_by_fwnode(struct fwnode_handle *fwnode)
> {
> + struct fwnode_reference_args args;
> + struct fwnode_handle *adap_fwnode;
> struct i2c_adapter *adapter;
> struct device *dev;
> + int err;
>
> if (!fwnode)
> return NULL;
>
> - dev = bus_find_device(&i2c_bus_type, NULL, fwnode,
> + adap_fwnode = fwnode_handle_get(fwnode);
> +
> + /* Walk extension busses (through i2c-parent) up to the adapter node */
> + while (fwnode_property_present(adap_fwnode, "i2c-parent")) {
> + /*
> + * A specific case exists for the i2c demux pinctrl. The i2c bus
> + * node related this component (the i2c demux pinctrl node
> + * itself) has an i2c-parent property set. This property is used
> + * by the i2c demux pinctrl component for the demuxing purpose
> + * and is not related to the extension bus feature.
> + *
> + * In this current i2c-parent walking, the i2c demux pinctrl
> + * node has to be considered as an adapter node and so, if
> + * the adap_fwnode node is an i2c demux pinctrl node, simply
> + * stop the i2c-parent walking.
> + */
> + if (fwnode_property_match_string(adap_fwnode, "compatible",
> + "i2c-demux-pinctrl") >= 0)
> + break;

I understand the unlikeliness of another demux driver showing up, yet
relying on compatible-values here is too easy to get stale. What about
checking if the i2c-parent property has more than one entry? That should
be only true for demuxers.

> +
> + /*
> + * i2c-parent property available in a i2c bus node means that
> + * this node is an extension bus node. In that case,
> + * continue i2c-parent walking up to the adapter node.
> + */
> + err = fwnode_property_get_reference_args(adap_fwnode, "i2c-parent",
> + NULL, 0, 0, &args);
> + if (err)
> + break;
> +
> + pr_debug("Find adapter for %pfw, use parent: %pfw\n", fwnode,
> + args.fwnode);

Is this useful when creating the overlays? I tend to ask you to remove
it one RFC phase is over. If it is useful, it should be at least
dev_dbg?

> +
> + fwnode_handle_put(adap_fwnode);
> + adap_fwnode = args.fwnode;
> + }
> +
> + dev = bus_find_device(&i2c_bus_type, NULL, adap_fwnode,
> i2c_dev_or_parent_fwnode_match);
> + fwnode_handle_put(adap_fwnode);
> if (!dev)
> return NULL;
>
> --
> 2.47.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]