# This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.564.2.4 -> 1.564.2.5 # drivers/usb/storage/shuttle_usbat.c 1.10 -> 1.11 # drivers/usb/storage/usb.c 1.38 -> 1.39 # drivers/usb/storage/transport.c 1.33 -> 1.34 # drivers/usb/storage/isd200.c 1.16 -> 1.17 # drivers/usb/storage/raw_bulk.c 1.3 -> 1.4 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/09/16 mdharm-usb@one-eyed-alien.net 1.564.2.5 # [PATCH] USB storage: add error checks, remove useless code # # This patch removes attempts to clear halts on a control endpoint (think # about it for a minute if you don't see why this is pointless....) and also # adds return-code checks for all places where halts are cleared. # # This _should_ be just redundant code, but recent tests suggest that this # is, in fact, not the case. People should _heavily_ test this patch. I'm # going to pause here for a while (in the patch stream) until we've got this # sorted out -- initial results on my test setup seem to show some problems # still remain. Where those problems are (HCD or usb-storage) remains to be # seen. # -------------------------------------------- # diff -Nru a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c --- a/drivers/usb/storage/isd200.c Mon Sep 16 15:00:59 2002 +++ b/drivers/usb/storage/isd200.c Mon Sep 16 15:00:59 2002 @@ -425,7 +425,8 @@ /* if we stall, we need to clear it before we go on */ if (result == -EPIPE) { US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); - usb_stor_clear_halt(us, pipe); + if (usb_stor_clear_halt(us, pipe) < 0) + return ISD200_TRANSPORT_FAILED; } /* did we send all the data? */ @@ -589,7 +590,8 @@ else if (result == -EPIPE) { /* if we stall, we need to clear it before we go on */ US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); - usb_stor_clear_halt(us, pipe); + if (usb_stor_clear_halt(us, pipe) < 0) + return ISD200_TRANSPORT_ERROR; } else if (result) return ISD200_TRANSPORT_ERROR; @@ -621,7 +623,8 @@ /* did the attempt to read the CSW fail? */ if (result == -EPIPE) { US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); - usb_stor_clear_halt(us, pipe); + if (usb_stor_clear_halt(us, pipe) < 0) + return ISD200_TRANSPORT_ERROR; /* get the status again */ US_DEBUGP("Attempting to get CSW (2nd try)...\n"); @@ -946,15 +949,6 @@ US_DEBUGP(" ISD200 Config Data was written successfully\n"); } else { US_DEBUGP(" Request to write ISD200 Config Data failed!\n"); - - /* STALL must be cleared when they are detected */ - if (result == -EPIPE) { - US_DEBUGP("-- Stall on control pipe. Clearing\n"); - result = usb_stor_clear_halt(us, - usb_sndctrlpipe(us->pusb_dev, 0)); - US_DEBUGP("-- usb_stor_clear_halt() returns %d\n", result); - - } retStatus = ISD200_ERROR; } @@ -1000,15 +994,6 @@ #endif } else { US_DEBUGP(" Request to get ISD200 Config Data failed!\n"); - - /* STALL must be cleared when they are detected */ - if (result == -EPIPE) { - US_DEBUGP("-- Stall on control pipe. Clearing\n"); - result = usb_stor_clear_halt(us, - usb_sndctrlpipe(us->pusb_dev, 0)); - US_DEBUGP("-- usb_stor_clear_halt() returns %d\n", result); - - } retStatus = ISD200_ERROR; } diff -Nru a/drivers/usb/storage/raw_bulk.c b/drivers/usb/storage/raw_bulk.c --- a/drivers/usb/storage/raw_bulk.c Mon Sep 16 15:00:59 2002 +++ b/drivers/usb/storage/raw_bulk.c Mon Sep 16 15:00:59 2002 @@ -67,12 +67,9 @@ // Check the return code for the command. if (result < 0) { - /* a stall is a fatal condition from the device */ + /* a stall indicates a protocol error */ if (result == -EPIPE) { - US_DEBUGP("-- Stall on control pipe. Clearing\n"); - result = usb_stor_clear_halt(us, pipe); - US_DEBUGP("-- usb_stor_clear_halt() returns %d\n", - result); + US_DEBUGP("-- Stall on control pipe\n"); return USB_STOR_TRANSPORT_FAILED; } @@ -102,7 +99,8 @@ US_DEBUGP("EPIPE: clearing endpoint halt for" " pipe 0x%x, stalled at %d bytes\n", pipe, *act_len); - usb_stor_clear_halt(us, pipe); + if (usb_stor_clear_halt(us, pipe) < 0) + return US_BULK_TRANSFER_FAILED; /* return US_BULK_TRANSFER_SHORT; */ } diff -Nru a/drivers/usb/storage/shuttle_usbat.c b/drivers/usb/storage/shuttle_usbat.c --- a/drivers/usb/storage/shuttle_usbat.c Mon Sep 16 15:00:59 2002 +++ b/drivers/usb/storage/shuttle_usbat.c Mon Sep 16 15:00:59 2002 @@ -348,10 +348,13 @@ * the bulk output pipe only the first time. */ - if (direction==SCSI_DATA_READ && i==0) - usb_stor_clear_halt(us, + if (direction==SCSI_DATA_READ && i==0) { + if (usb_stor_clear_halt(us, usb_sndbulkpipe(us->pusb_dev, - us->ep_out)); + us->ep_out)) < 0) + return USB_STOR_TRANSPORT_ERROR; + } + /* * Read status: is the device angry, or just busy? */ diff -Nru a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c --- a/drivers/usb/storage/transport.c Mon Sep 16 15:00:59 2002 +++ b/drivers/usb/storage/transport.c Mon Sep 16 15:00:59 2002 @@ -571,7 +571,8 @@ /* if we stall, we need to clear it before we go on */ if (result == -EPIPE) { US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); - usb_stor_clear_halt(us, pipe); + if (usb_stor_clear_halt(us, pipe) < 0) + return US_BULK_TRANSFER_FAILED; } /* did we abort this command? */ @@ -997,18 +998,9 @@ return US_BULK_TRANSFER_ABORTED; } - /* STALL must be cleared when it is detected */ + /* a stall indicates a protocol error */ if (result == -EPIPE) { - US_DEBUGP("-- Stall on control pipe. Clearing\n"); - result = usb_stor_clear_halt(us, - usb_sndctrlpipe(us->pusb_dev, 0)); - - /* did we abort this command? */ - if (atomic_read(&us->sm_state) == US_STATE_ABORTING) { - US_DEBUGP("usb_stor_control_msg(): transfer aborted\n"); - return US_BULK_TRANSFER_ABORTED; - } - + US_DEBUGP("-- Stall on control pipe\n"); return USB_STOR_TRANSPORT_FAILED; } @@ -1114,17 +1106,9 @@ return US_BULK_TRANSFER_ABORTED; } - /* a stall is a fatal condition from the device */ + /* a stall indicates a protocol error */ if (result == -EPIPE) { - US_DEBUGP("-- Stall on control pipe. Clearing\n"); - result = usb_stor_clear_halt(us, - usb_sndctrlpipe(us->pusb_dev, 0)); - - /* did we abort this command? */ - if (atomic_read(&us->sm_state) == US_STATE_ABORTING) { - US_DEBUGP("usb_stor_CB_transport(): transfer aborted\n"); - return US_BULK_TRANSFER_ABORTED; - } + US_DEBUGP("-- Stall on control pipe\n"); return USB_STOR_TRANSPORT_FAILED; } @@ -1182,15 +1166,6 @@ if (result == 1) return data; - /* if we get a STALL, clear the stall */ - if (result == -EPIPE) { - US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); - - /* Use usb_clear_halt() because this is not a - * scsi queued-command */ - usb_clear_halt(us->pusb_dev, pipe); - } - /* return the default -- no LUNs */ return 0; } @@ -1245,6 +1220,8 @@ US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n"); return US_BULK_TRANSFER_ABORTED; } + if (result < 0) + return USB_STOR_TRANSPORT_ERROR; result = -EPIPE; } else if (result) { /* unknown error -- we've got a problem */ @@ -1293,6 +1270,8 @@ US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n"); return US_BULK_TRANSFER_ABORTED; } + if (result < 0) + return USB_STOR_TRANSPORT_ERROR; /* get the status again */ US_DEBUGP("Attempting to get CSW (2nd try)...\n"); diff -Nru a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c --- a/drivers/usb/storage/usb.c Mon Sep 16 15:00:59 2002 +++ b/drivers/usb/storage/usb.c Mon Sep 16 15:00:59 2002 @@ -724,8 +724,7 @@ US_DEBUGP("Result from usb_set_configuration is %d\n", result); if (result == -EPIPE) { - US_DEBUGP("-- clearing stall on control interface\n"); - usb_clear_halt(dev, usb_sndctrlpipe(dev, 0)); + US_DEBUGP("-- stall on control interface\n"); } else if (result != 0) { /* it's not a stall, but another error -- time to bail */ US_DEBUGP("-- Unknown error. Rejecting device\n");