Booting from USB cdrom device doesn't work
Hi List, i have seabios running on a Thinkpad T60 with coreboot and seabios as Payload. When trying to boot from a USB cdrom, i get the following errors: Booting from DVD/CD... wait_td error - status=184007ff uhci_send_bulk failed USB transmission failed wait_td error - status=184007ff uhci_send_bulk failed USB transmission failed WARNING - Timeout at wait_td:427! uhci_send_bulk failed USB transmission failed WARNING - Timeout at wait_td:427! uhci_send_bulk failed USB transmission failed read capacity failed atapi_is_ready returned -1 WARNING - Timeout at wait_td:427! uhci_send_bulk failed USB transmission failed Boot failed: Could not read from CDROM (code 0003) I have no knowledge about USB, so if anybody could help me in interpreting those messages, or has an other idea what the problem is, that would be great. Thanks, Sven
On Sat, Oct 15, 2011 at 06:46:22PM +0200, Sven Schnelle wrote:
Hi List,
i have seabios running on a Thinkpad T60 with coreboot and seabios as Payload. When trying to boot from a USB cdrom, i get the following errors:
Booting from DVD/CD... wait_td error - status=184007ff uhci_send_bulk failed USB transmission failed
The device has responded with a STALL condition. Unfortunately, the current SeaBIOS code doesn't have the ability to clear the stall. I'm not sure why the device responds with a stall, but SeaBIOS should be able to handle it. Do other USB devices work okay? -Kevin
Hi Kevin, On 10/15/2011 09:43 PM, Kevin O'Connor wrote:
On Sat, Oct 15, 2011 at 06:46:22PM +0200, Sven Schnelle wrote:
Hi List,
i have seabios running on a Thinkpad T60 with coreboot and seabios as Payload. When trying to boot from a USB cdrom, i get the following errors:
Booting from DVD/CD... wait_td error - status=184007ff uhci_send_bulk failed USB transmission failed
The device has responded with a STALL condition. Unfortunately, the current SeaBIOS code doesn't have the ability to clear the stall. I'm not sure why the device responds with a stall, but SeaBIOS should be able to handle it.
Do other USB devices work okay?
Yes. I've investigated that a bit further in the meantime, and the problem is that seabios doesn't send a START STOP UNIT command before accessing the device. As soon as i send that command, i can successfully boot from that USB stick. I'll prepare a patch for that, but i'm not sure wether i should issue a TEST UNIT READY command before, or if it's safe to just send that command in any case. Regards, Sven
Some USB Mass storage devices need this command before being accessed. request sense in error case and retry the last command in case it says 'Unit attention' Signed-off-by: Sven Schnelle <svens@stackframe.org> --- src/blockcmd.c | 12 ++++++++++++ src/blockcmd.h | 11 +++++++++++ src/usb-msc.c | 44 ++++++++++++++++++++++++++++++++++---------- 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/blockcmd.c b/src/blockcmd.c index c9c6845..571f7cf 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -56,6 +56,18 @@ cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data) return cdb_cmd_data(op, &cmd, sizeof(*data)); } +int +cdb_start_stop_unit(struct disk_op_s *op, int stop, int start) +{ + struct cdb_start_stop_unit cmd; + memset(&cmd, 0, sizeof(cmd)); + cmd.command = CDB_CMD_START_STOP_UNIT; + cmd.loj_start = ((stop & 1) << 1) | (start & 1); + op->buf_fl = NULL; + op->count = 0; + return cdb_cmd_data(op, &cmd, 0); +} + // Request capacity int cdb_read_capacity(struct disk_op_s *op, struct cdbres_read_capacity *data) diff --git a/src/blockcmd.h b/src/blockcmd.h index 903c435..ebc1161 100644 --- a/src/blockcmd.h +++ b/src/blockcmd.h @@ -34,6 +34,7 @@ struct cdbres_read_capacity { #define CDB_CMD_INQUIRY 0x12 #define CDB_CMD_REQUEST_SENSE 0x03 +#define CDB_CMD_START_STOP_UNIT 0x1b struct cdb_request_sense { u8 command; @@ -44,6 +45,16 @@ struct cdb_request_sense { u8 pad[10]; } PACKED; +struct cdb_start_stop_unit { + u8 command; + u8 lun; + u8 reserved_02; + u8 reserved_03; + u8 loj_start; + u8 control; + u8 pad[10]; +} PACKED; + struct cdbres_request_sense { u8 errcode; u8 segment; diff --git a/src/usb-msc.c b/src/usb-msc.c index 13ef93e..deccf35 100644 --- a/src/usb-msc.c +++ b/src/usb-msc.c @@ -53,12 +53,14 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) if (!CONFIG_USB_MSC) return 0; +restart: dprintf(16, "usb_cmd_data id=%p write=%d count=%d bs=%d buf=%p\n" , op->drive_g, 0, op->count, blocksize, op->buf_fl); struct usbdrive_s *udrive_g = container_of( op->drive_g, struct usbdrive_s, drive); struct usb_pipe *bulkin = GET_GLOBAL(udrive_g->bulkin); struct usb_pipe *bulkout = GET_GLOBAL(udrive_g->bulkout); + struct cdbres_request_sense sense; // Setup command block wrapper. u32 bytes = blocksize * op->count; @@ -78,24 +80,42 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) if (ret) goto fail; - // Transfer data from device. - ret = usb_send_bulk(bulkin, USB_DIR_IN, op->buf_fl, bytes); - if (ret) - goto fail; - + if (blocksize) { + // Transfer data from device. + ret = usb_send_bulk(bulkin, USB_DIR_IN, op->buf_fl, bytes); + if (ret) + goto fail; + } // Transfer csw info. struct csw_s csw; ret = usb_send_bulk(bulkin, USB_DIR_IN , MAKE_FLATPTR(GET_SEG(SS), &csw), sizeof(csw)); if (ret) - goto fail; + goto fail; - if (!csw.bCSWStatus) + switch(csw.bCSWStatus) { + case 0: return DISK_RET_SUCCESS; - if (csw.bCSWStatus == 2) - goto fail; + case 2: + break; + case 1: + if (!cdb_get_sense(op, &sense)) { + dprintf(1, "SENSE: error %x, key %x, asc %x ascq %x\n", + sense.errcode, sense.flags & 0x0f, sense.asc, sense.ascq); + + switch(sense.flags & 0x0f) { + case 6: /* Unit Attention */ + goto restart; + default: + goto fail; + } + } + break; + }; - op->count -= csw.dCSWDataResidue / blocksize; + if(blocksize) + op->count -= csw.dCSWDataResidue / blocksize; + dprintf(1, "EBADTRACK\n"); return DISK_RET_EBADTRACK; fail: @@ -226,6 +246,10 @@ usb_msc_init(struct usb_pipe *pipe int ret = cdb_get_inquiry(&dop, &data); if (ret) goto fail; + + if (cdb_start_stop_unit(&dop, 0, 1)) + goto fail; + char vendor[sizeof(data.vendor)+1], product[sizeof(data.product)+1]; char rev[sizeof(data.rev)+1]; strtcpy(vendor, data.vendor, sizeof(vendor)); -- 1.7.7
On Sun, Oct 16, 2011 at 11:57:27AM +0200, Sven Schnelle wrote:
Some USB Mass storage devices need this command before being accessed. request sense in error case and retry the last command in case it says 'Unit attention'
Hi Sven, I was looking through your patch and noticed the following: [...]
+int +cdb_start_stop_unit(struct disk_op_s *op, int stop, int start) +{ + struct cdb_start_stop_unit cmd; + memset(&cmd, 0, sizeof(cmd)); + cmd.command = CDB_CMD_START_STOP_UNIT;
I don't see this command as required in the usb_msc_boot_1.0 spec - are drives guarenteed to support this command? [...]
@@ -53,12 +53,14 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize) if (!CONFIG_USB_MSC) return 0;
+restart: dprintf(16, "usb_cmd_data id=%p write=%d count=%d bs=%d buf=%p\n" , op->drive_g, 0, op->count, blocksize, op->buf_fl); [...] + case 1: + if (!cdb_get_sense(op, &sense)) { + dprintf(1, "SENSE: error %x, key %x, asc %x ascq %x\n", + sense.errcode, sense.flags & 0x0f, sense.asc, sense.ascq); + + switch(sense.flags & 0x0f) { + case 6: /* Unit Attention */ + goto restart;
This doesn't look correct - the cdb_get_sense will alter "op", so I don't think the command can be restarted. Also, should the cdb_get_sense fail, it could cause an infinite loop. Otherwise, both patches look okay to me. (I still need to boot test them though.) -Kevin
On 10/16/2011 11:57 AM, Sven Schnelle wrote:
Some USB Mass storage devices need this command before being accessed. request sense in error case and retry the last command in case it says 'Unit attention'
If it says UNIT ATTENTION you do not need to do START STOP UNIT. Sending REQUEST SENSE will clear the unit attention condition automatically. That said, sending START STOP UNIT at startup can indeed make sense for real hardware, especially a START STOP UNIT command with (start=1,loej=1) which would try closing the tray of a CD drive.
Signed-off-by: Sven Schnelle<svens@stackframe.org> --- src/blockcmd.c | 12 ++++++++++++ src/blockcmd.h | 11 +++++++++++ src/usb-msc.c | 44 ++++++++++++++++++++++++++++++++++---------- 3 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/src/blockcmd.c b/src/blockcmd.c index c9c6845..571f7cf 100644 --- a/src/blockcmd.c +++ b/src/blockcmd.c @@ -56,6 +56,18 @@ cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data) return cdb_cmd_data(op,&cmd, sizeof(*data)); }
+int +cdb_start_stop_unit(struct disk_op_s *op, int stop, int start) +{ + struct cdb_start_stop_unit cmd; + memset(&cmd, 0, sizeof(cmd)); + cmd.command = CDB_CMD_START_STOP_UNIT;
Please set bit 0 of byte 1 ("Immed") to 1. Otherwise, the command might take several seconds to complete on a CD and you might get a timeout in usb-{u,e}hci.c.
+ cmd.loj_start = ((stop& 1)<< 1) | (start& 1); + op->buf_fl = NULL; + op->count = 0; + return cdb_cmd_data(op,&cmd, 0); +}
Here, bit 0 means "start" and bit 1 means "load/eject" (not "stop"). The meaning is: start=0, loej=0 spin down the drive start=0, loej=1 eject the disk (open the tray) start=1, loej=0 spin up the drive start=1, loej=1 load the disk (close the tray) So what you are sending below is "start without loading". As I mentioned above, I think it's better if instead you just send the command once at startup, with start=loej=1. The function scsi_is_ready (formerly atapi_is_ready, but now used also for hard drives in my series that Kevin just committed) is perfect for this, because it is already equipped to poll for the device becoming ready. It would go somewhat like this: send TEST UNIT READY if successful, break send REQUEST SENSE if error, continue key = sense.flags & 0x0f; if (key == UNIT_ATTENTION) { /* TEST UNIT READY+REQUEST SENSE should have cleared the UNIT ATTENTION condition (e.g. POWER ON OCCURRED, or MEDIUM MAY HAVE CHANGED). Retry immediately. */ continue; } else if (key != NOT_READY) { printf ("Drive returned sense %x asc=%x ascq=%x\n", key, sense.asc, sense.ascq); return -1; } else if (in_progress) { /* Do not hit the drive continuously. */ usleep(200); } else { if (sense.asc == 0x3A && sense.ascq == 0x02) send START STOP UNIT(Immed=1,LoEj=1,Start=1); else if (sense.asc != 0x04 || sense.ascq != 0x01) continue; dprintf(1, "Waiting for device to detect medium... "); end = calc_future_tsc(30000); in_progress = 1; } What do you think? Paolo
participants (3)
-
Kevin O'Connor -
Paolo Bonzini -
Sven Schnelle