[PATCH 0/2] i2c: core-managed per-client directory in debugfs with example

[PATCH 0/2] i2c: core-managed per-client directory in debugfs with example

[Wolfram Sang]

I think there are enough i2c_drivers now using debugfs that it justifies
a central handling of an debugfs entry per i2c client.

Here is the I2C core code with an example conversion of the isl28022
driver. Quite convincing diffstat, I'd say.

Tested on a Renesas RZ/G3S SMARC board. The branch is based on 6.13-rc6
plus 'i2c/for-mergewindow' plus 'hwmon/next' and can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/g3s/isl28022

@Guenter: because both patches have dependencies in their respective
for-next branches, I suggest that I'll apply the I2C core patch soon and
resend the ISL patch during next merge window once my pull request is
included. Does that sound okay to you? And once both are in, I will send
further conversions as RFT patches aiming for 6.15?

Looking forward to further comments as well!

Happy hacking!


Wolfram Sang (2):
  i2c: add core-managed per-client directory in debugfs
  hwmon: (isl28022) Use per-client debugfs entry

 drivers/hwmon/isl28022.c    | 44 ++-----------------------------------
 drivers/i2c/i2c-core-base.c |  4 ++++
 include/linux/i2c.h         |  1 +
 3 files changed, 7 insertions(+), 42 deletions(-)

-- 
2.45.2

[PATCH 1/2] i2c: add core-managed per-client directory in debugfs

[Wolfram Sang]

More and more I2C client drivers use debugfs entries and currently they
need to manage a subdir for their files on their own. This means
inconsistent naming for these subdirs and they are scattered all over
the debugfs-tree as well. Not to mention the duplicated code.

Let the I2C core provide and maintain a proper directory per client.

Note: It was considered to save the additional pointer in 'struct
i2c_client' and only provide a subdir when requested via a helper
function. When sketching this approach, more and more corner cases
appeared, though, so the current solution with its simple and unabiguous
code was chosen.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/i2c-core-base.c | 4 ++++
 include/linux/i2c.h         | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index b072030a9105..00f171ebc01f 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1015,6 +1015,8 @@ 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));
 
@@ -1058,6 +1060,8 @@ 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);
 }
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 66fb3d6cf686..36de788dc7fe 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -347,6 +347,7 @@ struct i2c_client {
 	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
 #endif
 	void *devres_group_id;		/* ID of probe devres group	*/
+	struct dentry *debugfs;		/* per-client debugfs dir	*/
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
 
-- 
2.45.2

Re: [PATCH 1/2] i2c: add core-managed per-client directory in debugfs

[Guenter Roeck]

On Thu, Jan 09, 2025 at 01:21:10PM +0100, Wolfram Sang wrote:
> More and more I2C client drivers use debugfs entries and currently they
> need to manage a subdir for their files on their own. This means
> inconsistent naming for these subdirs and they are scattered all over
> the debugfs-tree as well. Not to mention the duplicated code.
> 
> Let the I2C core provide and maintain a proper directory per client.
> 
> Note: It was considered to save the additional pointer in 'struct
> i2c_client' and only provide a subdir when requested via a helper
> function. When sketching this approach, more and more corner cases
> appeared, though, so the current solution with its simple and unabiguous
> code was chosen.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/i2c/i2c-core-base.c | 4 ++++
>  include/linux/i2c.h         | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index b072030a9105..00f171ebc01f 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -1015,6 +1015,8 @@ 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));
>  
> @@ -1058,6 +1060,8 @@ 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);
>  }
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 66fb3d6cf686..36de788dc7fe 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -347,6 +347,7 @@ struct i2c_client {
>  	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
>  #endif
>  	void *devres_group_id;		/* ID of probe devres group	*/
> +	struct dentry *debugfs;		/* per-client debugfs dir	*/
>  };
>  #define to_i2c_client(d) container_of(d, struct i2c_client, dev)
>  
> -- 
> 2.45.2
> 

Re: [PATCH 1/2] i2c: add core-managed per-client directory in debugfs

[Wolfram Sang]

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

On Thu, Jan 09, 2025 at 01:21:10PM +0100, Wolfram Sang wrote:
> More and more I2C client drivers use debugfs entries and currently they
> need to manage a subdir for their files on their own. This means
> inconsistent naming for these subdirs and they are scattered all over
> the debugfs-tree as well. Not to mention the duplicated code.
> 
> Let the I2C core provide and maintain a proper directory per client.
> 
> Note: It was considered to save the additional pointer in 'struct
> i2c_client' and only provide a subdir when requested via a helper
> function. When sketching this approach, more and more corner cases
> appeared, though, so the current solution with its simple and unabiguous
> code was chosen.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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