| Message ID | 20181212143135.29147-2-lvivier@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb/storage: Implement block write support | expand |
On 2018-12-12 15:31, Laurent Vivier wrote: > to prepare write implementation > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > slof/fs/usb/dev-storage.fs | 39 +++++++++++++++++++------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/slof/fs/usb/dev-storage.fs b/slof/fs/usb/dev-storage.fs > index 94f8421..fe5af1c 100644 > --- a/slof/fs/usb/dev-storage.fs > +++ b/slof/fs/usb/dev-storage.fs > @@ -107,23 +107,23 @@ scsi-open > TO resp-size > TO resp-buffer > udev USB_PIPE_OUT td-buf td-buf-phys dma-buf-phys usb>cmd 1F > - usb-transfer-bulk IF \ transfer CBW > - resp-size IF > - d# 125 us > - udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size > - usb-transfer-bulk 1 = not IF \ transfer data > - usb-disk-debug? IF ." Data phase failed " cr THEN > - \ FALSE EXIT > - \ in case of a stall/halted endpoint we clear the halt > - \ Fall through and try reading the CSW > - THEN > - THEN > - d# 125 us > - udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D > - usb-transfer-bulk \ transfer CSW > - ELSE > - FALSE EXIT > + usb-transfer-bulk 1 = not IF In case you respin: Replace "= not" with "<>". (sorry for not noticing this in v1 already!) > + FALSE EXIT > THEN > + \ transfer CBW > + resp-size IF > + d# 125 us > + udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size > + usb-transfer-bulk 1 = not IF \ transfer data You only moved the code, but in case you respin, "= not" could be replaced with "<>" here, too. > + usb-disk-debug? IF ." Data phase failed " cr THEN > + \ FALSE EXIT > + \ in case of a stall/halted endpoint we clear the halt > + \ Fall through and try reading the CSW > + THEN > + THEN > + d# 125 us > + udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D > + usb-transfer-bulk \ transfer CSW > ; > > STRUCT \ cbw > @@ -189,12 +189,11 @@ CONSTANT cbw-length > > \ Send it > dma-buf-phys usb>data usb-buf-len > - do-bulk-command IF > - dma-buf usb>data usb-buf-addr usb-buf-len move > - ELSE > - ." USB-DISK: Bulk commad failed!" cr > + do-bulk-command 1 = not IF "= not" ==> "<>" Or maybe you could even simply say: do-bulk-command not IF ... since the C functions are simply returning "bool"s - and the IF-statement in Forth does not care whether the "true" value on the stack is -1 or 1. > + ." USB-DISK: Bulk command failed!" cr > 0 0 -1 EXIT > THEN > + dma-buf usb>data usb-buf-addr usb-buf-len move > > dma-buf usb>csw to csw-addr > csw-addr csw>sig l@ 55534253 <> IF > The code looks indeed better the way you cleaned this up! With or without the "= not" rework: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 12/12/2018 20:23, Thomas Huth wrote: > On 2018-12-12 15:31, Laurent Vivier wrote: >> to prepare write implementation >> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >> --- >> slof/fs/usb/dev-storage.fs | 39 +++++++++++++++++++------------------- >> 1 file changed, 19 insertions(+), 20 deletions(-) >> >> diff --git a/slof/fs/usb/dev-storage.fs b/slof/fs/usb/dev-storage.fs >> index 94f8421..fe5af1c 100644 >> --- a/slof/fs/usb/dev-storage.fs >> +++ b/slof/fs/usb/dev-storage.fs >> @@ -107,23 +107,23 @@ scsi-open >> TO resp-size >> TO resp-buffer >> udev USB_PIPE_OUT td-buf td-buf-phys dma-buf-phys usb>cmd 1F >> - usb-transfer-bulk IF \ transfer CBW >> - resp-size IF >> - d# 125 us >> - udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size >> - usb-transfer-bulk 1 = not IF \ transfer data >> - usb-disk-debug? IF ." Data phase failed " cr THEN >> - \ FALSE EXIT >> - \ in case of a stall/halted endpoint we clear the halt >> - \ Fall through and try reading the CSW >> - THEN >> - THEN >> - d# 125 us >> - udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D >> - usb-transfer-bulk \ transfer CSW >> - ELSE >> - FALSE EXIT >> + usb-transfer-bulk 1 = not IF > > In case you respin: Replace "= not" with "<>". > (sorry for not noticing this in v1 already!) > >> + FALSE EXIT >> THEN >> + \ transfer CBW >> + resp-size IF >> + d# 125 us >> + udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size >> + usb-transfer-bulk 1 = not IF \ transfer data > > You only moved the code, but in case you respin, "= not" could be > replaced with "<>" here, too. > >> + usb-disk-debug? IF ." Data phase failed " cr THEN >> + \ FALSE EXIT >> + \ in case of a stall/halted endpoint we clear the halt >> + \ Fall through and try reading the CSW >> + THEN >> + THEN >> + d# 125 us >> + udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D >> + usb-transfer-bulk \ transfer CSW >> ; >> >> STRUCT \ cbw >> @@ -189,12 +189,11 @@ CONSTANT cbw-length >> >> \ Send it >> dma-buf-phys usb>data usb-buf-len >> - do-bulk-command IF >> - dma-buf usb>data usb-buf-addr usb-buf-len move >> - ELSE >> - ." USB-DISK: Bulk commad failed!" cr >> + do-bulk-command 1 = not IF > > "= not" ==> "<>" > > Or maybe you could even simply say: > > do-bulk-command not IF > > ... since the C functions are simply returning "bool"s - and the > IF-statement in Forth does not care whether the "true" value on the > stack is -1 or 1. ... but "not" doesn't like it: 0 > 0 not . -1 ok 0 > 1 not . -2 ok 0 > -1 not . 0 ok I'm going to change the "=" by "<>" in the series. Thanks, Laurent
On 2018-12-12 21:02, Laurent Vivier wrote: > On 12/12/2018 20:23, Thomas Huth wrote: >> On 2018-12-12 15:31, Laurent Vivier wrote: >>> to prepare write implementation >>> >>> Signed-off-by: Laurent Vivier <lvivier@redhat.com> >>> --- >>> slof/fs/usb/dev-storage.fs | 39 +++++++++++++++++++------------------- >>> 1 file changed, 19 insertions(+), 20 deletions(-) >>> >>> diff --git a/slof/fs/usb/dev-storage.fs b/slof/fs/usb/dev-storage.fs >>> index 94f8421..fe5af1c 100644 >>> --- a/slof/fs/usb/dev-storage.fs >>> +++ b/slof/fs/usb/dev-storage.fs >>> @@ -107,23 +107,23 @@ scsi-open >>> TO resp-size >>> TO resp-buffer >>> udev USB_PIPE_OUT td-buf td-buf-phys dma-buf-phys usb>cmd 1F >>> - usb-transfer-bulk IF \ transfer CBW >>> - resp-size IF >>> - d# 125 us >>> - udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size >>> - usb-transfer-bulk 1 = not IF \ transfer data >>> - usb-disk-debug? IF ." Data phase failed " cr THEN >>> - \ FALSE EXIT >>> - \ in case of a stall/halted endpoint we clear the halt >>> - \ Fall through and try reading the CSW >>> - THEN >>> - THEN >>> - d# 125 us >>> - udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D >>> - usb-transfer-bulk \ transfer CSW >>> - ELSE >>> - FALSE EXIT >>> + usb-transfer-bulk 1 = not IF >> >> In case you respin: Replace "= not" with "<>". >> (sorry for not noticing this in v1 already!) >> >>> + FALSE EXIT >>> THEN >>> + \ transfer CBW >>> + resp-size IF >>> + d# 125 us >>> + udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size >>> + usb-transfer-bulk 1 = not IF \ transfer data >> >> You only moved the code, but in case you respin, "= not" could be >> replaced with "<>" here, too. >> >>> + usb-disk-debug? IF ." Data phase failed " cr THEN >>> + \ FALSE EXIT >>> + \ in case of a stall/halted endpoint we clear the halt >>> + \ Fall through and try reading the CSW >>> + THEN >>> + THEN >>> + d# 125 us >>> + udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D >>> + usb-transfer-bulk \ transfer CSW >>> ; >>> >>> STRUCT \ cbw >>> @@ -189,12 +189,11 @@ CONSTANT cbw-length >>> >>> \ Send it >>> dma-buf-phys usb>data usb-buf-len >>> - do-bulk-command IF >>> - dma-buf usb>data usb-buf-addr usb-buf-len move >>> - ELSE >>> - ." USB-DISK: Bulk commad failed!" cr >>> + do-bulk-command 1 = not IF >> >> "= not" ==> "<>" >> >> Or maybe you could even simply say: >> >> do-bulk-command not IF >> >> ... since the C functions are simply returning "bool"s - and the >> IF-statement in Forth does not care whether the "true" value on the >> stack is -1 or 1. > > ... but "not" doesn't like it: > > 0 > 0 not . -1 ok > 0 > 1 not . -2 ok > 0 > -1 not . 0 ok D'oh, sorry, I mix this up every couple of months ... you've got to use "0=" instead of "not" in that case: 0 > 0 0= . -1 ok 0 > 1 0= . 0 ok 0 > -1 0= . 0 ok Thomas
diff --git a/slof/fs/usb/dev-storage.fs b/slof/fs/usb/dev-storage.fs index 94f8421..fe5af1c 100644 --- a/slof/fs/usb/dev-storage.fs +++ b/slof/fs/usb/dev-storage.fs @@ -107,23 +107,23 @@ scsi-open TO resp-size TO resp-buffer udev USB_PIPE_OUT td-buf td-buf-phys dma-buf-phys usb>cmd 1F - usb-transfer-bulk IF \ transfer CBW - resp-size IF - d# 125 us - udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size - usb-transfer-bulk 1 = not IF \ transfer data - usb-disk-debug? IF ." Data phase failed " cr THEN - \ FALSE EXIT - \ in case of a stall/halted endpoint we clear the halt - \ Fall through and try reading the CSW - THEN - THEN - d# 125 us - udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D - usb-transfer-bulk \ transfer CSW - ELSE - FALSE EXIT + usb-transfer-bulk 1 = not IF + FALSE EXIT THEN + \ transfer CBW + resp-size IF + d# 125 us + udev USB_PIPE_IN td-buf td-buf-phys resp-buffer resp-size + usb-transfer-bulk 1 = not IF \ transfer data + usb-disk-debug? IF ." Data phase failed " cr THEN + \ FALSE EXIT + \ in case of a stall/halted endpoint we clear the halt + \ Fall through and try reading the CSW + THEN + THEN + d# 125 us + udev USB_PIPE_IN td-buf td-buf-phys dma-buf-phys usb>csw 0D + usb-transfer-bulk \ transfer CSW ; STRUCT \ cbw @@ -189,12 +189,11 @@ CONSTANT cbw-length \ Send it dma-buf-phys usb>data usb-buf-len - do-bulk-command IF - dma-buf usb>data usb-buf-addr usb-buf-len move - ELSE - ." USB-DISK: Bulk commad failed!" cr + do-bulk-command 1 = not IF + ." USB-DISK: Bulk command failed!" cr 0 0 -1 EXIT THEN + dma-buf usb>data usb-buf-addr usb-buf-len move dma-buf usb>csw to csw-addr csw-addr csw>sig l@ 55534253 <> IF
to prepare write implementation Signed-off-by: Laurent Vivier <lvivier@redhat.com> --- slof/fs/usb/dev-storage.fs | 39 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 20 deletions(-)