[PATCH ethtool 0/2] Adding downshift support to ethtool

[PATCH ethtool 0/2] Adding downshift support to ethtool

[Allan W. Nielsen]

Hi All,

This patch implements for set/get downshifting.

Downshifting can either be turned on/off, or it can be configured to a
specifc count.

If no "count" are provided, then it is up to the individual PHY driver to
configure a default count.

Best regards
Allan and Raju

Allan W. Nielsen (1):
  ethtool-copy.h:sync with net

Raju Lakkaraju (1):
  Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY
    downshift

 ethtool-copy.h | 34 ++++++++++++++--------
 ethtool.8.in   | 27 +++++++++++++++++
 ethtool.c      | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+), 12 deletions(-)

-- 
2.7.3

[PATCH ethtool 1/2] ethtool-copy.h:sync with net

[Allan W. Nielsen]

This covers kernel changes upto:

commit 67168af82f30bacbd734a4472670cba6b3d6fd36
Author: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Date:   Wed Nov 2 11:35:12 2016 +0530

    ethtool: (uapi) Add ETHTOOL_PHY_DOWNSHIFT to PHY tunables

    For operation in cabling environments that are incompatible with 1000BAST-T, PHY
    device may provide an automatic link speed downshift operation. When enabled,
    the device automatically changes its 1000BAST-T auto-negotiation to the next
    slower speed after a configured number of failed attempts at 1000BAST-T.  This
    feature is useful in setting up in networks using older cable installations that
    include only pairs A and B, and not pairs C and D.

    Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
    Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>

Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 ethtool-copy.h | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/ethtool-copy.h b/ethtool-copy.h
index 70748f5..040c5b5 100644
--- a/ethtool-copy.h
+++ b/ethtool-copy.h
@@ -10,14 +10,16 @@
  * Portions Copyright (C) Sun Microsystems 2008
  */
 
-#ifndef _LINUX_ETHTOOL_H
-#define _LINUX_ETHTOOL_H
+#ifndef _UAPI_LINUX_ETHTOOL_H
+#define _UAPI_LINUX_ETHTOOL_H
 
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <linux/if_ether.h>
 
+#ifndef __KERNEL__
 #include <limits.h> /* for INT_MAX */
+#endif
 
 /* All structures exposed to userland should be defined such that they
  * have the same layout for 32-bit and 64-bit userland.
@@ -114,15 +116,14 @@ struct ethtool_cmd {
 	__u32	reserved[2];
 };
 
-static __inline__ void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
+static inline void ethtool_cmd_speed_set(struct ethtool_cmd *ep,
 					 __u32 speed)
 {
-
-	ep->speed = (__u16)speed;
+	ep->speed = (__u16)(speed & 0xFFFF);
 	ep->speed_hi = (__u16)(speed >> 16);
 }
 
-static __inline__ __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
+static inline __u32 ethtool_cmd_speed(const struct ethtool_cmd *ep)
 {
 	return (ep->speed_hi << 16) | ep->speed;
 }
@@ -247,6 +248,14 @@ struct ethtool_tunable {
 	void	*data[0];
 };
 
+#define DOWNSHIFT_DEV_DEFAULT_COUNT	0xff
+#define DOWNSHIFT_DEV_DISABLE		0
+
+enum phy_tunable_id {
+	ETHTOOL_PHY_ID_UNSPEC,
+	ETHTOOL_PHY_DOWNSHIFT,
+};
+
 /**
  * struct ethtool_regs - hardware register dump
  * @cmd: Command number = %ETHTOOL_GREGS
@@ -878,12 +887,12 @@ struct ethtool_rx_flow_spec {
 #define ETHTOOL_RX_FLOW_SPEC_RING	0x00000000FFFFFFFFLL
 #define ETHTOOL_RX_FLOW_SPEC_RING_VF	0x000000FF00000000LL
 #define ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF 32
-static __inline__ __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie)
+static inline __u64 ethtool_get_flow_spec_ring(__u64 ring_cookie)
 {
 	return ETHTOOL_RX_FLOW_SPEC_RING & ring_cookie;
 };
 
-static __inline__ __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie)
+static inline __u64 ethtool_get_flow_spec_ring_vf(__u64 ring_cookie)
 {
 	return (ETHTOOL_RX_FLOW_SPEC_RING_VF & ring_cookie) >>
 				ETHTOOL_RX_FLOW_SPEC_RING_VF_OFF;
@@ -1312,7 +1321,8 @@ struct ethtool_per_queue_op {
 
 #define ETHTOOL_GLINKSETTINGS	0x0000004c /* Get ethtool_link_settings */
 #define ETHTOOL_SLINKSETTINGS	0x0000004d /* Set ethtool_link_settings */
-
+#define ETHTOOL_PHY_GTUNABLE	0x0000004e /* Get PHY tunable configuration */
+#define ETHTOOL_PHY_STUNABLE	0x0000004f /* Set PHY tunable configuration */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
@@ -1483,7 +1493,7 @@ enum ethtool_link_mode_bit_indices {
 
 #define SPEED_UNKNOWN		-1
 
-static __inline__ int ethtool_validate_speed(__u32 speed)
+static inline int ethtool_validate_speed(__u32 speed)
 {
 	return speed <= INT_MAX || speed == SPEED_UNKNOWN;
 }
@@ -1493,7 +1503,7 @@ static __inline__ int ethtool_validate_speed(__u32 speed)
 #define DUPLEX_FULL		0x01
 #define DUPLEX_UNKNOWN		0xff
 
-static __inline__ int ethtool_validate_duplex(__u8 duplex)
+static inline int ethtool_validate_duplex(__u8 duplex)
 {
 	switch (duplex) {
 	case DUPLEX_HALF:
@@ -1743,4 +1753,4 @@ struct ethtool_link_settings {
 	 * __u32 map_lp_advertising[link_mode_masks_nwords];
 	 */
 };
-#endif /* _LINUX_ETHTOOL_H */
+#endif /* _UAPI_LINUX_ETHTOOL_H */
-- 
2.7.3

[PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

From: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>

Add ethtool get and set tunable to access PHY drivers.

Ethtool Help: ethtool -h for PHY tunables
    ethtool --set-phy-tunable DEVNAME      Set PHY tunable
                [ downshift on|off|%d ]
    ethtool --get-phy-tunable DEVNAME      Get PHY tunable
                [ downshift ]

Ethtool ex:
  ethtool --set-phy-tuanble eth0 downshift on
  ethtool --set-phy-tuanble eth0 downshift off
  ethtool --set-phy-tuanble eth0 downshift 2

  ethtool --get-phy-tunable eth0 downshift

Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
Signed-off-by: Allan W. Nielsen <allan.nielsen@microsemi.com>
---
 ethtool.8.in | 27 ++++++++++++++++++
 ethtool.c    | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/ethtool.8.in b/ethtool.8.in
index 9631847..e1fd51f 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -340,6 +340,14 @@ ethtool \- query or control network driver and hardware settings
 .B2 tx-lpi on off
 .BN tx-timer
 .BN advertise
+.HP
+.B ethtool \-\-set\-phy\-tunable
+.I devname
+.B3 downshift on off N
+.HP
+.B ethtool \-\-get\-phy\-tunable
+.I devname
+.RB [ downshift ]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -947,6 +955,25 @@ Values are as for
 Sets the amount of time the device should stay in idle mode prior to asserting
 its Tx LPI (in microseconds). This has meaning only when Tx LPI is enabled.
 .RE
+.TP
+.B \-\-set\-phy\-tunable
+Sets the PHY tunable parameters.
+.RS 4
+.TP
+.A2 downshift on off
+Specifies whether downshift should be enabled
+.TP
+.BI downshift \ N
+Sets the PHY downshift count.
+.RE
+.TP
+.B \-\-get\-phy\-tunable
+Gets the PHY tunable parameters.
+.RS 4
+.TP
+.B downshift
+Gets the PHY downshift count/status.
+.RE
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 49ac94e..c9a0a1d 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4520,6 +4520,94 @@ static int do_seee(struct cmd_context *ctx)
 	return 0;
 }
 
+static int do_get_phy_tunable(struct cmd_context *ctx)
+{
+	int argc = ctx->argc;
+	char **argp = ctx->argp;
+	int err, i;
+	u8 downshift_changed;
+
+	if (argc < 1)
+		exit_bad_args();
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argp[i], "downshift")) {
+			downshift_changed = 1;
+			i += 1;
+		} else  {
+			exit_bad_args();
+		}
+	}
+
+	if (downshift_changed) {
+		struct ethtool_tunable ds;
+		u8 count = 0;
+
+		ds.cmd = ETHTOOL_PHY_GTUNABLE;
+		ds.id = ETHTOOL_PHY_DOWNSHIFT;
+		ds.type_id = ETHTOOL_TUNABLE_U8;
+		ds.len = 1;
+		ds.data[0] = &count;
+		err = send_ioctl(ctx, &ds);
+		if (err < 0) {
+			perror("Cannot Get PHY downshift count");
+			err = 87;
+		}
+		count = *((u8 *)&ds.data[0]);
+		if (count)
+			fprintf(stdout, "  Downshift count: %d\n",
+				count);
+		else
+			fprintf(stdout, " Downshift disabled\n");
+	}
+
+	return err;
+}
+
+static int do_set_phy_tunable(struct cmd_context *ctx)
+{
+	int argc = ctx->argc;
+	char **argp = ctx->argp;
+	int err, i;
+	u8 downshift_changed, downshift_wanted = 0;
+
+	for (i = 0; i < argc; i++) {
+		if (!strcmp(argp[i], "downshift")) {
+			downshift_changed = 1;
+			i += 1;
+			if (i >= argc)
+				exit_bad_args();
+			if (!strcmp(argp[i], "on"))
+				downshift_wanted = DOWNSHIFT_DEV_DEFAULT_COUNT;
+			else if (!strcmp(argp[i], "off"))
+				downshift_wanted = DOWNSHIFT_DEV_DISABLE;
+			else
+				downshift_wanted =
+					get_uint_range(argp[i], 0, 0xff);
+		} else  {
+			exit_bad_args();
+		}
+	}
+
+	if (downshift_changed) {
+		struct ethtool_tunable ds;
+		u8 count;
+
+		ds.cmd = ETHTOOL_PHY_STUNABLE;
+		ds.id = ETHTOOL_PHY_DOWNSHIFT;
+		ds.type_id = ETHTOOL_TUNABLE_U8;
+		ds.len = 1;
+		ds.data[0] = &count;
+		*((u8 *)&ds.data[0]) = downshift_wanted;
+		err = send_ioctl(ctx, &ds);
+		if (err < 0) {
+			perror("Cannot Set PHY downshift count");
+			err = 87;
+		}
+	}
+
+	return err;
+}
+
 #ifndef TEST_ETHTOOL
 int send_ioctl(struct cmd_context *ctx, void *cmd)
 {
@@ -4681,6 +4769,10 @@ static const struct option {
 	  "		[ advertise %x ]\n"
 	  "		[ tx-lpi on|off ]\n"
 	  "		[ tx-timer %d ]\n"},
+	{ "--set-phy-tunable", 1, do_set_phy_tunable, "Set PHY tunable",
+	  "		[ downshift on|off|%d ]\n"},
+	{ "--get-phy-tunable", 1, do_get_phy_tunable, "Get PHY tunable",
+	  "		[ downshift ]\n"},
 	{ "-h|--help", 0, show_usage, "Show this help" },
 	{ "--version", 0, do_version, "Show version number" },
 	{}
-- 
2.7.3

Re: [PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Andrew Lunn]

Hi Allen

> +++ b/ethtool.8.in
> @@ -340,6 +340,14 @@ ethtool \- query or control network driver and hardware settings
>  .B2 tx-lpi on off
>  .BN tx-timer
>  .BN advertise
> +.HP
> +.B ethtool \-\-set\-phy\-tunable
> +.I devname
> +.B3 downshift on off N
> +.HP

I don't think there is any other option which is on|off|N. The general
pattern would be

--set-phy-tunable downshift on|off [count N]

With count being optional.

This also allows avoiding the ambiguity of what

--set-phy-tunable downshift 0

means. To me, that means downshift after 0 retries. But it actually
seems to mean never downshift. You can then error out when somebody
does

--set-phy-tunable downshift on count 0
or
--set-phy-tunable downshift off count 42

Please also add a few sentences about what downshift is, and what N
means. Is it tries, or retries?

Thanks
       Andrew

Re: [PATCH ethtool 2/2] Ethtool: Implements ETHTOOL_PHY_GTUNABLE/ETHTOOL_PHY_STUNABLE and PHY downshift

[Allan W. Nielsen]

On 04/11/16 12:59, Andrew Lunn wrote:
> > +++ b/ethtool.8.in
> > @@ -340,6 +340,14 @@ ethtool \- query or control network driver and hardware settings
> >  .B2 tx-lpi on off
> >  .BN tx-timer
> >  .BN advertise
> > +.HP
> > +.B ethtool \-\-set\-phy\-tunable
> > +.I devname
> > +.B3 downshift on off N
> > +.HP
> 
> I don't think there is any other option which is on|off|N. The general
> pattern would be
> 
> --set-phy-tunable downshift on|off [count N]
> 
> With count being optional.
> 
> This also allows avoiding the ambiguity of what
> 
> --set-phy-tunable downshift 0
> 
> means. To me, that means downshift after 0 retries. But it actually
> seems to mean never downshift. You can then error out when somebody
> does
> 
> --set-phy-tunable downshift on count 0
> or
> --set-phy-tunable downshift off count 42
Okay, I do not have any strong feelings about the syntax here.

We could not find any other signature which had the same need, so we have been
looking for things that was "close" to...

What is being proposed is similar to "-s [ mdix auto|on|off ]".

What you request is similar to "-e|--eeprom-dump [ raw on|off ] [ offset N ]".

So, unless there are other comments, then we will change this in the next
version.

> Please also add a few sentences about what downshift is, and what N
> means. Is it tries, or retries?
Sure.

We will wait a few days to see what other comments we get.

Thanks for reviewing.

/Allan