[PATCH] sis900: add ethtool tests (link, eeprom)

[PATCH] sis900: add ethtool tests (link, eeprom)

[Sergej Benilov]

Add tests for ethtool: link test, EEPROM read test.
Correct a few typos, too.

Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
---
 drivers/net/ethernet/sis/sis900.c | 78 +++++++++++++++++++++++++++++--
 1 file changed, 74 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 9b036c857b1d..a781bce23ec8 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -262,7 +262,7 @@ static int sis900_get_mac_addr(struct pci_dev *pci_dev,
 	/* check to see if we have sane EEPROM */
 	signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
 	if (signature == 0xffff || signature == 0x0000) {
-		printk (KERN_WARNING "%s: Error EERPOM read %x\n",
+		printk (KERN_WARNING "%s: Error EEPROM read %x\n",
 			pci_name(pci_dev), signature);
 		return 0;
 	}
@@ -359,9 +359,9 @@ static int sis635_get_mac_addr(struct pci_dev *pci_dev,
  *
  *	SiS962 or SiS963 model, use EEPROM to store MAC address. And EEPROM
  *	is shared by
- *	LAN and 1394. When access EEPROM, send EEREQ signal to hardware first
- *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be access
- *	by LAN, otherwise is not. After MAC address is read from EEPROM, send
+ *	LAN and 1394. When accessing EEPROM, send EEREQ signal to hardware first
+ *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be accessed
+ *	by LAN, otherwise it is not. After MAC address is read from EEPROM, send
  *	EEDONE signal to refuse EEPROM access by LAN.
  *	The EEPROM map of SiS962 or SiS963 is different to SiS900.
  *	The signature field in SiS962 or SiS963 spec is meaningless.
@@ -2122,6 +2122,73 @@ static void sis900_get_wol(struct net_device *net_dev, struct ethtool_wolinfo *w
 	wol->supported = (WAKE_PHY | WAKE_MAGIC);
 }
 
+static const char sis900_gstrings_test[][ETH_GSTRING_LEN] = {
+	"Link test     (on/offline)",
+	"EEPROM read test   (on/offline)",
+};
+#define SIS900_TEST_LEN	ARRAY_SIZE(sis900_gstrings_test)
+
+static int sis900_eeprom_readtest(struct net_device *net_dev)
+{
+	struct sis900_private *sis_priv = netdev_priv(net_dev);
+	void __iomem *ioaddr = sis_priv->ioaddr;
+	int wait, ret = -EAGAIN;
+	u16 signature;
+
+	if (sis_priv->chipset_rev == SIS96x_900_REV) {
+	sw32(mear, EEREQ);
+	for (wait = 0; wait < 2000; wait++) {
+		if (sr32(mear) & EEGNT) {
+		 signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
+    	 ret = 0;
+		 break;
+		}
+		udelay(1);
+	  }		
+	sw32(mear, EEDONE);
+	}
+	else {		
+		signature = (u16) read_eeprom(ioaddr, EEPROMSignature);
+		if (signature != 0xffff && signature != 0x0000)
+			ret = 0;
+	}
+	return ret;
+}
+
+static void sis900_diag_test(struct net_device *netdev,
+	struct ethtool_test *test, u64 *data)
+{
+	struct sis900_private *nic = netdev_priv(netdev);
+	int i;
+
+	memset(data, 0, SIS900_TEST_LEN * sizeof(u64));
+	data[0] = !mii_link_ok(&nic->mii_info);
+    data[1] = sis900_eeprom_readtest(netdev);
+	for (i = 0; i < SIS900_TEST_LEN; i++)
+		test->flags |= data[i] ? ETH_TEST_FL_FAILED : 0;
+
+	msleep_interruptible(4 * 1000);
+}
+
+static int sis900_get_sset_count(struct net_device *netdev, int sset)
+{
+	switch (sset) {
+	case ETH_SS_TEST:
+		return SIS900_TEST_LEN;
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static void sis900_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
+{
+	switch (stringset) {
+	case ETH_SS_TEST:
+		memcpy(data, *sis900_gstrings_test, sizeof(sis900_gstrings_test));
+		break;
+	}
+}
+
 static const struct ethtool_ops sis900_ethtool_ops = {
 	.get_drvinfo 	= sis900_get_drvinfo,
 	.get_msglevel	= sis900_get_msglevel,
@@ -2132,6 +2199,9 @@ static const struct ethtool_ops sis900_ethtool_ops = {
 	.set_wol	= sis900_set_wol,
 	.get_link_ksettings = sis900_get_link_ksettings,
 	.set_link_ksettings = sis900_set_link_ksettings,
+	.self_test		= sis900_diag_test,
+	.get_strings	= sis900_get_strings,
+	.get_sset_count		= sis900_get_sset_count,
 };
 
 /**
-- 
2.17.1

Re: [PATCH] sis900: add ethtool tests (link, eeprom)

[Andrew Lunn]

On Mon, Jul 01, 2019 at 11:03:33AM +0200, Sergej Benilov wrote:
> Add tests for ethtool: link test, EEPROM read test.
> Correct a few typos, too.

Hi Sergej

Please split this up into two patches. The first one should fixing the
typos.

Rather than implementing a test for the EEPROM, add support for
ethtool --eeprom-dump. That is much more useful.

The link test does not show you anything which you cannot get via ip
link show. If there is no carrier, the link is down. So drop that.

The patch also has white space issues. Spaces where there should be
tabs. Please run ./scripts/checkpatch.pl.

     Andrew