[openfirmware] r865 - dev/usb2/device/storage

svn at openfirmware.info svn at openfirmware.info
Sun Aug 3 01:19:51 CEST 2008


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.




More information about the openfirmware mailing list