Hi Wolfram,
On 10/26/23 19:04, Wolfram Sang wrote:
> 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?
Yes exactly. I have an I2C device that creates MFD devices, and one of
them creates a child device which is an I2C host. So the hosts are not
directly linked to one another but in a spare manner.
>> 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.
It's pretty obvious what it does I would say.
Thanks,
Daniel
