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]
Date: Mon, 29 Apr 2024 17:46:50 +0200
From: Peter Rosin <peda@...ntia.se>
To: Farouk Bouabid <farouk.bouabid@...obroma-systems.com>,
 Wolfram Sang <wsa+renesas@...g-engineering.com>,
 Andi Shyti <andi.shyti@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Quentin Schulz <quentin.schulz@...obroma-systems.com>,
 Heiko Stuebner <heiko@...ech.de>
Cc: linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
 devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH 1/7] i2c: mux: add the ability to share mux-address with
 child nodes

Hi!

2024-04-26 at 18:49, Farouk Bouabid wrote:
> Allow the mux to have the same address as a child device. This is useful
> when the mux can only use an i2c-address that is used by a child device
> because no other addresses are free to use. eg. the mux can only use
> address 0x18 which is used by amc6821 connected to the mux.
> 
> Signed-off-by: Farouk Bouabid <farouk.bouabid@...obroma-systems.com>
> ---
>  drivers/i2c/i2c-mux.c   | 10 +++++++++-
>  include/linux/i2c-mux.h |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-mux.c b/drivers/i2c/i2c-mux.c
> index 57ff09f18c37..f5357dff8cc5 100644
> --- a/drivers/i2c/i2c-mux.c
> +++ b/drivers/i2c/i2c-mux.c
> @@ -331,7 +331,6 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  	priv->adap.owner = THIS_MODULE;
>  	priv->adap.algo = &priv->algo;
>  	priv->adap.algo_data = priv;
> -	priv->adap.dev.parent = &parent->dev;
>  	priv->adap.retries = parent->retries;
>  	priv->adap.timeout = parent->timeout;
>  	priv->adap.quirks = parent->quirks;
> @@ -348,6 +347,15 @@ int i2c_mux_add_adapter(struct i2c_mux_core *muxc,
>  	else
>  		priv->adap.class = class;
>  
> +	/*
> +	 * When creating the adapter, the node devices are checked for i2c address
> +	 * match with other devices on the parent adapter, among which is the mux itself.
> +	 * If a match is found the node device is not probed successfully.
> +	 * Allow the mux to have the same address as a child device by skipping this check.
> +	 */
> +	if (!(muxc->share_addr_with_children))
> +		priv->adap.dev.parent = &parent->dev;

This is a dirty hack that will not generally do the right thing.

The adapter device parent is not there solely for the purpose of
detecting address clashes, so the above has other implications
that are not desirable.

Therefore, NACK on this approach. It simply needs to be more involved.
Sorry.

Cheers,
Peter

> +
>  	/*
>  	 * Try to populate the mux adapter's of_node, expands to
>  	 * nothing if !CONFIG_OF.
> diff --git a/include/linux/i2c-mux.h b/include/linux/i2c-mux.h
> index 98ef73b7c8fd..17ac68bf1703 100644
> --- a/include/linux/i2c-mux.h
> +++ b/include/linux/i2c-mux.h
> @@ -21,6 +21,7 @@ struct i2c_mux_core {
>  	unsigned int mux_locked:1;
>  	unsigned int arbitrator:1;
>  	unsigned int gate:1;
> +	unsigned int share_addr_with_children:1;
>  
>  	void *priv;
>  
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ