Subject: sk(4) interrupt moderation timing fix, and sysctl support
To: None <tech-net@netbsd.org>
From: Jeff Rizzo <riz@tastylime.net>
List: tech-net
Date: 11/23/2005 20:10:49
--4SFOXa2GPu3tIq4H
Content-Type: multipart/mixed; boundary="jRHKVT23PllUwdXP"
Content-Disposition: inline


--jRHKVT23PllUwdXP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Here's another patch, to be applied in sys/dev/pci, which does two main
things:  takes into account the fact that not all sk chips use the same
interrupt moderation timer frequency (information taken from Marvell's
sk98lin Linux driver, at the suggestion of Thor), and also adds sysctl
support for changing the interrupt moderation timer at runtime.

I'd appreciate folks with sk boards, especially boards besides the ASUS
A8V motherboard and D-Link DGE-530T, to test this.  Unlike previous patches
I've sent, this one actually required me to think a little bit.  ;)
I modeled the sysctl piece after a combination of the FreeBSD driver, and
NetBSD's bge driver.

A few things I have questions on -

1. Should I bother supporting a per-board value for the interrupt moderation
timer?  In this patch, it's global, and only takes effect when sk_init()
is called. (ifconfig down/up is a good way to trigger this)

2. if_bge.c notes that the sysctl stuff will need to be made dynamic to
support LKM loading - should I bother with this now, or take it up later?

Thanks,
+j

--jRHKVT23PllUwdXP
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sk_intmod.diff"
Content-Transfer-Encoding: quoted-printable

Index: if_skreg.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/pci/if_skreg.h,v
retrieving revision 1.4
diff -u -r1.4 if_skreg.h
--- if_skreg.h	30 May 2005 04:35:22 -0000	1.4
+++ if_skreg.h	24 Nov 2005 04:02:02 -0000
@@ -357,18 +357,32 @@
 #define SK_YUKON		0xB0
 #define SK_YUKON_LITE		0xB1
 #define SK_YUKON_LP		0xB2
+#define SK_YUKON_XL		0xB3
+#define SK_YUKON_EC_U		0xB4
+#define SK_YUKON_EC		0xB6
+#define SK_YUKON_FE		0xB7
 #define SK_YUKON_FAMILY(x) ((x) & 0xB0)
 /* known revisions in SK_CONFIG */
 #define SK_YUKON_LITE_REV_A0	0x0 /* invented, see test in skc_attach */
 #define SK_YUKON_LITE_REV_A1	0x3
 #define SK_YUKON_LITE_REV_A3	0x7
=20
+#define SK_YUKON_EC_REV_A1	0x0
+#define SK_YUKON_EC_REV_A2	0x1
+#define SK_YUKON_EC_REV_A3	0x2
+
 #define SK_IMCTL_STOP	0x02
 #define SK_IMCTL_START	0x04
=20
-#define SK_IMTIMER_TICKS	54
-#define SK_IM_USECS(x)		((x) * SK_IMTIMER_TICKS)
-
+/* Number of ticks per usec for interrupt moderation */
+#define SK_IMTIMER_TICKS_GENESIS	54
+#define SK_IMTIMER_TICKS_YUKON		78
+#define SK_IMTIMER_TICKS_YUKON_EC	125
+#define SK_IM_USECS(x)		((x) * sk_imtimer_ticks)
+
+#define SK_IM_MIN	10
+#define SK_IM_DEFAULT	100
+#define SK_IM_MAX	10000
 /*
  * The SK_EPROM0 register contains a byte that describes the
  * amount of SRAM mounted on the NIC. The value also tells if
Index: if_sk.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/dev/pci/if_sk.c,v
retrieving revision 1.18
diff -u -r1.18 if_sk.c
--- if_sk.c	23 Nov 2005 18:56:22 -0000	1.18
+++ if_sk.c	24 Nov 2005 04:02:04 -0000
@@ -133,6 +133,7 @@
 #include <sys/device.h>
 #include <sys/queue.h>
 #include <sys/callout.h>
+#include <sys/sysctl.h>
=20
 #include <net/if.h>
 #include <net/if_dl.h>
@@ -243,6 +244,10 @@
 #define SK_WIN_CLRBIT_2(sc, reg, x)	\
 	sk_win_write_2(sc, reg, sk_win_read_2(sc, reg) & ~x)
=20
+/* interrupt moderation timer global XXX should this be per-interface? */
+static int sk_int_mod =3D SK_IM_DEFAULT;
+static int sk_int_mod_nodenum;
+
 /* supported device vendors */
 static const struct sk_product {
 	pci_vendor_id_t		sk_vendor;
@@ -996,6 +1001,8 @@
  */
 void sk_reset(struct sk_softc *sc)
 {
+	u_int32_t sk_imtimer_ticks;
+
 	DPRINTFN(2, ("sk_reset\n"));
=20
 	CSR_WRITE_2(sc, SK_CSR, SK_CSR_SW_RESET);
@@ -1031,10 +1038,23 @@
 	 * defers interrupts specified in the interrupt moderation
 	 * timer mask based on the timeout specified in the interrupt
 	 * moderation timer init register. Each bit in the timer
-	 * register represents 18.825ns, so to specify a timeout in
-	 * microseconds, we have to multiply by 54.
+	 * register represents one tick, so to specify a timeout in
+	 * microseconds, we have to multiply by the correct number of
+	 * ticks-per-microsecond.
 	 */
-        sk_win_write_4(sc, SK_IMTIMERINIT, SK_IM_USECS(100));
+	switch (sc->sk_type) {
+	case SK_GENESIS:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_GENESIS;
+		break;
+	case SK_YUKON_EC:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_YUKON_EC;
+		break;
+	default:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_YUKON;
+	}
+	aprint_verbose("%s: interrupt moderation is %d us\n",
+	    sc->sk_dev.dv_xname, sk_int_mod);
+        sk_win_write_4(sc, SK_IMTIMERINIT, SK_IM_USECS(sk_int_mod));
         sk_win_write_4(sc, SK_IMMR, SK_ISR_TX1_S_EOF|SK_ISR_TX2_S_EOF|
 	    SK_ISR_RX1_EOF|SK_ISR_RX2_EOF);
         sk_win_write_1(sc, SK_IMTIMERCTL, SK_IMCTL_START);
@@ -2481,6 +2501,7 @@
 	struct sk_softc		*sc =3D sc_if->sk_softc;
 	struct mii_data		*mii =3D &sc_if->sk_mii;
 	int			s;
+	u_int32_t		imr, sk_imtimer_ticks;
=20
 	DPRINTFN(1, ("sk_init\n"));
=20
@@ -2583,6 +2604,24 @@
 		return(ENOBUFS);
 	}
=20
+	/* Set interrupt moderation if changed via sysctl. */
+	switch (sc->sk_type) {
+	case SK_GENESIS:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_GENESIS;
+		break;
+	case SK_YUKON_EC:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_YUKON_EC;
+		break;
+	default:
+		sk_imtimer_ticks =3D SK_IMTIMER_TICKS_YUKON;
+	}
+	imr =3D sk_win_read_4(sc, SK_IMTIMERINIT);
+	if (imr !=3D SK_IM_USECS(sk_int_mod)) {
+		sk_win_write_4(sc, SK_IMTIMERINIT, SK_IM_USECS(sk_int_mod));
+		aprint_verbose("%s: interrupt moderation is %d us\n",
+		    sc->sk_dev.dv_xname, sk_int_mod);
+	}
+
 	/* Configure interrupt handling */
 	CSR_READ_4(sc, SK_ISSR);
 	if (sc_if->sk_port =3D=3D SK_PORT_A)
@@ -2785,3 +2824,70 @@
 	}
 }
 #endif
+
+static int
+sysctl_sk_verify(SYSCTLFN_ARGS)
+{
+	int error, t;
+	struct sysctlnode node;
+
+	node =3D *rnode;
+	t =3D *(int*)rnode->sysctl_data;
+	node.sysctl_data =3D &t;
+	error =3D sysctl_lookup(SYSCTLFN_CALL(&node));
+	if (error || newp =3D=3D NULL)
+		return (error);
+
+	if (node.sysctl_num =3D=3D sk_int_mod_nodenum){
+		if (t < SK_IM_MIN || t > SK_IM_MAX)
+			return (EINVAL);
+	} else
+		return (EINVAL);
+
+	*(int*)rnode->sysctl_data =3D t;
+
+	return (0);
+}
+
+/*
+ * Set up sysctl(3) MIB, hw.sk.*
+ */
+SYSCTL_SETUP(sysctl_sk, "sysctl sk subtree setup")
+{
+	int rc, sk_root_num;
+	const struct sysctlnode *node;
+
+	if ((rc =3D sysctl_createv(clog, 0, NULL, NULL,
+	    CTLFLAG_PERMANENT, CTLTYPE_NODE, "hw", NULL,
+	    NULL, 0, NULL, 0, CTL_HW, CTL_EOL)) !=3D 0) {
+		goto err;
+	}
+
+	if ((rc =3D sysctl_createv(clog, 0, NULL, &node,
+	    CTLFLAG_PERMANENT, CTLTYPE_NODE, "sk",
+	    SYSCTL_DESCR("sk interface controls"),
+	    NULL, 0, NULL, 0, CTL_HW, CTL_CREATE, CTL_EOL)) !=3D 0) {
+		goto err;
+	}
+
+	sk_root_num =3D node->sysctl_num;
+
+	/* sk interrupt moderation time in usecs */
+	if ((rc =3D sysctl_createv(clog, 0, NULL, &node,
+		    CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
+		    CTLTYPE_INT, "int_mod",
+		    SYSCTL_DESCR("sk interrupt moderation timer"),
+		    sysctl_sk_verify, 0,
+		    &sk_int_mod,
+		    0, CTL_HW, sk_root_num, CTL_CREATE,
+		    CTL_EOL)) !=3D 0) {
+		goto err;
+	}
+
+	sk_int_mod_nodenum =3D node->sysctl_num;
+
+	return;
+
+err:
+	printf("%s: syctl_createv failed (rc =3D %d)\n", __func__, rc);
+}

--jRHKVT23PllUwdXP--

--4SFOXa2GPu3tIq4H
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (NetBSD)

iQCVAwUBQ4U9SbOuUtxCgar5AQJLjgP/e6kue0eJo91JWm6VINAseHUuwvBW3fI+
o5ZghpIrQ/VS/KPdWhlOhNsOscYW7I+6FEyf+EXrtYdgQnZWwZrGGslhHC2BSAYZ
+OVleVufbFH1JEJHXT39qksVD7ql5nw5gaPj6NOaaRadEzLv76N+aAI3ExPSxMRu
o3Ej0Yva0r0=
=ZOwu
-----END PGP SIGNATURE-----

--4SFOXa2GPu3tIq4H--