[bug report] usb: Assimilate usb_get_descriptor() to linux

[bug report] usb: Assimilate usb_get_descriptor() to linux

[Andrew Goodbody]

Hello Philip Oberfichtner,

Commit 2fc8638403c7 ("usb: Assimilate usb_get_descriptor() to linux")
from Jun 4, 2024, leads to the following Smatch static checker
warning:

	common/usb.c:261 usb_control_msg()
	warn: should this be 'timeout == -1'

common/usb.c
     220 int usb_control_msg(struct usb_device *dev, unsigned int pipe,
     221                         unsigned char request, unsigned char 
requesttype,
     222                         unsigned short value, unsigned short index,
     223                         void *data, unsigned short size, int 
timeout)
     224 {
     225         ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, 
setup_packet, 1);
     226         int err;
     227
     228         if ((timeout == 0) && (!asynch_allowed)) {
     229                 /* request for a asynch control pipe is not 
allowed */
     230                 return -EINVAL;
     231         }
     232
     233         /* set setup command */
     234         setup_packet->requesttype = requesttype;
     235         setup_packet->request = request;
     236         setup_packet->value = cpu_to_le16(value);
     237         setup_packet->index = cpu_to_le16(index);
     238         setup_packet->length = cpu_to_le16(size);
     239         debug("usb_control_msg: request: 0x%X, requesttype: 
0x%X, " \
     240               "value 0x%X index 0x%X length 0x%X\n",
     241               request, requesttype, value, index, size);
     242         dev->status = USB_ST_NOT_PROC; /*not yet processed */
     243
     244         err = submit_control_msg(dev, pipe, data, size, 
setup_packet);
     245         if (err < 0)
     246                 return err;
     247         if (timeout == 0)
     248                 return (int)size;
     249
     250         /*
     251          * Wait for status to update until timeout expires, USB 
driver
     252          * interrupt handler may set the status when the USB 
operation has
     253          * been completed.
     254          */
     255         while (timeout--) {
     256                 if (!((volatile unsigned long)dev->status & 
USB_ST_NOT_PROC))
     257                         break;
     258                 mdelay(1);
     259         }
     260
--> 261         if (timeout == 0)
     262                 return -ETIMEDOUT;
     263
     264         if (dev->status)
     265                 return -1;
     266
     267         return dev->act_len;
     268 }

Regards,
Andrew Goodbody

Re: [bug report] usb: Assimilate usb_get_descriptor() to linux

[Philip Oberfichtner]

Hi Andrew,

On Thu, Jun 19, 2025 at 01:02:39PM +0100, Andrew Goodbody wrote:
> Hello Philip Oberfichtner,
> 
> Commit 2fc8638403c7 ("usb: Assimilate usb_get_descriptor() to linux")
> from Jun 4, 2024, leads to the following Smatch static checker
> warning:
> 
> 	common/usb.c:261 usb_control_msg()
> 	warn: should this be 'timeout == -1'

Thanks for pointing this out. Fixup is on the way.

Best regards,
Philip


> 
> common/usb.c
>     220 int usb_control_msg(struct usb_device *dev, unsigned int pipe,
>     221                         unsigned char request, unsigned char
> requesttype,
>     222                         unsigned short value, unsigned short index,
>     223                         void *data, unsigned short size, int
> timeout)
>     224 {
>     225         ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, setup_packet,
> 1);
>     226         int err;
>     227
>     228         if ((timeout == 0) && (!asynch_allowed)) {
>     229                 /* request for a asynch control pipe is not allowed
> */
>     230                 return -EINVAL;
>     231         }
>     232
>     233         /* set setup command */
>     234         setup_packet->requesttype = requesttype;
>     235         setup_packet->request = request;
>     236         setup_packet->value = cpu_to_le16(value);
>     237         setup_packet->index = cpu_to_le16(index);
>     238         setup_packet->length = cpu_to_le16(size);
>     239         debug("usb_control_msg: request: 0x%X, requesttype: 0x%X, "
> \
>     240               "value 0x%X index 0x%X length 0x%X\n",
>     241               request, requesttype, value, index, size);
>     242         dev->status = USB_ST_NOT_PROC; /*not yet processed */
>     243
>     244         err = submit_control_msg(dev, pipe, data, size,
> setup_packet);
>     245         if (err < 0)
>     246                 return err;
>     247         if (timeout == 0)
>     248                 return (int)size;
>     249
>     250         /*
>     251          * Wait for status to update until timeout expires, USB
> driver
>     252          * interrupt handler may set the status when the USB
> operation has
>     253          * been completed.
>     254          */
>     255         while (timeout--) {
>     256                 if (!((volatile unsigned long)dev->status &
> USB_ST_NOT_PROC))
>     257                         break;
>     258                 mdelay(1);
>     259         }
>     260
> --> 261         if (timeout == 0)
>     262                 return -ETIMEDOUT;
>     263
>     264         if (dev->status)
>     265                 return -1;
>     266
>     267         return dev->act_len;
>     268 }
> 
> Regards,
> Andrew Goodbody

-- 
=====================================================================
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: pro@denx.de
=====================================================================