Subject: Re: sk(4) interrupt moderation timing fix, and sysctl support
To: None <tech-net@NetBSD.org>
From: Jeff Rizzo <riz@NetBSD.org>
List: tech-net
Date: 11/26/2005 20:00:20
--7iMSBzlTiPOCCT2k
Content-Type: multipart/mixed; boundary="FCuugMFkClbJLl1L"
Content-Disposition: inline
--FCuugMFkClbJLl1L
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Nov 24, 2005 at 09:49:39AM -0800, Jeff Rizzo wrote:
> Jason Thorpe wrote:
>=20
> >
> > On Nov 23, 2005, at 8:10 PM, Jeff Rizzo wrote:
> >
> >> 1. Should I bother supporting a per-board value for the interrupt=20
> >> moderation
> >> timer? In this patch, it's global, and only takes effect when=20
> >> sk_init()
> >> is called. (ifconfig down/up is a good way to trigger this)
> >
> >
> > Seems like it should be per-instance rather than global, especially=20
> > if different revisions of the chip need to have different values.
>=20
>=20
> Well, the global value is made chip-specific before being applied to the
> actual board, but I'll put together a per-instance version anyway; I
> think that's probably best.
>=20
> >
> >> 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=20
> >> later?
OK, I've decided to go ahead and take care of both these items (at least
to a certain extent - sk doesn't have a detach routine at this point,
so the fact that the sysctl is easy to undo is kind of a moot point)
Here's a new patch - if this looks OK to folks, I'll commit it.
Thanks!
+j
--FCuugMFkClbJLl1L
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="sk.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 27 Nov 2005 03:51:48 -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_skvar.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_skvar.h,v
retrieving revision 1.6
diff -u -r1.6 if_skvar.h
--- if_skvar.h 30 May 2005 04:35:22 -0000 1.6
+++ if_skvar.h 27 Nov 2005 03:51:48 -0000
@@ -206,6 +206,8 @@
u_int32_t sk_ramsize; /* amount of RAM on NIC */
u_int32_t sk_pmd; /* physical media type */
u_int32_t sk_intrmask;
+ struct sysctllog *sk_clog;
+ int sk_int_mod;
bus_dma_tag_t sc_dmatag;
struct sk_if_softc *sk_if[2];
};
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 27 Nov 2005 03:51:50 -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,9 @@
#define SK_WIN_CLRBIT_2(sc, reg, x) \
sk_win_write_2(sc, reg, sk_win_read_2(sc, reg) & ~x)
=20
+static int sysctl_sk_verify(SYSCTLFN_PROTO);
+static int sk_root_num;
+
/* supported device vendors */
static const struct sk_product {
pci_vendor_id_t sk_vendor;
@@ -996,6 +1000,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 +1037,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, sc->sk_int_mod);
+ sk_win_write_4(sc, SK_IMTIMERINIT, SK_IM_USECS(sc->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);
@@ -1347,9 +1366,10 @@
const char *intrstr =3D NULL;
bus_addr_t iobase;
bus_size_t iosize;
- int s;
+ int s, rc, sk_nodenum;
u_int32_t command;
const char *revstr;
+ const struct sysctlnode *node;
=20
DPRINTFN(2, ("begin skc_attach\n"));
=20
@@ -1623,6 +1643,36 @@
/* Turn on the 'driver is loaded' LED. */
CSR_WRITE_2(sc, SK_LED, SK_LED_GREEN_ON);
=20
+ /* skc sysctl setup */
+
+ sc->sk_int_mod =3D SK_IM_DEFAULT;
+
+ if ((rc =3D sysctl_createv(&sc->sk_clog, 0, NULL, &node,
+ 0, CTLTYPE_NODE, sc->sk_dev.dv_xname,
+ SYSCTL_DESCR("skc per-controller controls"),
+ NULL, 0, NULL, 0, CTL_HW, sk_root_num, CTL_CREATE,
+ CTL_EOL)) !=3D 0) {
+ aprint_normal("%s: couldn't create sysctl node\n",
+ sc->sk_dev.dv_xname);
+ goto fail;
+ }
+
+ sk_nodenum =3D node->sysctl_num;
+
+ /* interrupt moderation time in usecs */
+ if ((rc =3D sysctl_createv(&sc->sk_clog, 0, NULL, &node,
+ CTLFLAG_READWRITE,
+ CTLTYPE_INT, "int_mod",
+ SYSCTL_DESCR("sk interrupt moderation timer"),
+ sysctl_sk_verify, 0,
+ &sc->sk_int_mod,
+ 0, CTL_HW, sk_root_num, sk_nodenum, CTL_CREATE,
+ CTL_EOL)) !=3D 0) {
+ aprint_normal("%s: couldn't create int_mod sysctl node\n",
+ sc->sk_dev.dv_xname);
+ goto fail;
+ }
+
fail:
splx(s);
}
@@ -2481,6 +2531,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 +2634,25 @@
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(sc->sk_int_mod)) {
+ sk_win_write_4(sc, SK_IMTIMERINIT,
+ SK_IM_USECS(sc->sk_int_mod));
+ aprint_verbose("%s: interrupt moderation is %d us\n",
+ sc->sk_dev.dv_xname, sc->sk_int_mod);
+ }
+
/* Configure interrupt handling */
CSR_READ_4(sc, SK_ISSR);
if (sc_if->sk_port =3D=3D SK_PORT_A)
@@ -2785,3 +2855,53 @@
}
}
#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 (t < SK_IM_MIN || t > SK_IM_MAX)
+ return (EINVAL);
+
+ *(int*)rnode->sysctl_data =3D t;
+
+ return (0);
+}
+
+/*
+ * Set up sysctl(3) MIB, hw.sk.* - Individual controllers will be
+ * set up in skc_attach()
+ */
+SYSCTL_SETUP(sysctl_sk, "sysctl sk subtree setup")
+{
+ int rc;
+ const struct sysctlnode *node;
+
+ if ((rc =3D sysctl_createv(clog, 0, NULL, NULL,
+ 0, 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,
+ 0, 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;
+ return;
+
+err:
+ printf("%s: syctl_createv failed (rc =3D %d)\n", __func__, rc);
+}
--FCuugMFkClbJLl1L--
--7iMSBzlTiPOCCT2k
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (NetBSD)
iQCVAwUBQ4kvVLOuUtxCgar5AQKtrwQAh7+UnFCKMSEorr2vFE2l+opiV6x3TOyH
gaZEkjUNzoicPCdtlhb9fzhT647hUp9z9UJZtpfu2ZIpzKnYDwlUWD684f33EI6H
BhbeUofAE8F5sDrAQf/By//+DPQBsekmV6VBkMBgI9xdb/a7cLb89nVVI+9pTV4j
5EPi+NsiZm8=
=ZLss
-----END PGP SIGNATURE-----
--7iMSBzlTiPOCCT2k--