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.
>
> Note that jc42 devices are registered correctly and work with and without the change.
>
> # grep . /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-*/name
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0018/name:jc42
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0019/name:jc42
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-001a/name:jc42
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-001b/name:jc42
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0050/name:spd
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0051/name:spd
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0052/name:spd
> /sys/devices/pci0000:00/0000:00:1f.3/i2c-12/12-0053/name:spd
>
> # sensors|grep -A4 jc42-i2c
> jc42-i2c-12-1b
> Adapter: SMBus I801 adapter at 3000
> temp1: +33.2°C (low = +0.0°C)
> (high = +91.0°C, hyst = +91.0°C)
> (crit = +95.0°C, hyst = +95.0°C)
> --
> jc42-i2c-12-19
> Adapter: SMBus I801 adapter at 3000
> temp1: +33.5°C (low = +0.0°C)
> (high = +91.0°C, hyst = +91.0°C)
> (crit = +95.0°C, hyst = +95.0°C)
> --
> jc42-i2c-12-1a
> Adapter: SMBus I801 adapter at 3000
> temp1: +33.5°C (low = +0.0°C)
> (high = +91.0°C, hyst = +91.0°C)
> (crit = +95.0°C, hyst = +95.0°C)
> --
> jc42-i2c-12-18
> Adapter: SMBus I801 adapter at 3000
> temp1: +33.2°C (low = +0.0°C)
> (high = +91.0°C, hyst = +91.0°C)
> (crit = +95.0°C, hyst = +95.0°C)
>
>
> dmesg:
> [ 0.000000] DMI: Dell Inc. PowerEdge T110 II/0PM2CW, BIOS 2.10.0 05/24/2018
> (...)
> [ 7.681132] i2c_dev: i2c /dev entries driver
> [ 7.687116] i2c i2c-12: 4/4 memory slots populated (from DMI)
> [ 7.690623] at24 12-0050: 256 byte spd EEPROM, read-only
> [ 7.691812] i2c i2c-12: Successfully instantiated SPD at 0x50
> [ 7.698246] at24 12-0051: 256 byte spd EEPROM, read-only
> [ 7.699465] i2c i2c-12: Successfully instantiated SPD at 0x51
> [ 7.700043] i2c i2c-12: Failed to register i2c client jc42 at 0x19 (-16)
> [ 7.700047] i2c i2c-12: Failed creating jc42 at 0x19
> [ 7.705248] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:1f.3/i2c-12/12-001a'
> [ 7.711617] <TASK>
> [ 7.712612] dump_stack_lvl+0x37/0x4a
> [ 7.712612] sysfs_warn_dup+0x55/0x61
> [ 7.715616] sysfs_create_dir_ns+0xa6/0xd2
> [ 7.716620] kobject_add_internal+0xc3/0x1c0
> [ 7.716620] kobject_add+0xba/0xe4
> [ 7.719615] ? device_add+0x53/0x726
> [ 7.720611] device_add+0x132/0x726
> [ 7.720611] i2c_new_client_device+0x1ee/0x246
> [ 7.723616] at24_probe+0x5f8/0x666
> [ 7.724642] ? __pfx_at24_read+0x10/0x10
> [ 7.724642] ? __pfx_at24_write+0x10/0x10
> [ 7.724642] ? __pfx___device_attach_driver+0x10/0x10
> [ 7.727619] i2c_device_probe+0x1b7/0x240
> [ 7.728612] really_probe+0x101/0x248
> [ 7.728612] __driver_probe_device+0xbb/0xed
> [ 7.731620] driver_probe_device+0x1a/0x72
> [ 7.732621] __device_attach_driver+0x82/0x96
> [ 7.732621] bus_for_each_drv+0xa6/0xd4
> [ 7.732621] __device_attach+0xa8/0x12a
> [ 7.735619] bus_probe_device+0x31/0x95
> [ 7.736614] device_add+0x265/0x726
> [ 7.736614] i2c_new_client_device+0x1ee/0x246
> [ 7.739618] i2c_register_spd+0x1a1/0x1ed
> [ 7.740613] i801_probe+0x589/0x603
> [ 7.740613] ? up_write+0x37/0x4d
> [ 7.740613] ? kernfs_add_one+0x104/0x126
> [ 7.743618] ? __raw_spin_unlock_irqrestore+0x14/0x29
> [ 7.744612] pci_device_probe+0xbe/0x12f
> [ 7.744612] really_probe+0x101/0x248
> [ 7.744612] __driver_probe_device+0xbb/0xed
> [ 7.747618] driver_probe_device+0x1a/0x72
> [ 7.748612] __driver_attach_async_helper+0x2d/0x42
> [ 7.748612] async_run_entry_fn+0x25/0xa0
> [ 7.748612] process_scheduled_works+0x193/0x291
> [ 7.748612] worker_thread+0x1c5/0x21f
> [ 7.751619] ? __pfx_worker_thread+0x10/0x10
> [ 7.752611] kthread+0xf6/0xfe
> [ 7.752611] ? __pfx_kthread+0x10/0x10
> [ 7.752611] ret_from_fork+0x23/0x35
> [ 7.755621] ? __pfx_kthread+0x10/0x10
> [ 7.756613] ret_from_fork_asm+0x1b/0x30
> [ 7.756613] </TASK>
> [ 7.759637] i2c i2c-12: Failed to register i2c client jc42 at 0x1a (-17)
> [ 7.760815] at24 12-0052: 256 byte spd EEPROM, read-only
> [ 7.762047] i2c i2c-12: Successfully instantiated SPD at 0x52
> [ 7.765252] i2c i2c-12: Failed to register i2c client jc42 at 0x1b (-16)
> [ 7.766126] at24 12-0053: 256 byte spd EEPROM, read-only
> [ 7.767584] i2c i2c-12: Successfully instantiated SPD at 0x53
>
> Thanks,
> Krzysztof
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.
From c64b23a583fb987e93528033bf6033918cbaf105 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@xxxxxxxxx>
Date: Tue, 2 Jul 2024 21:54:47 +0200
Subject: [PATCH 1/2] i2c: Include device name in check i2c_check_addr_busy()
Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
---
drivers/i2c/i2c-core-base.c | 50 ++++++++++++++++++++++++-------------
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 49fdcb3eb..a6c2974b9 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -779,55 +779,67 @@ int i2c_check_7bit_addr_validity_strict(unsigned short addr)
return 0;
}
-static int __i2c_check_addr_busy(struct device *dev, void *addrp)
+static int __i2c_check_addr_busy(struct device *dev, void *data)
{
struct i2c_client *client = i2c_verify_client(dev);
- int addr = *(int *)addrp;
+ struct i2c_client *cl = data;
+
+ if (client && i2c_encode_flags_to_addr(client) == cl->addr) {
+ if (strcmp(client->name, cl->name))
+ return -EBUSY;
+ else
+ /*
+ * If we find a device with the same name on the address,
+ * then assume we have been instantiated by other means already
+ */
+ return -EEXIST;
+ }
- if (client && i2c_encode_flags_to_addr(client) == addr)
- return -EBUSY;
return 0;
}
/* walk up mux tree */
-static int i2c_check_mux_parents(struct i2c_adapter *adapter, int addr)
+static int i2c_check_mux_parents(struct i2c_adapter *adapter, struct i2c_client *cl)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
int result;
- result = device_for_each_child(&adapter->dev, &addr,
- __i2c_check_addr_busy);
+ result = device_for_each_child(&adapter->dev, cl, __i2c_check_addr_busy);
if (!result && parent)
- result = i2c_check_mux_parents(parent, addr);
+ result = i2c_check_mux_parents(parent, cl);
return result;
}
/* recurse down mux tree */
-static int i2c_check_mux_children(struct device *dev, void *addrp)
+static int i2c_check_mux_children(struct device *dev, void *cl)
{
int result;
if (dev->type == &i2c_adapter_type)
- result = device_for_each_child(dev, addrp,
- i2c_check_mux_children);
+ result = device_for_each_child(dev, cl, i2c_check_mux_children);
else
- result = __i2c_check_addr_busy(dev, addrp);
+ result = __i2c_check_addr_busy(dev, cl);
return result;
}
-static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr)
+static int i2c_check_addr_busy(struct i2c_adapter *adapter, int addr, const char *name)
{
struct i2c_adapter *parent = i2c_parent_is_i2c_adapter(adapter);
+ struct i2c_client cl = {};
int result = 0;
+ cl.addr = addr;
+ if (name)
+ strscpy(cl.name, name, sizeof(cl.name));
+
if (parent)
- result = i2c_check_mux_parents(parent, addr);
+ result = i2c_check_mux_parents(parent, &cl);
if (!result)
- result = device_for_each_child(&adapter->dev, &addr,
+ result = device_for_each_child(&adapter->dev, &cl,
i2c_check_mux_children);
return result;
@@ -963,7 +975,9 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
}
/* Check for address business */
- status = i2c_check_addr_busy(adap, i2c_encode_flags_to_addr(client));
+ status = i2c_check_addr_busy(adap, i2c_encode_flags_to_addr(client), client->name);
+ if (status == -EEXIST)
+ goto out_err_silent;
if (status)
goto out_err;
@@ -2444,7 +2458,7 @@ static int i2c_detect_address(struct i2c_client *temp_client,
}
/* Skip if already in use (7 bit, no need to encode flags) */
- if (i2c_check_addr_busy(adapter, addr))
+ if (i2c_check_addr_busy(adapter, addr, NULL))
return 0;
/* Make sure there is something at this address */
@@ -2559,7 +2573,7 @@ i2c_new_scanned_device(struct i2c_adapter *adap,
}
/* Check address availability (7 bit, no need to encode flags) */
- if (i2c_check_addr_busy(adap, addr_list[i])) {
+ if (i2c_check_addr_busy(adap, addr_list[i], NULL)) {
dev_dbg(&adap->dev,
"Address 0x%02x already in use, not probing\n",
addr_list[i]);
--
2.45.2
From 98c5a9e018c188064dea2debae7651e8b1e6d258 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@xxxxxxxxx>
Date: Sun, 30 Jun 2024 22:56:13 +0200
Subject: [PATCH 2/2] i2c: Serialize client device instantiation
Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
---
drivers/i2c/i2c-core-base.c | 45 ++++++++++++++++++++++++++++++-------
include/linux/i2c.h | 3 +++
2 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index a6c2974b9..2fd3a5432 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -928,7 +928,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources,
}
/**
- * i2c_new_client_device - instantiate an i2c device
+ * __i2c_new_client_device - instantiate an i2c device
* @adap: the adapter managing the device
* @info: describes one I2C device; bus_num is ignored
* Context: can sleep
@@ -944,7 +944,7 @@ int i2c_dev_irq_from_resources(const struct resource *resources,
* i2c_unregister_device(); or an ERR_PTR to describe the error.
*/
struct i2c_client *
-i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
+__i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
{
struct i2c_client *client;
bool need_put = false;
@@ -1025,6 +1025,20 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
kfree(client);
return ERR_PTR(status);
}
+EXPORT_SYMBOL_GPL(__i2c_new_client_device);
+
+
+struct i2c_client *
+i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
+{
+ struct i2c_client *cl;
+
+ rt_mutex_lock_nested(&adap->client_lock, i2c_adapter_depth(adap));
+ cl = __i2c_new_client_device(adap, info);
+ rt_mutex_unlock(&adap->client_lock);
+
+ return cl;
+}
EXPORT_SYMBOL_GPL(i2c_new_client_device);
/**
@@ -1516,6 +1530,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
adap->locked_flags = 0;
rt_mutex_init(&adap->bus_lock);
+ rt_mutex_init(&adap->client_lock);
rt_mutex_init(&adap->mux_lock);
mutex_init(&adap->userspace_clients_lock);
INIT_LIST_HEAD(&adap->userspace_clients);
@@ -2457,13 +2472,15 @@ static int i2c_detect_address(struct i2c_client *temp_client,
return err;
}
+ rt_mutex_lock_nested(&adapter->client_lock, i2c_adapter_depth(adapter));
+
/* Skip if already in use (7 bit, no need to encode flags) */
if (i2c_check_addr_busy(adapter, addr, NULL))
- return 0;
+ goto out;
/* Make sure there is something at this address */
if (!i2c_default_probe(adapter, addr))
- return 0;
+ goto out;
/* Finally call the custom detection function */
memset(&info, 0, sizeof(struct i2c_board_info));
@@ -2472,7 +2489,9 @@ static int i2c_detect_address(struct i2c_client *temp_client,
if (err) {
/* -ENODEV is returned if the detection fails. We catch it
here as this isn't an error. */
- return err == -ENODEV ? 0 : err;
+ if (err == -ENODEV)
+ err = 0;
+ goto out;
}
/* Consistency check */
@@ -2500,7 +2519,9 @@ static int i2c_detect_address(struct i2c_client *temp_client,
dev_err(&adapter->dev, "Failed creating %s at 0x%02x\n",
info.type, info.addr);
}
- return 0;
+out:
+ rt_mutex_unlock(&adapter->client_lock);
+ return err;
}
static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver)
@@ -2559,11 +2580,14 @@ i2c_new_scanned_device(struct i2c_adapter *adap,
unsigned short const *addr_list,
int (*probe)(struct i2c_adapter *adap, unsigned short addr))
{
+ struct i2c_client *cl;
int i;
if (!probe)
probe = i2c_default_probe;
+ rt_mutex_lock_nested(&adap->client_lock, i2c_adapter_depth(adap));
+
for (i = 0; addr_list[i] != I2C_CLIENT_END; i++) {
/* Check address validity */
if (i2c_check_7bit_addr_validity_strict(addr_list[i]) < 0) {
@@ -2587,11 +2611,16 @@ i2c_new_scanned_device(struct i2c_adapter *adap,
if (addr_list[i] == I2C_CLIENT_END) {
dev_dbg(&adap->dev, "Probing failed, no device found\n");
- return ERR_PTR(-ENODEV);
+ cl = ERR_PTR(-ENODEV);
+ goto out;
}
info->addr = addr_list[i];
- return i2c_new_client_device(adap, info);
+ cl = __i2c_new_client_device(adap, info);
+
+out:
+ rt_mutex_unlock(&adap->client_lock);
+ return cl;
}
EXPORT_SYMBOL_GPL(i2c_new_scanned_device);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 424acb98c..981b16147 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -457,6 +457,8 @@ struct i2c_board_info {
*/
struct i2c_client *
i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info);
+struct i2c_client *
+__i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *info);
/* If you don't know the exact address of an I2C device, use this variant
* instead, which can probe for device presence in a list of possible
@@ -725,6 +727,7 @@ struct i2c_adapter {
/* data fields that are valid for all devices */
const struct i2c_lock_operations *lock_ops;
struct rt_mutex bus_lock;
+ struct rt_mutex client_lock;
struct rt_mutex mux_lock;
int timeout; /* in jiffies */
--
2.45.2
