diff mbox series

ethtool: dsa: mv88e6xxx: add pretty dump for 88E6352 SERDES

Message ID 20200716175526.14005-1-cphealy@gmail.com
State Changes Requested
Delegated to: Michal Kubecek
Headers show
Series ethtool: dsa: mv88e6xxx: add pretty dump for 88E6352 SERDES | expand

Commit Message

Chris Healy July 16, 2020, 5:55 p.m. UTC 
From: Andrew Lunn <andrew@lunn.ch>

In addition to the port registers, the device can provide the
SERDES/PCS registers. Dump these, and for a few of the important
SGMII/1000Base-X registers decode the bits.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Chris Healy <cphealy@gmail.com>
---
 dsa.c | 196 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 195 insertions(+), 1 deletion(-)

Comments

Michal Kubecek July 19, 2020, 9:16 p.m. UTC | #1 
On Thu, Jul 16, 2020 at 10:55:26AM -0700, Chris Healy wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> 
> In addition to the port registers, the device can provide the
> SERDES/PCS registers. Dump these, and for a few of the important
> SGMII/1000Base-X registers decode the bits.
> 
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Chris Healy <cphealy@gmail.com>
> ---
[...]
> +	case 32 + 0:
> +		REG(reg - 32, "Fiber Control", val);

Could you give these "32" (and similar below) a name?

[...]
> @@ -667,6 +850,17 @@ static int dsa_mv88e6xxx_dump_regs(struct ethtool_regs *regs)
>  		else
>  			REG(i, "", data[i]);
>  
> +	/* Dump the SERDES registers, if provided */
> +	if (regs->len > 32 * 2) {

sizeof(u16) would be easier to read, IMHO

> +		printf("\n%s Switch Port SERDES Registers\n", sw->name);
> +		printf("-------------------------------------\n");
> +		for (i = 32; i < regs->len / 2; i++)
> +			if (sw->dump)
> +				sw->dump(i, data[i]);
> +			else
> +				REG(i, "", data[i]);

In the dump handler above you subtract 32 (offset of SERDES registers,
IIUC) from register number but in the generic branch you don't, this
seems inconsistent.

Michal

> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.21.3
>
Chris Healy July 20, 2020, 6:42 p.m. UTC | #2 
On Sun, Jul 19, 2020 at 2:16 PM Michal Kubecek <mkubecek@suse.cz> wrote:
>
> On Thu, Jul 16, 2020 at 10:55:26AM -0700, Chris Healy wrote:
> > From: Andrew Lunn <andrew@lunn.ch>
> >
> > In addition to the port registers, the device can provide the
> > SERDES/PCS registers. Dump these, and for a few of the important
> > SGMII/1000Base-X registers decode the bits.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Signed-off-by: Chris Healy <cphealy@gmail.com>
> > ---
> [...]
> > +     case 32 + 0:
> > +             REG(reg - 32, "Fiber Control", val);
>
> Could you give these "32" (and similar below) a name?

Yep, will do in v2.
>
> [...]
> > @@ -667,6 +850,17 @@ static int dsa_mv88e6xxx_dump_regs(struct ethtool_regs *regs)
> >               else
> >                       REG(i, "", data[i]);
> >
> > +     /* Dump the SERDES registers, if provided */
> > +     if (regs->len > 32 * 2) {
>
> sizeof(u16) would be easier to read, IMHO

Agreed, will do in v2.
>
> > +             printf("\n%s Switch Port SERDES Registers\n", sw->name);
> > +             printf("-------------------------------------\n");
> > +             for (i = 32; i < regs->len / 2; i++)
> > +                     if (sw->dump)
> > +                             sw->dump(i, data[i]);
> > +                     else
> > +                             REG(i, "", data[i]);
>
> In the dump handler above you subtract 32 (offset of SERDES registers,
> IIUC) from register number but in the generic branch you don't, this
> seems inconsistent.

You are correct.  I tested this code path and it was reporting
incorrectly.  Fix coming in v2.

>
> Michal
>
> > +     }
> > +
> >       return 0;
> >  }
> >
> > --
> > 2.21.3
> >
diff mbox series

Patch

diff --git a/dsa.c b/dsa.c
index 50a171b..a5045fc 100644
--- a/dsa.c
+++ b/dsa.c
@@ -405,21 +405,204 @@  static void dsa_mv88e6352(int reg, u16 val)
 	case 19:
 		REG(reg, "Rx Frame Counter", val);
 		break;
+	case 20 ... 21:
+		REG(reg, "Reserved", val);
+		break;
 	case 22:
 		REG(reg, "LED Control", val);
 		break;
+	case 23:
+		REG(reg, "Reserved", val);
+		break;
 	case 24:
 		REG(reg, "Tag Remap 0-3", val);
 		break;
 	case 25:
 		REG(reg, "Tag Remap 4-7", val);
 		break;
+	case 26:
+		REG(reg, "Reserved", val);
+		break;
 	case 27:
 		REG(reg, "Queue Counters", val);
 		break;
-	default:
+	case 28 ... 31:
 		REG(reg, "Reserved", val);
 		break;
+	case 32 + 0:
+		REG(reg - 32, "Fiber Control", val);
+		FIELD("Fiber Reset", "%u", !!(val & 0x8000));
+		FIELD("Loopback", "%u", !!(val & 0x4000));
+		FIELD("Speed", "%s",
+		      (val & (0x2000 | 0x0040)) == 0x0000 ? "10 Mbps" :
+		      (val & (0x2000 | 0x0040)) == 0x2000 ? "100 Mbps" :
+		      (val & (0x2000 | 0x0040)) == 0x0040 ? "1000 Mbps" :
+		      (val & (0x2000 | 0x0040)) == (0x2000 | 0x0040) ?
+		      "Reserved" : "?");
+		FIELD("Autoneg Enable", "%u", !!(val & 0x1000));
+		FIELD("Power down", "%u", !!(val & 0x0800));
+		FIELD("Isolate", "%u", !!(val & 0x0400));
+		FIELD("Restart Autoneg", "%u", !!(val & 0x0200));
+		FIELD("Duplex", "%s", val & 0x0100 ? "Full" : "Half");
+		break;
+	case 32 + 1:
+		REG(reg - 32, "Fiber Status", val);
+		FIELD("100Base-X FD",  "%u", !!(val & 0x4000));
+		FIELD("100Base-X HD",  "%u", !!(val & 0x2000));
+		FIELD("Autoneg Complete", "%u", !!(val & 0x0020));
+		FIELD("Remote Fault", "%u", !!(val & 0x0010));
+		FIELD("Autoneg Ability", "%u", !!(val & 0x0008));
+		FIELD("Link Status", "%s", val & 0x0004 ? "Up" : "Down");
+		break;
+	case 32 + 2:
+		REG(reg - 32, "PHY ID 1", val);
+		break;
+	case 32 + 3:
+		REG(reg - 32, "PHY ID 2", val);
+		break;
+	case 32 + 4:
+		REG(reg - 32, "Fiber Autoneg Advertisement", val);
+		FIELD("Remote Fault", "%s",
+		      (val & 0x3000) == 0x0000 ? "No error, link OK" :
+		      (val & 0x3000) == 0x1000 ? "Link failure" :
+		      (val & 0x3000) == 0x2000 ? "Offline" :
+		      (val & 0x3000) == 0x3000 ? "Autoneg Error" : "?");
+		FIELD("Pause", "%s",
+		      (val & 0x0180) == 0x0000 ? "No Pause" :
+		      (val & 0x0180) == 0x0080 ? "Symmetric Pause" :
+		      (val & 0x0180) == 0x0100 ? "Asymmetric Pause" :
+		      (val & 0x0180) == 0x0180 ? "Symmetric & Asymmetric Pause" :
+		      "?");
+		FIELD("1000BaseX HD", "%u", !!(val & 0x0040));
+		FIELD("1000BaseX FD", "%u", !!(val & 0x0020));
+		break;
+	case 32 + 5:
+		REG(reg - 32, "Fiber Link Autoneg Ability", val);
+		FIELD("Acknowledge", "%u", !!(val & 0x4000));
+		FIELD("Remote Fault", "%s",
+		      (val & 0x3000) == 0x0000 ? "No error, link OK" :
+		      (val & 0x3000) == 0x1000 ? "Link failure" :
+		      (val & 0x3000) == 0x2000 ? "Offline" :
+		      (val & 0x3000) == 0x3000 ? "Autoneg Error" : "?");
+		FIELD("Pause", "%s",
+		      (val & 0x0180) == 0x0000 ? "No Pause" :
+		      (val & 0x0180) == 0x0080 ? "Symmetric Pause" :
+		      (val & 0x0180) == 0x0100 ? "Asymmetric Pause" :
+		      (val & 0x0180) == 0x0180 ? "Symmetric & Asymmetric Pause" :
+		      "?");
+		FIELD("1000BaseX HD", "%u", !!(val & 0x0040));
+		FIELD("1000BaseX FD", "%u", !!(val & 0x0020));
+		break;
+	case 32 + 6:
+		REG(reg - 32, "Fiber Autoneg Expansion", val);
+		FIELD("Link Partner Next Page Ability", "%u", !!(val & 0x0008));
+		FIELD("Page Received", "%u", !!(val & 0x0002));
+		FIELD("Link Partner Autoneg Ability", "%u", !!(val & 0x0001));
+		break;
+	case 32 + 7:
+		REG(reg - 32, "Fiber Next Page Transmit", val);
+		break;
+	case 32 + 8:
+		REG(reg - 32, "Fiber Link Partner Next Page", val);
+		break;
+	case 32 + 9 ... 32 + 14:
+		REG(reg - 32, "Reserved", val);
+		break;
+	case 32 + 15:
+		REG(reg - 32, "Extended Status", val);
+		break;
+	case 32 + 16:
+		REG(reg - 32, "Fiber Specific Control", val);
+		FIELD("Fiber Transmit FIFO Depth", "%s",
+		      (val & 0xc000) == 0x0000 ? "16 Bits" :
+		      (val & 0xc000) == 0x4000 ? "24 Bits" :
+		      (val & 0xc000) == 0x8000 ? "32 Bits" :
+		      (val & 0xc000) == 0xc000 ? "40 Bits" : "?");
+		FIELD("SERDES Loopback", "%u", !!(val & 0x1000));
+		FIELD("Force Link Good", "%u", !!(val & 0x0400));
+		FIELD("MAC Interface Power Down", "%u", !!(val & 0x0008));
+		FIELD("Mode", "%s",
+		      (val & 0x0003) == 0x0000 ? "100BaseFX" :
+		      (val & 0x0003) == 0x0001 ? "1000BaseX" :
+		      (val & 0x0003) == 0x0002 ? "SGMII System" :
+		      (val & 0x0003) == 0x0003 ? "SGMII Media" : "?");
+		break;
+	case 32 + 17:
+		REG(reg - 32, "Fiber Specific Status", val);
+		FIELD("Speed", "%s",
+		      (val & 0xc000) == 0x0000 ? "10 Mbps" :
+		      (val & 0xc000) == 0x4000 ? "100 Mbps" :
+		      (val & 0xc000) == 0x8000 ? "1000 Mbps" :
+		      (val & 0xc000) == 0xc000 ? "Reserved" : "?");
+		FIELD("Duplex", "%s", val & 0x2000 ? "Full" : "Half");
+		FIELD("Page Received", "%u", !!(val & 0x1000));
+		FIELD("Speed/Duplex Resolved", "%u", !!(val & 0x0800));
+		FIELD("Link", "%s", val & 0x0400 ? "Up" : "Down");
+		FIELD("Sync", "%u", !!(val & 0x0020));
+		FIELD("Energy Detect", "%s", val & 0x010 ? "False" : "True");
+		FIELD("Transmit Pause", "%u", !!(val & 0x0008));
+		FIELD("Receive Pause", "%u", !!(val & 0x00004));
+		break;
+	case 32 + 18:
+		REG(reg - 32, "Fiber Interrupt Enable", val);
+		FIELD("Speed Changed", "%u", !!(val & 0x4000));
+		FIELD("Duplex Changed", "%u", !!(val & 0x2000));
+		FIELD("Page Received", "%u", !!(val & 0x1000));
+		FIELD("Autoneg Complete", "%u", !!(val & 0x0800));
+		FIELD("Link Status Change", "%u", !!(val & 0x0400));
+		FIELD("Symbol Error", "%u", !!(val & 0x0200));
+		FIELD("False Carrier", "%u", !!(val & 0x0100));
+		FIELD("Energy Detect", "%u", !!(val & 0x0010));
+		break;
+	case 32 + 19:
+		REG(reg - 32, "Fiber Interrupt Status", val);
+		FIELD("Speed Changed", "%u", !!(val & 0x4000));
+		FIELD("Duplex Changed", "%u", !!(val & 0x2000));
+		FIELD("Page Received", "%u", !!(val & 0x1000));
+		FIELD("Autoneg Complete", "%u", !!(val & 0x0800));
+		FIELD("Link Status Change", "%u", !!(val & 0x0400));
+		FIELD("Symbol Error", "%u", !!(val & 0x0200));
+		FIELD("False Carrier", "%u", !!(val & 0x0100));
+		FIELD("Energy Detect", "%u", !!(val & 0x0010));
+		break;
+	case 32 + 20:
+		REG(reg - 32, "Reserved", val);
+		break;
+	case 32 + 21:
+		REG(reg - 32, "Fiber Receive Error Counter", val);
+		break;
+	case 32 + 22:
+		REG(reg - 32, "Reserved", val);
+		break;
+	case 32 + 23:
+		REG(reg - 32, "PRBS Control", val);
+		break;
+	case 32 + 24:
+		REG(reg - 32, "PRBS Error Counter LSB", val);
+		break;
+	case 32 + 25:
+		REG(reg - 32, "PRBS Error Counter MSB", val);
+		break;
+	case 32 + 26:
+		REG(reg - 32, "Fiber Specific Control 2", val);
+		FIELD("1000BaseX Noise Filtering", "%u", !!(val & 0x4000));
+		FIELD("1000BaseFX Noise Filtering", "%u", !!(val & 0x2000));
+		FIELD("SERDES Autoneg Bypass Enable", "%u", !!(val & 0x0040));
+		FIELD("SERDES Autoneg Bypass Status", "%u", !!(val & 0x0020));
+		FIELD("Fiber Transmitter Disable", "%u", !!(val & 0x0008));
+		FIELD("SGMII/Fiber Output Amplitude", "%s",
+		      (val & 0x0007) == 0x0000 ? "14mV" :
+		      (val & 0x0007) == 0x0001 ? "112mV" :
+		      (val & 0x0007) == 0x0002 ? "210mV" :
+		      (val & 0x0007) == 0x0003 ? "308mV" :
+		      (val & 0x0007) == 0x0004 ? "406mV" :
+		      (val & 0x0007) == 0x0005 ? "504mV" :
+		      (val & 0x0007) == 0x0006 ? "602mV" :
+		      (val & 0x0007) == 0x0007 ? "700mV" : "?");
+		break;
+	default:
+		REG(reg - 32, "Reserved", val);
+		break;
 	}
 };
 
@@ -667,6 +850,17 @@  static int dsa_mv88e6xxx_dump_regs(struct ethtool_regs *regs)
 		else
 			REG(i, "", data[i]);
 
+	/* Dump the SERDES registers, if provided */
+	if (regs->len > 32 * 2) {
+		printf("\n%s Switch Port SERDES Registers\n", sw->name);
+		printf("-------------------------------------\n");
+		for (i = 32; i < regs->len / 2; i++)
+			if (sw->dump)
+				sw->dump(i, data[i]);
+			else
+				REG(i, "", data[i]);
+	}
+
 	return 0;
 }