| Message ID | 20250127153938.34630-1-linux@roeck-us.net |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: Fix core-managed per-client debugfs handling | expand |
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!
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); }
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(-)