From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Date: Mon, 30 Mar 2015 15:35:03 +0200 Subject: [U-Boot] [PATCH] usb: Early failure when the first descriptor read fails, one way or another In-Reply-To: <201503300147.57183.marex@denx.de> References: <1427563438-25427-1-git-send-email-contact@paulk.fr> <201503281826.20603.marex@denx.de> <1427563902.8267.12.camel@collins> <201503300147.57183.marex@denx.de> Message-ID: <1427722503.2521.9.camel@collins> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Le lundi 30 mars 2015 ? 01:47 +0200, Marek Vasut a ?crit : > On Saturday, March 28, 2015 at 06:31:42 PM, Paul Kocialkowski wrote: > > Le samedi 28 mars 2015 ? 18:26 +0100, Marek Vasut a ?crit : > > > On Saturday, March 28, 2015 at 06:23:58 PM, Paul Kocialkowski wrote: > > > > When using an USB1 device on a controller that only supports USB2 (e.g. > > > > EHCI), reading the first descriptor will fail (read 0 byte), so we can > > > > abort the process at this point instead of failing later and wasting > > > > time. > > > > > > > > Signed-off-by: Paul Kocialkowski > > > > --- > > > > > > > > common/usb.c | 5 ++++- > > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/common/usb.c b/common/usb.c > > > > index 32e15cd..d1b3316 100644 > > > > --- a/common/usb.c > > > > +++ b/common/usb.c > > > > @@ -950,7 +950,7 @@ int usb_new_device(struct usb_device *dev) > > > > > > > > */ > > > > > > > > #ifndef CONFIG_USB_XHCI > > > > > > > > err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64); > > > > > > > > - if (err < 0) { > > > > + if (err < sizeof(struct usb_device_descriptor)) { > > > > > > > > debug("usb_new_device: usb_get_descriptor() failed\n"); > > > > return 1; > > > > > > > > } > > > > > > > > @@ -990,6 +990,9 @@ int usb_new_device(struct usb_device *dev) > > > > > > > > case 64: > > > > dev->maxpacketsize = PACKET_SIZE_64; > > > > break; > > > > > > > > + default: > > > > + debug("usb_new_device: invalid max packet size\n"); > > > > > > Hi, > > > > > > since this is an error, this should probably be a printf(). Also, > > > to make the error message useful, it should state the invalid value > > > due to which it failed. > > > > Well, it is not unexpected behaviour in my use case (but I reckon it may > > be generally speaking). > > Why is this not unexpected in your case please ? Because using an USB low speed device with an EHCI controller should normally fail (that is, the first descriptor read returns 0 bytes), or at least it does on the sunxi platforms. I thought there was no need to report that as an error, but I changed my mind. Since the end result is that the USB device won't work, I think it makes sense to print an error. > > If you're not convinced by this, I can still make a v2 with printf, I > > just don't think it's a necessity. > > Let me just understand what you're seeing a bit better first please . At this point, v2 of this patch was sent as: usb: Early failure when the first descriptor read fails or is invalid Let me know if you need some more context on this. -- Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: http://www.replicant.us/ Blog: http://blog.replicant.us/ Wiki/tracker/forums: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: