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

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- 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 --]