[PATCH] i2c: Fix core-managed per-client debugfs handling

[PATCH] i2c: Fix core-managed per-client debugfs handling

[Guenter Roeck]

Per-driver debugfs entries are created in the device probe function and
released in the device remove function. The common debugfs directory
introduced with commit d06905d68610 ("i2c: add core-managed per-client
directory in debugfs") is added when a device is registered, not when it
is probed, and it is removed when the device is unregistered. As result,
debugfs entries added by a driver are not deleted when a device remove
function is called since that does not necessarily result in device
unregistration. If the probe function is then called again, the debugfs
entries will already exist, which will result in error messages such as

	debugfs: File 'test' in directory '3-0020' already present!

if 'test' was a debugfs file created during the first call to probe().

This is easy to reproduce by executing "modprobe -r" followed by "modprobe"
with a driver using the debugfs pointer created by the i2c subsystem.

The debugfs directory should be created when a device is probed, not when
it is registered. It should be removed when the device is removed, not
when it is unregistered. Change the code accordingly.

Also clear the client->debugfs if creating the debugfs directory fails.
This simplifies I2C client driver code if it needs to call dentry
functions which do not validate dentry pointers passed as argument.

Fixes: d06905d68610 ("i2c: add core-managed per-client directory in debugfs")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/i2c/i2c-core-base.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index c24ccefb015e..6ae6313b2ea1 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -583,6 +583,11 @@ static int i2c_device_probe(struct device *dev)
 		goto err_detach_pm_domain;
 	}
 
+	client->debugfs = debugfs_create_dir(dev_name(&client->dev),
+					     client->adapter->debugfs);
+	if (IS_ERR(client->debugfs))
+		client->debugfs = NULL;
+
 	if (driver->probe)
 		status = driver->probe(client);
 	else
@@ -602,6 +607,7 @@ static int i2c_device_probe(struct device *dev)
 	return 0;
 
 err_release_driver_resources:
+	debugfs_remove_recursive(client->debugfs);
 	devres_release_group(&client->dev, client->devres_group_id);
 err_detach_pm_domain:
 	dev_pm_domain_detach(&client->dev, do_power_on);
@@ -627,6 +633,8 @@ static void i2c_device_remove(struct device *dev)
 		driver->remove(client);
 	}
 
+	debugfs_remove_recursive(client->debugfs);
+
 	devres_release_group(&client->dev, client->devres_group_id);
 
 	dev_pm_domain_detach(&client->dev, true);
@@ -1015,8 +1023,6 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
 	if (status)
 		goto out_remove_swnode;
 
-	client->debugfs = debugfs_create_dir(dev_name(&client->dev), adap->debugfs);
-
 	dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n",
 		client->name, dev_name(&client->dev));
 
@@ -1061,7 +1067,6 @@ void i2c_unregister_device(struct i2c_client *client)
 	if (ACPI_COMPANION(&client->dev))
 		acpi_device_clear_enumerated(ACPI_COMPANION(&client->dev));
 
-	debugfs_remove_recursive(client->debugfs);
 	device_remove_software_node(&client->dev);
 	device_unregister(&client->dev);
 }
-- 
2.45.2

Re: [PATCH] i2c: Fix core-managed per-client debugfs handling

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

On Mon, Jan 27, 2025 at 07:39:38AM -0800, Guenter Roeck wrote:
> Per-driver debugfs entries are created in the device probe function and
> released in the device remove function. The common debugfs directory
> introduced with commit d06905d68610 ("i2c: add core-managed per-client
> directory in debugfs") is added when a device is registered, not when it
> is probed, and it is removed when the device is unregistered. As result,
> debugfs entries added by a driver are not deleted when a device remove
> function is called since that does not necessarily result in device
> unregistration. If the probe function is then called again, the debugfs
> entries will already exist, which will result in error messages such as
> 
> 	debugfs: File 'test' in directory '3-0020' already present!
> 
> if 'test' was a debugfs file created during the first call to probe().
> 
> This is easy to reproduce by executing "modprobe -r" followed by "modprobe"
> with a driver using the debugfs pointer created by the i2c subsystem.
> 
> The debugfs directory should be created when a device is probed, not when
> it is registered. It should be removed when the device is removed, not
> when it is unregistered. Change the code accordingly.
> 
> Also clear the client->debugfs if creating the debugfs directory fails.
> This simplifies I2C client driver code if it needs to call dentry
> functions which do not validate dentry pointers passed as argument.
> 
> Fixes: d06905d68610 ("i2c: add core-managed per-client directory in debugfs")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Applied to for-current, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]