[patch] fix uplcom(4) clear stall logic for PL2303HX
Mark Johnston
markjdb at gmail.com
Wed Nov 21 08:43:09 UTC 2012
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);
}
More information about the freebsd-usb
mailing list