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);
 }
 
_______________________________________________
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"

Reply via email to