Re: [PATCH 1/1] usb: Assimilate usb_get_descriptor() to linux

[Philip Oberfichtner]

Good point. See V2.

On Fri, May 17, 2024 at 11:16:19PM +0200, Marek Vasut wrote:
> On 5/17/24 11:18 AM, Philip Oberfichtner wrote:
> > Before this commit, usb_get_descriptor() failed for some flakey USB
> > devices. We hereby adopt the more robust linux implementation [1].
> 
> Can you include which exact device fails and how? Details of the failure are
> valuable for the next person who runs into such a bug. Search engines index
> the ML posts too, so they become searchable using those.
> 
> Which kernel version is this imported from ? Commit ID would be good to add.
> 
> [...]

-- 
=
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-22  Fax: +49-8142-66989-80   Email: p...@denx.de
=

Re: [PATCH 1/1] usb: Assimilate usb_get_descriptor() to linux

[Marek Vasut]

On 5/17/24 11:18 AM, Philip Oberfichtner wrote:

Before this commit, usb_get_descriptor() failed for some flakey USB
devices. We hereby adopt the more robust linux implementation [1].


Can you include which exact device fails and how? Details of the failure 
are valuable for the next person who runs into such a bug. Search 
engines index the ML posts too, so they become searchable using those.


Which kernel version is this imported from ? Commit ID would be good to add.

[...]

[PATCH 1/1] usb: Assimilate usb_get_descriptor() to linux

[Philip Oberfichtner]

Before this commit, usb_get_descriptor() failed for some flakey USB
devices. We hereby adopt the more robust linux implementation [1].

Signed-off-by: Philip Oberfichtner 

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/usb/core/message.c?h=v6.9#n781
---
 common/usb.c | 37 ++---
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 84b10f5c7d..f5b21c883f 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -214,8 +214,9 @@ int usb_int_msg(struct usb_device *dev, unsigned long pipe,
  * clear keyboards LEDs). For data transfers, (storage transfers) we don't
  * allow control messages with 0 timeout, by previousely resetting the flag
  * asynch_allowed (usb_disable_asynch(1)).
- * returns the transferred length if OK or -1 if error. The transferred length
- * and the current status are stored in the dev->act_len and dev->status.
+ * returns the transferred length if OK, otherwise a negative error code. The
+ * transferred length and the current status are stored in the dev->act_len and
+ * dev->status.
  */
 int usb_control_msg(struct usb_device *dev, unsigned int pipe,
unsigned char request, unsigned char requesttype,
@@ -257,11 +258,14 @@ int usb_control_msg(struct usb_device *dev, unsigned int 
pipe,
break;
mdelay(1);
}
+
+   if (timeout == 0)
+   return -ETIMEDOUT;
+
if (dev->status)
return -1;
 
return dev->act_len;
-
 }
 
 /*---
@@ -562,10 +566,29 @@ int usb_clear_halt(struct usb_device *dev, int pipe)
 static int usb_get_descriptor(struct usb_device *dev, unsigned char type,
unsigned char index, void *buf, int size)
 {
-   return usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
-  USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
-  (type << 8) + index, 0, buf, size,
-  USB_CNTL_TIMEOUT);
+   int i;
+   int result;
+
+   if (size <= 0)  /* No point in asking for no data */
+   return -EINVAL;
+
+   memset(buf, 0, size);   /* Make sure we parse really received data */
+
+   for (i = 0; i < 3; ++i) {
+   /* retry on length 0 or error; some devices are flakey */
+   result = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+USB_REQ_GET_DESCRIPTOR, USB_DIR_IN,
+(type << 8) + index, 0, buf, size,
+USB_CNTL_TIMEOUT);
+   if (result <= 0 && result != -ETIMEDOUT)
+   continue;
+   if (result > 1 && ((u8 *)buf)[1] != type) {
+   result = -ENODATA;
+   continue;
+   }
+   break;
+   }
+   return result;
 }
 
 /**
-- 
2.39.2