Commit bcee5ba0 authored by Matthew Dharm's avatar Matthew Dharm Committed by Greg Kroah-Hartman

[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.
parent b2bc6be6
...@@ -425,7 +425,8 @@ static int isd200_transfer_partial( struct us_data *us, ...@@ -425,7 +425,8 @@ static int isd200_transfer_partial( struct us_data *us,
/* if we stall, we need to clear it before we go on */ /* if we stall, we need to clear it before we go on */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); 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? */ /* did we send all the data? */
...@@ -589,7 +590,8 @@ int isd200_Bulk_transport( struct us_data *us, Scsi_Cmnd *srb, ...@@ -589,7 +590,8 @@ int isd200_Bulk_transport( struct us_data *us, Scsi_Cmnd *srb,
else if (result == -EPIPE) { else if (result == -EPIPE) {
/* if we stall, we need to clear it before we go on */ /* if we stall, we need to clear it before we go on */
US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); 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) } else if (result)
return ISD200_TRANSPORT_ERROR; return ISD200_TRANSPORT_ERROR;
...@@ -621,7 +623,8 @@ int isd200_Bulk_transport( struct us_data *us, Scsi_Cmnd *srb, ...@@ -621,7 +623,8 @@ int isd200_Bulk_transport( struct us_data *us, Scsi_Cmnd *srb,
/* did the attempt to read the CSW fail? */ /* did the attempt to read the CSW fail? */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); 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 */ /* get the status again */
US_DEBUGP("Attempting to get CSW (2nd try)...\n"); US_DEBUGP("Attempting to get CSW (2nd try)...\n");
...@@ -946,15 +949,6 @@ int isd200_write_config( struct us_data *us ) ...@@ -946,15 +949,6 @@ int isd200_write_config( struct us_data *us )
US_DEBUGP(" ISD200 Config Data was written successfully\n"); US_DEBUGP(" ISD200 Config Data was written successfully\n");
} else { } else {
US_DEBUGP(" Request to write ISD200 Config Data failed!\n"); 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; retStatus = ISD200_ERROR;
} }
...@@ -1000,15 +994,6 @@ int isd200_read_config( struct us_data *us ) ...@@ -1000,15 +994,6 @@ int isd200_read_config( struct us_data *us )
#endif #endif
} else { } else {
US_DEBUGP(" Request to get ISD200 Config Data failed!\n"); 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; retStatus = ISD200_ERROR;
} }
......
...@@ -67,12 +67,9 @@ usb_storage_send_control(struct us_data *us, ...@@ -67,12 +67,9 @@ usb_storage_send_control(struct us_data *us,
// Check the return code for the command. // Check the return code for the command.
if (result < 0) { if (result < 0) {
/* a stall is a fatal condition from the device */ /* a stall indicates a protocol error */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("-- Stall on control pipe. Clearing\n"); US_DEBUGP("-- Stall on control pipe\n");
result = usb_stor_clear_halt(us, pipe);
US_DEBUGP("-- usb_stor_clear_halt() returns %d\n",
result);
return USB_STOR_TRANSPORT_FAILED; return USB_STOR_TRANSPORT_FAILED;
} }
...@@ -102,7 +99,8 @@ usb_storage_raw_bulk(struct us_data *us, int direction, unsigned char *data, ...@@ -102,7 +99,8 @@ usb_storage_raw_bulk(struct us_data *us, int direction, unsigned char *data,
US_DEBUGP("EPIPE: clearing endpoint halt for" US_DEBUGP("EPIPE: clearing endpoint halt for"
" pipe 0x%x, stalled at %d bytes\n", " pipe 0x%x, stalled at %d bytes\n",
pipe, *act_len); 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; */ /* return US_BULK_TRANSFER_SHORT; */
} }
......
...@@ -348,10 +348,13 @@ int usbat_rw_block_test(struct us_data *us, ...@@ -348,10 +348,13 @@ int usbat_rw_block_test(struct us_data *us,
* the bulk output pipe only the first time. * the bulk output pipe only the first time.
*/ */
if (direction==SCSI_DATA_READ && i==0) if (direction==SCSI_DATA_READ && i==0) {
usb_stor_clear_halt(us, if (usb_stor_clear_halt(us,
usb_sndbulkpipe(us->pusb_dev, 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? * Read status: is the device angry, or just busy?
*/ */
......
...@@ -571,7 +571,8 @@ int usb_stor_transfer_partial(struct us_data *us, char *buf, int length) ...@@ -571,7 +571,8 @@ int usb_stor_transfer_partial(struct us_data *us, char *buf, int length)
/* if we stall, we need to clear it before we go on */ /* if we stall, we need to clear it before we go on */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("clearing endpoint halt for pipe 0x%x\n", pipe); 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? */ /* did we abort this command? */
...@@ -997,18 +998,9 @@ int usb_stor_CBI_transport(Scsi_Cmnd *srb, struct us_data *us) ...@@ -997,18 +998,9 @@ int usb_stor_CBI_transport(Scsi_Cmnd *srb, struct us_data *us)
return US_BULK_TRANSFER_ABORTED; return US_BULK_TRANSFER_ABORTED;
} }
/* STALL must be cleared when it is detected */ /* a stall indicates a protocol error */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("-- Stall on control pipe. Clearing\n"); US_DEBUGP("-- Stall on control pipe\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;
}
return USB_STOR_TRANSPORT_FAILED; return USB_STOR_TRANSPORT_FAILED;
} }
...@@ -1114,17 +1106,9 @@ int usb_stor_CB_transport(Scsi_Cmnd *srb, struct us_data *us) ...@@ -1114,17 +1106,9 @@ int usb_stor_CB_transport(Scsi_Cmnd *srb, struct us_data *us)
return US_BULK_TRANSFER_ABORTED; return US_BULK_TRANSFER_ABORTED;
} }
/* a stall is a fatal condition from the device */ /* a stall indicates a protocol error */
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("-- Stall on control pipe. Clearing\n"); US_DEBUGP("-- Stall on control pipe\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;
}
return USB_STOR_TRANSPORT_FAILED; return USB_STOR_TRANSPORT_FAILED;
} }
...@@ -1182,15 +1166,6 @@ int usb_stor_Bulk_max_lun(struct us_data *us) ...@@ -1182,15 +1166,6 @@ int usb_stor_Bulk_max_lun(struct us_data *us)
if (result == 1) if (result == 1)
return data; 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 the default -- no LUNs */
return 0; return 0;
} }
...@@ -1245,6 +1220,8 @@ int usb_stor_Bulk_transport(Scsi_Cmnd *srb, struct us_data *us) ...@@ -1245,6 +1220,8 @@ int usb_stor_Bulk_transport(Scsi_Cmnd *srb, struct us_data *us)
US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n"); US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n");
return US_BULK_TRANSFER_ABORTED; return US_BULK_TRANSFER_ABORTED;
} }
if (result < 0)
return USB_STOR_TRANSPORT_ERROR;
result = -EPIPE; result = -EPIPE;
} else if (result) { } else if (result) {
/* unknown error -- we've got a problem */ /* unknown error -- we've got a problem */
...@@ -1293,6 +1270,8 @@ int usb_stor_Bulk_transport(Scsi_Cmnd *srb, struct us_data *us) ...@@ -1293,6 +1270,8 @@ int usb_stor_Bulk_transport(Scsi_Cmnd *srb, struct us_data *us)
US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n"); US_DEBUGP("usb_stor_Bulk_transport(): transfer aborted\n");
return US_BULK_TRANSFER_ABORTED; return US_BULK_TRANSFER_ABORTED;
} }
if (result < 0)
return USB_STOR_TRANSPORT_ERROR;
/* get the status again */ /* get the status again */
US_DEBUGP("Attempting to get CSW (2nd try)...\n"); US_DEBUGP("Attempting to get CSW (2nd try)...\n");
......
...@@ -724,8 +724,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum, ...@@ -724,8 +724,7 @@ static void * storage_probe(struct usb_device *dev, unsigned int ifnum,
US_DEBUGP("Result from usb_set_configuration is %d\n", result); US_DEBUGP("Result from usb_set_configuration is %d\n", result);
if (result == -EPIPE) { if (result == -EPIPE) {
US_DEBUGP("-- clearing stall on control interface\n"); US_DEBUGP("-- stall on control interface\n");
usb_clear_halt(dev, usb_sndctrlpipe(dev, 0));
} else if (result != 0) { } else if (result != 0) {
/* it's not a stall, but another error -- time to bail */ /* it's not a stall, but another error -- time to bail */
US_DEBUGP("-- Unknown error. Rejecting device\n"); US_DEBUGP("-- Unknown error. Rejecting device\n");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment