Subject: Re: kern/36276 (ucycom causes kernel panic when accessing Delorme USB Earthmate LT-20 GPS)
To: None <gnats-bugs@netbsd.org>
From: Nick Hudson <skrll@netbsd.org>
List: netbsd-bugs
Date: 06/21/2007 08:27:38
--Boundary-00=_rhieGDlp4vAv9/6
Content-Type: text/plain;
  charset="iso-8859-6"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On Tuesday 22 May 2007 21:28, skrll@netbsd.org wrote:
> Synopsis: ucycom causes kernel panic when accessing Delorme USB Earthmate 
LT-20 GPS

Can you try this patch. It deals with the transfer length in the callback
differently to yours (and it works for me ;)

I'll deal with the whitespace stuff later.

Thanks,
Nick

--Boundary-00=_rhieGDlp4vAv9/6
Content-Type: text/x-diff;
  charset="iso-8859-6";
  name="ucycom.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename="ucycom.diff"

Index: dev/usb/ucycom.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/ucycom.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 ucycom.c
--- dev/usb/ucycom.c	4 Mar 2007 06:02:49 -0000	1.17
+++ dev/usb/ucycom.c	21 Jun 2007 07:18:51 -0000
@@ -129,6 +129,7 @@ struct ucycom_softc {
 	size_t			sc_olen; /* output report length */
 
 	uint8_t			*sc_obuf;
+	int			sc_wlen;
 
 	/* settings */
 	uint32_t		sc_baud;
@@ -157,6 +158,7 @@ const struct cdevsw ucycom_cdevsw = {
 
 Static int ucycomparam(struct tty *, struct termios *);
 Static void ucycomstart(struct tty *);
+Static void ucycomwritecb(usbd_xfer_handle, usbd_private_handle, usbd_status);
 Static void ucycom_intr(struct uhidev *, void *, u_int);
 Static int ucycom_configure(struct ucycom_softc *, uint32_t, uint8_t);
 Static void tiocm_to_ucycom(struct ucycom_softc *, u_long, int);
@@ -168,13 +170,14 @@ Static void ucycom_rts(struct ucycom_sof
 #endif
 Static void ucycom_cleanup(struct ucycom_softc *sc);
 
-#ifdef UCYCOM_DEBUG
+#ifdef UCYCOM_DEBUGX
 Static void ucycom_get_cfg(struct ucycom_softc *);
 #endif
 
 Static const struct usb_devno ucycom_devs[] = {
 	{ USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_USBRS232 },
 	{ USB_VENDOR_DELORME, USB_PRODUCT_DELORME_EARTHMATE },
+	{ USB_VENDOR_DELORME, USB_PRODUCT_DELORME_EARTHMATE_LT20 },
 };
 #define ucycom_lookup(v, p) usb_lookup(ucycom_devs, v, p)
 
@@ -452,8 +455,9 @@ Static void
 ucycomstart(struct tty *tp)
 {
 	struct ucycom_softc *sc = ucycom_cd.cd_devs[UCYCOMUNIT(tp->t_dev)];
+	usbd_status err;
 	u_char *data;
-	int cnt, len, err, s;
+	int cnt, len, s;
 
 	if (sc->sc_dying)
 		return;
@@ -552,7 +556,7 @@ ucycomstart(struct tty *tp)
 		    sc->sc_olen));
 		goto out;
 	}
-	splx(s);
+	sc->sc_wlen = len;
 
 #ifdef UCYCOM_DEBUG
 	if (ucycomdebug > 5) {
@@ -566,26 +570,64 @@ ucycomstart(struct tty *tp)
 		}
 	}
 #endif
-	err = uhidev_write(sc->sc_hdev.sc_parent, sc->sc_obuf, sc->sc_olen);
 
-	if (err) {
-		DPRINTF(("ucycomstart: error doing uhidev_write = %d\n", err));
-	}
+	DPRINTFN(4,("ucycomstart: %d chars\n", len));
+	usbd_setup_xfer(sc->sc_hdev.sc_parent->sc_oxfer,
+	    sc->sc_hdev.sc_parent->sc_opipe, (usbd_private_handle)sc,
+	    sc->sc_obuf, sc->sc_olen, 0 /* USBD_NO_COPY */, USBD_NO_TIMEOUT,
+	    ucycomwritecb);
 
+	/* What can we do on error? */
+	err = usbd_transfer(sc->sc_hdev.sc_parent->sc_oxfer);
 #ifdef UCYCOM_DEBUG
-	ucycom_get_cfg(sc);
+	if (err != USBD_IN_PROGRESS)
+		DPRINTF(("ucycomstart: err=%s\n", usbd_errstr(err)));
 #endif
-	DPRINTFN(4,("ucycomstart: req %d chars did %d chars\n", cnt, len));
 
- 	s = spltty();
+out:
+	splx(s);
+}
+
+Static void
+ucycomwritecb(usbd_xfer_handle xfer, usbd_private_handle p, usbd_status status)
+{
+	struct ucycom_softc *sc = (struct ucycom_softc *)p;
+	struct tty *tp = sc->sc_tty;
+	usbd_status stat;
+	int len, s;
+
+	if (status == USBD_CANCELLED || sc->sc_dying)
+		goto error;
+
+	if (status) {
+		DPRINTF(("ucycomwritecb: status=%d\n", status));
+		usbd_clear_endpoint_stall(sc->sc_hdev.sc_parent->sc_opipe);
+		/* XXX we should restart after some delay. */
+		goto error;
+	}
+
+	usbd_get_xfer_status(xfer, NULL, NULL, &len, &stat);
+
+	if (status != USBD_NORMAL_COMPLETION) {
+		DPRINTFN(4,("ucycomwritecb: status = %d\n", status));
+		goto error;
+	}
+
+	DPRINTFN(4,("ucycomwritecb: did %d/%d chars\n", sc->sc_wlen, len));
+
+	s = spltty();
 	CLR(tp->t_state, TS_BUSY);
 	if (ISSET(tp->t_state, TS_FLUSH))
 		CLR(tp->t_state, TS_FLUSH);
 	else
-		ndflush(&tp->t_outq, len);
+		ndflush(&tp->t_outq, sc->sc_wlen);
 	(*tp->t_linesw->l_start)(tp);
+	splx(s);
+	return;
 
-out:
+error:
+	s = spltty();
+	CLR(tp->t_state, TS_BUSY);
 	splx(s);
 }
 
@@ -1036,7 +1078,7 @@ ucycom_set_status(struct ucycom_softc *s
 	}
 }
 
-#ifdef UCYCOM_DEBUG
+#ifdef UCYCOM_DEBUGX
 Static void
 ucycom_get_cfg(struct ucycom_softc *sc)
 {

--Boundary-00=_rhieGDlp4vAv9/6--