[PATCH net-next] net: dsa: mt7530: Add debugfs support

[PATCH net-next] net: dsa: mt7530: Add debugfs support

[Lorenzo Bianconi]

Introduce debugfs support for mt7530 dsa switch.
Add the capability to read or write device registers through debugfs:

$echo 0x7ffc > regidx
$cat regval
0x75300000

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/dsa/mt7530-mmio.c | 12 +++++++++-
 drivers/net/dsa/mt7530.c      | 41 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mt7530.h      |  5 +++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mt7530-mmio.c b/drivers/net/dsa/mt7530-mmio.c
index b74a230a3f13..cedb046ea2a3 100644
--- a/drivers/net/dsa/mt7530-mmio.c
+++ b/drivers/net/dsa/mt7530-mmio.c
@@ -60,7 +60,17 @@ mt7988_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
 
-	return dsa_register_switch(priv->ds);
+	ret = dsa_register_switch(priv->ds);
+	if (ret)
+		return ret;
+
+	ret = mt7530_register_debugfs(priv);
+	if (ret) {
+		dsa_unregister_switch(priv->ds);
+		return ret;
+	}
+
+	return 0;
 }
 
 static void mt7988_remove(struct platform_device *pdev)
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 598434d8d6e4..18cb42a771e8 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -3,6 +3,7 @@
  * Mediatek MT7530 DSA Switch driver
  * Copyright (C) 2017 Sean Wang <sean.wang@mediatek.com>
  */
+#include <linux/debugfs.h>
 #include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
 #include <linux/iopoll.h>
@@ -271,6 +272,28 @@ mt7530_clear(struct mt7530_priv *priv, u32 reg, u32 val)
 	mt7530_rmw(priv, reg, val, 0);
 }
 
+static int
+mt7530_reg_set(void *data, u64 val)
+{
+	struct mt7530_priv *priv = data;
+
+	mt7530_write(priv, priv->debugfs_reg, val);
+
+	return 0;
+}
+
+static int
+mt7530_reg_get(void *data, u64 *val)
+{
+	struct mt7530_priv *priv = data;
+
+	*val = mt7530_read(priv, priv->debugfs_reg);
+
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops, mt7530_reg_get, mt7530_reg_set, "0x%08llx\n");
+
 static int
 mt7530_fdb_cmd(struct mt7530_priv *priv, enum mt7530_fdb_cmd cmd, u32 *rsp)
 {
@@ -3218,6 +3241,22 @@ const struct mt753x_info mt753x_table[] = {
 };
 EXPORT_SYMBOL_GPL(mt753x_table);
 
+int
+mt7530_register_debugfs(struct mt7530_priv *priv)
+{
+	priv->debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
+	if (IS_ERR(priv->debugfs_dir))
+		return PTR_ERR(priv->debugfs_dir);
+
+	debugfs_create_u32("regidx", 0600, priv->debugfs_dir,
+			   &priv->debugfs_reg);
+	debugfs_create_file_unsafe("regval", 0600, priv->debugfs_dir, priv,
+				   &fops);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(mt7530_register_debugfs);
+
 int
 mt7530_probe_common(struct mt7530_priv *priv)
 {
@@ -3252,6 +3291,8 @@ EXPORT_SYMBOL_GPL(mt7530_probe_common);
 void
 mt7530_remove_common(struct mt7530_priv *priv)
 {
+	debugfs_remove(priv->debugfs_dir);
+
 	if (priv->irq)
 		mt7530_free_irq(priv);
 
diff --git a/drivers/net/dsa/mt7530.h b/drivers/net/dsa/mt7530.h
index 2ea4e24628c6..b7568c1c6d5e 100644
--- a/drivers/net/dsa/mt7530.h
+++ b/drivers/net/dsa/mt7530.h
@@ -798,6 +798,8 @@ struct mt753x_info {
  * @create_sgmii:	Pointer to function creating SGMII PCS instance(s)
  * @active_cpu_ports:	Holding the active CPU ports
  * @mdiodev:		The pointer to the MDIO device structure
+ * @debugfs_dir:	Debugfs entry point
+ * @debugfs_reg:	Selected register to read or write through debugfs
  */
 struct mt7530_priv {
 	struct device		*dev;
@@ -825,6 +827,8 @@ struct mt7530_priv {
 	int (*create_sgmii)(struct mt7530_priv *priv);
 	u8 active_cpu_ports;
 	struct mdio_device *mdiodev;
+	struct dentry *debugfs_dir;
+	u32 debugfs_reg;
 };
 
 struct mt7530_hw_vlan_entry {
@@ -861,6 +865,7 @@ static inline void INIT_MT7530_DUMMY_POLL(struct mt7530_dummy_poll *p,
 	p->reg = reg;
 }
 
+int mt7530_register_debugfs(struct mt7530_priv *priv);
 int mt7530_probe_common(struct mt7530_priv *priv);
 void mt7530_remove_common(struct mt7530_priv *priv);
 
-- 
2.45.1

Re: [PATCH net-next] net: dsa: mt7530: Add debugfs support

[Vladimir Oltean]

Hi Lorenzo,

On Wed, May 29, 2024 at 12:54:37PM +0200, Lorenzo Bianconi wrote:
> Introduce debugfs support for mt7530 dsa switch.
> Add the capability to read or write device registers through debugfs:
> 
> $echo 0x7ffc > regidx
> $cat regval
> 0x75300000
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---

Apart from the obvious NACK on permitting user space to alter random
registers outside of the driver's control.

Have you looked at /sys/kernel/debug/regmap/? Or at ethtool --register-dump?

Re: [PATCH net-next] net: dsa: mt7530: Add debugfs support

[Andrew Lunn]

On Wed, May 29, 2024 at 12:54:37PM +0200, Lorenzo Bianconi wrote:
> Introduce debugfs support for mt7530 dsa switch.
> Add the capability to read or write device registers through debugfs:
> 
> $echo 0x7ffc > regidx
> $cat regval
> 0x75300000
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

In addition to Vladimirs NACK, you can also take a look at how
mv88e6xxx exports registers and tables using devlink regions.

    Andrew

---
pw-bot: cr

Re: [PATCH net-next] net: dsa: mt7530: Add debugfs support

[Lorenzo Bianconi]

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

> Hi Lorenzo,
> 
> On Wed, May 29, 2024 at 12:54:37PM +0200, Lorenzo Bianconi wrote:
> > Introduce debugfs support for mt7530 dsa switch.
> > Add the capability to read or write device registers through debugfs:
> > 
> > $echo 0x7ffc > regidx
> > $cat regval
> > 0x75300000
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> 
> Apart from the obvious NACK on permitting user space to alter random
> registers outside of the driver's control.
> 
> Have you looked at /sys/kernel/debug/regmap/? Or at ethtool --register-dump?

ack, regmap sysfs is fine to dump registers value.
It was just very handy for me to have the capability to change registers
during development (moreover we already have something similar in mt76).
Anyway it is probably something more related to development so I do not
have a strong opinion on it and we discard the patch.

Regards,
Lorenzo

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

Re: [PATCH net-next] net: dsa: mt7530: Add debugfs support

[Lorenzo Bianconi]

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

> On Wed, May 29, 2024 at 12:54:37PM +0200, Lorenzo Bianconi wrote:
> > Introduce debugfs support for mt7530 dsa switch.
> > Add the capability to read or write device registers through debugfs:
> > 
> > $echo 0x7ffc > regidx
> > $cat regval
> > 0x75300000
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> In addition to Vladimirs NACK, you can also take a look at how
> mv88e6xxx exports registers and tables using devlink regions.

ack, right. Anyway in my case (mt7530-mmio) regmap is enough to dump register
values. Thanks for the pointer.

Regards,
Lorenzo

> 
>     Andrew
> 
> ---
> pw-bot: cr

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