On 23.07.2024 16:12, Krzysztof Olędzki wrote: > On 06.07.2024 at 18:42, Krzysztof Olędzki wrote: >> On 02.07.2024 at 13:25, Heiner Kallweit wrote: >>> On 23.06.2024 20:47, Krzysztof Olędzki wrote: >>>> Hi, >>>> >>>> After upgrading kernel to Linux 6.6.34 on one of my systems, I noticed "sysfs: cannot create duplicate filename" and i2c registration errors in dmesg, please see below. >>>> >>>> This seems to be related to https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-6.6.y&id=4d5ace787273cb159bfdcf1c523df957938b3e42 - reverting the change fixes the problem. >> >> <CUT> >> >>> >>> Could you please test whether the attached two experimental patches fix the issue for you? >>> They serialize client device instantiation per I2C adapter, and include the client device >>> name in the check whether a bus address is busy. >> >> Sadly, they crash the kernel. >> >> I will get serial console attached there next week, so will be able to capture the full crash. >> For now, I was able to obtain a photo. I'm very sorry for the quality, just wanted to provide >> something for now. > > Sorry it took me so long - my attempts to coordinate setting up serial console > were not successful, so it had to wait for me to go there in person... > > I have attached complete dmesg, summary: > > [ 10.905953] rtmutex deadlock detected > [ 10.909959] WARNING: CPU: 5 PID: 83 at kernel/locking/rtmutex.c:1642 __rt_mutex_slowlock_locked.constprop.0+0x10f/0x1a5 > [ 10.920961] CPU: 5 PID: 83 Comm: kworker/u16:3 Not tainted 6.6.34-o5 #1 > [ 10.929970] Hardware name: Dell Inc. PowerEdge T110 II/0PM2CW, BIOS 2.10.0 05/24/2018 > [ 10.938954] Workqueue: events_unbound async_run_entry_fn > > > [ 11.336954] BUG: scheduling while atomic: kworker/u16:3/83/0x00000002 > [ 11.342953] Preemption disabled at: > [ 11.342953] [<0000000000000000>] 0x0 > [ 11.350953] CPU: 5 PID: 83 Comm: kworker/u16:3 Tainted: G W 6.6.34-o5 #1 > [ 11.361954] Hardware name: Dell Inc. PowerEdge T110 II/0PM2CW, BIOS 2.10.0 05/24/2018 > [ 11.369953] Workqueue: events_unbound async_run_entry_fn > Thanks a lot for the comprehensive info. Reason for the deadlock is that calls to i2c_new_client_device() can be nested. So another solution approach is needed. I'd appreciate if you could test also the new version below. > > > Krzysztof Heiner diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c index b63f75e44..c1074d409 100644 --- a/drivers/i2c/i2c-core-base.c +++ b/drivers/i2c/i2c-core-base.c @@ -915,6 +915,29 @@ int i2c_dev_irq_from_resources(const struct resource *resources, return 0; } +/** + * Serialize device instantiation in case it can be instantiated explicitly + * and by auto-detection + */ +static int i2c_test_and_set_addr_in_instantiation(struct i2c_adapter *adap, + const struct i2c_client *client) +{ + if (!(client->flags & I2C_CLIENT_TEN) && + !(client->flags & I2C_CLIENT_SLAVE) && + test_and_set_bit(client->addr, adap->addrs_in_instantiation)) + return -EBUSY; + + return 0; +} + +static void i2c_clear_addr_in_instantiation(struct i2c_adapter *adap, + const struct i2c_client *client) +{ + if (!(client->flags & I2C_CLIENT_TEN) && + !(client->flags & I2C_CLIENT_SLAVE)) + clear_bit(client->addr, adap->addrs_in_instantiation); +} + /** * i2c_new_client_device - instantiate an i2c device * @adap: the adapter managing the device @@ -962,6 +985,10 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf goto out_err_silent; } + status = i2c_test_and_set_addr_in_instantiation(adap, client); + if (status) + goto out_err_silent; + /* Check for address business */ status = i2c_check_addr_busy(adap, i2c_encode_flags_to_addr(client)); if (status) @@ -993,6 +1020,8 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n", client->name, dev_name(&client->dev)); + i2c_clear_addr_in_instantiation(adap, client); + return client; out_remove_swnode: @@ -1004,6 +1033,7 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf dev_err(&adap->dev, "Failed to register i2c client %s at 0x%02x (%d)\n", client->name, client->addr, status); + i2c_clear_addr_in_instantiation(adap, client); out_err_silent: if (need_put) put_device(&client->dev); diff --git a/include/linux/i2c.h b/include/linux/i2c.h index 07e33bbc9..31486455f 100644 --- a/include/linux/i2c.h +++ b/include/linux/i2c.h @@ -761,6 +761,9 @@ struct i2c_adapter { struct regulator *bus_regulator; struct dentry *debugfs; + + /* covers 7bit addresses only */ + DECLARE_BITMAP(addrs_in_instantiation, 1 << 7); }; #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev) -- 2.46.0
