diff mbox series

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

Message ID 0999545cf558ded50087e174096bb631e59b5583.1716979901.git.lorenzo@kernel.org (mailing list archive)
State New
Headers show
Series [net-next] net: dsa: mt7530: Add debugfs support | expand

Commit Message

Lorenzo Bianconi May 29, 2024, 10:54 a.m. UTC
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(-)

Comments

Vladimir Oltean May 29, 2024, 1:31 p.m. UTC | #1
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?
Andrew Lunn May 29, 2024, 2:41 p.m. UTC | #2
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
Lorenzo Bianconi May 29, 2024, 3:06 p.m. UTC | #3
> 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
Lorenzo Bianconi May 29, 2024, 3:08 p.m. UTC | #4
> 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
diff mbox series

Patch

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);