From owner-freebsd-usb@FreeBSD.ORG Wed Nov 21 08:43:09 2012 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id F2ECC55D for ; Wed, 21 Nov 2012 08:43:08 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-ia0-f182.google.com (mail-ia0-f182.google.com [209.85.210.182]) by mx1.freebsd.org (Postfix) with ESMTP id B475B8FC15 for ; Wed, 21 Nov 2012 08:43:08 +0000 (UTC) Received: by mail-ia0-f182.google.com with SMTP id x2so6182591iad.13 for ; Wed, 21 Nov 2012 00:43:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=FETzZJ+JxFUqJZRyjPAzUtK17fcHJxZq6S1fZEdXjpY=; b=iYBtfYGG7Q6Sneetgi6cyMw3SpiO81K1AGGiQ9o1/GprFjZ09XLoT7EoK03aNUvmtl 9+2KDUhvv6Zo+6uJfHuvrMik4CrqjV9vPI0Ux743YcezC6iIFqSM5aVoiP9gcPRuUusK EDXS12+Z6Vjku6Uwzf0dLyhoinUSue9yw1+vN0+5KdPKfk4ppN7WtoFWFf5J9LDDCnav tf/HX12jr7uc1gcTNe1kGx5NpCla00XaQbn+ZikKphr6O8fj9dkRyxtviEG3wupdgXF5 ayIAt7dUwwd4Ttsx93NRODUWYtMqMdbs5AUNaKsndys+wKpDPZclLAdrYE+ImO3rpKil /qUg== Received: by 10.43.50.197 with SMTP id vf5mr16515973icb.13.1353487387703; Wed, 21 Nov 2012 00:43:07 -0800 (PST) Received: from oddish ([66.11.160.25]) by mx.google.com with ESMTPS id wm10sm11520788igc.2.2012.11.21.00.43.06 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 21 Nov 2012 00:43:06 -0800 (PST) Date: Wed, 21 Nov 2012 03:43:02 -0500 From: Mark Johnston To: Hans Petter Selasky Subject: Re: [patch] fix uplcom(4) clear stall logic for PL2303HX Message-ID: <20121121084302.GA4136@oddish> References: <20121120025722.GA3338@oddish> <201211200815.19192.hselasky@c2i.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201211200815.19192.hselasky@c2i.net> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-usb@freebsd.org X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 21 Nov 2012 08:43:09 -0000 On Tue, Nov 20, 2012 at 08:15:19AM +0100, Hans Petter Selasky wrote: > On Tuesday 20 November 2012 03:57:22 Mark Johnston wrote: > > > > 1. What exactly is the purpose of clearing ENDPOINT_HALT when a > > userspace program attaches to a device? Is it just to make sure that the > > device fw is in some known good state before starting to transmit data? > > The purpose is to ensure that the so-called data toggle is reset to zero. If a > packet is sent using the wrong data-toggle, it will simply get dropped. This > is not so important for Serial devices, but for other device classes it is. > > If a USB device does not support clear-stall, it is not complying with the > basics of the USB standards defined at usb.org, and I think it is not USB > certified either. That seems to be the case unfortunately, i.e. the device is responding with USB_ERROR_STALLED when it receives a clear stall request over the intr or bulk transfers. > > > > > 2. uplcom(4) tries to clear any stalls after an error in its r/w and > > intr callbacks. Is there some way I can trigger an error so that I can > > test and fix that code too? > > > > Does the device choke on clear-stall? > > You can test that simply by adding a sysctl to the code, which you set to make > the code go to the error case upon transfer completion. > > I suggest you look at storage/umass.c and the reset states for an example on > how to make a non-default asynchronous control transfer. > > When you have a patch I can commit it. > It doesn't seem like the vendor-specific commands help at all in clearing errors. I spent some time playing around with the patch at [1], which adds some reset callbacks that submit the vendor commands, but I couldn't get the device to recover - I just get more STALLED errors back. Looking at the Linux driver (pl2303) doesn't reveal anything helpful - I don't really see any kind of error-handling code. At any rate, my original patch below fixes my problems. I don't like the fact that the error handling is broken, but at least everything (seems) to clean itself up nicely in the error case - the driver just disconnects and reattaches. Thanks, -Mark [1] https://www.student.cs.uwaterloo.ca/~m6johnst/patch/uplcom_reset_logic.patch diff --git a/sys/dev/usb/serial/uplcom.c b/sys/dev/usb/serial/uplcom.c index 4549bad..4998c3f 100644 --- a/sys/dev/usb/serial/uplcom.c +++ b/sys/dev/usb/serial/uplcom.c @@ -433,10 +433,19 @@ uplcom_attach(device_t dev) goto detach; } /* clear stall at first run */ - mtx_lock(&sc->sc_mtx); - usbd_xfer_set_stall(sc->sc_xfer[UPLCOM_BULK_DT_WR]); - usbd_xfer_set_stall(sc->sc_xfer[UPLCOM_BULK_DT_RD]); - mtx_unlock(&sc->sc_mtx); + if (sc->sc_chiptype != TYPE_PL2303HX) { + /* HX variants seem to lock up after a clear stall request. */ + mtx_lock(&sc->sc_mtx); + usbd_xfer_set_stall(sc->sc_xfer[UPLCOM_BULK_DT_WR]); + usbd_xfer_set_stall(sc->sc_xfer[UPLCOM_BULK_DT_RD]); + mtx_unlock(&sc->sc_mtx); + } else { + if (uplcom_pl2303_do(sc->sc_udev, UT_WRITE_VENDOR_DEVICE, + UPLCOM_SET_REQUEST, 8, 0, 0) || + uplcom_pl2303_do(sc->sc_udev, UT_WRITE_VENDOR_DEVICE, + UPLCOM_SET_REQUEST, 9, 0, 0)) + goto detach; + } error = ucom_attach(&sc->sc_super_ucom, &sc->sc_ucom, 1, sc, &uplcom_callback, &sc->sc_mtx); @@ -555,9 +564,6 @@ uplcom_pl2303_init(struct usb_device *udev, uint8_t chiptype) if (err) return (EIO); - if (uplcom_pl2303_do(udev, UT_WRITE_VENDOR_DEVICE, UPLCOM_SET_REQUEST, 8, 0, 0) - || uplcom_pl2303_do(udev, UT_WRITE_VENDOR_DEVICE, UPLCOM_SET_REQUEST, 9, 0, 0)) - return (EIO); return (0); }