Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-10 Thread Vishal Annapurve
Hi Greg,

There was problem with my email client.
Hope now it is fine.

---
[PATCH 1/3] usb: storage: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.

As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/usb/storage/cypress_atacb.c |  1 +
 drivers/usb/storage/isd200.c|  2 +-
 drivers/usb/storage/transport.c |  8 
 drivers/usb/storage/usb.c   | 10 ++
 4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c 
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct scsi_cmnd 
*srb, struct us_data *us)
 */
if ((srb-result != (DID_ERROR  16) 
srb-result != (DID_ABORT  16)) 
+   srb-result != (DID_TIME_OUT  16) 
save_cmnd[2]  0x20) {
struct scsi_eh_save ses;
unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
/* abort processing: the bulk-only transport requires a reset
 * following an abort */
Handle_Abort:
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;

/* permit the reset transfer to take place */
clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
 * short-circuit all other processing
 */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   usb_stor_dbg(us, -- command was aborted\n);
-   srb-result = DID_ABORT  16;
+   usb_stor_dbg(-- command was aborted because of timeout\n);
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}

@@ -717,8 +717,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, ses);

if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   usb_stor_dbg(us, -- auto-sense aborted\n);
-   srb-result = DID_ABORT  16;
+   usb_stor_dbg(-- auto-sense aborted due to timeout\n);
+   srb-result = DID_TIME_OUT  16;

/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)

/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   us-srb-result = DID_ABORT  16;
+   us-srb-result = DID_TIME_OUT  16;
goto SkipForAbort;
}

@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);

/* indicate that the command is done */
-   if (us-srb-result != DID_ABORT  16) {
+   if ((us-srb-result != DID_ABORT  16) 
+   (us-srb-result != DID_TIME_OUT  16)) {
usb_stor_dbg(us, scsi cmd done, result=0x%x\n,
 us-srb-result);
us-srb-scsi_done(us-srb);
@@ -390,8 +391,9 @@ SkipForAbort:

/* If an abort request was received we need to signal that
 * the abort has finished.  The proper test for this is
-* the TIMED_OUT flag, not srb-result == DID_ABORT, because
-* the timeout might have occurred after the command had
+* the TIMED_OUT flag, not srb-result == DID_ABORT or
+* srb-result 

Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Greg KH
On Sat, Nov 16, 2013 at 12:23:50PM +0530, Vishal Annapurve wrote:
 Hi,
 
 Here are the updated patches: 

Can you please resend these in a format which I can apply them in
without having to hand-edit all three of them?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Vishal Annapurve

Hi Greg,

Attached are the patches.

Regards,
Vishal

On Monday 09 December 2013 07:29 AM, Greg KH wrote:

On Sat, Nov 16, 2013 at 12:23:50PM +0530, Vishal Annapurve wrote:

Hi,

Here are the updated patches:   

Can you please resend these in a format which I can apply them in
without having to hand-edit all three of them?

thanks,

greg k-h




01-PATCH-usb-storage-Proper-cmd-result-assignment.patch.gz
Description: GNU Zip compressed data


02-keucr-Proper-cmd-result-assignment.patch.gz
Description: GNU Zip compressed data


03-usb-storage-uas-Proper-cmd-result-assignment.patch.gz
Description: GNU Zip compressed data


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Oliver Neukum
On Mon, 2013-12-09 at 15:14 +0530, Vishal Annapurve wrote:
 Hi Greg,
 
 Attached are the patches.
Hi,

please send patches in the clear, inline, one patch per mail
and with a Signed-off-by line, as described in the Documentation
directory of the kernel.

Regards
Oliver


--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Greg KH
On Mon, Dec 09, 2013 at 03:14:07PM +0530, Vishal Annapurve wrote:
 Hi Greg,
 
 Attached are the patches.

As Oliver said, I can't take these, please read
Documentation/SubmittingPatches for how to do this properly.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Vishal Annapurve

Hi Greg,

Does this look fine? I will send over other patches
once you confirm.

Patch set 1:
-
[PATCH 1/3] usb: storage: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.

As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/usb/storage/cypress_atacb.c |  1 +
 drivers/usb/storage/isd200.c|  2 +-
 drivers/usb/storage/transport.c |  8 
 drivers/usb/storage/usb.c   | 10 ++
 4 files changed, 12 insertions(+), 9 deletions(-)
---
diff --git a/drivers/usb/storage/cypress_atacb.c 
b/drivers/usb/storage/cypress_atacb.c

index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -168,6 +168,7 @@ static void cypress_atacb_passthrough(struct 
scsi_cmnd *srb, struct us_data *us)

  */
 if ((srb-result != (DID_ERROR  16) 
 srb-result != (DID_ABORT  16)) 
+srb-result != (DID_TIME_OUT  16) 
 save_cmnd[2]  0x20) {
 struct scsi_eh_save ses;
 unsigned char regs[8];
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 599d8bf..ffd5d58 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -703,7 +703,7 @@ static void isd200_invoke_transport( struct us_data *us,
 /* abort processing: the bulk-only transport requires a reset
  * following an abort */
 Handle_Abort:
-srb-result = DID_ABORT  16;
+srb-result = DID_TIME_OUT  16;

 /* permit the reset transfer to take place */
 clear_bit(US_FLIDX_ABORTING, us-dflags);
diff --git a/drivers/usb/storage/transport.c 
b/drivers/usb/storage/transport.c

index 22c7d43..6a90161 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -607,8 +607,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd 
*srb, struct us_data *us)

  * short-circuit all other processing
  */
 if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-usb_stor_dbg(us, -- command was aborted\n);
-srb-result = DID_ABORT  16;
+usb_stor_dbg(-- command was aborted because of timeout\n);
+srb-result = DID_TIME_OUT  16;
 goto Handle_Errors;
 }

@@ -717,8 +717,8 @@ Retry_Sense:
 scsi_eh_restore_cmnd(srb, ses);

 if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-usb_stor_dbg(us, -- auto-sense aborted\n);
-srb-result = DID_ABORT  16;
+usb_stor_dbg(-- auto-sense aborted due to timeout\n);
+srb-result = DID_TIME_OUT  16;

 /* If SANE_SENSE caused this problem, disable it */
 if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index 5c4fe07..04a68eb 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -325,7 +325,7 @@ static int usb_stor_control_thread(void * __us)

 /* has the command timed out *already* ? */
 if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-us-srb-result = DID_ABORT  16;
+us-srb-result = DID_TIME_OUT  16;
 goto SkipForAbort;
 }

@@ -379,7 +379,8 @@ static int usb_stor_control_thread(void * __us)
 scsi_lock(host);

 /* indicate that the command is done */
-if (us-srb-result != DID_ABORT  16) {
+if ((us-srb-result != DID_ABORT  16) 
+(us-srb-result != DID_TIME_OUT  16)) {
 usb_stor_dbg(us, scsi cmd done, result=0x%x\n,
  us-srb-result);
 us-srb-scsi_done(us-srb);
@@ -390,8 +391,9 @@ SkipForAbort:

 /* If an abort request was received we need to signal that
  * the abort has finished.  The proper test for this is
- * the TIMED_OUT flag, not srb-result == DID_ABORT, because
- * the timeout might have occurred after the command had
+ * the TIMED_OUT flag, not srb-result == DID_ABORT or
+ * srb-result == DID_TIMEOUT , because the timeout might
+ * have occurred after the command had
  * already completed with a different result code. */
 if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
 complete((us-notify));
---

Regards,
Vishal

On Monday 09 December 2013 04:53 PM, Greg KH wrote:

On Mon, Dec 09, 2013 at 

Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-12-09 Thread Greg KH
On Tue, Dec 10, 2013 at 10:53:59AM +0530, Vishal Annapurve wrote:
 Hi Greg,
 
 Does this look fine? I will send over other patches
 once you confirm.

I'm not going to confirm a broken format...

Please go read the file I pointed you at as to how to properly do
this.

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-11-18 Thread Alan Stern
On Sat, 16 Nov 2013, Vishal Annapurve wrote:

 Hi,
 
 Here are the updated patches: 
 
 [PATCH 1/3] usb: storage: Proper cmd result assignment
 
 This change replaces DID_ABORT with DID_TIMEOUT as a command result
 whenever US_FLIDX_TIMED_OUT bit is set.
 
 This change is made to bring USB storage inline with a recent change:
 
 commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
 [SCSI] Handle disk devices which can not process medium access commands
 We have experienced several devices which fail in a fashion we do not
 currently handle gracefully in SCSI. After a failure these devices will
 respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
 but any command accessing the storage medium will time out.
 
 As the USB storage was setting command result as aborted rather than
 timed out, SCSI layer was not recognizing the above mentioned failure
 pattern.
 
 Signed-off-by: Vishal Annapurve vannapu...@nvidia.com

Acked-by: Alan Stern st...@rowland.harvard.edu

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-11-15 Thread Vishal Annapurve
 the timeout might
+* have occurred after the command had
 * already completed with a different result code. */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
complete((us-notify));
-- 
1.8.4.2

Regards,
Vishal

-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Sunday, October 27, 2013 1:04 AM
To: Vishal Annapurve
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

On Sat, 26 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 Here is the new patch:
 
 From: Vishal Annapurve vannapu...@nvidia.com
 Date: Sat, 26 Oct 2013 21:10:11 +0530
 Subject: [PATCH] usb: storage: Proper cmd result assignment
 
 This change replaces DID_ABORT with DID_TIMEOUT as a command result 
 whenever US_FLIDX_TIMED_OUT bit is set.
 
 This change is made to bring USB storage inline with a recent change:
 
 commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
 [SCSI] Handle disk devices which can not process medium access 
 commands We have experienced several devices which fail in a fashion 
 we do not currently handle gracefully in SCSI. After a failure these 
 devices will respond to the SCSI primary command set (INQUIRY, TEST 
 UNIT READY, etc.) but any command accessing the storage medium will time out.
 
 As the USB storage was setting command result as aborted rather than 
 timed out, SCSI layer was not recognizing the above mentioned failure 
 pattern.
 
 Change-Id: Ic58e2247fed11649f4dbea56382354ba2fe0be1b
 ---
  drivers/staging/keucr/transport.c   | 6 +++---
  drivers/staging/keucr/usb.c | 5 +++--

Those two files aren't part of usb-storage; they belong to a different driver.  
It would be better to have a separate patch for them, and you should copy that 
patch to the driver's maintainer.

  drivers/usb/storage/cypress_atacb.c | 1 +
  drivers/usb/storage/isd200.c| 2 +-
  drivers/usb/storage/transport.c | 8 
  drivers/usb/storage/usb.c   | 9 +
  6 files changed, 17 insertions(+), 14 deletions(-)

Otherwise this is okay.  You might also want to submit a third patch for the 
uas driver.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-11-15 Thread Vishal Annapurve
Hi,

[PATCH 2/3] keucr: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.

As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/staging/keucr/transport.c | 6 +++---
 drivers/staging/keucr/usb.c   | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/keucr/transport.c 
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
 
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
 
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- auto-sense aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) {
@@ -488,7 +488,7 @@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
 
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c
index a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
 
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   us-srb-result = DID_ABORT  16;
+   us-srb-result = DID_TIME_OUT  16;
goto SkipForAbort;
}
 
@@ -209,7 +209,8 @@ static int usb_stor_control_thread(void *__us)
scsi_lock(host);
 
/* indicate that the command is done */
-   if (us-srb-result != DID_ABORT  16) {
+   if ((us-srb-result != DID_ABORT  16) 
+   (us-srb-result != DID_TIME_OUT  16)) {
us-srb-scsi_done(us-srb);
} else {
 SkipForAbort:
-- 
1.8.4.2

Regards,
Vishal

-Original Message-
From: Vishal Annapurve 
Sent: Saturday, November 16, 2013 12:24 PM
To: 'Alan Stern'
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

Hi,

Here are the updated patches:   

[PATCH 1/3] usb: storage: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever 
US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands We have 
experienced several devices which fail in a fashion we do not currently handle 
gracefully in SCSI. After a failure these devices will respond to the SCSI 
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing 
the storage medium will time out.

As the USB storage was setting command result as aborted rather than timed out, 
SCSI layer was not recognizing the above mentioned failure pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/usb/storage/cypress_atacb.c |  1 +
 drivers/usb/storage/isd200.c|  2 +-
 drivers/usb/storage/transport.c |  8 
 drivers/usb/storage/usb.c   | 10 ++
 4 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/storage/cypress_atacb.c 
b/drivers/usb/storage/cypress_atacb.c
index 8514a2d..3477ca19 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage

RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-11-15 Thread Vishal Annapurve
Hi,

[PATCH 3/3] usb: storage: uas: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result
whenever US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands
We have experienced several devices which fail in a fashion we do not
currently handle gracefully in SCSI. After a failure these devices will
respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
but any command accessing the storage medium will time out.

As the USB storage was setting command result as aborted rather than
timed out, SCSI layer was not recognizing the above mentioned failure
pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/usb/storage/uas.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index d966b59..3a4a44e 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -271,7 +271,10 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
usb_free_urb(cmdinfo-data_out_urb);
if (cmdinfo-state  COMMAND_ABORTED) {
scmd_printk(KERN_INFO, cmnd, abort completed\n);
-   cmnd-result = DID_ABORT  16;
+   /* Better to check for US_FLIDX_TIMED_OUT
+* bit settings.
+*/
+   cmnd-result = DID_TIME_OUT  16;
}
cmnd-scsi_done(cmnd);
return 0;
-- 
1.8.4.2

Regards,
Vishal

-Original Message-
From: Vishal Annapurve 
Sent: Saturday, November 16, 2013 12:25 PM
To: 'Alan Stern'
Cc: 'Ming Lei'; 'Linux Kernel Mailing List'; 'linux-usb'
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

Hi,

[PATCH 2/3] keucr: Proper cmd result assignment

This change replaces DID_ABORT with DID_TIMEOUT as a command result whenever 
US_FLIDX_TIMED_OUT bit is set.

This change is made to bring USB storage inline with a recent change:

commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

[SCSI] Handle disk devices which can not process medium access commands We have 
experienced several devices which fail in a fashion we do not currently handle 
gracefully in SCSI. After a failure these devices will respond to the SCSI 
primary command set (INQUIRY, TEST UNIT READY, etc.) but any command accessing 
the storage medium will time out.

As the USB storage was setting command result as aborted rather than timed out, 
SCSI layer was not recognizing the above mentioned failure pattern.

Signed-off-by: Vishal Annapurve vannapu...@nvidia.com
---
 drivers/staging/keucr/transport.c | 6 +++---
 drivers/staging/keucr/usb.c   | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/keucr/transport.c 
b/drivers/staging/keucr/transport.c
index aeb2186..6f61c20 100644
--- a/drivers/staging/keucr/transport.c
+++ b/drivers/staging/keucr/transport.c
@@ -353,7 +353,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
 
@@ -412,7 +412,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
 
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- auto-sense aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
if (temp_result != USB_STOR_TRANSPORT_GOOD) { @@ -488,7 +488,7 
@@ void ENE_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
we need to short-circuit all other processing */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
/* pr_info(-- command was aborted\n); */
-   srb-result = DID_ABORT  16;
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
 
diff --git a/drivers/staging/keucr/usb.c b/drivers/staging/keucr/usb.c index 
a84ee63..26ec64c 100644
--- a/drivers/staging/keucr/usb.c
+++ b/drivers/staging/keucr/usb.c
@@ -181,7 +181,7 @@ static int usb_stor_control_thread(void *__us)
 
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   us-srb-result = DID_ABORT  16;
+   us-srb-result = DID_TIME_OUT  16;
goto SkipForAbort;
}
 
@@ -209,7 +209,8 @@ static int usb_stor_control_thread(void *__us)
scsi_lock(host

RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-26 Thread Vishal Annapurve
..f68e4c6 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -608,8 +608,8 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, 
struct us_data *us)
 * short-circuit all other processing
 */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   US_DEBUGP(-- command was aborted\n);
-   srb-result = DID_ABORT  16;
+   US_DEBUGP(-- command was aborted because of timeout\n);
+   srb-result = DID_TIME_OUT  16;
goto Handle_Errors;
}
 
@@ -721,8 +721,8 @@ Retry_Sense:
scsi_eh_restore_cmnd(srb, ses);
 
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   US_DEBUGP(-- auto-sense aborted\n);
-   srb-result = DID_ABORT  16;
+   US_DEBUGP(-- auto-sense aborted due to timeout\n);
+   srb-result = DID_TIME_OUT  16;
 
/* If SANE_SENSE caused this problem, disable it */
if (sense_size != US_SENSE_SIZE) {
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index c0d0915..64eaca6 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -290,7 +290,7 @@ static int usb_stor_control_thread(void * __us)
 
/* has the command timed out *already* ? */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-   us-srb-result = DID_ABORT  16;
+   us-srb-result = DID_TIME_OUT  16;
goto SkipForAbort;
}
 
@@ -344,18 +344,19 @@ static int usb_stor_control_thread(void * __us)
scsi_lock(host);
 
/* indicate that the command is done */
-   if (us-srb-result != DID_ABORT  16) {
+   if ((us-srb-result != (DID_TIME_OUT  16)) 
+   (us-srb-result != (DID_ABORT  16)))  {
US_DEBUGP(scsi cmd done, result=0x%x\n, 
   us-srb-result);
us-srb-scsi_done(us-srb);
} else {
 SkipForAbort:
-   US_DEBUGP(scsi command aborted\n);
+   US_DEBUGP(scsi command aborted due to timeout\n);
}
 
/* If an abort request was received we need to signal that
 * the abort has finished.  The proper test for this is
-* the TIMED_OUT flag, not srb-result == DID_ABORT, because
+* the TIMED_OUT flag, not srb-result == DID_TIME_OUT, because
 * the timeout might have occurred after the command had
 * already completed with a different result code. */
if (test_bit(US_FLIDX_TIMED_OUT, us-dflags)) {
-- 
1.8.4

Regards,
Vishal

-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Monday, October 21, 2013 7:23 PM
To: Vishal Annapurve
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

On Sat, 19 Oct 2013, Vishal Annapurve wrote:

 Hi,
 
 Attaching the new patch which will replace all the occurrences of 
 DID_ABORT with DID_TIMOUT in USB Storage whenever it sees 
 US_FLIDX_TIMED_OUT bit is set.

It seems okay, but you forgot to update the isd200.c and cypress_atacb.c files.

Also, in the future please put your patches inline with the rest of the email, 
not as a separate attachment.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-26 Thread Alan Stern
On Sat, 26 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 Here is the new patch:
 
 From: Vishal Annapurve vannapu...@nvidia.com
 Date: Sat, 26 Oct 2013 21:10:11 +0530
 Subject: [PATCH] usb: storage: Proper cmd result assignment
 
 This change replaces DID_ABORT with DID_TIMEOUT as a command result
 whenever US_FLIDX_TIMED_OUT bit is set.
 
 This change is made to bring USB storage inline with a recent change:
 
 commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
 [SCSI] Handle disk devices which can not process medium access commands
 We have experienced several devices which fail in a fashion we do not
 currently handle gracefully in SCSI. After a failure these devices will
 respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
 but any command accessing the storage medium will time out.
 
 As the USB storage was setting command result as aborted rather than
 timed out, SCSI layer was not recognizing the above mentioned failure
 pattern.
 
 Change-Id: Ic58e2247fed11649f4dbea56382354ba2fe0be1b
 ---
  drivers/staging/keucr/transport.c   | 6 +++---
  drivers/staging/keucr/usb.c | 5 +++--

Those two files aren't part of usb-storage; they belong to a different 
driver.  It would be better to have a separate patch for them, and you 
should copy that patch to the driver's maintainer.

  drivers/usb/storage/cypress_atacb.c | 1 +
  drivers/usb/storage/isd200.c| 2 +-
  drivers/usb/storage/transport.c | 8 
  drivers/usb/storage/usb.c   | 9 +
  6 files changed, 17 insertions(+), 14 deletions(-)

Otherwise this is okay.  You might also want to submit a third patch 
for the uas driver.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-26 Thread Greg KH
On Sat, 26 Oct 2013, Vishal Annapurve wrote:
 Hi Alan,
 
 Here is the new patch:
 
 From: Vishal Annapurve vannapu...@nvidia.com
 Date: Sat, 26 Oct 2013 21:10:11 +0530
 Subject: [PATCH] usb: storage: Proper cmd result assignment
 
 This change replaces DID_ABORT with DID_TIMEOUT as a command result
 whenever US_FLIDX_TIMED_OUT bit is set.
 
 This change is made to bring USB storage inline with a recent change:
 
 commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
 [SCSI] Handle disk devices which can not process medium access commands
 We have experienced several devices which fail in a fashion we do not
 currently handle gracefully in SCSI. After a failure these devices will
 respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
 but any command accessing the storage medium will time out.
 
 As the USB storage was setting command result as aborted rather than
 timed out, SCSI layer was not recognizing the above mentioned failure
 pattern.
 
 Change-Id: Ic58e2247fed11649f4dbea56382354ba2fe0be1b

What's this line for?  (yeah, I know where it comes from, the point is
it doesn't belong here...)

Also, no signed-off-by, so I can't apply it...

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-18 Thread Alan Stern
On Fri, 18 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more
 meanings than timed out then maybe it would be best to override the
 results after usb-storage is done with the command maybe in scsi layer
 itself who aborted it in the first place.

US_FLIDX_TIMED_OUT has one very specific meaning: command_abort() was
called.

Since command_abort() is the .eh_abort_handler routine, 
US_FLIDX_TIMED_OUT means that the SCSI layer decided to abort the 
command.

Does the SCSI layer ever abort a command for any reason other than a
timeout?  If not, you may conclude that US_FLIDX_TIMED_OUT indicates 
a timeout.  But if it does, you should not make this conclusion.

 My concern was that overriding the result in usb storage or scsi layers
 will have more side effects than doing it in scsiglue.c.
 And by scsi-usb storage bridge what I meant was specifically the code in 
 scsiglue.
 
 Question about your last mail, do you want to change all the occurrences of
 DID_ABORT from usb-storage to DID_TIMEOUT?

Put it this way: There's no good reason for changing some of them but 
not all of them.

And if you're going to change them at all, it makes no sense to first 
set the result to DID_ABORT and then change it to DID_TIMEOUT.  You 
should simply set it to DID_TIMEOUT in the first place.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-17 Thread Vishal Annapurve
Hi Alan,

What I wanted to say was If the bit US_FLIDX_TIMED_OUT can have more
meanings than timed out then maybe it would be best to override the
results after usb-storage is done with the command maybe in scsi layer
itself who aborted it in the first place.

My concern was that overriding the result in usb storage or scsi layers
will have more side effects than doing it in scsiglue.c.
And by scsi-usb storage bridge what I meant was specifically the code in 
scsiglue.

Question about your last mail, do you want to change all the occurrences of
DID_ABORT from usb-storage to DID_TIMEOUT?

Regards,
Vishal

-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Tuesday, October 15, 2013 10:22 PM
To: Vishal Annapurve
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

On Tue, 15 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 USB storage maybe just has to say that the abort occurred. By setting 
 the US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the 
 reason was time out and the command is being aborted.

No.  By setting the US_FLIDX_TIMED_OUT bit, usb-storage indicates that the 
command was aborted.  This doesn't indicate anything about the reason for the 
abort.  (Maybe this bit's name wasn't chosen very well.)

 Now, it's arguable whether to change the implication of 
 US_FLIDX_TIMED_OUT bit for scsi - USB storage bridge or for entire usb 
 storage

I don't understand this.  What's the difference between scsi - USB storage 
bridge and entire usb storage?  Aren't they the same thing?

  Or maybe scsi has
 decided to abort so it should override the result.

Of course the SCSI midlayer has decided to abort.  That's the only way this bit 
can get set.  But usb-storage doesn't know why SCSI decided to abort.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-16 Thread Alan Stern
On Wed, 16 Oct 2013, Ming Lei wrote:

  Of course the SCSI midlayer has decided to abort.  That's the only way
  this bit can get set.  But usb-storage doesn't know why SCSI decided to
  abort.
 
 usb-storage may know if it is caused by timeout via .eh_timed_out callback
 if it wants to know.

Ah, good point.

Vishal, since the only way an abort can occur is when the 
.eh_timeout_out callback runs, I think it makes sense to set the result 
code to DID_TIME_OUT instead of DID_ABORT in all cases.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Ming Lei
On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve vannapu...@nvidia.com wrote:
 Hi,

 There was a recent commit in mainline for the scsi devices which do not
 respond properly to medium access command:

 commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8

 [SCSI] Handle disk devices which can not process medium access commands
 We have experienced several devices which fail in a fashion we do not
 currently handle gracefully in SCSI. After a failure these devices will
 respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
 but any command accessing the storage medium will time out.

 I came across a USB drive which showed similar problem and what I see is
 usb storage is still not able to cope with such devices properly.

 The control flow downwards is like:
 scsi_times_out -- Setting cmd-result as DID_TIME_OUT
 scsi_error_handler
 scsi_unjam_host
 scsi_eh_abort_cmds
 command_abort(sets US_FLIDX_TIMED_OUT for us-dflags
   calls stop_transport,
   and waits for)usb_stor_control_thread
 (which is waiting for
transport
 call to return inside

 usb_stor_invoke_transport)
both
 usb_stor_control_thread and

 usb_stor_invoke_transport
 check for us-dflags
 timed_out bit and
  set the result as
 DID_ABORT
  and signal completion
 for command_abort
  to complete
 ..
 sd_eh_action
 checks for cmd-result and finds out that it's DID_ABORT rather than
 DID_TIME_OUT.

 This patch updates the command result to be TIME_OUT explicitly before
 returning from command_abort in scsiglue.c.

 I would like to know if this patch can work out for such USB Storage
 devices? What would be the better way to do the same?

Looks your diagnose is correct, and patch should be doable, the
only side effect is that previous returned DID_ABORT in srb-result
is replaced with DID_TIME_OUT now.

Another way is to implement .eh_timed_out callback to return different
error for the two failure, but looks not a big deal.

Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Alan Stern
On Tue, 15 Oct 2013, Ming Lei wrote:

 On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve vannapu...@nvidia.com 
 wrote:
  Hi,
 
  There was a recent commit in mainline for the scsi devices which do not
  respond properly to medium access command:
 
  commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
  [SCSI] Handle disk devices which can not process medium access commands
  We have experienced several devices which fail in a fashion we do not
  currently handle gracefully in SCSI. After a failure these devices will
  respond to the SCSI primary command set (INQUIRY, TEST UNIT READY, etc.)
  but any command accessing the storage medium will time out.
 
  I came across a USB drive which showed similar problem and what I see is
  usb storage is still not able to cope with such devices properly.
 
  The control flow downwards is like:
  scsi_times_out -- Setting cmd-result as DID_TIME_OUT
  scsi_error_handler
  scsi_unjam_host
  scsi_eh_abort_cmds
  command_abort(sets US_FLIDX_TIMED_OUT for us-dflags
calls stop_transport,
and waits for)usb_stor_control_thread
  (which is waiting for
 transport
  call to return inside
 
  usb_stor_invoke_transport)
 both
  usb_stor_control_thread and
 
  usb_stor_invoke_transport
  check for us-dflags
  timed_out bit and
   set the result as
  DID_ABORT
   and signal completion
  for command_abort
   to complete
  ..
  sd_eh_action
  checks for cmd-result and finds out that it's DID_ABORT rather than
  DID_TIME_OUT.
 
  This patch updates the command result to be TIME_OUT explicitly before
  returning from command_abort in scsiglue.c.
 
  I would like to know if this patch can work out for such USB Storage
  devices? What would be the better way to do the same?
 
 Looks your diagnose is correct, and patch should be doable, the
 only side effect is that previous returned DID_ABORT in srb-result
 is replaced with DID_TIME_OUT now.
 
 Another way is to implement .eh_timed_out callback to return different
 error for the two failure, but looks not a big deal.

Instead of overriding the result code, a better way to do this would
simply be to change the places where srb-result is currently set to
DID_ABORT  16.  Just make it DID_TIME_OUT  16 instead.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Vishal Annapurve
Hi Alan,

This issue seems more related to the devices using SCSI protocol and the
changes otherwise will be at more places giving the same end result.

I think as the comment says over the command_abort function,
intentional result change should only happen in case of timeout.

Regards,
Vishal

-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Tuesday, October 15, 2013 5:55 PM
To: Ming Lei
Cc: Vishal Annapurve; Linux Kernel Mailing List; linux-usb
Subject: Re: [PATCH] usb-storage: scsiglue: Changing the command result

On Tue, 15 Oct 2013, Ming Lei wrote:

 On Thu, Sep 26, 2013 at 7:35 PM, Vishal Annapurve vannapu...@nvidia.com 
 wrote:
  Hi,
 
  There was a recent commit in mainline for the scsi devices which do 
  not respond properly to medium access command:
 
  commit18a4d0a22ed6c54b67af7718c305cd010f09ddf8
 
  [SCSI] Handle disk devices which can not process medium access 
  commands We have experienced several devices which fail in a fashion 
  we do not currently handle gracefully in SCSI. After a failure these 
  devices will respond to the SCSI primary command set (INQUIRY, TEST 
  UNIT READY, etc.) but any command accessing the storage medium will time 
  out.
 
  I came across a USB drive which showed similar problem and what I 
  see is usb storage is still not able to cope with such devices properly.
 
  The control flow downwards is like:
  scsi_times_out -- Setting cmd-result as DID_TIME_OUT 
  scsi_error_handler scsi_unjam_host scsi_eh_abort_cmds
  command_abort(sets US_FLIDX_TIMED_OUT for us-dflags
calls stop_transport,
and waits for)usb_stor_control_thread
  (which is waiting for
 
  transport call to return inside
 
  usb_stor_invoke_transport)
 
  both usb_stor_control_thread and
 
  usb_stor_invoke_transport
  check for 
  us-dflags timed_out bit and
   set the result 
  as DID_ABORT
   and signal 
  completion for command_abort
   to complete 
  ..
  sd_eh_action
  checks for cmd-result and finds out that it's DID_ABORT rather than 
  DID_TIME_OUT.
 
  This patch updates the command result to be TIME_OUT explicitly 
  before returning from command_abort in scsiglue.c.
 
  I would like to know if this patch can work out for such USB Storage 
  devices? What would be the better way to do the same?
 
 Looks your diagnose is correct, and patch should be doable, the only 
 side effect is that previous returned DID_ABORT in srb-result is 
 replaced with DID_TIME_OUT now.
 
 Another way is to implement .eh_timed_out callback to return different 
 error for the two failure, but looks not a big deal.

Instead of overriding the result code, a better way to do this would simply be 
to change the places where srb-result is currently set to DID_ABORT  16.  
Just make it DID_TIME_OUT  16 instead.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Alan Stern
On Tue, 15 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 This issue seems more related to the devices using SCSI protocol and the
 changes otherwise will be at more places giving the same end result.
 
 I think as the comment says over the command_abort function,
 intentional result change should only happen in case of timeout.

usb-storage doesn't know _why_ a command was aborted; it knows only 
that the abort occurred.

If you look carefully at the code, you'll see that the result is set to 
DID_ABORT only when the US_FLIDX_TIMED_OUT bit is set, and this bit 
gets set only when a SCSI abort occurs.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Vishal Annapurve
Hi Alan,

USB storage maybe just has to say that the abort occurred. By setting the
US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was
time out and the command is being aborted.
 
Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT
bit for scsi - USB storage bridge or for entire usb storage Or maybe scsi has
decided to abort so it should override the result.

Regards,
Vishal

-Original Message-
From: Alan Stern [mailto:st...@rowland.harvard.edu] 
Sent: Tuesday, October 15, 2013 7:03 PM
To: Vishal Annapurve
Cc: Ming Lei; Linux Kernel Mailing List; linux-usb
Subject: RE: [PATCH] usb-storage: scsiglue: Changing the command result

On Tue, 15 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 This issue seems more related to the devices using SCSI protocol and 
 the changes otherwise will be at more places giving the same end result.
 
 I think as the comment says over the command_abort function, 
 intentional result change should only happen in case of timeout.

usb-storage doesn't know _why_ a command was aborted; it knows only that the 
abort occurred.

If you look carefully at the code, you'll see that the result is set to 
DID_ABORT only when the US_FLIDX_TIMED_OUT bit is set, and this bit gets set 
only when a SCSI abort occurs.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Alan Stern
On Tue, 15 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,
 
 USB storage maybe just has to say that the abort occurred. By setting the
 US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was
 time out and the command is being aborted.

No.  By setting the US_FLIDX_TIMED_OUT bit, usb-storage indicates that
the command was aborted.  This doesn't indicate anything about the
reason for the abort.  (Maybe this bit's name wasn't chosen very well.)

 Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT
 bit for scsi - USB storage bridge or for entire usb storage

I don't understand this.  What's the difference between scsi - USB 
storage bridge and entire usb storage?  Aren't they the same thing?

  Or maybe scsi has
 decided to abort so it should override the result.

Of course the SCSI midlayer has decided to abort.  That's the only way 
this bit can get set.  But usb-storage doesn't know why SCSI decided to 
abort.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] usb-storage: scsiglue: Changing the command result

2013-10-15 Thread Ming Lei
On Wed, Oct 16, 2013 at 4:22 AM, Alan Stern st...@rowland.harvard.edu wrote:
 On Tue, 15 Oct 2013, Vishal Annapurve wrote:

 Hi Alan,

 USB storage maybe just has to say that the abort occurred. By setting the
 US_FLIDX_TIMED_OUT bit USB storage is getting signaled that the reason was
 time out and the command is being aborted.

 No.  By setting the US_FLIDX_TIMED_OUT bit, usb-storage indicates that
 the command was aborted.  This doesn't indicate anything about the
 reason for the abort.  (Maybe this bit's name wasn't chosen very well.)

 Now, it's arguable whether to change the implication of US_FLIDX_TIMED_OUT
 bit for scsi - USB storage bridge or for entire usb storage

 I don't understand this.  What's the difference between scsi - USB
 storage bridge and entire usb storage?  Aren't they the same thing?

  Or maybe scsi has
 decided to abort so it should override the result.

 Of course the SCSI midlayer has decided to abort.  That's the only way
 this bit can get set.  But usb-storage doesn't know why SCSI decided to
 abort.

usb-storage may know if it is caused by timeout via .eh_timed_out callback
if it wants to know.


Thanks,
-- 
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html