Re: [PATCH net-next] net: dsa: lantiq_gswip: Add support for dumping the registers

From: Hauke Mehrtens
Date: Sun Apr 11 2021 - 18:19:21 EST

On 4/11/21 10:55 PM, Martin Blumenstingl wrote:
Add support for .get_regs_len and .get_regs so it is easier to find out
about the state of the ports on the GSWIP hardware. For this we
specifically add the GSWIP_MAC_PSTATp(port) and GSWIP_MDIO_STATp(port)
register #defines as these contain the current port status (as well as
the result of the auto polling mechanism). Other global and per-port
registers which are also considered useful are included as well.

Acked-by: Hauke Mehrtens <hauke@xxxxxxxxxx>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
---
drivers/net/dsa/lantiq_gswip.c | 83 ++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)

diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 314ae78bbdd6..d3cfc72644ff 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -90,6 +90,21 @@
GSWIP_MDIO_PHY_LINK_MASK | \
GSWIP_MDIO_PHY_SPEED_MASK | \
GSWIP_MDIO_PHY_FDUP_MASK)
+#define GSWIP_MDIO_STATp(p) (0x16 + (p))
+#define GSWIP_MDIO_STAT_RXACT BIT(10)
+#define GSWIP_MDIO_STAT_TXACT BIT(9)
+#define GSWIP_MDIO_STAT_CLK_STOP_CAPAB BIT(8)
+#define GSWIP_MDIO_STAT_EEE_CAPABLE BIT(7)
+#define GSWIP_MDIO_STAT_PACT BIT(6)
+#define GSWIP_MDIO_STAT_LSTAT BIT(5)
+#define GSWIP_MDIO_STAT_SPEED_M10 0x00
+#define GSWIP_MDIO_STAT_SPEED_M100 0x08
+#define GSWIP_MDIO_STAT_SPEED_1G 0x10
+#define GSWIP_MDIO_STAT_SPEED_RESERVED 0x18
+#define GSWIP_MDIO_STAT_SPEED_MASK 0x18
+#define GSWIP_MDIO_STAT_FDUP BIT(2)
+#define GSWIP_MDIO_STAT_RXPAUEN BIT(1)
+#define GSWIP_MDIO_STAT_TXPAUEN BIT(0)
/* GSWIP MII Registers */
#define GSWIP_MII_CFGp(p) (0x2 * (p))
@@ -195,6 +210,19 @@
#define GSWIP_PCE_DEFPVID(p) (0x486 + ((p) * 0xA))
#define GSWIP_MAC_FLEN 0x8C5
+#define GSWIP_MAC_PSTATp(p) (0x900 + ((p) * 0xC))
+#define GSWIP_MAC_PSTAT_PACT BIT(11)
+#define GSWIP_MAC_PSTAT_GBIT BIT(10)
+#define GSWIP_MAC_PSTAT_MBIT BIT(9)
+#define GSWIP_MAC_PSTAT_FDUP BIT(8)
+#define GSWIP_MAC_PSTAT_RXPAU BIT(7)
+#define GSWIP_MAC_PSTAT_TXPAU BIT(6)
+#define GSWIP_MAC_PSTAT_RXPAUEN BIT(5)
+#define GSWIP_MAC_PSTAT_TXPAUEN BIT(4)
+#define GSWIP_MAC_PSTAT_LSTAT BIT(3)
+#define GSWIP_MAC_PSTAT_CRS BIT(2)
+#define GSWIP_MAC_PSTAT_TXLPI BIT(1)
+#define GSWIP_MAC_PSTAT_RXLPI BIT(0)
#define GSWIP_MAC_CTRL_0p(p) (0x903 + ((p) * 0xC))
#define GSWIP_MAC_CTRL_0_PADEN BIT(8)
#define GSWIP_MAC_CTRL_0_FCS_EN BIT(7)
@@ -701,6 +729,57 @@ static void gswip_port_disable(struct dsa_switch *ds, int port)
GSWIP_SDMA_PCTRLp(port));
}
+static int gswip_get_regs_len(struct dsa_switch *ds, int port)
+{
+ return 17 * sizeof(u32);
+}
+
+static void gswip_get_regs(struct dsa_switch *ds, int port,
+ struct ethtool_regs *regs, void *_p)
+{
+ struct gswip_priv *priv = ds->priv;
+ u32 *p = _p;
+
+ regs->version = gswip_switch_r(priv, GSWIP_VERSION);

This is the bump format version not the HW version, see here:
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/ethtool.h#L298

I would prefer to just return 1 here, for this format. When we add an extra register we would change this to 2 and so on.

These are not all registers of this switch, but for now this looks good.

+
+ memset(p, 0xff, 17 * sizeof(u32));
+
+ p[0] = gswip_mdio_r(priv, GSWIP_MDIO_GLOB);
+ p[1] = gswip_mdio_r(priv, GSWIP_MDIO_CTRL);
+ p[2] = gswip_mdio_r(priv, GSWIP_MDIO_MDC_CFG0);
+ p[3] = gswip_mdio_r(priv, GSWIP_MDIO_MDC_CFG1);
+
+ if (!dsa_is_cpu_port(priv->ds, port)) {
+ p[4] = gswip_mdio_r(priv, GSWIP_MDIO_PHYp(port));
+ p[5] = gswip_mdio_r(priv, GSWIP_MDIO_STATp(port));
+ p[6] = gswip_mii_r(priv, GSWIP_MII_CFGp(port));

Please add:
#define GSWIP_MDIO_EEEp(p) (0x1C + (p))

+ }
+
+ switch (port) {
+ case 0:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU0);
+ break;
+ case 1:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU1);
+ break;
+ case 5:
+ p[7] = gswip_mii_r(priv, GSWIP_MII_PCDU5);
+ break;
+ default:
+ break;
+ }
+
+ p[8] = gswip_switch_r(priv, GSWIP_PCE_PCTRL_0p(port));
+ p[9] = gswip_switch_r(priv, GSWIP_PCE_VCTRL(port));
+ p[10] = gswip_switch_r(priv, GSWIP_PCE_DEFPVID(port));
+ p[11] = gswip_switch_r(priv, GSWIP_MAC_FLEN);
+ p[12] = gswip_switch_r(priv, GSWIP_MAC_PSTATp(port));
+ p[13] = gswip_switch_r(priv, GSWIP_MAC_CTRL_0p(port));
+ p[14] = gswip_switch_r(priv, GSWIP_MAC_CTRL_2p(port));
+ p[15] = gswip_switch_r(priv, GSWIP_FDMA_PCTRLp(port));
+ p[16] = gswip_switch_r(priv, GSWIP_SDMA_PCTRLp(port));
+}
+
static int gswip_pce_load_microcode(struct gswip_priv *priv)
{
int i;
@@ -1795,6 +1874,8 @@ static const struct dsa_switch_ops gswip_xrx200_switch_ops = {
.setup = gswip_setup,
.port_enable = gswip_port_enable,
.port_disable = gswip_port_disable,
+ .get_regs_len = gswip_get_regs_len,
+ .get_regs = gswip_get_regs,
.port_bridge_join = gswip_port_bridge_join,
.port_bridge_leave = gswip_port_bridge_leave,
.port_fast_age = gswip_port_fast_age,
@@ -1819,6 +1900,8 @@ static const struct dsa_switch_ops gswip_xrx300_switch_ops = {
.setup = gswip_setup,
.port_enable = gswip_port_enable,
.port_disable = gswip_port_disable,
+ .get_regs_len = gswip_get_regs_len,
+ .get_regs = gswip_get_regs,
.port_bridge_join = gswip_port_bridge_join,
.port_bridge_leave = gswip_port_bridge_leave,
.port_fast_age = gswip_port_fast_age,