The usb_control_msg and usb_bulk_msg functions in usb.c use a while loop and delay to wait for a USB control message to be processed. iopoll provides a macro to do this for us which can be used instead.
Replace the while loops with calls to the read_poll_timeout macro. Also add a check for timeout being negative at the start of the functions and return -EINVAL in that case, since a negative timeout doesn't make sense. This also fixes a bug where usb_control_msg would check if timeout was equal to 0 to determine if a timeout occurred, when it should have been checking if it was -1 instead. This caused -1 to be returned by the function in most cases when a timeout occurred instead of the correct error code of -ETIMEDOUT. usb_bulk_msg now returns -ETIMEDOUT instead of -EIO on timeout. Signed-off-by: Ronan Dalton <[email protected]> Cc: Marek Vasut <[email protected]> Cc: Philip Oberfichtner <[email protected]> --- Changes for v2: - Changed headline, previously called: usb: Fix timeout check in usb_control_msg - Fix timeout check bug by using iopoll instead of just fixing the if statement - Update usb_bulk_msg to also use iopoll as well Changes for v3: - No changes Changes for v4: - Return -ETIMEDOUT from usb_bulk_msg function on timeout - Update commit message to mention usb_bulk_msg return value change common/usb.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/common/usb.c b/common/usb.c index 1c139d86e90..488ec0532b1 100644 --- a/common/usb.c +++ b/common/usb.c @@ -40,6 +40,7 @@ #include <errno.h> #include <usb.h> #include <linux/delay.h> +#include <linux/iopoll.h> #define USB_BUFSIZ 512 @@ -224,8 +225,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, void *data, unsigned short size, int timeout) { ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, setup_packet, 1); + volatile unsigned long usb_status; int err; + if (timeout < 0) + return -EINVAL; + if ((timeout == 0) && (!asynch_allowed)) { /* request for a asynch control pipe is not allowed */ return -EINVAL; @@ -253,14 +258,12 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, * interrupt handler may set the status when the USB operation has * been completed. */ - while (timeout--) { - if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC)) - break; - mdelay(1); - } + err = read_poll_timeout((volatile unsigned long), usb_status, + !(usb_status & USB_ST_NOT_PROC), 1000, + timeout * 1000UL, dev->status); - if (timeout == 0) - return -ETIMEDOUT; + if (err == -ETIMEDOUT) + return err; if (dev->status) return -1; @@ -276,22 +279,29 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe, int usb_bulk_msg(struct usb_device *dev, unsigned int pipe, void *data, int len, int *actual_length, int timeout) { + volatile unsigned long usb_status; + int err; + if (len < 0) return -EINVAL; + if (timeout < 0) + return -EINVAL; + dev->status = USB_ST_NOT_PROC; /*not yet processed */ if (submit_bulk_msg(dev, pipe, data, len) < 0) return -EIO; - while (timeout--) { - if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC)) - break; - mdelay(1); - } + err = read_poll_timeout((volatile unsigned long), usb_status, + !(usb_status & USB_ST_NOT_PROC), 1000, + timeout * 1000UL, dev->status); *actual_length = dev->act_len; + if (err == -ETIMEDOUT) + return err; + if (dev->status == 0) return 0; else -- 2.53.0
