[coreboot-gerrit] New patch to review for coreboot: c42707a libpayload: usb: Remove automatic clear_stall() calls from transfers

Marc Jones (marc.jones@se-eng.com) gerrit at coreboot.org
Tue Oct 28 04:39:15 CET 2014


Marc Jones (marc.jones at se-eng.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/7223

-gerrit

commit c42707a3a18908fbf161a803ae65d7c616dd1084
Author: Julius Werner <jwerner at chromium.org>
Date:   Tue Apr 8 13:37:39 2014 -0700

    libpayload: usb: Remove automatic clear_stall() calls from transfers
    
    We've recently fixed a problem where an external hard drive would choke
    due to one too many CLEAR_FEATURE(HALT) commands in the XHCI stack with
    "libpayload: usb: xhci: Fix STALL endpoint handling". Clearing stall
    conditions from within the transfer function is wrong in general... this
    is really something that is host controller agnostic and should be left
    to the higher-level driver to decide. The mass storage driver (the only
    one that should really encounter stalls right now) already contains the
    proper amount of clear_stall() calls... any more than that is redundant
    and as we found out potentially dangerous.
    
    This patch removes automatic clear stalls from UHCI and OHCI drivers as
    well to make things consistent between host controllers.
    
    BUG=chromium:192866
    TEST=None. I could borrow the original hard drive from Shawn and compile
    a Snow to only use the OHCI driver to reproduce/verify this, but alas, I
    am lazy (and it's really not that important).
    
    Original-Change-Id: Ie1e4d4d2d70fa4abf8b4dabd33b10d6d4012048a
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/193732
    Original-Reviewed-by: Stefan Reinauer <reinauer at chromium.org>
    (cherry picked from commit d46e183f3e7e0b0130becdefa6fd3ef8097df54b)
    Signed-off-by: Marc Jones <marc.jones at se-eng.com>
    
    Change-Id: Ie8f4ab3db8ec0d9a2d1e91c62967833e59c46700
---
 payloads/libpayload/drivers/usb/ohci.c | 3 +--
 payloads/libpayload/drivers/usb/uhci.c | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/payloads/libpayload/drivers/usb/ohci.c b/payloads/libpayload/drivers/usb/ohci.c
index 143f738..3aa7810 100644
--- a/payloads/libpayload/drivers/usb/ohci.c
+++ b/payloads/libpayload/drivers/usb/ohci.c
@@ -602,8 +602,7 @@ ohci_bulk (endpoint_t *ep, int dalen, u8 *src, int finalize)
 		result = dalen - result;
 		if (ep->direction == IN && data != src)
 			memcpy(src, data, result);
-	} else
-		clear_stall(ep);
+	}
 
 	return result;
 }
diff --git a/payloads/libpayload/drivers/usb/uhci.c b/payloads/libpayload/drivers/usb/uhci.c
index 2e3249e..cdba3de 100644
--- a/payloads/libpayload/drivers/usb/uhci.c
+++ b/payloads/libpayload/drivers/usb/uhci.c
@@ -452,8 +452,6 @@ uhci_bulk (endpoint_t *ep, int size, u8 *data, int finalize)
 		size -= maxpsize;
 	}
 	if (run_schedule (ep->dev, tds) == 1) {
-		usb_debug("Stalled. Trying to clean up.\n");
-		clear_stall (ep);
 		free (tds);
 		return -1;
 	}



More information about the coreboot-gerrit mailing list