Author: wmb Date: 2008-08-03 01:19:50 +0200 (Sun, 03 Aug 2008) New Revision: 865
Modified: dev/usb2/device/storage/hacom.fth dev/usb2/device/storage/scsi.fth dev/usb2/device/storage/scsicom.fth dev/usb2/device/storage/scsidisk.fth Log: OLPC trac 7774 - restructured the internal error code propagation and device initialization for USB mass storage devices to cope with devices that mis-report the data transfer status in the Command Status Wrapper protocol phase. Some devices report "success" even when they don't return the requested data. One such device reports "success" after failing to transfer the data for the "Read Capacity" command. With the restructuring, the actual data transfer length as reported by the lower-level USB transport is propagated all the way up to the top level caller, so it can notice that the command didn't really succeed. I also added a "Test Unit Ready" step at the beginning of the disk "open" method, thus making the system handle multi-card readers with several LUNs more cleanly.
Modified: dev/usb2/device/storage/hacom.fth =================================================================== --- dev/usb2/device/storage/hacom.fth 2008-07-30 23:49:08 UTC (rev 864) +++ dev/usb2/device/storage/hacom.fth 2008-08-02 23:19:50 UTC (rev 865) @@ -38,9 +38,9 @@ dup 2 = if \ check (tapes, especially) for MEDIA NOT PRESENT: if the \ media's not there the command is not retryable - drop sense-buf h# c + c@ h# 3a = sense-buf h# d + c@ 0= and if - -1 else 1 - then exit + drop ( ) + sense-buf h# c + c@ h# 3a = sense-buf h# d + c@ 0= and ( not-present? ) + if -1 else 1 then exit then
\ Media-error(3) is not retryable @@ -82,72 +82,18 @@ then ;
- headers
create sense-cmd 3 c, 0 c, 0 c, 0 c, ff c, 0 c,
-: get-sense ( -- ) \ Issue REQUEST SENSE, which is not supposed to fail - sense-buf ff true sense-cmd 6 execute-command 0= if drop then +: get-sense ( -- failed? ) \ Issue REQUEST SENSE + sense-buf ff true sense-cmd 6 execute-command ( actual cswStatus ) + if drop true else 8 < then ;
\ Give the device a little time to recover before retrying the command. -: delay-retry ( -- ) 2000 0 do loop ; +: delay-retry ( -- ) 1 ms ;
-0 value statbyte \ Local variable used by retry? - -\ RETRY? is used by RETRY-COMMAND to determine whether or not to retry the -\ command, considering the following factors: -\ - Success or failure of the command at the hardware level (failure at -\ this level is usually fatal, except in the case of an incoming bus reset) -\ - The value of the status byte returned by the command -\ - The condition indicated by the sense bytes -\ - The number of previous retries -\ -\ The input arguments are as returned by "scsi-exec" -\ On output, the top of the stack is true if the command is to be retried, -\ otherwise the top of the stack is false and the results that should be -\ returned by retry-command are underneath it; those results indicate the type -\ of error that occurred. - -: retry? ( hw-result | statbyte 0 -- true | [[sensebuf] f-hw] error? false ) - case - 0 of to statbyte endof \ No hardware error; continue checking - bus-reset of true exit endof \ Retry after incoming bus reset - ( hw-result ) true false exit \ Other hardware errors are fatal - endcase - - statbyte 0= if false false exit then \ If successful, return "no-error" - - statbyte 2 and if \ "Check Condition", so get extended status - get-sense classify-sense case ( -1|0|1 ) - \ If the sense information says "no sense", return "no-error" - 0 of false false exit endof - - \ If the error is fatal, return "sense-buf,valid,statbyte" - -1 of sense-buf false statbyte false exit endof - endcase - - \ Otherwise, the error was retryable. However, if we have - \ have already retried the specified number of times, don't - \ retry again; instead return sense buffer and status. - #retries 0= if sense-buf false statbyte false exit then - then - - \ Don't retry if vendor-unique, reserved, intermediate, or - \ "condition met/good" bits are set. Return "no-sense,status" - statbyte h# f5 and if true statbyte false exit then - - \ Don't retry if we have already retried the specified number - \ of times. Return "no-sense,status" - #retries 0= if true statbyte false exit then - - \ Otherwise, it was either a busy or a retryable check condition, - \ so we retry. - - true -; - \ RETRY-COMMAND executes a SCSI command. If a check condition is indicated, \ performs a "get-sense" command. If the sense bytes indicate a non-fatal \ condition (e.g. power-on reset occurred, not ready yet, or recoverable @@ -162,19 +108,6 @@
\ #retries is number of times to retry (0: don't retry, -1: retry forever) \ -\ sensebuf is the address of the sense buffer; it is present only -\ if f-hw is 0 and error? is non-zero. The length of the sense buffer -\ is 8 bytes plus the value in byte 7 of the sense buffer. -\ -\ f-hw is non-zero if there is a hardware error -- dma fails, select fails, -\ etc -- or if the status byte was neither 0 (okay) nor 2 (check condition) -\ -\ error? is non-zero if there is a transaction error. If error? is 0, -\ f-hw and sensebuf are not returned. -\ -\ If sensebuf is returned, the contents are valid until the next call to -\ retry-command. sensebuf becomes inaccessable when this package is closed. -\ \ dma-dir is necessary because it is not always possible to infer the DMA \ direction from the command.
@@ -189,35 +122,53 @@
external
-: retry-command ( dma-buf dma-len dma-dir cmdbuf cmdlen #retries -- ... ) - ( ... -- [[sensebuf] f-hw] error? ) +\ errcode values: 0: okay -1: phase error otherwise: sense-key + +: retry-command? ( dma-buf dma-len dma-dir cmdbuf cmdlen #retries -- actual errcode ) to #retries to clen to cbuf to direction-in to dlen to dbuf
begin - dbuf dlen direction-in cbuf clen execute-command ( hwerr | stat 0 ) - retry? - while - #retries 1- to #retries - delay-retry - repeat -; + dbuf dlen direction-in cbuf clen execute-command ( actual cswStatus )
-headers + dup 0= if drop 0 exit then \ Exit reporting success + dup 2 > if drop -1 exit then \ Exit reporting invalid CSW result code
-\ Collapses the complete error information returned by retry-command into -\ a single error/no-error flag. + 1 = if ( actual ) + \ Do get-sense to determine what to do next + get-sense if ( actual ) + \ Treat a get-sense failure like a phase error; just retry the command + -1 ( actual errcode ) + else ( actual ) + classify-sense case ( actual -1|0|1 ) + \ If the sense information says "no sense", return "no-error" + 0 of 0 exit endof
-: error? ( false | true true | sensebuf false true -- error? ) - dup if swap 0= if nip then then + \ If the error is fatal, return the sense-key + -1 of sense-buf 2+ c@ exit endof + endcase + sense-buf 2+ c@ ( actual errcode ) + then + else ( actual ) + -1 \ Was phase error ( actual errcode ) + then ( actual errcode ) + + \ If we get here, the command is retryable - either a phase error + \ or a non-fatal sense code + + #retries 1- dup to #retries ( actual errcode #retries ) + while ( actual errcode ) + 2drop ( ) + delay-retry + repeat ( actual errcode ) ;
external
-\ Simplified "retry-command" routine for commands with no data transfer phase +\ Simplified routine for commands with no data transfer phase \ and simple error checking requirements.
: no-data-command ( cmdbuf -- error? ) - >r 0 0 true r> 6 -1 retry-command error? + >r 0 0 true r> 6 -1 retry-command? nip ;
\ short-data-command executes a command with the following characteristics: @@ -229,9 +180,9 @@ \ The buffer contents become invalid when another SCSI command is \ executed, or when the driver is closed.
-: short-data-command ( data-len cmdbuf cmdlen -- true | buffer false ) - >r >r inq-buf swap true r> r> -1 retry-command ( retry-cmd-results ) - error? dup 0= if inq-buf swap then +: short-data-command ( data-len cmdbuf cmdlen #retries -- true | buffer len false ) + >r >r >r inq-buf swap true r> r> r> retry-command? ( actual error-code ) + if drop true else inq-buf swap false then ;
headers @@ -256,7 +207,7 @@ \ supposed to respond with "check condition". \ However, empirically, on MC2 EVT1, 8 proves insufficient.
- inq-buf ff true inquiry-cmd 6 10 retry-command error? + inq-buf ff true inquiry-cmd 6 10 retry-command? nip ;
headers @@ -318,7 +269,7 @@ headers
\ The diagnose command is useful for generic SCSI devices. -\ It executes bothe "test-unit-ready" and "send-diagnostic" +\ It executes both the "test-unit-ready" and "send-diagnostic" \ commands, decoding the error status information they return.
create test-unit-rdy-cmd 0 c, 0 c, 0 c, 0 c, 0 c, 0 c, @@ -331,19 +282,22 @@
: diagnose ( -- flag ) 0 0 true test-unit-rdy-cmd 6 -1 ( dma$ dir cmd$ #retries ) - retry-command if ( [ sensebuf ] hardware-error? ) - ." Test unit ready failed - " ( [ sensebuf ] hardware-error? ) - if ( ) - ." hardware error (no such device?)" cr ( ) - else ( sensebuf ) - ." extended status = " cr ( sensebuf ) - base @ >r hex ( sensebuf ) - 8 bounds ?do i 3 u.r loop cr ( ) + retry-command? ?dup if ( actual error-code ) + nip ( error-code ) + ." Test unit ready failed - " ( error-code ) + dup -1 if ( error-code ) + ." phase error " . cr ( ) + else ( error-code ) + ." Sense code " . ( ) + ." extended status = " cr ( ) + base @ >r hex ( ) + sense-buf 8 bounds ?do i 3 u.r loop cr ( ) r> base ! then true - else - send-diagnostic ( fail? ) + else ( actual ) + drop ( ) + send-diagnostic ( fail? ) then ;
Modified: dev/usb2/device/storage/scsi.fth =================================================================== --- dev/usb2/device/storage/scsi.fth 2008-07-30 23:49:08 UTC (rev 864) +++ dev/usb2/device/storage/scsi.fth 2008-08-02 23:19:50 UTC (rev 865) @@ -111,49 +111,56 @@
d# 15,000 constant bulk-timeout
-: (execute-command) ( data-adr,len dir cbw-adr,len -- hwresult | statbyte 0 ) +: (execute-command) ( data-adr,len dir cbw-adr,len -- actual-len cswStatus ) debug? if 2dup " dump" evaluate cr then
bulk-out-pipe bulk-out ( data-adr,len dir usberr ) - USB_ERR_CRC invert and ( data-adr,len dir usberr' ) - ?dup if nip nip nip exit then ( data-adr,len dir ) + USB_ERR_CRC invert and if ( data-adr,len dir ) + transport-reset 3drop 0 2 exit ( actual=0 status=retry ) + then ( data-adr,len dir )
- over if + over if ( data-adr,len dir ) if ( data-adr,len ) - bulk-in-pipe bulk-in 2drop ( ) - else - bulk-out-pipe bulk-out drop ( ) - then + bulk-in-pipe bulk-in ( actual usberror ) + else ( data-adr,len ) + tuck bulk-out-pipe bulk-out ( len usberror ) + dup if nip 0 swap then ( len' usberror ) + then ( usberror ) else ( data-adr,len dir ) - drop 2drop ( ) - then + drop nip 0 ( len usberror ) + then ( actual usberror )
- get-csw ( len usberr ) - ?dup if nip exit then + get-csw ( actual usberror csw-len csw-usberror ) + + rot drop ( actual csw-len csw-usberror ) + + ?dup if nip exit then ( actual csw-len csw-usberror ) + drop ( actual ) + debug? if csw /csw " dump" evaluate cr then
- drop csw >csw-stat c@ ( cswStatus ) - case - 0 of 0 0 endof \ No error - 1 of 2 0 endof \ Error, "check condition" - 2 of transport-reset \ Phase error, reset recovery - USB_ERR_STALL endof - ( default ) 0 0 rot \ Reserved error - endcase + csw >csw-stat c@ ( actual cswStatus ) + dup 2 = if transport-reset then ( actual cswStatus ) + \ Values are: + \ 0: No error - command is finished + \ 1: Error - do get-sense and possibly retry + \ 2: Phase error - retry after transport-reset + \ else: Invalid status code - abort command ;
external
-: execute-command ( data-adr,len dir cmd-adr,len -- hwresult | statbyte 0 ) - execute-command-hook - over c@ h# 1b = 2 pick 4 + c@ 1 = and >r \ Start command? +: execute-command ( data-adr,len dir cmd-adr,len -- actual cswStatus ) + execute-command-hook ( data$ dir cmd$ ) + over c@ h# 1b = ( data$ dir cmd$ flag ) + 2 pick 4 + c@ 1 = and >r ( data$ dir cmd$ r: Start-command? ) 2over 2swap wrap-cbw ( data-adr,len dir cbw-adr,len ) - (execute-command) - r> if dup 0= if nip 0 then then \ Fake ok + (execute-command) ( actual cswStatus ) + r> if drop 0 then \ Fake ok if it's a start commmand ;
: set-address ( lun -- )
Modified: dev/usb2/device/storage/scsicom.fth =================================================================== --- dev/usb2/device/storage/scsicom.fth 2008-07-30 23:49:08 UTC (rev 864) +++ dev/usb2/device/storage/scsicom.fth 2008-08-02 23:19:50 UTC (rev 865) @@ -15,14 +15,11 @@ : parent-set-address ( lun -- ) " set-address" $call-parent ;
-\ Calls the parent device's "retry-command" method. The parent device is +\ Calls the parent device's "retry-command?" method. The parent device is \ assumed to be a driver for a SCSI host adapter (device-type = "scsi")
-: retry-command ( dma-addr dma-len dma-dir cmd-addr cmd-len #retries -- ... ) - ( ... -- false ) \ No error - ( ... -- true true ) \ Hardware error - ( ... -- sensebuf false true ) \ Fatal error with extended status - " retry-command" $call-parent +: retry-command? ( dma-addr dma-len dma-dir cmd-addr cmd-len #retries -- actual errcode ) + " retry-command?" $call-parent ;
@@ -30,7 +27,7 @@
: no-data-command ( cmdbuf -- error? ) " no-data-command" $call-parent ;
-: short-data-command ( data-len cmdbuf cmdlen -- true | buffer false ) +: short-data-command ( data-len cmdbuf cmdlen #retries -- true | buffer len false ) " short-data-command" $call-parent ;
@@ -60,7 +57,7 @@ external : device-present? ( lun -- present? ) parent-set-address - " inquiry" $call-parent invert + " inquiry" $call-parent 0= ;
: eject ( -- )
Modified: dev/usb2/device/storage/scsidisk.fth =================================================================== --- dev/usb2/device/storage/scsidisk.fth 2008-07-30 23:49:08 UTC (rev 864) +++ dev/usb2/device/storage/scsidisk.fth 2008-08-02 23:19:50 UTC (rev 865) @@ -37,22 +37,23 @@ then ;
+\ Checks to see if a device is ready
+: unit-ready? ( -- ready? ) + " "(00 00 00 00 00 00)" drop no-data-command 0= +; + \ Ensures that the disk is spinning, but doesn't wait forever
create sstart-cmd h# 1b c, 0 c, 0 c, 0 c, 1 c, 0 c,
: timed-spin ( -- error? ) - \ Set timeout to 45 sec: some large (>1GB) drives take - \ up to 30 secs to spin up. - d# 45 d# 1000 * set-timeout - - 0 0 true sstart-cmd 6 -1 retry-command if ( true | sensebuf false ) + 0 0 true sstart-cmd 6 -1 retry-command? nip ?dup if ( error-code ) \ true on top of the stack indicates a hardware error. \ We don't treat "illegal request" as an error because some drives \ don't support the start command. Everything else other than \ success is considered an error. - if true else 2+ c@ 5 <> then ( error? ) + 5 <> ( error? ) else ( ) false ( false ) then ( error? ) @@ -62,20 +63,48 @@
create read-capacity-cmd h# 25 c, 0 c, 0 c, 0 c, 0 c, 0 c, 0 c, 0 c, 0 c, 0 c,
+: get-capacity ( -- false | block-size #blocks false true ) + 8 read-capacity-cmd 0a 0 short-data-command if ( ) + false + else ( adr len ) + 8 <> if drop false exit then ( adr ) + dup 4 + 4c@ swap 4c@ 1+ false true + then +; + +[ifdef] notdef +\ This is a "read for nothing", discarding the result. It's a +\ workaround for a problem with the "Silicon Motion SMI331" controller +\ as used in the "Transcend TS2GUSD-S3" USB / MicroSD reader. That +\ device stalls "read capacity" commands until you do the first block +\ read. The first block read stalls too, but afterwards everything works. +: nonce-read ( -- ) + d# 512 dma-alloc >r + r@ d# 512 true " "(28 00 00 00 00 00 00 00 01 00)" ( data$ in? cmd$ ) + 0 retry-command? 2drop + r> d# 512 dma-free +; +[then] + : read-block-extent ( -- true | block-size #blocks false ) - \ First try "read capacity" - data returned in bytes 4,5,6,7 - \ The SCSI-2 standard requires disk devices to implement - \ the "read capacity" command. - - \ Retry it a few times just in case that helps + \ Try "read capacity" a few times. Support for that command is + \ mandatory, but some devices aren't ready for it immediately. d# 20 0 do - 8 read-capacity-cmd 0a short-data-command 0= if ( ) - dup 4 + 4c@ swap 4c@ 1+ false - unloop exit - then + get-capacity if unloop exit then ( ) d# 200 ms loop
+[ifdef] notdef + \ At least one device stalls read-capacity until the first block read + nonce-read + + \ Retry it a few more times + d# 18 0 do + get-capacity if unloop exit then ( ) + d# 200 ms + loop +[then] + \ If it fails, we just guess. Some devices violate the spec and \ fail to implement read_capacity d# 512 h# ffffffff false @@ -97,10 +126,13 @@
\ Return true for error, otherwise disk geometry and false : geometry ( -- true | sectors/track #heads #cylinders false ) - d# 36 mode-sense-geometry 6 short-data-command if true exit then >r - r@ d# 17 + c@ r@ d# 14 + 3c@ ( heads cylinders ) - 2dup * r> d# 4 + 4c@ ( heads cylinders heads*cylinders #blocks ) - swap / -rot ( sectors/track heads cylinders ) + d# 36 mode-sense-geometry 6 2 ( len cmd$ #retries ) + short-data-command if true exit then ( adr len ) + d# 36 <> if drop true exit then ( adr ) + >r ( r: adr ) + r@ d# 17 + c@ r@ d# 14 + 3c@ ( heads cylinders ) + 2dup * r> d# 4 + 4c@ ( heads cylinders heads*cylinders #blocks ) + swap / -rot ( sectors/track heads cylinders ) false ; [then] @@ -143,9 +175,9 @@ [then] swap ( addr dir cmdlen #blks ) dup >r ( addr input? cmdlen #blks ) - block-size * -rot cmdbuf swap -1 ( addr #bytes input? cmd cmdlen #retries ) - retry-command if ( [ sensebuf ] hw? ) - 0= if drop then r> drop 0 + block-size * -rot cmdbuf swap -1 ( data-adr,len in? cmd-adr,len #retries ) + retry-command? nip if ( r: #blks ) + r> drop 0 else r> then ( actual# ) @@ -162,8 +194,14 @@ \ Methods used by external clients
: open ( -- flag ) - my-unit " set-address" $call-parent + my-unit parent-set-address
+ \ Set timeout to 45 sec: some large (>1GB) drives take + \ up to 30 secs to spin up. + d# 45 d# 1000 * set-timeout + + unit-ready? 0= if false exit then + \ It might be a good idea to do an inquiry here to determine the \ device configuration, checking the result to see if the device \ really is a disk.