[PATCH v2 0/3] i2c: create debugfs entry per adapter

[PATCH v2 0/3] i2c: create debugfs entry per adapter

[Wolfram Sang]

Two drivers already implement custom debugfs handling for their
i2c_adapter and more will come. So, let the core create a debugfs
directory per adapter and pass that to drivers for their debugfs files.
Convert the two existing users as well. Tested on a Renesas Lager board
with the i2c-gpio driver. npcm7xx was tested using QEMU with the
quanta-gsj target.

Changes since v1:
* rebased to something close to 6.7-rc1 which includes i2c/for-mergewindow
* tested npcm7xx with QEMU

Wolfram Sang (3):
  i2c: create debugfs entry per adapter
  i2c: gpio: move to per-adapter debugfs directory
  i2c: npcm7xx: move to per-adapter debugfs directory

 drivers/i2c/busses/i2c-gpio.c    | 34 ++++------------------
 drivers/i2c/busses/i2c-npcm7xx.c | 49 +++++---------------------------
 drivers/i2c/i2c-core-base.c      | 11 +++++++
 include/linux/i2c.h              |  2 ++
 4 files changed, 26 insertions(+), 70 deletions(-)

-- 
2.35.1

[PATCH v2 1/3] i2c: create debugfs entry per adapter

[Wolfram Sang]

Two drivers already implement custom debugfs handling for their
i2c_adapter and more will come. So, let the core create a debugfs
directory per adapter and pass that to drivers for their debugfs files.

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

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index eac90a3cf61a..f6c828bbd166 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -16,6 +16,7 @@
 #include <linux/acpi.h>
 #include <linux/clk/clk-conf.h>
 #include <linux/completion.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/errno.h>
@@ -67,6 +68,8 @@ static int i2c_detect(struct i2c_adapter *adapter, struct i2c_driver *driver);
 static DEFINE_STATIC_KEY_FALSE(i2c_trace_msg_key);
 static bool is_registered;
 
+static struct dentry *i2c_debugfs_root;
+
 int i2c_transfer_trace_reg(void)
 {
 	static_branch_inc(&i2c_trace_msg_key);
@@ -1524,6 +1527,8 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 		goto out_list;
 	}
 
+	adap->debugfs = debugfs_create_dir(dev_name(&adap->dev), i2c_debugfs_root);
+
 	res = i2c_setup_smbus_alert(adap);
 	if (res)
 		goto out_reg;
@@ -1563,6 +1568,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
 	return 0;
 
 out_reg:
+	debugfs_remove_recursive(adap->debugfs);
 	init_completion(&adap->dev_released);
 	device_unregister(&adap->dev);
 	wait_for_completion(&adap->dev_released);
@@ -1764,6 +1770,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
 	i2c_host_notify_irq_teardown(adap);
 
+	debugfs_remove_recursive(adap->debugfs);
+
 	/* wait until all references to the device are gone
 	 *
 	 * FIXME: This is old code and should ideally be replaced by an
@@ -2061,6 +2069,8 @@ static int __init i2c_init(void)
 
 	is_registered = true;
 
+	i2c_debugfs_root = debugfs_create_dir("i2c", NULL);
+
 #ifdef CONFIG_I2C_COMPAT
 	i2c_adapter_compat_class = class_compat_register("i2c-adapter");
 	if (!i2c_adapter_compat_class) {
@@ -2099,6 +2109,7 @@ static void __exit i2c_exit(void)
 #ifdef CONFIG_I2C_COMPAT
 	class_compat_unregister(i2c_adapter_compat_class);
 #endif
+	debugfs_remove_recursive(i2c_debugfs_root);
 	bus_unregister(&i2c_bus_type);
 	tracepoint_synchronize_unregister();
 }
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 0dae9db27538..cb93a054cdec 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -746,6 +746,8 @@ struct i2c_adapter {
 
 	struct irq_domain *host_notify_domain;
 	struct regulator *bus_regulator;
+
+	struct dentry *debugfs;
 };
 #define to_i2c_adapter(d) container_of(d, struct i2c_adapter, dev)
 
-- 
2.35.1

[PATCH v2 2/3] i2c: gpio: move to per-adapter debugfs directory

[Wolfram Sang]

The I2C core now provides a per-adapter debugfs directory. Use it
instead of creating a custom one.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-gpio.c | 34 ++++++----------------------------
 1 file changed, 6 insertions(+), 28 deletions(-)

diff --git a/drivers/i2c/busses/i2c-gpio.c b/drivers/i2c/busses/i2c-gpio.c
index fb35a75fe0e3..9c8531137354 100644
--- a/drivers/i2c/busses/i2c-gpio.c
+++ b/drivers/i2c/busses/i2c-gpio.c
@@ -25,7 +25,6 @@ struct i2c_gpio_private_data {
 	struct i2c_algo_bit_data bit_data;
 	struct i2c_gpio_platform_data pdata;
 #ifdef CONFIG_I2C_GPIO_FAULT_INJECTOR
-	struct dentry *debug_dir;
 	/* these must be protected by bus lock */
 	struct completion scl_irq_completion;
 	u64 scl_irq_data;
@@ -72,7 +71,6 @@ static int i2c_gpio_getscl(void *data)
 }
 
 #ifdef CONFIG_I2C_GPIO_FAULT_INJECTOR
-static struct dentry *i2c_gpio_debug_dir;
 
 #define setsda(bd, val)	((bd)->setsda((bd)->data, val))
 #define setscl(bd, val)	((bd)->setscl((bd)->data, val))
@@ -258,41 +256,23 @@ static void i2c_gpio_fault_injector_init(struct platform_device *pdev)
 {
 	struct i2c_gpio_private_data *priv = platform_get_drvdata(pdev);
 
-	/*
-	 * If there will be a debugfs-dir per i2c adapter somewhen, put the
-	 * 'fault-injector' dir there. Until then, we have a global dir with
-	 * all adapters as subdirs.
-	 */
-	if (!i2c_gpio_debug_dir)
-		i2c_gpio_debug_dir = debugfs_create_dir("i2c-fault-injector", NULL);
-
-	priv->debug_dir = debugfs_create_dir(pdev->name, i2c_gpio_debug_dir);
-
 	init_completion(&priv->scl_irq_completion);
 
-	debugfs_create_file_unsafe("incomplete_address_phase", 0200, priv->debug_dir,
+	debugfs_create_file_unsafe("incomplete_address_phase", 0200, priv->adap.debugfs,
 				   priv, &fops_incomplete_addr_phase);
-	debugfs_create_file_unsafe("incomplete_write_byte", 0200, priv->debug_dir,
+	debugfs_create_file_unsafe("incomplete_write_byte", 0200, priv->adap.debugfs,
 				   priv, &fops_incomplete_write_byte);
 	if (priv->bit_data.getscl) {
-		debugfs_create_file_unsafe("inject_panic", 0200, priv->debug_dir,
+		debugfs_create_file_unsafe("inject_panic", 0200, priv->adap.debugfs,
 					   priv, &fops_inject_panic);
-		debugfs_create_file_unsafe("lose_arbitration", 0200, priv->debug_dir,
+		debugfs_create_file_unsafe("lose_arbitration", 0200, priv->adap.debugfs,
 					   priv, &fops_lose_arbitration);
 	}
-	debugfs_create_file_unsafe("scl", 0600, priv->debug_dir, priv, &fops_scl);
-	debugfs_create_file_unsafe("sda", 0600, priv->debug_dir, priv, &fops_sda);
-}
-
-static void i2c_gpio_fault_injector_exit(struct platform_device *pdev)
-{
-	struct i2c_gpio_private_data *priv = platform_get_drvdata(pdev);
-
-	debugfs_remove_recursive(priv->debug_dir);
+	debugfs_create_file_unsafe("scl", 0600, priv->adap.debugfs, priv, &fops_scl);
+	debugfs_create_file_unsafe("sda", 0600, priv->adap.debugfs, priv, &fops_sda);
 }
 #else
 static inline void i2c_gpio_fault_injector_init(struct platform_device *pdev) {}
-static inline void i2c_gpio_fault_injector_exit(struct platform_device *pdev) {}
 #endif /* CONFIG_I2C_GPIO_FAULT_INJECTOR*/
 
 /* Get i2c-gpio properties from DT or ACPI table */
@@ -475,8 +455,6 @@ static void i2c_gpio_remove(struct platform_device *pdev)
 	struct i2c_gpio_private_data *priv;
 	struct i2c_adapter *adap;
 
-	i2c_gpio_fault_injector_exit(pdev);
-
 	priv = platform_get_drvdata(pdev);
 	adap = &priv->adap;
 
-- 
2.35.1

[PATCH v2 3/3] i2c: npcm7xx: move to per-adapter debugfs directory

[Wolfram Sang]

The I2C core now provides a per-adapter debugfs directory. Use it
instead of creating a custom one.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-npcm7xx.c | 49 +++++---------------------------
 1 file changed, 7 insertions(+), 42 deletions(-)

diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
index ae4bae63ad4f..54181b3f1919 100644
--- a/drivers/i2c/busses/i2c-npcm7xx.c
+++ b/drivers/i2c/busses/i2c-npcm7xx.c
@@ -326,7 +326,6 @@ struct npcm_i2c {
 	u8 slv_rd_buf[MAX_I2C_HW_FIFO_SIZE];
 	u8 slv_wr_buf[MAX_I2C_HW_FIFO_SIZE];
 #endif
-	struct dentry *debugfs; /* debugfs device directory */
 	u64 ber_cnt;
 	u64 rec_succ_cnt;
 	u64 rec_fail_cnt;
@@ -2250,27 +2249,15 @@ static const struct i2c_algorithm npcm_i2c_algo = {
 #endif
 };
 
-/* i2c debugfs directory: used to keep health monitor of i2c devices */
-static struct dentry *npcm_i2c_debugfs_dir;
-
 static void npcm_i2c_init_debugfs(struct platform_device *pdev,
 				  struct npcm_i2c *bus)
 {
-	struct dentry *d;
-
-	if (!npcm_i2c_debugfs_dir)
-		return;
-	d = debugfs_create_dir(dev_name(&pdev->dev), npcm_i2c_debugfs_dir);
-	if (IS_ERR_OR_NULL(d))
-		return;
-	debugfs_create_u64("ber_cnt", 0444, d, &bus->ber_cnt);
-	debugfs_create_u64("nack_cnt", 0444, d, &bus->nack_cnt);
-	debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt);
-	debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt);
-	debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt);
-	debugfs_create_u64("tx_complete_cnt", 0444, d, &bus->tx_complete_cnt);
-
-	bus->debugfs = d;
+	debugfs_create_u64("ber_cnt", 0444, bus->adap.debugfs, &bus->ber_cnt);
+	debugfs_create_u64("nack_cnt", 0444, bus->adap.debugfs, &bus->nack_cnt);
+	debugfs_create_u64("rec_succ_cnt", 0444, bus->adap.debugfs, &bus->rec_succ_cnt);
+	debugfs_create_u64("rec_fail_cnt", 0444, bus->adap.debugfs, &bus->rec_fail_cnt);
+	debugfs_create_u64("timeout_cnt", 0444, bus->adap.debugfs, &bus->timeout_cnt);
+	debugfs_create_u64("tx_complete_cnt", 0444, bus->adap.debugfs, &bus->tx_complete_cnt);
 }
 
 static int npcm_i2c_probe_bus(struct platform_device *pdev)
@@ -2362,7 +2349,6 @@ static void npcm_i2c_remove_bus(struct platform_device *pdev)
 	unsigned long lock_flags;
 	struct npcm_i2c *bus = platform_get_drvdata(pdev);
 
-	debugfs_remove_recursive(bus->debugfs);
 	spin_lock_irqsave(&bus->lock, lock_flags);
 	npcm_i2c_disable(bus);
 	spin_unlock_irqrestore(&bus->lock, lock_flags);
@@ -2385,28 +2371,7 @@ static struct platform_driver npcm_i2c_bus_driver = {
 	}
 };
 
-static int __init npcm_i2c_init(void)
-{
-	int ret;
-
-	npcm_i2c_debugfs_dir = debugfs_create_dir("npcm_i2c", NULL);
-
-	ret = platform_driver_register(&npcm_i2c_bus_driver);
-	if (ret) {
-		debugfs_remove_recursive(npcm_i2c_debugfs_dir);
-		return ret;
-	}
-
-	return 0;
-}
-module_init(npcm_i2c_init);
-
-static void __exit npcm_i2c_exit(void)
-{
-	platform_driver_unregister(&npcm_i2c_bus_driver);
-	debugfs_remove_recursive(npcm_i2c_debugfs_dir);
-}
-module_exit(npcm_i2c_exit);
+module_platform_driver(npcm_i2c_bus_driver);
 
 MODULE_AUTHOR("Avi Fishman <avi.fishman@gmail.com>");
 MODULE_AUTHOR("Tali Perry <tali.perry@nuvoton.com>");
-- 
2.35.1

Re: [PATCH v2 3/3] i2c: npcm7xx: move to per-adapter debugfs directory

[Tali Perry]

On Mon, Nov 13, 2023 at 12:59 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> The I2C core now provides a per-adapter debugfs directory. Use it
> instead of creating a custom one.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-npcm7xx.c | 49 +++++---------------------------
>  1 file changed, 7 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c
> index ae4bae63ad4f..54181b3f1919 100644
> --- a/drivers/i2c/busses/i2c-npcm7xx.c
> +++ b/drivers/i2c/busses/i2c-npcm7xx.c
> @@ -326,7 +326,6 @@ struct npcm_i2c {
>         u8 slv_rd_buf[MAX_I2C_HW_FIFO_SIZE];
>         u8 slv_wr_buf[MAX_I2C_HW_FIFO_SIZE];
>  #endif
> -       struct dentry *debugfs; /* debugfs device directory */
>         u64 ber_cnt;
>         u64 rec_succ_cnt;
>         u64 rec_fail_cnt;
> @@ -2250,27 +2249,15 @@ static const struct i2c_algorithm npcm_i2c_algo = {
>  #endif
>  };
>
> -/* i2c debugfs directory: used to keep health monitor of i2c devices */
> -static struct dentry *npcm_i2c_debugfs_dir;
> -
>  static void npcm_i2c_init_debugfs(struct platform_device *pdev,
>                                   struct npcm_i2c *bus)
>  {
> -       struct dentry *d;
> -
> -       if (!npcm_i2c_debugfs_dir)
> -               return;
> -       d = debugfs_create_dir(dev_name(&pdev->dev), npcm_i2c_debugfs_dir);
> -       if (IS_ERR_OR_NULL(d))
> -               return;
> -       debugfs_create_u64("ber_cnt", 0444, d, &bus->ber_cnt);
> -       debugfs_create_u64("nack_cnt", 0444, d, &bus->nack_cnt);
> -       debugfs_create_u64("rec_succ_cnt", 0444, d, &bus->rec_succ_cnt);
> -       debugfs_create_u64("rec_fail_cnt", 0444, d, &bus->rec_fail_cnt);
> -       debugfs_create_u64("timeout_cnt", 0444, d, &bus->timeout_cnt);
> -       debugfs_create_u64("tx_complete_cnt", 0444, d, &bus->tx_complete_cnt);
> -
> -       bus->debugfs = d;
> +       debugfs_create_u64("ber_cnt", 0444, bus->adap.debugfs, &bus->ber_cnt);
> +       debugfs_create_u64("nack_cnt", 0444, bus->adap.debugfs, &bus->nack_cnt);
> +       debugfs_create_u64("rec_succ_cnt", 0444, bus->adap.debugfs, &bus->rec_succ_cnt);
> +       debugfs_create_u64("rec_fail_cnt", 0444, bus->adap.debugfs, &bus->rec_fail_cnt);
> +       debugfs_create_u64("timeout_cnt", 0444, bus->adap.debugfs, &bus->timeout_cnt);
> +       debugfs_create_u64("tx_complete_cnt", 0444, bus->adap.debugfs, &bus->tx_complete_cnt);
>  }
>
>  static int npcm_i2c_probe_bus(struct platform_device *pdev)
> @@ -2362,7 +2349,6 @@ static void npcm_i2c_remove_bus(struct platform_device *pdev)
>         unsigned long lock_flags;
>         struct npcm_i2c *bus = platform_get_drvdata(pdev);
>
> -       debugfs_remove_recursive(bus->debugfs);
>         spin_lock_irqsave(&bus->lock, lock_flags);
>         npcm_i2c_disable(bus);
>         spin_unlock_irqrestore(&bus->lock, lock_flags);
> @@ -2385,28 +2371,7 @@ static struct platform_driver npcm_i2c_bus_driver = {
>         }
>  };
>
> -static int __init npcm_i2c_init(void)
> -{
> -       int ret;
> -
> -       npcm_i2c_debugfs_dir = debugfs_create_dir("npcm_i2c", NULL);
> -
> -       ret = platform_driver_register(&npcm_i2c_bus_driver);
> -       if (ret) {
> -               debugfs_remove_recursive(npcm_i2c_debugfs_dir);
> -               return ret;
> -       }
> -
> -       return 0;
> -}
> -module_init(npcm_i2c_init);
> -
> -static void __exit npcm_i2c_exit(void)
> -{
> -       platform_driver_unregister(&npcm_i2c_bus_driver);
> -       debugfs_remove_recursive(npcm_i2c_debugfs_dir);
> -}
> -module_exit(npcm_i2c_exit);
> +module_platform_driver(npcm_i2c_bus_driver);
>
>  MODULE_AUTHOR("Avi Fishman <avi.fishman@gmail.com>");
>  MODULE_AUTHOR("Tali Perry <tali.perry@nuvoton.com>");
> --
> 2.35.1
>

Test pass on npcm8xx device.

Reviewed-by: Tali Perry <tali.perry1@gmail.com>

Thanks Wolfram for the patch!

Re: [PATCH v2 0/3] i2c: create debugfs entry per adapter

[Wolfram Sang]

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

On Sun, Nov 12, 2023 at 05:54:40PM -0500, Wolfram Sang wrote:
> Two drivers already implement custom debugfs handling for their
> i2c_adapter and more will come. So, let the core create a debugfs
> directory per adapter and pass that to drivers for their debugfs files.
> Convert the two existing users as well. Tested on a Renesas Lager board
> with the i2c-gpio driver. npcm7xx was tested using QEMU with the
> quanta-gsj target.
> 
> Changes since v1:
> * rebased to something close to 6.7-rc1 which includes i2c/for-mergewindow
> * tested npcm7xx with QEMU
> 
> Wolfram Sang (3):
>   i2c: create debugfs entry per adapter
>   i2c: gpio: move to per-adapter debugfs directory
>   i2c: npcm7xx: move to per-adapter debugfs directory
> 

Applied to for-next, thanks!


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