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

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

[Martin Blumenstingl]

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@hauke-m.de>
Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 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);
+
+	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));
+	}
+
+	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,
-- 
2.31.1

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

[Hauke Mehrtens]

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@hauke-m.de>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>   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,
> 

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

[Andrew Lunn]

On Sun, Apr 11, 2021 at 10:55:11PM +0200, 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.

Although this is O.K, there has been a trend towards using devlink
regions for this, and other register sets in the switch. Take a look
at drivers/net/dsa/mv88e6xxx/devlink.c.

There is a userspace tool for the mv88e6xxx devlink regions here:

https://github.com/lunn/mv88e6xxx_dump

and a few people have forked it and modified it for other DSA
switches. At some point we might want to try to merge the forks back
together so we have one tool to dump any switch.

	 Andrew

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

[Martin Blumenstingl]

Hi Andrew,

On Mon, Apr 12, 2021 at 1:16 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sun, Apr 11, 2021 at 10:55:11PM +0200, 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.
>
> Although this is O.K, there has been a trend towards using devlink
> regions for this, and other register sets in the switch. Take a look
> at drivers/net/dsa/mv88e6xxx/devlink.c.
>
> There is a userspace tool for the mv88e6xxx devlink regions here:
>
> https://github.com/lunn/mv88e6xxx_dump
>
> and a few people have forked it and modified it for other DSA
> switches. At some point we might want to try to merge the forks back
> together so we have one tool to dump any switch.
actually I was wondering if there is some way to make the registers
"easier to read" in userspace.
It turns out there is :-)

Hauke, which approach do you recommend?:
- update this patch with your suggestion and ask Andrew to still merge
it soon-ish
- put this topic somewhere on my or your TODO-list and come up with a
devlink solution at some point in the future


Best regards,
Martin

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

[Andrew Lunn]

On Tue, Apr 13, 2021 at 12:24:49AM +0200, Martin Blumenstingl wrote:
> Hi Andrew,
> 
> On Mon, Apr 12, 2021 at 1:16 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Sun, Apr 11, 2021 at 10:55:11PM +0200, 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.
> >
> > Although this is O.K, there has been a trend towards using devlink
> > regions for this, and other register sets in the switch. Take a look
> > at drivers/net/dsa/mv88e6xxx/devlink.c.
> >
> > There is a userspace tool for the mv88e6xxx devlink regions here:
> >
> > https://github.com/lunn/mv88e6xxx_dump
> >
> > and a few people have forked it and modified it for other DSA
> > switches. At some point we might want to try to merge the forks back
> > together so we have one tool to dump any switch.
> actually I was wondering if there is some way to make the registers
> "easier to read" in userspace.

You can add decoding to ethtool. The marvell chips have this, to some
extent. But the ethtool API is limited to just port registers, and
there can be a lot more registers which are not associated to a
port. devlink gives you access to these additional registers.

      Andrew

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

[Martin Blumenstingl]

On Tue, Apr 13, 2021 at 1:45 AM Andrew Lunn <andrew@lunn.ch> wrote:
[...]
> > > and a few people have forked it and modified it for other DSA
> > > switches. At some point we might want to try to merge the forks back
> > > together so we have one tool to dump any switch.
> > actually I was wondering if there is some way to make the registers
> > "easier to read" in userspace.
>
> You can add decoding to ethtool. The marvell chips have this, to some
> extent. But the ethtool API is limited to just port registers, and
> there can be a lot more registers which are not associated to a
> port. devlink gives you access to these additional registers.
oh, then that's actually also a problem with my patch:
the .get_regs implementation currently also uses five registers which
are not related to the specific port.
noted in case I re-send this as .get_regs patch instead of moving over
to devlink.

Thanks for the hints as always!


Martin

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

[Florian Fainelli]

On 4/13/2021 12:25 PM, Martin Blumenstingl wrote:
> On Tue, Apr 13, 2021 at 1:45 AM Andrew Lunn <andrew@lunn.ch> wrote:
> [...]
>>>> and a few people have forked it and modified it for other DSA
>>>> switches. At some point we might want to try to merge the forks back
>>>> together so we have one tool to dump any switch.
>>> actually I was wondering if there is some way to make the registers
>>> "easier to read" in userspace.
>>
>> You can add decoding to ethtool. The marvell chips have this, to some
>> extent. But the ethtool API is limited to just port registers, and
>> there can be a lot more registers which are not associated to a
>> port. devlink gives you access to these additional registers.
> oh, then that's actually also a problem with my patch:
> the .get_regs implementation currently also uses five registers which
> are not related to the specific port.
> noted in case I re-send this as .get_regs patch instead of moving over
> to devlink.

In premise there is nothing that prevents you from returning the port
registers as well as the global registers for any .get_regs() call. This
is not really beautiful or clean but as far as register dumping goes it
would get the job done. devlink is better organized in that you can dump
global and per-port registers and they show up as separate regions.
--
Florian

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

[Hauke Mehrtens]

On 4/13/21 12:24 AM, Martin Blumenstingl wrote:
> Hi Andrew,
> 
> On Mon, Apr 12, 2021 at 1:16 AM Andrew Lunn <andrew@lunn.ch> wrote:
>>
>> On Sun, Apr 11, 2021 at 10:55:11PM +0200, 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.
>>
>> Although this is O.K, there has been a trend towards using devlink
>> regions for this, and other register sets in the switch. Take a look
>> at drivers/net/dsa/mv88e6xxx/devlink.c.
>>
>> There is a userspace tool for the mv88e6xxx devlink regions here:
>>
>> https://github.com/lunn/mv88e6xxx_dump
>>
>> and a few people have forked it and modified it for other DSA
>> switches. At some point we might want to try to merge the forks back
>> together so we have one tool to dump any switch.
> actually I was wondering if there is some way to make the registers
> "easier to read" in userspace.
> It turns out there is :-)
> 
> Hauke, which approach do you recommend?:
> - update this patch with your suggestion and ask Andrew to still merge
> it soon-ish
> - put this topic somewhere on my or your TODO-list and come up with a
> devlink solution at some point in the future

Would you work on the devlink solution in the next weeks?
I think this is part of the ABI when we add it, can we later remove the 
ethtool registers when we add devlink support or is this not allowed?

Hauke