| Message ID | 20231018094613.849007-1-daniel@zonque.org |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: core: fix lockdep warning for sparsely nested adapter chain | expand |
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@zonque.org> > --- > 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
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, > 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. > > Signed-off-by: Daniel Mack <daniel@zonque.org> Applied to for-next, thanks!
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))) - depth++; + for (parent = adapter->dev.parent; parent; parent = parent->parent) + if (parent->type == &i2c_adapter_type) + depth++; WARN_ONCE(depth >= MAX_LOCKDEP_SUBCLASSES, "adapter depth exceeds lockdep subclass limit\n");
When adapters are chained in a sparse manner (with intermediate MFD devices, 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. Signed-off-by: Daniel Mack <daniel@zonque.org> --- 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. drivers/i2c/i2c-core-base.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)