Hi Daniel,
On Wed, Oct 18, 2023 at 11:46:13AM +0200, Daniel Mack wrote:
> When adapters are chained in a sparse manner (with intermediate MFD devices,
So, you have an MFD including an i2c-mux or something?
> for instance) the code currently fails to use the correct subclass for
> the adapter's bus_lock which leads to false-positive lockdep warnings.
>
> Fix this by walking the entire pedigree of the device and count all
> adapters along the way instead of just checking the immediate parent.
Sounds reasonable to me.
>
> Signed-off-by: Daniel Mack <daniel@xxxxxxxxxx>
> ---
> This hit me when during the development of a driver stack that isn't
> submitted mainline yet. This patch could however be discussed
> independently I think.
Yes, it can :)
>
> drivers/i2c/i2c-core-base.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 60746652fd52..4692a1e5ea0a 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1189,9 +1189,11 @@ static void i2c_adapter_dev_release(struct device *dev)
> unsigned int i2c_adapter_depth(struct i2c_adapter *adapter)
> {
> unsigned int depth = 0;
> + struct device *parent;
>
> - while ((adapter = i2c_parent_is_i2c_adapter(adapter)))
I never noticed we overwrite the 'adapter' function argument. Much
better with your version and the local variable.
> - depth++;
> + for (parent = adapter->dev.parent; parent; parent = parent->parent)
> + if (parent->type == &i2c_adapter_type)
> + depth++;
I am not sure myself. Is the code explaining itself or should we add a
short comment why we use a for-loop? I tend to leave it as is.
Thanks for this patch!
Wolfram
Attachment:
signature.asc
Description: PGP signature
