Re: [patch] fix uplcom(4) clear stall logic for PL2303HX
On Thursday 22 November 2012 06:09:19 Mark Johnston wrote: On Wed, Nov 21, 2012 at 11:07:20PM +0100, Hans Petter Selasky wrote: Here you go: http://svnweb.freebsd.org/changeset/base/243380 Please test and verify. --HPS Yep, it's working. Thanks a lot! When I was debugging the problem I noticed the UQ_OPEN_CLEARSTALL quirk, and then later discovered that it no longer has any effect. Is there any reason for keeping it around? Or can it be removed (patch below)? Thanks, -Mark Done: http://svnweb.freebsd.org/changeset/base/243435 --HPS ___ 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
Re: [patch] fix uplcom(4) clear stall logic for PL2303HX
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
Re: [patch] fix uplcom(4) clear stall logic for PL2303HX
On Wednesday 21 November 2012 09:43:02 Mark Johnston wrote: Mark Johnston Here you go: http://svnweb.freebsd.org/changeset/base/243380 Please test and verify. --HPS ___ 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
Re: [patch] fix uplcom(4) clear stall logic for PL2303HX
On Wed, Nov 21, 2012 at 11:07:20PM +0100, Hans Petter Selasky wrote: Here you go: http://svnweb.freebsd.org/changeset/base/243380 Please test and verify. --HPS Yep, it's working. Thanks a lot! When I was debugging the problem I noticed the UQ_OPEN_CLEARSTALL quirk, and then later discovered that it no longer has any effect. Is there any reason for keeping it around? Or can it be removed (patch below)? Thanks, -Mark diff --git a/share/man/man4/usb_quirk.4 b/share/man/man4/usb_quirk.4 index 65deafa..f017e01 100644 --- a/share/man/man4/usb_quirk.4 +++ b/share/man/man4/usb_quirk.4 @@ -76,8 +76,6 @@ mouse sends an unknown leading byte mouse has Z-axis reversed .It UQ_NO_STRINGS string descriptors are broken -.It UQ_OPEN_CLEARSTALL -device needs clear endpoint stall .It UQ_POWER_CLAIM hub lies about power status .It UQ_SPUR_BUT_UP diff --git a/sys/dev/usb/quirk/usb_quirk.c b/sys/dev/usb/quirk/usb_quirk.c index fe15a0a..8f35584 100644 --- a/sys/dev/usb/quirk/usb_quirk.c +++ b/sys/dev/usb/quirk/usb_quirk.c @@ -508,7 +508,6 @@ static const char *usb_quirk_str[USB_QUIRK_MAX] = { [UQ_MS_LEADING_BYTE]= UQ_MS_LEADING_BYTE, [UQ_MS_REVZ]= UQ_MS_REVZ, [UQ_NO_STRINGS] = UQ_NO_STRINGS, - [UQ_OPEN_CLEARSTALL]= UQ_OPEN_CLEARSTALL, [UQ_POWER_CLAIM]= UQ_POWER_CLAIM, [UQ_SPUR_BUT_UP]= UQ_SPUR_BUT_UP, [UQ_SWAP_UNICODE] = UQ_SWAP_UNICODE, diff --git a/sys/dev/usb/quirk/usb_quirk.h b/sys/dev/usb/quirk/usb_quirk.h index 32a60a1..15d5f15 100644 --- a/sys/dev/usb/quirk/usb_quirk.h +++ b/sys/dev/usb/quirk/usb_quirk.h @@ -54,7 +54,6 @@ enum { UQ_MS_LEADING_BYTE, /* mouse sends an unknown leading byte */ UQ_MS_REVZ, /* mouse has Z-axis reversed */ UQ_NO_STRINGS, /* string descriptors are broken */ - UQ_OPEN_CLEARSTALL, /* device needs clear endpoint stall */ UQ_POWER_CLAIM, /* hub lies about power status */ UQ_SPUR_BUT_UP, /* spurious mouse button up events */ UQ_SWAP_UNICODE,/* has some Unicode strings swapped */ ___ 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
[patch] fix uplcom(4) clear stall logic for PL2303HX
Hello all, I recently bought a PL-2303 USB to serial converter (VID 0x067b, DID 0x2303) to use with an ARM board. When I start cu(1) and power on the board no messages show up, and if I enter any input, cu exits without printing anything and uplcom detaches and reinitializes itself. No error messages are printed by the kernel - all I see is that uplcom(4) disconnects and reattaches. After some debugging I found that when the USB stack sends a ENDPOINT_HALT clear to the OUT bulk endpoint, the hw seems to respond with an endpoint stalled error, and after that, the uplcom callbacks are called with error set to USB_ERROR_CANCELLED. I looked at the Linux driver for this device and found the following code in pl2303.c: if (priv-type != HX) { usb_clear_halt(serial-dev, port-write_urb-pipe); usb_clear_halt(serial-dev, port-read_urb-pipe); } else { /* reset upstream data pipes */ pl2303_vendor_write(8, 0, serial); pl2303_vendor_write(9, 0, serial); } Unfortunately, I couldn't find any datasheets which indicate what these vendor-specific commands mean. However I ended up with the patch below, and now the device works perfectly. :) In particular, it seems that we don't want to send a clear ENDPOINT_HALT request to the HX variant of the device, which is what I have. I noticed that the device still seems to work if I omit the vendor commands, and being a total USB newbie I thought I'd ask the following questions: 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? 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? I'm happy to test alternative fixes, provide debug output, etc., etc. Thanks, -Mark diff --git a/sys/dev/usb/serial/uplcom.c b/sys/dev/usb/serial/uplcom.c index 4549bad..4d9ac32 100644 --- a/sys/dev/usb/serial/uplcom.c +++ b/sys/dev/usb/serial/uplcom.c @@ -433,10 +433,18 @@ 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) { + 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 +563,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
Re: [patch] fix uplcom(4) clear stall logic for PL2303HX
On Tuesday 20 November 2012 03:57:22 Mark Johnston wrote: Hello all, I recently bought a PL-2303 USB to serial converter (VID 0x067b, DID 0x2303) to use with an ARM board. When I start cu(1) and power on the board no messages show up, and if I enter any input, cu exits without printing anything and uplcom detaches and reinitializes itself. No error messages are printed by the kernel - all I see is that uplcom(4) disconnects and reattaches. After some debugging I found that when the USB stack sends a ENDPOINT_HALT clear to the OUT bulk endpoint, the hw seems to respond with an endpoint stalled error, and after that, the uplcom callbacks are called with error set to USB_ERROR_CANCELLED. I looked at the Linux driver for this device and found the following code in pl2303.c: if (priv-type != HX) { usb_clear_halt(serial-dev, port-write_urb-pipe); usb_clear_halt(serial-dev, port-read_urb-pipe); } else { /* reset upstream data pipes */ pl2303_vendor_write(8, 0, serial); pl2303_vendor_write(9, 0, serial); } Unfortunately, I couldn't find any datasheets which indicate what these vendor-specific commands mean. However I ended up with the patch below, and now the device works perfectly. :) In particular, it seems that we don't want to send a clear ENDPOINT_HALT request to the HX variant of the device, which is what I have. I noticed that the device still seems to work if I omit the vendor commands, and being a total USB newbie I thought I'd ask the following questions: 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? Hi, 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. 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. --HPS ___ 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