[PATCH v2 4/4] usb: Convert ad-hoc poll/timeout sequences in usb.c to iopoll

[Ronan Dalton]

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.

Signed-off-by: Ronan Dalton <ronan.dalton at alliedtelesis.co.nz>
Cc: Marek Vasut <marex at denx.de>
Cc: Philip Oberfichtner <pro at denx.de>
---
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

Notes:
- usb_bulk_msg could be updated to return -ETIMEDOUT on the timeout
  case. Currently it looks like -EIO is returned if a timeout occurs and
  callers can't tell if a timeout occurred as such. The -EIO return code
  is also inconsistent with usb_control_msg which returns -1.

 common/usb.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 1c139d86e90..4defda93a06 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,19 +279,22 @@ 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;
+
 	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);
-	}
+	read_poll_timeout((volatile unsigned long), usb_status,
+			  !(usb_status & USB_ST_NOT_PROC), 1000,
+			  timeout * 1000UL, dev->status);
 
 	*actual_length = dev->act_len;
 
-- 
2.53.0