[coreboot] New patch to review for coreboot: 38762c8 libpayload: Detach unresponsive usb mass storage devices
Nico Huber (nico.huber@secunet.com)
gerrit at coreboot.org
Wed Jun 20 17:36:38 CEST 2012
Nico Huber (nico.huber at secunet.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1121
-gerrit
commit 38762c83cc8071aebdaaa35cb5befe6682f9fdc3
Author: Nico Huber <nico.huber at secunet.com>
Date: Fri Jun 1 09:50:11 2012 +0200
libpayload: Detach unresponsive usb mass storage devices
This enables logical detachment of unresponsive usb devices (i.e.
devices not responding to control transfers) in the usb mass storage
driver. Without the detection of unresponsive devices we wait way too
long for the device to become ready.
Change-Id: I8b8cf327f49dde25afaca4d3066f16ea86b99d3d
Signed-off-by: Nico Huber <nico.huber at secunet.com>
---
payloads/libpayload/drivers/usb/usb.c | 23 +++--
payloads/libpayload/drivers/usb/usbmsc.c | 151 +++++++++++++++++++----------
payloads/libpayload/include/usb/usb.h | 2 +-
3 files changed, 116 insertions(+), 60 deletions(-)
diff --git a/payloads/libpayload/drivers/usb/usb.c b/payloads/libpayload/drivers/usb/usb.c
index 07735a2..aa47c4a 100644
--- a/payloads/libpayload/drivers/usb/usb.c
+++ b/payloads/libpayload/drivers/usb/usb.c
@@ -205,7 +205,7 @@ set_configuration (usbdev_t *dev)
dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0);
}
-void
+int
clear_feature (usbdev_t *dev, int endp, int feature, int rtype)
{
dev_req_t dr;
@@ -216,7 +216,7 @@ clear_feature (usbdev_t *dev, int endp, int feature, int rtype)
dr.wValue = feature;
dr.wIndex = endp;
dr.wLength = 0;
- dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0);
+ return dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0);
}
int
@@ -227,9 +227,9 @@ clear_stall (endpoint_t *ep)
int rtype = gen_bmRequestType (host_to_device, standard_type,
endp ? endp_recp : dev_recp);
- clear_feature (dev, endp, ENDPOINT_HALT, rtype);
+ int ret = clear_feature (dev, endp, ENDPOINT_HALT, rtype);
ep->toggle = 0;
- return 0;
+ return ret;
}
/* returns free address or -1 */
@@ -458,12 +458,21 @@ set_address (hci_t *controller, int speed, int hubport, int hubaddr)
return adr;
}
+/*
+ * Should be called by the hub drivers whenever a physical detach occurs
+ * and can be called by usb class drivers if they are unsatisfied with a
+ * malfunctioning device.
+ */
void
usb_detach_device(hci_t *controller, int devno)
{
- controller->devices[devno]->destroy (controller->devices[devno]);
- free(controller->devices[devno]);
- controller->devices[devno] = 0;
+ /* check if device exists, as we may have
+ been called yet by the usb class driver */
+ if (controller->devices[devno]) {
+ controller->devices[devno]->destroy (controller->devices[devno]);
+ free(controller->devices[devno]);
+ controller->devices[devno] = NULL;
+ }
}
int
diff --git a/payloads/libpayload/drivers/usb/usbmsc.c b/payloads/libpayload/drivers/usb/usbmsc.c
index fd6517a..d1e3302 100644
--- a/payloads/libpayload/drivers/usb/usbmsc.c
+++ b/payloads/libpayload/drivers/usb/usbmsc.c
@@ -106,10 +106,22 @@ typedef struct {
unsigned char bCSWStatus;
} __attribute__ ((packed)) csw_t;
+enum {
+ /*
+ * MSC commands can be
+ * successful,
+ * fail with proper response or
+ * fail totally, which results in detaching of the usb device.
+ * In the latter case the caller has to make sure, that he won't
+ * use the device any more.
+ */
+ MSC_COMMAND_OK = 0, MSC_COMMAND_FAIL, MSC_COMMAND_DETACHED
+};
+
static int
request_sense (usbdev_t *dev);
-static void
+static int
reset_transport (usbdev_t *dev)
{
dev_req_t dr;
@@ -124,9 +136,17 @@ reset_transport (usbdev_t *dev)
dr.wValue = 0;
dr.wIndex = 0;
dr.wLength = 0;
- dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0);
- clear_stall (MSC_INST (dev)->bulk_in);
- clear_stall (MSC_INST (dev)->bulk_out);
+
+ /* if any of these fails, detach device, as we are lost */
+ if (dev->controller->control (dev, OUT, sizeof (dr), &dr, 0, 0) ||
+ clear_stall (MSC_INST (dev)->bulk_in) ||
+ clear_stall (MSC_INST (dev)->bulk_out)) {
+ printf ("Detaching unresponsive device.\n");
+ usb_detach_device (dev->controller, dev->address);
+ return MSC_COMMAND_DETACHED;
+ }
+ /* return fail as we are only called in case of failure */
+ return MSC_COMMAND_FAIL;
}
/* device may stall this command, so beware! */
@@ -177,20 +197,18 @@ get_csw (endpoint_t *ep, csw_t *csw)
clear_stall (ep);
if (ep->dev->controller->bulk
(ep, sizeof (csw_t), (u8 *) csw, 1)) {
- reset_transport (ep->dev);
- return 1;
+ return reset_transport (ep->dev);
}
}
if (csw->dCSWTag != tag) {
- reset_transport (ep->dev);
- return 1;
+ return reset_transport (ep->dev);
}
- return 0;
+ return MSC_COMMAND_OK;
}
static int
execute_command (usbdev_t *dev, cbw_direction dir, const u8 *cb, int cblen,
- u8 *buf, int buflen)
+ u8 *buf, int buflen, int residue_ok)
{
cbw_t cbw;
csw_t csw;
@@ -202,43 +220,48 @@ execute_command (usbdev_t *dev, cbw_direction dir, const u8 *cb, int cblen,
wrap_cbw (&cbw, buflen, dir, cb, cblen);
if (dev->controller->
bulk (MSC_INST (dev)->bulk_out, sizeof (cbw), (u8 *) &cbw, 0)) {
- reset_transport (dev);
- return 1;
+ return reset_transport (dev);
}
if (buflen > 0) {
if (dir == cbw_direction_data_in) {
if (dev->controller->
bulk (MSC_INST (dev)->bulk_in, buflen, buf, 0)) {
clear_stall (MSC_INST (dev)->bulk_in);
- return 1;
+ return MSC_COMMAND_FAIL;
}
} else {
if (dev->controller->
bulk (MSC_INST (dev)->bulk_out, buflen, buf, 0)) {
clear_stall (MSC_INST (dev)->bulk_out);
- return 1;
+ return MSC_COMMAND_FAIL;
}
}
}
- if (get_csw (MSC_INST (dev)->bulk_in, &csw))
- return 1;
- if (always_succeed == 1) {
- // return success, regardless of message
- return 0;
- }
- if (csw.bCSWStatus == 2) {
- // phase error, reset transport
- reset_transport (dev);
- return 1;
- }
- if (csw.bCSWStatus == 0) {
- // no error, exit
- return 0;
+ int ret = get_csw (MSC_INST (dev)->bulk_in, &csw);
+ if (ret) {
+ return ret;
+ } else if (always_succeed == 1) {
+ /* return success, regardless of message */
+ return MSC_COMMAND_OK;
+ } else if (csw.bCSWStatus == 2) {
+ /* phase error, reset transport */
+ return reset_transport (dev);
+ } else if (csw.bCSWStatus == 0) {
+ if ((csw.dCSWDataResidue == 0) || residue_ok)
+ /* no error, exit */
+ return MSC_COMMAND_OK;
+ else
+ /* missed some bytes */
+ return MSC_COMMAND_FAIL;
+ } else {
+ if (cb[0] == 0x03)
+ /* requesting sense failed, that's bad */
+ return MSC_COMMAND_FAIL;
+ /* error "check condition" or reserved error */
+ ret = request_sense (dev);
+ /* return fail or the status of request_sense if it's worse */
+ return ret ? ret : MSC_COMMAND_FAIL;
}
- if (cb[0] != 0x03) /* 0x03 == request sense */
- // error "check condition" or reserved error
- request_sense (dev);
- return 1;
}
typedef struct {
@@ -309,7 +332,8 @@ readwrite_blocks (usbdev_t *dev, int start, int n, cbw_direction dir, u8 *buf)
cb.numblocks = htonw (n);
return execute_command (dev, dir, (u8 *) &cb, sizeof (cb), buf,
- n * MSC_INST(dev)->blocksize);
+ n * MSC_INST(dev)->blocksize, 0)
+ != MSC_COMMAND_OK ? 1 : 0;
}
/* Only request it, we don't interpret it.
@@ -324,7 +348,7 @@ request_sense (usbdev_t *dev)
cb.command = 0x3;
return execute_command (dev, cbw_direction_data_in, (u8 *) &cb,
- sizeof (cb), buf, 19);
+ sizeof (cb), buf, 19, 1);
}
static int
@@ -333,7 +357,7 @@ test_unit_ready (usbdev_t *dev)
cmdblock6_t cb;
memset (&cb, 0, sizeof (cb)); // full initialization for T-U-R
return execute_command (dev, cbw_direction_data_out, (u8 *) &cb,
- sizeof (cb), 0, 0);
+ sizeof (cb), 0, 0, 0);
}
static int
@@ -344,10 +368,10 @@ spin_up (usbdev_t *dev)
cb.command = 0x1b;
cb.lun = 1;
return execute_command (dev, cbw_direction_data_out, (u8 *) &cb,
- sizeof (cb), 0, 0);
+ sizeof (cb), 0, 0, 0);
}
-static void
+static int
read_capacity (usbdev_t *dev)
{
cmdblock_t cb;
@@ -356,12 +380,20 @@ read_capacity (usbdev_t *dev)
u8 buf[8];
debug ("Reading capacity of mass storage device.\n");
- int count = 0;
- while ((count++ < 20)
- &&
- (execute_command
- (dev, cbw_direction_data_in, (u8 *) &cb, sizeof (cb), buf,
- 8) == 1));
+ int count = 0, ret;
+ while (count++ < 20) {
+ switch (ret = execute_command
+ (dev, cbw_direction_data_in, (u8 *) &cb,
+ sizeof (cb), buf, 8, 0)) {
+ case MSC_COMMAND_OK:
+ break;
+ case MSC_COMMAND_FAIL:
+ continue;
+ default: /* if it's worse return */
+ return ret;
+ }
+ break;
+ }
if (count >= 20) {
// still not successful, assume 2tb in 512byte sectors, which is just the same garbage as any other number, but probably more usable.
printf (" assuming 2 TB with 512-byte sectors as READ CAPACITY didn't answer.\n");
@@ -377,6 +409,7 @@ read_capacity (usbdev_t *dev)
MSC_INST (dev)->numblocks > 1000000
? (MSC_INST (dev)->numblocks / 1000) * MSC_INST (dev)->blocksize / 1000 :
MSC_INST (dev)->numblocks * MSC_INST (dev)->blocksize / 1000 / 1000);
+ return MSC_COMMAND_OK;
}
void
@@ -444,12 +477,21 @@ usb_msc_init (usbdev_t *dev)
printf (" Waiting for device to become ready...");
timeout = 30 * 10; /* SCSI/ATA specs say we have to wait up to 30s. Ugh */
- while (test_unit_ready (dev) && --timeout) {
- mdelay (100);
- if (!(timeout % 10))
- printf (".");
+ while (timeout--) {
+ switch (test_unit_ready (dev)) {
+ case MSC_COMMAND_OK:
+ break;
+ case MSC_COMMAND_FAIL:
+ mdelay (100);
+ if (!(timeout % 10))
+ printf (".");
+ continue;
+ default: /* if it's worse return */
+ return;
+ }
+ break;
}
- if (test_unit_ready (dev)) {
+ if (timeout < 0) {
printf ("timeout. Device not ready. Still trying...\n");
} else {
printf ("ok.\n");
@@ -458,15 +500,20 @@ usb_msc_init (usbdev_t *dev)
debug (" spin up");
for (i = 0; i < 30; i++) {
debug (".");
- if (!spin_up (dev)) {
+ switch (spin_up (dev)) {
+ case MSC_COMMAND_OK:
debug (" OK.");
break;
+ case MSC_COMMAND_FAIL:
+ mdelay (100);
+ continue;
+ default: /* if it's worse return */
+ return;
}
- mdelay (100);
+ break;
}
debug ("\n");
- read_capacity (dev);
- if (usbdisk_create)
+ if ((read_capacity (dev) == MSC_COMMAND_OK) && usbdisk_create)
usbdisk_create (dev);
}
diff --git a/payloads/libpayload/include/usb/usb.h b/payloads/libpayload/include/usb/usb.h
index ee9c50b..ecfee54 100644
--- a/payloads/libpayload/include/usb/usb.h
+++ b/payloads/libpayload/include/usb/usb.h
@@ -215,7 +215,7 @@ void init_device_entry (hci_t *controller, int num);
void set_feature (usbdev_t *dev, int endp, int feature, int rtype);
void get_status (usbdev_t *dev, int endp, int rtype, int len, void *data);
-void clear_feature (usbdev_t *dev, int endp, int feature, int rtype);
+int clear_feature (usbdev_t *dev, int endp, int feature, int rtype);
int clear_stall (endpoint_t *ep);
void usb_nop_init (usbdev_t *dev);
More information about the coreboot
mailing list