Date: Tue, 28 Dec 2004 21:28:33 +0100 From: Bernd Walter <ticso@cicely12.cicely.de> To: John Birrell <jb@cimlogic.com.au> Cc: Julian Elischer <julian@elischer.org> Subject: Re: USB problems Message-ID: <20041228202832.GN81585@cicely12.cicely.de> In-Reply-To: <20041228195436.GA61668@freebsd3.cimlogic.com.au> References: <20041228010938.GA39686@freebsd3.cimlogic.com.au> <41D0C63F.3000702@elischer.org> <20041228025836.GA53223@freebsd3.cimlogic.com.au> <20041228152212.GM81585@cicely12.cicely.de> <20041228195436.GA61668@freebsd3.cimlogic.com.au>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Dec 29, 2004 at 06:54:36AM +1100, John Birrell wrote:
> On Tue, Dec 28, 2004 at 04:22:13PM +0100, Bernd Walter wrote:
> > Why is the quirk required?
> > OK - you get a stall on control endpoint, but this shouldn't harm
> > anything, as stalls on control enpoints are automatically cleared on
> > the next setup phase and the Philips controllers do that on their own.
> > If you get in trouble then the firmware on the device does more wrong
> > than just stalling the endpoint.
>
> The quirk gets around the problem that FreeBSD turns the stall status
> of the control endpoint into an error which aborts any further processing.
Well, getting a stall on control endpoint is an error, but getting an
error on string descriptors shouldn't stop further processing.
> The string descriptors are returned normally by the device, except for the
> fact that it sets the stall bit on the control endpoint.
Here is the device broken, unless FreeBSD's request is not correct.
> (If the handling of the stall status is changed, the quirk wouldn't be
> necessary)
Acording to your description things are not handled very well in both
the device and FreeBSD.
> > IIRC some USB2.0 firmware stall the control enpoint after transfering
> > legal data if they have been asked in an USB1.1 format.
> > I think we had this a while back for high speed hubs.
>
> The firmware in this device doesn't test the format. It unconditionally
> stalls the control endpoint in many cases (like getting a device or
> configuration descriptor and requesting less bytes than the full
> descriptor; getting a string descriptor; getting configuration; getting
> status).
The device shouldn't stall on requesting partial descriptors.
IIRC FreeBSD changed the way how descriptors are fetched normaly in
that it doesn't trust the size fields anymore and asks for more data
than required.
> > Maybe there are broken comments left, but a stalled control endpoint
> > doesn't require any special handling other than taken this a an error.
>
> In usbd_do_request_flags_pipe() there is code that detects a stalled
> control endpoint and then tries to clear the stall. In the case of the
> devices I have, this code just fails because it gets a stall error too.
usbd_do_request_flags_pipe() shouldn't clear a stall condition on
control endpoints anywhere than possibly in the host controller.
It's perfectly legal for a device to refuse an unstall request for the
control endpoint.
But speaking of usbd_do_request_flags_pipe(), I don't see anything
wrong here - it tries to unstall in case of a received stall for
unstalling broken devices.
The unstall itself gets a stall too for most devices, but that is
ignored and shouldn't harm.
> The FreeBSD code *never* takes into account that the control endpoint
> stall will be cleared with the next setup packet.
Maybe the host controller needs any kind of unstall mimic, but not the
device.
The question is more what happens with further requests after that
error.
> I think that a control endpoint transfer should not return USBD_STALLED
> to the caller unless the length received is less than that requested
> and ~SHORT_TRANSER_OK. A stall on the control endpoint should only be an
> error if the controller has a problem with the protocol (i.e. it is
> a protocol stall, not a device stall);
I had something like this localy running (cut & paste'd):
Index: usbdi.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.76
diff -u -r1.76 usbdi.c
--- usbdi.c 26 May 2002 22:00:06 -0000 1.76
+++ usbdi.c 15 Jun 2003 21:32:34 -0000
@@ -931,8 +931,17 @@
usbd_do_request_flags(usbd_device_handle dev, usb_device_request_t *req,
void *data, u_int16_t flags, int *actlen, u_int32_t timo)
{
- return (usbd_do_request_flags_pipe(dev, dev->default_pipe, req,
- data, flags, actlen, timo));
+ int actlen2;
+ usbd_status ret;
+
+ ret = usbd_do_request_flags_pipe(dev, dev->default_pipe, req,
+ data, flags, &actlen2, timo);
+ if (actlen != NULL)
+ *actlen = actlen2;
+ if (ret == USBD_STALLED && actlen2 == UGETW(req->wLength)) {
+ ret = USBD_NORMAL_COMPLETION;
+ }
+ return (ret);
}
usbd_status
But it's wrong.
You just don't know if the received data is correct when receiving
a stall - especially for requests without data.
This is something to hand up to the driver which might have a clue
about a specific device brokenness.
--
B.Walter BWCT http://www.bwct.de
bernd@bwct.de info@bwct.de
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20041228202832.GN81585>