i2c_del_adapter() can be called recursively if it has an i2c mux on the bus. This results in a deadlock. We use the lock to protect from parallel unregistering of clients in case driver and adapter are removed at the same time. The fix approach is based on the fact that the used iterators are klist-based. So it's safe to remove list elements during the iteration, and we don't have to acquire the core lock. As a result we just have to prevent that i2c_unregister_device() is executed in parallel for the same client. Use an atomic bit op for this purpose. Fixes: 56a50667cbcf ("i2c: Replace list-based mechanism for handling auto-detected clients") Reported-by: Herve Codina <herve.codina@xxxxxxxxxxx> Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> --- drivers/i2c/i2c-core-base.c | 9 +++------ include/linux/i2c.h | 3 ++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index 6eade239f..44b861eb4 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -1059,6 +1059,9 @@ void i2c_unregister_device(struct i2c_client *client) if (IS_ERR_OR_NULL(client)) return; + if (test_and_set_bit(I2C_CLIENT_UNREGISTERING, &client->flags)) + return; + if (client->dev.of_node) { of_node_clear_flag(client->dev.of_node, OF_POPULATED); of_node_put(client->dev.of_node); @@ -1354,7 +1357,6 @@ delete_device_store(struct device *dev, struct device_attribute *attr, return -EINVAL; } - mutex_lock(&core_lock); /* Make sure the device was added through sysfs */ child_dev = device_find_child(&adap->dev, &addr, __i2c_find_user_addr); if (child_dev) { @@ -1364,7 +1366,6 @@ delete_device_store(struct device *dev, struct device_attribute *attr, dev_err(dev, "Can't find userspace-created device at %#x\n", addr); count = -ENOENT; } - mutex_unlock(&core_lock); return count; } @@ -1745,10 +1746,8 @@ void i2c_del_adapter(struct i2c_adapter *adap) * we can't remove the dummy devices during the first pass: they * could have been instantiated by real devices wishing to clean * them up properly, so we give them a chance to do that first. */ - mutex_lock(&core_lock); device_for_each_child(&adap->dev, NULL, __unregister_client); device_for_each_child(&adap->dev, NULL, __unregister_dummy); - mutex_unlock(&core_lock); /* device name is gone after device_unregister */ dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name); @@ -2002,12 +2001,10 @@ static int __i2c_unregister_detected_client(struct device *dev, void *argp) */ void i2c_del_driver(struct i2c_driver *driver) { - mutex_lock(&core_lock); /* Satisfy __must_check, function can't fail */ if (driver_for_each_device(&driver->driver, NULL, NULL, __i2c_unregister_detected_client)) { } - mutex_unlock(&core_lock); driver_unregister(&driver->driver); pr_debug("driver [%s] unregistered\n", driver->driver.name); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index c31fd1dba..c9b6c0f50 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -325,7 +325,7 @@ struct i2c_driver { * managing the device. */ struct i2c_client { - unsigned short flags; /* div., see below */ + unsigned long flags; /* div., see below */ #define I2C_CLIENT_PEC 0x04 /* Use Packet Error Checking */ #define I2C_CLIENT_TEN 0x10 /* we have a ten bit chip address */ /* Must equal I2C_M_TEN below */ @@ -334,6 +334,7 @@ struct i2c_client { #define I2C_CLIENT_WAKE 0x80 /* for board_info; true iff can wake */ #define I2C_CLIENT_AUTO 0x100 /* client was auto-detected */ #define I2C_CLIENT_USER 0x200 /* client was userspace-created */ +#define I2C_CLIENT_UNREGISTERING 0x400 /* client is being unregistered */ #define I2C_CLIENT_SCCB 0x9000 /* Use Omnivision SCCB protocol */ /* Must match I2C_M_STOP|IGNORE_NAK */ -- 2.48.1