[coreboot-gerrit] [S] Change in coreboot[master]: libpayload/uhci: Return expected length for control/bulk transfers

[Martin L Roth (Code Review)]

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75349?usp=email )

Change subject: libpayload/uhci: Return expected length for control/bulk transfers
......................................................................

libpayload/uhci: Return expected length for control/bulk transfers

Libpayload's USB API was changed in commit e9738dbe2b (libpayload: Make
USB transfer functions return amount of bytes). However, the UHCI driver
was never adapted. Instead of returning 0 for success, we can return the
expected data length as a best effort. We won't be able to catch short
transfers this way, but previously working cases will work again.

Change-Id: I31d7de495a46af401e2cbe5a3b8f6349facad8ff
Signed-off-by: Nico Huber 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/75349
Tested-by: build bot (Jenkins) 
Reviewed-by: Paul Menzel 
Reviewed-by: Julius Werner 
---
M payloads/libpayload/drivers/usb/uhci.c
1 file changed, 13 insertions(+), 11 deletions(-)

Approvals:
  Paul Menzel: Looks good to me, but someone else must approve
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/payloads/libpayload/drivers/usb/uhci.c b/payloads/libpayload/drivers/usb/uhci.c
index 538107e..eb252cd 100644
--- a/payloads/libpayload/drivers/usb/uhci.c
+++ b/payloads/libpayload/drivers/usb/uhci.c
@@ -310,7 +310,7 @@
 }

static int
-uhci_control(usbdev_t *dev, direction_t dir, int drlen, void *devreq, int dalen,
+uhci_control(usbdev_t *dev, direction_t dir, int drlen, void *devreq, const int dalen,
          unsigned char *data)
 {
    int endp = 0;		/* this is control: always 0 */
@@ -339,6 +339,7 @@
    	TD_STATUS_ACTIVE;

int toggle = 1;
+	int len_left = dalen;
    for (i = 1; i < count; i++) {
    	switch (dir) {
    		case SETUP: tds[i].token = UHCI_SETUP; break;
@@ -347,14 +348,14 @@
    	}
    	tds[i].token |= dev->address << TD_DEVADDR_SHIFT |
    		endp << TD_EP_SHIFT |
-			maxlen(min(mlen, dalen)) << TD_MAXLEN_SHIFT |
+			maxlen(min(mlen, len_left)) << TD_MAXLEN_SHIFT |
    		toggle << TD_TOGGLE_SHIFT;
    	tds[i].bufptr = virt_to_phys(data);
    	tds[i].ctrlsts = (3 << TD_COUNTER_SHIFT) |
    		(dev->speed?TD_LOWSPEED:0) |
    		TD_STATUS_ACTIVE;
    	toggle ^= 1;
-		dalen -= mlen;
+		len_left -= mlen;
    	data += mlen;
    }

@@ -374,7 +375,7 @@
    				  qh_data);
    int result;
    if (td == 0) {
-		result = 0;
+		result = dalen; /* TODO: We should return the actually transferred length. */
    } else {
    	usb_debug("control packet, req %x\n", req);
    	td_dump(td);
@@ -438,25 +439,26 @@

/* finalize == 1: if data is of packet aligned size, add a zero length packet */
 static int
-uhci_bulk(endpoint_t *ep, int size, u8 *data, int finalize)
+uhci_bulk(endpoint_t *ep, const int dalen, u8 *data, int finalize)
 {
    int maxpsize = ep->maxpacketsize;
    if (maxpsize == 0)
    	fatal("MaxPacketSize == 0!!!");
-	int numpackets = (size + maxpsize - 1) / maxpsize;
-	if (finalize && ((size % maxpsize) == 0)) {
+	int numpackets = (dalen + maxpsize - 1) / maxpsize;
+	if (finalize && ((dalen % maxpsize) == 0)) {
    	numpackets++;
    }
    if (numpackets == 0)
    	return 0;
    td_t *tds = create_schedule(numpackets);
    int i = 0, toggle = ep->toggle;
-	while ((size > 0) || ((size == 0) && (finalize != 0))) {
-		fill_schedule(&tds[i], ep, min(size, maxpsize), data,
+	int len_left = dalen;
+	while ((len_left > 0) || ((len_left == 0) && (finalize != 0))) {
+		fill_schedule(&tds[i], ep, min(len_left, maxpsize), data,
    		       &toggle);
    	i++;
    	data += maxpsize;
-		size -= maxpsize;
+		len_left -= maxpsize;
    }
    if (run_schedule(ep->dev, tds) == 1) {
    	free(tds);
@@ -464,7 +466,7 @@
    }
    ep->toggle = toggle;
    free(tds);
-	return 0;
+	return dalen; /* TODO: We should return the actually transferred length. */
 }

typedef struct {