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=l... - 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)