NetBSD Problem Report #36276

From martin@duskware.de  Fri May  4 21:22:12 2007
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 1988E63BA19
	for <gnats-bugs@gnats.netbsd.org>; Fri,  4 May 2007 21:22:12 +0000 (UTC)
Message-Id: <20070504211547.7A65163B853@narn.NetBSD.org>
Date: Fri,  4 May 2007 21:15:47 +0000 (UTC)
From: dhowland@users.sourceforge.net
Reply-To: dhowland@users.sourceforge.net
To: netbsd-bugs-owner@NetBSD.org
Subject: ucycom causes kernel panic when accessing Delorme USB Earthmate LT-20 GPS
X-Send-Pr-Version: www-1.0

>Number:         36276
>Category:       kern
>Synopsis:       ucycom causes kernel panic when accessing Delorme USB Earthmate LT-20 GPS
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    skrll
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri May 04 21:25:00 +0000 2007
>Closed-Date:    Sat Feb 20 09:11:45 +0000 2021
>Last-Modified:  Sat Feb 20 09:11:45 +0000 2021
>Originator:     David Howland
>Release:        NetBSD 4.99.18
>Organization:
>Environment:
NetBSD micron 4.99.18 NetBSD 4.99.18 (GENERIC) #10: Fri May  4 12:20:18 EDT 2007  root@micron:/usr/obj/sys/arch/i386/compile/GENERIC i386
>Description:
The ucycom driver causes a kernel panic when trying to access my USB GPS device.

Running -current on a i386 and a GENERIC kernel.

Support for the Delorme USB Earthmate LT-20 GPS device is not yet working.  Nick Hudson has committed the USB vendor and product codes, but the ucycom driver does not yet take ownership of the device like it is supposed to.

I have modified my kernel to make the ucycom driver recognize the device.

The dmesg report looks like this:

uhidev0 at uhub0 port 1 configuration 1 interface 0
uhidev0: DeLorme Publishing DeLorme USB Earthmate, rev 1.00/0.01, addr 
2, iclass 3/0
ucycom0 at uhidev0

I did a MAKEDEV ttyY0 to create the /dev entry.

However...

Any access to /dev/ttyY0 results in an immediate kernel panic.

Here is the backtrace from GDB:


#21 0xc010b677 in calltrap ()
#22 0xc043c66d in sleepq_enqueue (sq=0xc0a299b0, pri=22, wchan=0xc15fb700,
     wmesg=0xc091c0fd "uhidevwi", sobj=0xc098d5d0) at 
/usr/src/sys/kern/kern_sleepq.c:216
#23 0xc043ca4a in sleepq_block (sq=0xc0a299b0, pri=22, wchan=0xc15fb700,
     wmesg=0xc091c0fd "uhidevwi", timo=0, catch=true, sobj=0xc098d5d0)
     at /usr/src/sys/kern/kern_sleepq.c:295
#24 0xc043f003 in ltsleep (ident=0xc15fb700, priority=278, 
wmesg=0xc091c0fd "uhidevwi", timo=0,
     interlock=0x0) at /usr/src/sys/kern/kern_synch.c:492
#25 0xc06a60f4 in usbd_intr_transfer (xfer=0xc15fb700, pipe=0xc164d300, 
flags=0, timeout=0,
     buf=0xc15bf620, size=0xcaf31e1c, lbl=0xc091c0fd "uhidevwi")
     at /usr/src/sys/dev/usb/usbdi_util.c:495
#26 0xc06b4936 in uhidev_write (sc=0x0, data=0xc15bf620, len=32)
     at /usr/src/sys/dev/usb/uhidev.c:644
#27 0xc06b91e5 in ucycomstart (tp=0xcaf02b84) at 
/usr/src/sys/dev/usb/ucycom.c:569
#28 0xc045fde8 in ttstart (tp=0xc098d5d0) at /usr/src/sys/kern/tty.c:1566
#29 0xc04620ac in ttyinput_wlock (c=-1063085648, tp=0xcaf02b84) at 
/usr/src/sys/kern/tty.c:692
#30 0xc046440d in ttyinput (c=50, tp=0xc098d5d0) at 
/usr/src/sys/kern/tty.c:716
#31 0xc06b8bbe in ucycom_intr (addr=0xc15c0d00, ibuf=0xc15bf600, len=32)
     at /usr/src/sys/dev/usb/ucycom.c:913
#32 0xc06b4db4 in uhidev_intr (xfer=0xc1524800, addr=0xc15c1e80, 
status=USBD_NORMAL_COMPLETION)
     at /usr/src/sys/dev/usb/uhidev.c:455
#33 0xc06a53ea in usb_transfer_complete (xfer=0xc1524800) at 
/usr/src/sys/dev/usb/usbdi.c:824
#34 0xc030da04 in uhci_idone (ii=<value optimized out>) at 
/usr/src/sys/dev/usb/uhci.c:1529
#35 0xc030e4d1 in uhci_softintr (v=0xc1527000) at 
/usr/src/sys/dev/usb/uhci.c:1404
#36 0xc04d2c5f in softintr_dispatch (which=1) at 
/usr/src/sys/arch/x86/x86/softintr.c:104
#37 0xc010b33a in Xsoftnet ()


The only thing I notice is the worrying "sc=0x0" argument to uhidev_write.

If I compile the ucycom driver with #define UCYCOM_DEBUG  and  ucycomdebug=11,

cat /dev/ttyY0 results in the following output before the crash.


ucycomopen: unit=0
ucycomopen: sc=0xc15e7300
ucycomopen: tp=0xcaf02754
ucycomparam: baud=0
ucycomparam: l_modem
ucycomopen: sc->sc_obuf=0xc15bf740
ucycomread: sc=0xc15e7300, tp=0xcaf02754, uio=0xcc0fcbb0, flag=0
ucycom_intr: ibuf[0..30) = 24 50 53 54 4d 56 45 52 2c 47 50 53 20 56 65 72 73 69 6f 6e 20 34 2e 32 30 2e 35 20 52 65
ucycom_intr: char=0x24
ucycomstart(32): sc->sc_obuf[0] = 0
sc->sc_obuf[1] = 1
ucycomstart(32): data = 24
ucycomstart: sc->sc_obuf[0..32) = 00 01 24 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

This is where the kernel panics with a page fault.
the first 'sync' in ddb gives...

syncing disks... ucycom_intr: ibuf[0..30) = 24 50 53 54 4d 56 45 52 2c 47 50 53 20 56 65 72 73 69 6f 6e 20 34 2e 32 30 2e 35 20 52 65
ucycom_intr: char=0x24
ucycomstart: no go, state=0x4c
ucycom_intr: char=0x50
ucycomstart: no go, state=0x4c
...
ucycom_intr: char=0x52
ucycomstart: no go, state=0x4c
ucycom_intr: char=0x65
ucycomstart: no go, state=0x4c

Second 'sync' finally makes it dump core and reboot.


I'm fairly sure that the LT-20 should work the same as the original USB earthmate that is already supported by the ucycom driver (they have the same cypress comm. controller), however, even if the driver is not ready, it should not result in a kernel panic.


>How-To-Repeat:
Start with a -current kernel from after May 3rd, 2007.
Patch src/sys/dev/usb/ucycom.c as follows:

--- ucycom.c.orig	2007-05-02 23:25:30.000000000 -0400
+++ ucycom.c
@@ -175,6 +175,7 @@ Static void ucycom_get_cfg(struct ucycom
  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)


Then compile a kernel and plug in a Delorme USB Earthmate LT-20 GPS.

then, as root, try to access /dev/ttyY0

>Fix:
no fix yet

>Release-Note:

>Audit-Trail:

State-Changed-From-To: open->analyzed
State-Changed-By: skrll@netbsd.org
State-Changed-When: Tue, 08 May 2007 06:48:38 +0000
State-Changed-Why:
The problem here is that uhidev_write can sleep and therefore should not
be called from interrupt context. ucycomstart should make used of 
usbd_setup_xfer and a callback.


From: David Howland <dhowland@users.sourceforge.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/36276 (ucycom causes kernel panic when accessing Delorme
 USB Earthmate LT-20 GPS)
Date: Thu, 17 May 2007 00:23:31 -0400

 I have modified the ucycom driver to use a usbd_setup_xfer and a 
 callback.  Specifically, I have reimplemented ucycomstart().  It should 
 be correct since its mostly copied from ucom.  I have tested it with my 
 Delorme USB Earthmate LT-20 GPS.  (this patch also adds support for that 
 device)

 The patch is included.  Sorry for all the extra junk in the diff, my 
 text editor automatically removes trailing spaces from lines.  Also, I 
 have left (2) comments in the file that log the steps I took to update 
 the file, I assume that those will be removed if my changes are committed.

 ********************************************************

 --- ucycom.c.orig	2007-05-08 14:41:32.000000000 -0400
 +++ ucycom.c	2007-05-17 00:08:30.000000000 -0400
 @@ -5,7 +5,7 @@
    * All rights reserved.
    *
    * This code is derived from software contributed to The NetBSD Foundation
 - * by Nick Hudson
 + * by Nick Hudson
    *
    * Redistribution and use in source and binary forms, with or without
    * modification, are permitted provided that the following conditions
 @@ -157,6 +157,7 @@

   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);
 @@ -175,6 +176,7 @@
   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)

 @@ -319,7 +321,7 @@
   	sc = ucycom_cd.cd_devs[unit];

   	DPRINTF(("ucycomopen: sc=%p\n", sc));
 -
 +
   	if (sc == NULL)
   		return (ENXIO);

 @@ -363,7 +365,7 @@
   			SET(t.c_cflag, CRTSCTS);
   		if (ISSET(sc->sc_swflags, TIOCFLAG_MDMBUF))
   			SET(t.c_cflag, MDMBUF);
 -		
 +
   		tp->t_ospeed = 0;
   		(void) ucycomparam(tp, &t);
   		tp->t_iflag = TTYDEF_IFLAG;
 @@ -374,7 +376,7 @@

   		/* Allocate an output report buffer */
   		sc->sc_obuf = malloc(sc->sc_olen, M_USBDEV, M_WAITOK);
 -	
 +
   		DPRINTF(("ucycomopen: sc->sc_obuf=%p\n", sc->sc_obuf));

   #if 0
 @@ -448,12 +450,22 @@
   	return (0);
   }

 +/*
 + *  Modified to use usbd_transfer like ucom rather than uhidev_write
 + *    - Remove uhidev_write
 + *    - Add usbd_setup_xfer
 + *    - 'cnt' becomes 'len'
 + *    - Change 'err' from int to usbd_status
 + *    - Replace all parameters to usbd_setup_xfer with equivalents from 
 uhidev_write
 + *    - Moved the wrap-up stuff to the callback function
 + */
   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;
 @@ -490,7 +502,7 @@
   	}

   	SET(tp->t_state, TS_BUSY);
 -	
 +
   	/*
   	 * The 8 byte output report uses byte 0 for control and byte
   	 * count.
 @@ -498,7 +510,7 @@
   	 * The 32 byte output report uses byte 0 for control. Byte 1
   	 * is used for byte count.
   	 */
 -	memset(sc->sc_obuf, 0, sc->sc_olen);	
 +	memset(sc->sc_obuf, 0, sc->sc_olen);
   	len = cnt;
   	switch (sc->sc_olen) {
   	case 8:
 @@ -506,7 +518,7 @@
   			DPRINTF(("ucycomstart(8): big buffer %d chars\n", len));
   			len = sc->sc_olen - 1;
   		}
 -	
 +
   		memcpy(sc->sc_obuf + 1, data, len);
   		sc->sc_obuf[0] = len | sc->sc_mcr;

 @@ -523,13 +535,13 @@
   		}
   #endif
   		break;
 -	
 +
   	case 32:
   		if (cnt > sc->sc_olen - 2) {
   			DPRINTF(("ucycomstart(32): big buffer %d chars\n", len));
   			len = sc->sc_olen - 2;
   		}
 -		
 +
   		memcpy(sc->sc_obuf + 2, data, len);
   		sc->sc_obuf[0] = sc->sc_mcr;
   		sc->sc_obuf[1] = len;
 @@ -546,7 +558,7 @@
   		}
   #endif
   		break;
 -	
 +
   	default:
           	DPRINTFN(2,("ucycomstart: unknown output report size (%zd)\n",
   		    sc->sc_olen));
 @@ -566,16 +578,79 @@
   		}
   	}
   #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,
 +			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
 +	if (err != USBD_IN_PROGRESS)
 +		DPRINTF(("ucycomstart: err=%s\n", usbd_errstr(err)));
 +#endif
 +
 +out:
 +	splx(s);
 +}
 +
 +/*
 + *  Copied from ucom
 + *    - removed call to rnd_add_uint32 (no need to complicate things)
 + *    - change 'cc' to 'len', get 'len' by the same method as ucycomstart
 + *    - change usbd_clear_endpoint to something similar to 
 usbd_intr_transfer()
 + *    - change ucom_softc to ucycom_softc
 + */
 +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;
 +	int cnt, len;
 +	int s;
 +
 +	DPRINTFN(5,("ucycomwritecb: status=%d\n", status));
 +
 +	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;
 +	}
 +
 +	/* determine len the same way it was determined in ucycomstart */
 +	/* rather than doing this, should we have just stored len in softc? */
 +	cnt = ndqb(&tp->t_outq, 0);
 +	len = cnt;
 +	switch (sc->sc_olen) {
 +		case 8:
 +			if (cnt > sc->sc_olen - 1)
 +				len = sc->sc_olen - 1;
 +			break;
 +		case 32:
 +			if (cnt > sc->sc_olen - 2)
 +				len = sc->sc_olen - 2;
 +			break;
 +		default:
 +			goto error;
   	}

   #ifdef UCYCOM_DEBUG
 -	ucycom_get_cfg(sc);
 +	if (ucycomdebug) {
 +		u_int32_t cc;
 +
 +		usbd_get_xfer_status(xfer, NULL, NULL, &cc, NULL);
 +		DPRINTF(("ucycomwritecb: len = %d, cnt = %d, cc = %d\n", len, cnt, cc));
 +
 +		ucycom_get_cfg(sc);
 +	}
   #endif
 -	DPRINTFN(4,("ucycomstart: req %d chars did %d chars\n", cnt, len));
 +
 +	DPRINTFN(4,("ucycomwritecb: req %d chars did %d chars\n", cnt, len));

    	s = spltty();
   	CLR(tp->t_state, TS_BUSY);
 @@ -584,8 +659,12 @@
   	else
   		ndflush(&tp->t_outq, len);
   	(*tp->t_linesw->l_start)(tp);
 +	splx(s);
 +	return;

 -out:
 +error:
 +	s = spltty();
 +	CLR(tp->t_state, TS_BUSY);
   	splx(s);
   }

 @@ -803,7 +882,7 @@
   	struct ucycom_softc *sc = ucycom_cd.cd_devs[UCYCOMUNIT(dev)];
   	struct tty *tp = sc->sc_tty;
   	int err;
 -	
 +
   	DPRINTF(("ucycompoll: sc=%p, tp=%p, events=%d, lwp=%p\n", sc, tp, 
 events, l));

   	if (sc->sc_dying)
 @@ -873,7 +952,7 @@
   	int (*rint)(int , struct tty *) = tp->t_linesw->l_rint;
   	uint8_t *cp = ibuf;
   	int s, n, st, chg;
 -	
 +
   	/* We understand 8 byte and 32 byte input records */
   	switch (len) {
   	case 8:
 @@ -986,13 +1065,13 @@
   ucycom_dtr(struct ucycom_softc *sc, int set)
   {
   	uint8_t old;
 -	
 +
   	old = sc->sc_mcr;
   	if (set)
   		SET(sc->sc_mcr, UCYCOM_DTR);
   	else
   		CLR(sc->sc_mcr, UCYCOM_DTR);
 -	
 +
   	if (old ^ sc->sc_mcr)
   		ucycom_set_status(sc);
   }
 @@ -1002,13 +1081,13 @@
   ucycom_rts(struct ucycom_softc *sc, int set)
   {
   	uint8_t old;
 -	
 +
   	old = sc->sc_msr;
   	if (set)
   		SET(sc->sc_mcr, UCYCOM_RTS);
   	else
   		CLR(sc->sc_mcr, UCYCOM_RTS);
 -	
 +
   	if (old ^ sc->sc_mcr)
   		ucycom_set_status(sc);
   }
 @@ -1024,7 +1103,7 @@
   		    sc->sc_olen));
   		return;
   	}
 -	
 +
   	DPRINTF(("ucycom_set_status: %d\n", sc->sc_mcr));

   	memset(sc->sc_obuf, 0, sc->sc_olen);
 @@ -1042,7 +1121,7 @@
   {
   	int err, cfg, baud;
   	uint8_t report[5];
 -	
 +
   	err = uhidev_get_report(&sc->sc_hdev, UHID_FEATURE_REPORT,
   	    report, sc->sc_flen);
   	cfg = report[4];

 ********************************************************

Responsible-Changed-From-To: kern-bug-people->skrll
Responsible-Changed-By: skrll@netbsd.org
Responsible-Changed-When: Tue, 22 May 2007 20:28:42 +0000
Responsible-Changed-Why:
I'm working on this.


From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@netbsd.org
Cc: kern-bug-people@netbsd.org, netbsd-bugs@netbsd.org,
	gnats-admin@netbsd.org, dhowland@users.sourceforge.net
Subject: Re: kern/36276 (ucycom causes kernel panic when accessing Delorme USB Earthmate LT-20 GPS)
Date: Thu, 21 Jun 2007 08:27:38 +0100

 --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--

From: David Howland <dhowland@users.sourceforge.net>
To: Nick Hudson <skrll@netbsd.org>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/36276 (ucycom causes kernel panic when accessing Delorme
 USB Earthmate LT-20 GPS)
Date: Thu, 21 Jun 2007 17:05:40 -0400

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

 I have tried the patch.  I see the changes you made.  I knew that the 
 transfer length thing was probably not desirable, but I was unsure as to 
 when it is appropriate to change the softc bits.

 The driver does not work for me.  However, my original patch didn't work 
 either.  I am currently unable to explain why.

 With your patch (and mine), it seems to be able to make transfers 
 properly, however it does not seem to make the LT-20 device happy.  The 
 GPS will simply shut down after a number of transfers.

 At this point, I have no idea what to do to fix the problem, since the 
 driver never crashes or shows any errors.  Debug output seems to show 
 perfect communication.  Without documentation for the GPS device I am 
 using, I have no idea what its problem is.  My last hope is to contact 
 the author of the OpenBSD driver and ask if he is familiar with what I 
 am seeing.

 -d

From: Nick Hudson <skrll@netbsd.org>
To: David Howland <dhowland@users.sourceforge.net>
Cc: gnats-bugs@netbsd.org
Subject: Re: kern/36276 (ucycom causes kernel panic when accessing Delorme USB Earthmate LT-20 GPS)
Date: Fri, 22 Jun 2007 08:47:38 +0100

 On Thursday 21 June 2007 22:05, David Howland wrote:
 > Nick Hudson wrote:
 > > Can you try this patch. It deals with the transfer length in the callback
 > > differently to yours (and it works for me ;)
 >
 > I have tried the patch.  I see the changes you made.  I knew that the
 > transfer length thing was probably not desirable, but I was unsure as to
 > when it is appropriate to change the softc bits.
 >
 > The driver does not work for me.  However, my original patch didn't work
 > either.  I am currently unable to explain why.
 >
 > With your patch (and mine), it seems to be able to make transfers
 > properly, however it does not seem to make the LT-20 device happy.  The
 > GPS will simply shut down after a number of transfers.

 I wonder if it's flow control related

 > At this point, I have no idea what to do to fix the problem, since the
 > driver never crashes or shows any errors.  Debug output seems to show
 > perfect communication.  Without documentation for the GPS device I am
 > using, I have no idea what its problem is.  My last hope is to contact
 > the author of the OpenBSD driver and ask if he is familiar with what I
 > am seeing.

 Please let me know if you get anything back.

 Thanks,
 Nick

From: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/36276 CVS commit: src/sys/dev/usb
Date: Thu, 6 Aug 2009 07:07:31 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Thu Aug  6 07:07:31 UTC 2009

 Modified Files:
 	src/sys/dev/usb: ucycom.c

 Log Message:
 Make ucycomstart use usbd_setup_xfer and a callback so that it doesn't
 (attempt to) sleep in interrupt context.

 From David Howland in PR 36276 with changes from me.

 XXX Still losing data on input.


 To generate a diff of this commit:
 cvs rdiff -u -r1.28 -r1.29 src/sys/dev/usb/ucycom.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: analyzed->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sat, 20 Feb 2021 09:11:45 +0000
State-Changed-Why:
Original bug is fixed. Please raise new PR if it's still a problem.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: gnats-precook-prs,v 1.4 2018/12/21 14:20:20 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.