Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 21 Apr 2006 18:01:07 GMT
From:      Marcel Moolenaar <marcel@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 95806 for review
Message-ID:  <200604211801.k3LI17b1075886@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=95806

Change 95806 by marcel@marcel_nfs on 2006/04/21 18:00:26

	Brainwave: have puc_intr() only handle serdev devices. Non-serdev
	devices (including non-serdev serial drivers) will have their
	interrupt handler setup by the bus implementation. This makes
	live so much easier as it assures that serdev devices with fast
	interrupt handlers don't get pessimized by Giant-locked parallel
	ports (by virtue of having puc(4) re-setup its interrupt handler
	as MPSAFE). If there are no serdev children, puc(4) tears down
	its handler.

Affected files ...

.. //depot/projects/uart/dev/puc/puc.c#33 edit

Differences ...

==== //depot/projects/uart/dev/puc/puc.c#33 (text+ko) ====

@@ -58,8 +58,6 @@
 	int		p_rclk;
 
 	int		p_hasintr:1;
-	int 		p_fastintr:1;
-	int		p_giantintr:1;
 
 	driver_intr_t	*p_ih;
 	serdev_intr_t	*p_ihsrc[PUC_ISRCCNT];
@@ -135,7 +133,6 @@
 	u_long dev, devs;
 	int i, idx, ipend, isrc;
 
-	/* handle serdev I/F compliant devices first. */
 	ipend = 0;
 	idx = 0, dev = 1UL;
 	devs = sc->sc_serdevs;
@@ -148,26 +145,6 @@
 		ipend |= port->p_ipend;
 	}
 
-	if (ipend == 0) {
-		idx = 0, dev = 1UL;
-		devs = ~sc->sc_serdevs & ((1UL << sc->sc_nports) - 1);
-		while (devs != 0UL) {
-			while ((devs & dev) == 0UL)
-				idx++, dev <<= 1;
-			devs &= ~dev;
-			port = &sc->sc_port[idx];
-			if (port->p_ih != NULL) {
-				if (port->p_giantintr) {
-			                mtx_lock(&Giant);
-					(*port->p_ih)(port->p_iharg);
-					mtx_unlock(&Giant);
-				} else
-					(*port->p_ih)(port->p_iharg);
-			}
-		}
-		return;
-	}
-
 	i = 0, isrc = SER_INT_OVERRUN;
 	while (ipend) {
 		while (i < PUC_ISRCCNT && !(ipend & isrc))
@@ -322,6 +299,13 @@
 		}
 	}
 
+	/*
+	 * If there are no serdev devices, then our interrupt handler
+	 * will do nothing. Tear it down.
+	 */
+	if (sc->sc_serdevs == 0UL)
+		bus_teardown_intr(dev, sc->sc_ires, sc->sc_icookie);
+
 	return (0);
 
 fail:
@@ -375,7 +359,8 @@
 	if (error)
 		return (error);
 
-	bus_teardown_intr(dev, sc->sc_ires, sc->sc_icookie);
+	if (sc->sc_serdevs != 0UL)
+		bus_teardown_intr(dev, sc->sc_ires, sc->sc_icookie);
 	bus_release_resource(dev, SYS_RES_IRQ, sc->sc_irid, sc->sc_ires);
 
 	for (idx = 0; idx < PUC_PCI_BARS; idx++) {
@@ -550,7 +535,7 @@
 	struct puc_port *port;
 	struct puc_softc *sc;
 	device_t originator;
-	int i, isrc;
+	int i, isrc, serdev;
 
 	sc = device_get_softc(dev);
 
@@ -570,37 +555,35 @@
 		return (ENXIO);
 
 	/*
-	 * If one of our children doesn't have a fast interrupt handler,
-	 * downgrade our interrupt handler from a fast handler to a
-	 * MP safe handler. We acquire giant on a need to have basis.
+	 * Have non-serdev ports handled by the bus implementation. It
+	 * supports multiple handlers for a single interrupt as it is,
+	 * so we wouldn't add value if we did it ourselves.
 	 */
-	if (sc->sc_fastintr && !(flags & INTR_FAST)) {
-		sc->sc_fastintr = 0;
-		bus_teardown_intr(dev, sc->sc_ires, sc->sc_icookie);
-		bus_setup_intr(dev, sc->sc_ires, INTR_TYPE_TTY | INTR_MPSAFE,
-		    puc_intr, sc, &sc->sc_icookie);
-	}
-
-	port->p_hasintr = 1;
-	port->p_fastintr = (flags & INTR_FAST) ? 1 : 0;
-	port->p_giantintr = (flags & (INTR_FAST | INTR_MPSAFE)) ? 0 : 1;
-	port->p_ih = ihand;
-	port->p_iharg = arg;
-
-	*cookiep = originator;
-
+	serdev = 0;
 	if (port->p_type == PUC_TYPE_SERIAL) {
 		i = 0, isrc = SER_INT_OVERRUN;
 		while (i < PUC_ISRCCNT) {
 			port->p_ihsrc[i] = SERDEV_IHAND(originator, isrc);
-			if (port->p_ihsrc[i] != NULL) {
-				sc->sc_serdevs |= 1UL << (port->p_nr - 1);
-				port->p_ih = NULL;
-			}
+			if (port->p_ihsrc[i] != NULL)
+				serdev = 1;
 			i++, isrc <<= 1;
 		}
 	}
+	if (!serdev)
+		return (BUS_SETUP_INTR(device_get_parent(dev), originator,
+		    sc->sc_ires, flags, ihand, arg, cookiep));
+
+	/* We demand that serdev devices use fast interrupts. */
+	if (!(flags & INTR_FAST))
+		return (ENXIO);
+
+	sc->sc_serdevs |= 1UL << (port->p_nr - 1);
+
+	port->p_hasintr = 1;
+	port->p_ih = ihand;
+	port->p_iharg = arg;
 
+	*cookiep = port;
 	return (0);
 }
 
@@ -609,9 +592,12 @@
     void *cookie)
 {
 	struct puc_port *port;
+	struct puc_softc *sc;
 	device_t originator;
 	int i;
 
+	sc = device_get_softc(dev);
+
 	/* Get our immediate child. */
 	originator = child;
 	while (child != NULL && device_get_parent(child) != dev)
@@ -622,23 +608,24 @@
 	port = device_get_ivars(child);
 	KASSERT(port != NULL, ("%s %d", __func__, __LINE__));
 
-	if (res != port->p_ires || cookie != originator)
+	if (res != port->p_ires)
 		return (EINVAL);
 	if (rman_get_device(port->p_ires) != originator)
 		return (ENXIO);
+
 	if (!port->p_hasintr)
-		return (ENXIO);
+		return (BUS_TEARDOWN_INTR(device_get_parent(dev), originator,
+		    sc->sc_ires, cookie));
+
+	if (cookie != port)
+		return (EINVAL);
 
 	port->p_hasintr = 0;
-	port->p_fastintr = 0;
-	port->p_giantintr = 0;
 	port->p_ih = NULL;
 	port->p_iharg = NULL;
 
-	if (port->p_type == PUC_TYPE_SERIAL) {
-		for (i = 0; i < PUC_ISRCCNT; i++)
-			port->p_ihsrc[i] = NULL;
-	}
+	for (i = 0; i < PUC_ISRCCNT; i++)
+		port->p_ihsrc[i] = NULL;
 
 	return (0);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200604211801.k3LI17b1075886>