kern/39642: tcom(4) driver is broken

[khym%sloth.azeotrope.org@localhost]

>Number:         39642
>Category:       kern
>Synopsis:       tcom(4) driver is broken
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Sep 28 02:15:00 +0000 2008
>Originator:     Dave Huang
>Release:        NetBSD 4.99.72
>Organization:
        
>Environment:
        
        
System: NetBSD sloth.azeotrope.org 4.99.72 NetBSD 4.99.72 (FOXY) #4: Sat Sep 27 
20:02:31 CDT 2008 
khym%cheetah.azeotrope.org@localhost:/usr/obj.i386/sys/arch/i386/compile/FOXY 
i386
Architecture: i386
Machine: i386
>Description:
        The tcom(4) driver probes and attaches com ports, but opening a tty
device causes fatal page fault in supervisor mode. It looks like the changes
in http://mail-index.netbsd.org/source-changes/2008/03/14/msg003488.html
were made for com(4) devices, but not for the devices that attached com(4)
as a slave device (i.e., multiport serial cards).

>How-To-Repeat:
        Boot -current kernel on a machine that has a multiport serial card
that uses the tcom(4) driver, then watch it crash when the boot process
runs "ttyflags -a". Or boot single-user, and do "stty -f /dev/tty02"
(assuming tty02 is one of the tcom ports).

>Fix:
        I'm not sure if this is correct, but it seems to work for me. I
think something similar will need to be done to the other multiport serial
cards in dev/isa that have the same basic structure. On further inspection,
it looks like ast(4) was already fixed (PR kern/38776); however, boca, ioat,
moxa, and rtfps look like they're still broken. My patch does things
differently than the way ast was fixed; perhaps the ast way is better;
I don't actually know anything about the kernel config mechanism.

Index: tcom.c
===================================================================
RCS file: /cvsroot/src/sys/dev/isa/tcom.c,v
retrieving revision 1.16
diff -u -r1.16 tcom.c
--- tcom.c      28 Apr 2008 20:23:52 -0000      1.16
+++ tcom.c      28 Sep 2008 01:37:59 -0000
@@ -91,28 +91,27 @@
 #define        STATUS_SIZE     8               /* 8 bytes reserved for irq 
status */
 
 struct tcom_softc {
-       struct device sc_dev;
+       device_t sc_dev;
        void *sc_ih;
 
        bus_space_tag_t sc_iot;
        int sc_iobase;
 
        int sc_alive;                   /* mask of slave units attached */
-       void *sc_slaves[NSLAVES];       /* com device unit numbers */
+       device_t sc_slaves[NSLAVES];    /* com slave devices */
        bus_space_handle_t sc_slaveioh[NSLAVES];
        bus_space_handle_t sc_statusioh;
 };
 
-int tcomprobe(struct device *, struct cfdata *, void *);
-void tcomattach(struct device *, struct device *, void *);
+int tcomprobe(struct device *, cfdata_t, void *);
+void tcomattach(struct device *, device_t, void *);
 int tcomintr(void *);
 
-CFATTACH_DECL(tcom, sizeof(struct tcom_softc),
+CFATTACH_DECL_NEW(tcom, sizeof(struct tcom_softc),
     tcomprobe, tcomattach, NULL, NULL);
 
 int
-tcomprobe(struct device *parent, struct cfdata *self,
-    void *aux)
+tcomprobe(struct device *parent, cfdata_t self, void *aux)
 {
        struct isa_attach_args *ia = aux;
        bus_space_tag_t iot = ia->ia_iot;
@@ -181,9 +180,9 @@
 }
 
 void
-tcomattach(struct device *parent, struct device *self, void *aux)
+tcomattach(struct device *parent, device_t self, void *aux)
 {
-       struct tcom_softc *sc = (void *)self;
+       struct tcom_softc *sc = device_private(self);
        struct isa_attach_args *ia = aux;
        struct commulti_attach_args ca;
        bus_space_tag_t iot = ia->ia_iot;
@@ -191,6 +190,7 @@
 
        printf("\n");
 
+       sc->sc_dev = self;
        sc->sc_iot = ia->ia_iot;
        sc->sc_iobase = ia->ia_io[0].ir_addr;
 
@@ -199,14 +199,14 @@
                if (!com_is_console(iot, iobase, &sc->sc_slaveioh[i]) &&
                    bus_space_map(iot, iobase, COM_NPORTS, 0,
                        &sc->sc_slaveioh[i])) {
-                       aprint_error_dev(&sc->sc_dev, "can't map i/o space for 
slave %d\n", i);
+                       aprint_error_dev(sc->sc_dev, "can't map i/o space for 
slave %d\n", i);
                        return;
                }
        }
 
        if (bus_space_map(iot, sc->sc_iobase + STATUS_OFFSET, STATUS_SIZE, 0,
            &sc->sc_statusioh)) {
-               aprint_error_dev(&sc->sc_dev, "can't map status space\n");
+               aprint_error_dev(sc->sc_dev, "can't map status space\n");
                return;
        }
 
@@ -242,7 +242,7 @@
        for (;;) {
 #define        TRY(n) \
                if (bits & (1 << (n))) \
-                       comintr(sc->sc_slaves[n]);
+                       comintr(device_private(sc->sc_slaves[n]));
                TRY(0);
                TRY(1);
                TRY(2);

>Unformatted: