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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z-5O3-FSsHbn27lW@shikoro>
Date: Thu, 3 Apr 2025 11:03:27 +0200
From: Wolfram Sang <wsa+renesas@...g-engineering.com>
To: Herve Codina <herve.codina@...tlin.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-i2c@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Luca Ceresoli <luca.ceresoli@...tlin.com>,
	Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [RFC PATCH 1/3] i2c: core: Follow i2c-parent when retrieving an
 adapter from node


> 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@...tlin.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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)