Re: [SeaBIOS] [PATCH] USB MSC: send START/STOP UNIT command before accessing device

[Paolo Bonzini]

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
>
> 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