linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c
@ 2025-10-03 10:46 Shi Hao
  2025-10-03 11:39 ` Greg KH
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Shi Hao @ 2025-10-03 10:46 UTC (permalink / raw)
  To: gregkh; +Cc: shuah, linux-usb, Shi Hao

Previously, only a few USB subclasses had srb->cmd_len translated to 6 or 12 bytes, which was insufficient for some SCSI opcodes. This patch extends translation to the correct opcode ranges so that command lengths are set properly.

Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
---
 drivers/usb/storage/transport.c | 41 ++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 1aa1bd26c81f..4dd6bfa86c23 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -718,12 +718,41 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 
 		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
 
-		/* FIXME: we must do the protocol translation here */
-		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
-				us->subclass == USB_SC_CYP_ATACB)
-			srb->cmd_len = 6;
-		else
-			srb->cmd_len = 12;
+		/* Protocol translation per SCSI opcode group */
+                switch(us->subclass)
+                {
+                case USB_SC_UFI:
+                case USB_SC_8020:
+                case USB_SC_8070:
+                case USB_SC_QIC:
+                         srb->cmd_len = 12 ;             /* ATAPI/UFI devices always use 12-byte CDBs */
+                         break;
+
+                case USB_SC_RBC:
+                case USB_SC_SCSI:
+                case USB_SC_CYP_ATACB:                   /* Determine cmd_len based on SCSI opcode group */
+
+                         if(opcode <= 0x1F)             /* Group 0 */
+                         {
+                                 srb->cmd_len = 6 ;
+                         }else if( opcode <= 0x7F)      /* Group 1 & 2 */
+                         {
+                                 srb->cmd_len = 10;
+                         }else if(opcode <= 0x9F )      /* Group 5 */
+                         {
+                                srb->cmd_len = 16 ;
+                         }else if(opcode <=0xBF)        /* Group 6 */
+                         {
+                                srb->cmd_len = 12 ;
+                         }else if( opcode <=0xDF)       /* Group 7 */
+                         {
+                                srb->cmd_len = 16;
+                         }else{
+                                ;                       /* Leaving cmd_len  value unchanged  for  0xE0–0xFF vendor-specific*/
+
+                         }
+                        break;
+                }
 
 		/* issue the auto-sense command */
 		scsi_set_resid(srb, 0);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c
  2025-10-03 10:46 [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c Shi Hao
@ 2025-10-03 11:39 ` Greg KH
  2025-10-03 11:40 ` Greg KH
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2025-10-03 11:39 UTC (permalink / raw)
  To: Shi Hao; +Cc: shuah, linux-usb

On Fri, Oct 03, 2025 at 04:16:33PM +0530, Shi Hao wrote:
> Previously, only a few USB subclasses had srb->cmd_len translated to 6 or 12 bytes, which was insufficient for some SCSI opcodes. This patch extends translation to the correct opcode ranges so that command lengths are set properly.

Please take a look at the kernel submission guidelines that says not to
say things like "this patch" :)

Also, please wrap your changelog lines.

> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

What commit id does this fix?

What devices are not working without this change?  Given the age of this
code, and the fact that no new devices should be using it, it's odd that
this is just showing up now.  What changed to cause this to be relevant?

> ---
>  drivers/usb/storage/transport.c | 41 ++++++++++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 1aa1bd26c81f..4dd6bfa86c23 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -718,12 +718,41 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  
>  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
>  
> -		/* FIXME: we must do the protocol translation here */
> -		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> -				us->subclass == USB_SC_CYP_ATACB)
> -			srb->cmd_len = 6;
> -		else
> -			srb->cmd_len = 12;
> +		/* Protocol translation per SCSI opcode group */
> +                switch(us->subclass)
> +                {
> +                case USB_SC_UFI:
> +                case USB_SC_8020:
> +                case USB_SC_8070:
> +                case USB_SC_QIC:
> +                         srb->cmd_len = 12 ;             /* ATAPI/UFI devices always use 12-byte CDBs */
> +                         break;
> +
> +                case USB_SC_RBC:
> +                case USB_SC_SCSI:
> +                case USB_SC_CYP_ATACB:                   /* Determine cmd_len based on SCSI opcode group */
> +
> +                         if(opcode <= 0x1F)             /* Group 0 */
> +                         {
> +                                 srb->cmd_len = 6 ;
> +                         }else if( opcode <= 0x7F)      /* Group 1 & 2 */
> +                         {
> +                                 srb->cmd_len = 10;
> +                         }else if(opcode <= 0x9F )      /* Group 5 */
> +                         {
> +                                srb->cmd_len = 16 ;
> +                         }else if(opcode <=0xBF)        /* Group 6 */
> +                         {
> +                                srb->cmd_len = 12 ;
> +                         }else if( opcode <=0xDF)       /* Group 7 */
> +                         {
> +                                srb->cmd_len = 16;
> +                         }else{
> +                                ;                       /* Leaving cmd_len  value unchanged  for  0xE0–0xFF vendor-specific*/
> +
> +                         }
> +                        break;
> +                }

You ignored my bot's response saying that this shows a bunch of
scripts/checkpatch.pl errors.  Why?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c
  2025-10-03 10:46 [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c Shi Hao
  2025-10-03 11:39 ` Greg KH
@ 2025-10-03 11:40 ` Greg KH
  2025-10-06 12:31 ` [PATCH v3] usb-storage: Protocol translation for scsi opcode Shi Hao
  2025-10-09 21:15 ` [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c kernel test robot
  3 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2025-10-03 11:40 UTC (permalink / raw)
  To: Shi Hao; +Cc: shuah, linux-usb

On Fri, Oct 03, 2025 at 04:16:33PM +0530, Shi Hao wrote:
> Previously, only a few USB subclasses had srb->cmd_len translated to 6 or 12 bytes, which was insufficient for some SCSI opcodes. This patch extends translation to the correct opcode ranges so that command lengths are set properly.
> 
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> ---
>  drivers/usb/storage/transport.c | 41 ++++++++++++++++++++++++++++-----
>  1 file changed, 35 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 1aa1bd26c81f..4dd6bfa86c23 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -718,12 +718,41 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  
>  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
>  
> -		/* FIXME: we must do the protocol translation here */
> -		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> -				us->subclass == USB_SC_CYP_ATACB)
> -			srb->cmd_len = 6;
> -		else
> -			srb->cmd_len = 12;
> +		/* Protocol translation per SCSI opcode group */
> +                switch(us->subclass)
> +                {
> +                case USB_SC_UFI:
> +                case USB_SC_8020:
> +                case USB_SC_8070:
> +                case USB_SC_QIC:
> +                         srb->cmd_len = 12 ;             /* ATAPI/UFI devices always use 12-byte CDBs */
> +                         break;
> +
> +                case USB_SC_RBC:
> +                case USB_SC_SCSI:
> +                case USB_SC_CYP_ATACB:                   /* Determine cmd_len based on SCSI opcode group */
> +
> +                         if(opcode <= 0x1F)             /* Group 0 */
> +                         {
> +                                 srb->cmd_len = 6 ;
> +                         }else if( opcode <= 0x7F)      /* Group 1 & 2 */
> +                         {
> +                                 srb->cmd_len = 10;
> +                         }else if(opcode <= 0x9F )      /* Group 5 */
> +                         {
> +                                srb->cmd_len = 16 ;
> +                         }else if(opcode <=0xBF)        /* Group 6 */
> +                         {
> +                                srb->cmd_len = 12 ;
> +                         }else if( opcode <=0xDF)       /* Group 7 */
> +                         {
> +                                srb->cmd_len = 16;
> +                         }else{
> +                                ;                       /* Leaving cmd_len  value unchanged  for  0xE0–0xFF vendor-specific*/
> +
> +                         }
> +                        break;
> +                }
>  
>  		/* issue the auto-sense command */
>  		scsi_set_resid(srb, 0);
> -- 
> 2.51.0
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch contains warnings and/or errors noticed by the
  scripts/checkpatch.pl tool.

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] usb-storage: Protocol translation for scsi opcode
  2025-10-03 10:46 [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c Shi Hao
  2025-10-03 11:39 ` Greg KH
  2025-10-03 11:40 ` Greg KH
@ 2025-10-06 12:31 ` Shi Hao
  2025-10-06 13:47   ` Alan Stern
  2025-10-09 21:15 ` [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c kernel test robot
  3 siblings, 1 reply; 14+ messages in thread
From: Shi Hao @ 2025-10-06 12:31 UTC (permalink / raw)
  To: gregkh; +Cc: linux-usb, shuah, Shi Hao

Protocol translation for correct cmd_len for old legacy usb device
subclasses  USB_SC_RBC,USB_SC_SCSI  and USB_SC_CYP_ATACB as per
their scsi opcode.In the old code There was a 'FIXME' comment stating
that there  was a  need to set up for a proper SCSI subclass
protocol translation for those legacy usb subclasses .

Previous protocol translation is enough for modern usb subclasses
but it is insufficient for old usb devices for which it had a
'FIXME' comment on it. Also it was  only padding to 6 or to 12 bytes
leaving cmd_len value not properly set for those devices .

To avoid this  set proper SCSI  subclass protocol  translation based
on their opcodes and determine correct cmd_len for proper command
length translation as per their need .

Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

---

change v1:
- update subjects for the commit
- update commit  description
- check checkpatch.pl script
- update From:  and signed-off name
 - wrap git commit changelog
change v2:
- check checkpatch.pl script
- update verion history in the commit

---
 drivers/usb/storage/transport.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 1aa1bd26c81f..ed9f3b9292f3 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -718,12 +718,31 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 
 		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
 
-		/* FIXME: we must do the protocol translation here */
-		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
-				us->subclass == USB_SC_CYP_ATACB)
-			srb->cmd_len = 6;
-		else
+		/* Protocol translation per scsi opcode group */
+		switch (us->subclass) {
+		case USB_SC_UFI:
+		case USB_SC_8020:
+		case USB_SC_8070:
+		case USB_SC_QIC:
 			srb->cmd_len = 12;
+			break;
+		/* Determine cmd_len based on scsi opcode group */
+		case USB_SC_RBC:
+		case USB_SC_SCSI:
+		case USB_SC_CYP_ATACB:
+			if (srb->cmnd[0] <= 0x1F)
+				srb->cmd_len = 6;
+			else if (srb->cmnd[0] <= 0x7F)
+				srb->cmd_len = 10;
+			else if (srb->cmnd[0] <= 0x9F)
+				srb->cmd_len = 16;
+			else if (srb->cmnd[0] <= 0xBF)
+				srb->cmd_len = 12;
+			else if (srb->cmnd[0] <= 0xDF)
+				srb->cmd_len = 16;
+			else
+
+				break; }
 
 		/* issue the auto-sense command */
 		scsi_set_resid(srb, 0);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] usb-storage: Protocol translation for scsi opcode
  2025-10-06 12:31 ` [PATCH v3] usb-storage: Protocol translation for scsi opcode Shi Hao
@ 2025-10-06 13:47   ` Alan Stern
  2025-10-07 11:37     ` [PATCH v4] usb-storage v4: Simplify protocol translation Shi Hao
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Stern @ 2025-10-06 13:47 UTC (permalink / raw)
  To: Shi Hao; +Cc: gregkh, linux-usb, shuah

On Mon, Oct 06, 2025 at 06:01:21PM +0530, Shi Hao wrote:
> Protocol translation for correct cmd_len for old legacy usb device
> subclasses  USB_SC_RBC,USB_SC_SCSI  and USB_SC_CYP_ATACB as per
> their scsi opcode.In the old code There was a 'FIXME' comment stating
> that there  was a  need to set up for a proper SCSI subclass
> protocol translation for those legacy usb subclasses .
> 
> Previous protocol translation is enough for modern usb subclasses
> but it is insufficient for old usb devices for which it had a
> 'FIXME' comment on it. Also it was  only padding to 6 or to 12 bytes
> leaving cmd_len value not properly set for those devices .
> 
> To avoid this  set proper SCSI  subclass protocol  translation based
> on their opcodes and determine correct cmd_len for proper command
> length translation as per their need .
> 
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> 
> ---
> 
> change v1:
> - update subjects for the commit
> - update commit  description
> - check checkpatch.pl script
> - update From:  and signed-off name
>  - wrap git commit changelog
> change v2:
> - check checkpatch.pl script
> - update verion history in the commit
> 
> ---
>  drivers/usb/storage/transport.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 1aa1bd26c81f..ed9f3b9292f3 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -718,12 +718,31 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  
>  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
>  
> -		/* FIXME: we must do the protocol translation here */
> -		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> -				us->subclass == USB_SC_CYP_ATACB)
> -			srb->cmd_len = 6;
> -		else
> +		/* Protocol translation per scsi opcode group */
> +		switch (us->subclass) {
> +		case USB_SC_UFI:
> +		case USB_SC_8020:
> +		case USB_SC_8070:
> +		case USB_SC_QIC:

I think it's a bad idea to enumerate these subclasses here.  The 
original code only tests for RBC, SCSI, and ATACB; you should stick with 
that approach and replace the case labels above with "default:".

Or just keep the original "if" statement instead of using "switch" -- 
that would be even simpler.

>  			srb->cmd_len = 12;
> +			break;
> +		/* Determine cmd_len based on scsi opcode group */
> +		case USB_SC_RBC:
> +		case USB_SC_SCSI:
> +		case USB_SC_CYP_ATACB:
> +			if (srb->cmnd[0] <= 0x1F)
> +				srb->cmd_len = 6;
> +			else if (srb->cmnd[0] <= 0x7F)
> +				srb->cmd_len = 10;
> +			else if (srb->cmnd[0] <= 0x9F)
> +				srb->cmd_len = 16;
> +			else if (srb->cmnd[0] <= 0xBF)
> +				srb->cmd_len = 12;
> +			else if (srb->cmnd[0] <= 0xDF)
> +				srb->cmd_len = 16;
> +			else
> +
> +				break; }

srb->cmd_len needs to be set to _some_ reasonable value when cmnd[0] >= 
0xe0.  It looks like a line of code got left out and the "break" was 
indented too far.

Also, the placement of the closing '}' is wrong; it should be on 
a line of its own.

Alan Stern

>  
>  		/* issue the auto-sense command */
>  		scsi_set_resid(srb, 0);
> -- 
> 2.51.0
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] usb-storage v4: Simplify protocol translation
  2025-10-06 13:47   ` Alan Stern
@ 2025-10-07 11:37     ` Shi Hao
  2025-10-07 14:35       ` Alan Stern
  0 siblings, 1 reply; 14+ messages in thread
From: Shi Hao @ 2025-10-07 11:37 UTC (permalink / raw)
  To: stern; +Cc: gregkh, i.shihao.999, linux-usb

Simplify protocol translation for usb subclasses.

As suggested by Alan Stern  to remove the switch case labels  with
'default'  or keep the  previous  if statements  instead of the switch
cases and advised  reasonable value when cmnd[0] >= 0xe0 .

keep  those usb subclasses to default and simplify logic removing
switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
which fallbacks  to value 6 which is old cmd_len for those subclasses.

Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

---

change v1:
- update subjects for the commit
- update commit  description
- check checkpatch.pl script
- update From:  and signed-off name
change v2:
- check checkpatch.pl script
- update verion history in the commit
- wrap git commit changlog message
- update git commit  message body
change v3:
- simplfy protocol  translation logic
- set reasonable value when >= cmnd[0]
- remove switch case statements

---

 drivers/usb/storage/transport.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 4d01f70f39ac..14cc608052d9 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
 
 		/* Protocol translation per scsi opcode group */
-		switch (us->subclass) {
-		case USB_SC_UFI:
-		case USB_SC_8020:
-		case USB_SC_8070:
-		case USB_SC_QIC:
-			srb->cmd_len = 12;
-			break;
-		/* Determine cmd_len based on scsi opcode group */
-		case USB_SC_RBC:
-		case USB_SC_SCSI:
-		case USB_SC_CYP_ATACB:
+		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
+				us->subclass == USB_SC_CYP_ATACB) {
+		/* Determine cmd_len based on SCSI opcode group */
 			if (srb->cmnd[0] <= 0x1F)
 				srb->cmd_len = 6;
 			else if (srb->cmnd[0] <= 0x7F)
@@ -741,9 +733,10 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 			else if (srb->cmnd[0] <= 0xDF)
 				srb->cmd_len = 16;
 			else
-
-				break;
-					}
+				srb->cmd_len = 6;
+		} else {
+			srb->cmd_len = 12;
+		}
 
 		/* issue the auto-sense command */
 		scsi_set_resid(srb, 0);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] usb-storage v4: Simplify protocol translation
  2025-10-07 11:37     ` [PATCH v4] usb-storage v4: Simplify protocol translation Shi Hao
@ 2025-10-07 14:35       ` Alan Stern
  2025-10-08  7:04         ` [PATCH v5] usb-storage: " Shi Hao
  0 siblings, 1 reply; 14+ messages in thread
From: Alan Stern @ 2025-10-07 14:35 UTC (permalink / raw)
  To: Shi Hao; +Cc: gregkh, linux-usb

On Tue, Oct 07, 2025 at 05:07:32PM +0530, Shi Hao wrote:
> Simplify protocol translation for usb subclasses.
> 
> As suggested by Alan Stern  to remove the switch case labels  with
> 'default'  or keep the  previous  if statements  instead of the switch
> cases and advised  reasonable value when cmnd[0] >= 0xe0 .
> 
> keep  those usb subclasses to default and simplify logic removing
> switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
> which fallbacks  to value 6 which is old cmd_len for those subclasses.
> 
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> 
> ---

Is this meant to be version 4 of your original patch, or is it version 1 
of a new patch that is supposed to be applied on top of version 3 of the 
original patch?  It looks like you haven't made up your mind about this.  
The Subject: line and changelog below indicate one thing, but the 
description above and the diff indicate another.

Since none of the versions of your original patch have been accepted, 
you should make this a new version of the original patch.  Please 
reformulate it that way and submit the result as version 5.

Alan Stern

> change v1:
> - update subjects for the commit
> - update commit  description
> - check checkpatch.pl script
> - update From:  and signed-off name
> change v2:
> - check checkpatch.pl script
> - update verion history in the commit
> - wrap git commit changlog message
> - update git commit  message body
> change v3:
> - simplfy protocol  translation logic
> - set reasonable value when >= cmnd[0]
> - remove switch case statements
> 
> ---
> 
>  drivers/usb/storage/transport.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 4d01f70f39ac..14cc608052d9 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
>  
>  		/* Protocol translation per scsi opcode group */
> -		switch (us->subclass) {
> -		case USB_SC_UFI:
> -		case USB_SC_8020:
> -		case USB_SC_8070:
> -		case USB_SC_QIC:
> -			srb->cmd_len = 12;
> -			break;
> -		/* Determine cmd_len based on scsi opcode group */
> -		case USB_SC_RBC:
> -		case USB_SC_SCSI:
> -		case USB_SC_CYP_ATACB:
> +		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> +				us->subclass == USB_SC_CYP_ATACB) {
> +		/* Determine cmd_len based on SCSI opcode group */
>  			if (srb->cmnd[0] <= 0x1F)
>  				srb->cmd_len = 6;
>  			else if (srb->cmnd[0] <= 0x7F)
> @@ -741,9 +733,10 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  			else if (srb->cmnd[0] <= 0xDF)
>  				srb->cmd_len = 16;
>  			else
> -
> -				break;
> -					}
> +				srb->cmd_len = 6;
> +		} else {
> +			srb->cmd_len = 12;
> +		}
>  
>  		/* issue the auto-sense command */
>  		scsi_set_resid(srb, 0);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v5] usb-storage: Simplify protocol translation
  2025-10-07 14:35       ` Alan Stern
@ 2025-10-08  7:04         ` Shi Hao
  2025-10-08  7:31           ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Shi Hao @ 2025-10-08  7:04 UTC (permalink / raw)
  To: stern; +Cc: gregkh, i.shihao.999, linux-usb

Simplify protocol translation for usb subclasses.

As suggested by Alan Stern  to remove the switch case labels  with
'default'  or keep the  previous  if statements  instead of the switch
cases and advised  reasonable value when cmnd[0] >= 0xe0 .

keep  those usb subclasses to default and simplify logic removing
switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
which fallbacks  to value 6 which is old cmd_len for those subclasses.

Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

---

changes v5:
- Reformulate as  v5  of original patch series
- No code changes ,clarified versioning history

changes v4:
- Simplify  protocol translation logic
- Set reasonable default when cmnd[0] >= 0xE0
- Remove switch-case statement

changes v3:
- Wrap  commit message lines properly
- Improve commit description
- fix  checkpatch.pl script errors

changes v2:
- Update subject commit message
- fix  checkpatch.pl warnings
- match from: and sign-off: name
---
 drivers/usb/storage/transport.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 4d01f70f39ac..14cc608052d9 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
 
 		/* Protocol translation per scsi opcode group */
-		switch (us->subclass) {
-		case USB_SC_UFI:
-		case USB_SC_8020:
-		case USB_SC_8070:
-		case USB_SC_QIC:
-			srb->cmd_len = 12;
-			break;
-		/* Determine cmd_len based on scsi opcode group */
-		case USB_SC_RBC:
-		case USB_SC_SCSI:
-		case USB_SC_CYP_ATACB:
+		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
+				us->subclass == USB_SC_CYP_ATACB) {
+		/* Determine cmd_len based on SCSI opcode group */
 			if (srb->cmnd[0] <= 0x1F)
 				srb->cmd_len = 6;
 			else if (srb->cmnd[0] <= 0x7F)
@@ -741,9 +733,10 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 			else if (srb->cmnd[0] <= 0xDF)
 				srb->cmd_len = 16;
 			else
-
-				break;
-					}
+				srb->cmd_len = 6;
+		} else {
+			srb->cmd_len = 12;
+		}
 
 		/* issue the auto-sense command */
 		scsi_set_resid(srb, 0);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] usb-storage: Simplify protocol translation
  2025-10-08  7:04         ` [PATCH v5] usb-storage: " Shi Hao
@ 2025-10-08  7:31           ` Greg KH
  2025-10-08  9:18             ` ShiHao
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2025-10-08  7:31 UTC (permalink / raw)
  To: Shi Hao; +Cc: stern, linux-usb

On Wed, Oct 08, 2025 at 12:34:20PM +0530, Shi Hao wrote:
> Simplify protocol translation for usb subclasses.
> 
> As suggested by Alan Stern  to remove the switch case labels  with
> 'default'  or keep the  previous  if statements  instead of the switch
> cases and advised  reasonable value when cmnd[0] >= 0xe0 .
> 
> keep  those usb subclasses to default and simplify logic removing
> switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
> which fallbacks  to value 6 which is old cmd_len for those subclasses.
> 
> Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> 
> ---
> 
> changes v5:
> - Reformulate as  v5  of original patch series
> - No code changes ,clarified versioning history
> 
> changes v4:
> - Simplify  protocol translation logic
> - Set reasonable default when cmnd[0] >= 0xE0
> - Remove switch-case statement
> 
> changes v3:
> - Wrap  commit message lines properly
> - Improve commit description
> - fix  checkpatch.pl script errors
> 
> changes v2:
> - Update subject commit message
> - fix  checkpatch.pl warnings
> - match from: and sign-off: name
> ---
>  drivers/usb/storage/transport.c | 21 +++++++--------------
>  1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 4d01f70f39ac..14cc608052d9 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
>  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
>  
>  		/* Protocol translation per scsi opcode group */
> -		switch (us->subclass) {
> -		case USB_SC_UFI:
> -		case USB_SC_8020:
> -		case USB_SC_8070:
> -		case USB_SC_QIC:
> -			srb->cmd_len = 12;
> -			break;
> -		/* Determine cmd_len based on scsi opcode group */
> -		case USB_SC_RBC:
> -		case USB_SC_SCSI:
> -		case USB_SC_CYP_ATACB:
> +		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> +				us->subclass == USB_SC_CYP_ATACB) {
> +		/* Determine cmd_len based on SCSI opcode group */

Always remember to run scripts/checkpatch.pl before sending changes out
to others :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] usb-storage: Simplify protocol translation
  2025-10-08  7:31           ` Greg KH
@ 2025-10-08  9:18             ` ShiHao
  2025-10-08 10:04               ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: ShiHao @ 2025-10-08  9:18 UTC (permalink / raw)
  To: Greg KH; +Cc: stern, linux-usb, gregkh

On Wed, Oct 08, 2025 at 09:31:59AM +0200, Greg KH wrote:
> On Wed, Oct 08, 2025 at 12:34:20PM +0530, Shi Hao wrote:
> > Simplify protocol translation for usb subclasses.
> > 
> > As suggested by Alan Stern  to remove the switch case labels  with
> > 'default'  or keep the  previous  if statements  instead of the switch
> > cases and advised  reasonable value when cmnd[0] >= 0xe0 .
> > 
> > keep  those usb subclasses to default and simplify logic removing
> > switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
> > which fallbacks  to value 6 which is old cmd_len for those subclasses.
> > 
> > Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> > 
> > ---
> > 
> > changes v5:
> > - Reformulate as  v5  of original patch series
> > - No code changes ,clarified versioning history
> > 
> > changes v4:
> > - Simplify  protocol translation logic
> > - Set reasonable default when cmnd[0] >= 0xE0
> > - Remove switch-case statement
> > 
> > changes v3:
> > - Wrap  commit message lines properly
> > - Improve commit description
> > - fix  checkpatch.pl script errors
> > 
> > changes v2:
> > - Update subject commit message
> > - fix  checkpatch.pl warnings
> > - match from: and sign-off: name
> > ---
> >  drivers/usb/storage/transport.c | 21 +++++++--------------
> >  1 file changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> > index 4d01f70f39ac..14cc608052d9 100644
> > --- a/drivers/usb/storage/transport.c
> > +++ b/drivers/usb/storage/transport.c
> > @@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
> >  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
> >  
> >  		/* Protocol translation per scsi opcode group */
> > -		switch (us->subclass) {
> > -		case USB_SC_UFI:
> > -		case USB_SC_8020:
> > -		case USB_SC_8070:
> > -		case USB_SC_QIC:
> > -			srb->cmd_len = 12;
> > -			break;
> > -		/* Determine cmd_len based on scsi opcode group */
> > -		case USB_SC_RBC:
> > -		case USB_SC_SCSI:
> > -		case USB_SC_CYP_ATACB:
> > +		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> > +				us->subclass == USB_SC_CYP_ATACB) {
> > +		/* Determine cmd_len based on SCSI opcode group */
> 
> Always remember to run scripts/checkpatch.pl before sending changes out
> to others :(
> 
> thanks,
> 
> greg k-h


Hi greg ,

Thanks for the reminder. I ran scripts/checkpatch.pl --strict on
drivers/usb/storage/transport.c and all checkpatch.pl has been
resolved .If i need to make another patch for this new change
please let me know .Once again thanks Alan stern and greg k-h
for reviewing and giving your precious time .

Thanks shihao

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v5] usb-storage: Simplify protocol translation
  2025-10-08  9:18             ` ShiHao
@ 2025-10-08 10:04               ` Greg KH
  2025-10-08 11:09                 ` [PATCH v6] usb-storage: Fix comment indentation level Shi Hao
  0 siblings, 1 reply; 14+ messages in thread
From: Greg KH @ 2025-10-08 10:04 UTC (permalink / raw)
  To: ShiHao; +Cc: stern, linux-usb

On Wed, Oct 08, 2025 at 02:48:12PM +0530, ShiHao wrote:
> On Wed, Oct 08, 2025 at 09:31:59AM +0200, Greg KH wrote:
> > On Wed, Oct 08, 2025 at 12:34:20PM +0530, Shi Hao wrote:
> > > Simplify protocol translation for usb subclasses.
> > > 
> > > As suggested by Alan Stern  to remove the switch case labels  with
> > > 'default'  or keep the  previous  if statements  instead of the switch
> > > cases and advised  reasonable value when cmnd[0] >= 0xe0 .
> > > 
> > > keep  those usb subclasses to default and simplify logic removing
> > > switch cases and set 6 as a reasonable value when cmnd[0] >= 0xe0
> > > which fallbacks  to value 6 which is old cmd_len for those subclasses.
> > > 
> > > Signed-off-by: Shi Hao <i.shihao.999@gmail.com>
> > > 
> > > ---
> > > 
> > > changes v5:
> > > - Reformulate as  v5  of original patch series
> > > - No code changes ,clarified versioning history
> > > 
> > > changes v4:
> > > - Simplify  protocol translation logic
> > > - Set reasonable default when cmnd[0] >= 0xE0
> > > - Remove switch-case statement
> > > 
> > > changes v3:
> > > - Wrap  commit message lines properly
> > > - Improve commit description
> > > - fix  checkpatch.pl script errors
> > > 
> > > changes v2:
> > > - Update subject commit message
> > > - fix  checkpatch.pl warnings
> > > - match from: and sign-off: name
> > > ---
> > >  drivers/usb/storage/transport.c | 21 +++++++--------------
> > >  1 file changed, 7 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> > > index 4d01f70f39ac..14cc608052d9 100644
> > > --- a/drivers/usb/storage/transport.c
> > > +++ b/drivers/usb/storage/transport.c
> > > @@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
> > >  		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
> > >  
> > >  		/* Protocol translation per scsi opcode group */
> > > -		switch (us->subclass) {
> > > -		case USB_SC_UFI:
> > > -		case USB_SC_8020:
> > > -		case USB_SC_8070:
> > > -		case USB_SC_QIC:
> > > -			srb->cmd_len = 12;
> > > -			break;
> > > -		/* Determine cmd_len based on scsi opcode group */
> > > -		case USB_SC_RBC:
> > > -		case USB_SC_SCSI:
> > > -		case USB_SC_CYP_ATACB:
> > > +		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
> > > +				us->subclass == USB_SC_CYP_ATACB) {
> > > +		/* Determine cmd_len based on SCSI opcode group */
> > 
> > Always remember to run scripts/checkpatch.pl before sending changes out
> > to others :(
> > 
> > thanks,
> > 
> > greg k-h
> 
> 
> Hi greg ,
> 
> Thanks for the reminder. I ran scripts/checkpatch.pl --strict on
> drivers/usb/storage/transport.c and all checkpatch.pl has been
> resolved .If i need to make another patch for this new change
> please let me know .Once again thanks Alan stern and greg k-h
> for reviewing and giving your precious time .

The comment is indented to the wrong level here in your change.  So
as-is, I can not take this patch, sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v6] usb-storage: Fix comment indentation level
  2025-10-08 10:04               ` Greg KH
@ 2025-10-08 11:09                 ` Shi Hao
  2025-10-08 11:21                   ` Greg KH
  0 siblings, 1 reply; 14+ messages in thread
From: Shi Hao @ 2025-10-08 11:09 UTC (permalink / raw)
  To: gregkh; +Cc: i.shihao.999, linux-usb, stern

Fix comment  indentation level .

The previous comment was indented at the wrong level, breaking
kernel coding style consistency. The indentation has been corrected
to align with the relevant code block.

Signed-off-by: Shi Hao <i.shihao.999@gmail.com>

---

changes v6:
-  Fix comment indentation level
-  Fix checkpatch.pl script errors

changes v5:
- Reformulate as  v5  of original patch series
- No code changes ,clarified versioning history

changes v4:
- Simplify  protocol translation logic
- Set reasonable default when cmnd[0] >= 0xE0
- Remove switch-case statement

changes v3:
- Wrap  commit message lines properly
- Improve commit description
- fix  checkpatch.pl script errors

changes v2:
- Update subject commit message
- fix  checkpatch.pl warnings
- match from: and sign-off: name
---
 drivers/usb/storage/transport.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 4d01f70f39ac..09012cafc536 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -719,17 +719,9 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 		scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
 
 		/* Protocol translation per scsi opcode group */
-		switch (us->subclass) {
-		case USB_SC_UFI:
-		case USB_SC_8020:
-		case USB_SC_8070:
-		case USB_SC_QIC:
-			srb->cmd_len = 12;
-			break;
-		/* Determine cmd_len based on scsi opcode group */
-		case USB_SC_RBC:
-		case USB_SC_SCSI:
-		case USB_SC_CYP_ATACB:
+		if (us->subclass == USB_SC_RBC || us->subclass == USB_SC_SCSI ||
+		    us->subclass == USB_SC_CYP_ATACB) {
+			/* Determine cmd_len based on SCSI opcode group */
 			if (srb->cmnd[0] <= 0x1F)
 				srb->cmd_len = 6;
 			else if (srb->cmnd[0] <= 0x7F)
@@ -741,9 +733,10 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
 			else if (srb->cmnd[0] <= 0xDF)
 				srb->cmd_len = 16;
 			else
-
-				break;
-					}
+				srb->cmd_len = 6;
+		} else {
+			srb->cmd_len = 12;
+		}
 
 		/* issue the auto-sense command */
 		scsi_set_resid(srb, 0);
--
2.51.0

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v6] usb-storage: Fix comment indentation level
  2025-10-08 11:09                 ` [PATCH v6] usb-storage: Fix comment indentation level Shi Hao
@ 2025-10-08 11:21                   ` Greg KH
  0 siblings, 0 replies; 14+ messages in thread
From: Greg KH @ 2025-10-08 11:21 UTC (permalink / raw)
  To: Shi Hao; +Cc: linux-usb, stern

On Wed, Oct 08, 2025 at 04:39:10PM +0530, Shi Hao wrote:
> Fix comment  indentation level .
> 
> The previous comment was indented at the wrong level, breaking
> kernel coding style consistency. The indentation has been corrected
> to align with the relevant code block.

This does not describe what this patch does.

Please take a break, relax, and come back in a day or so and start from
a "clean" tree to implement this after taking a look at the kernel
documentation for how to submit patches again.

You are making patches on top of patches, all of which will not work
here.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c
  2025-10-03 10:46 [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c Shi Hao
                   ` (2 preceding siblings ...)
  2025-10-06 12:31 ` [PATCH v3] usb-storage: Protocol translation for scsi opcode Shi Hao
@ 2025-10-09 21:15 ` kernel test robot
  3 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2025-10-09 21:15 UTC (permalink / raw)
  To: Shi Hao, gregkh; +Cc: llvm, oe-kbuild-all, shuah, linux-usb, Shi Hao

Hi Shi,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus linus/master v6.17 next-20251009]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shi-Hao/usb-fix-protocol-translation-in-drivers-usb-storage-transport-c/20251003-184944
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20251003104635.24998-1-i.shihao.999%40gmail.com
patch subject: [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20251010/202510100437.u4ye4wKW-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251010/202510100437.u4ye4wKW-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510100437.u4ye4wKW-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/storage/transport.c:735:29: error: use of undeclared identifier 'opcode'
     735 |                          if(opcode <= 0x1F)             /* Group 0 */
         |                             ^
   drivers/usb/storage/transport.c:738:36: error: use of undeclared identifier 'opcode'
     738 |                          }else if( opcode <= 0x7F)      /* Group 1 & 2 */
         |                                    ^
   drivers/usb/storage/transport.c:741:35: error: use of undeclared identifier 'opcode'
     741 |                          }else if(opcode <= 0x9F )      /* Group 5 */
         |                                   ^
   drivers/usb/storage/transport.c:744:35: error: use of undeclared identifier 'opcode'
     744 |                          }else if(opcode <=0xBF)        /* Group 6 */
         |                                   ^
   drivers/usb/storage/transport.c:747:36: error: use of undeclared identifier 'opcode'
     747 |                          }else if( opcode <=0xDF)       /* Group 7 */
         |                                    ^
   5 errors generated.


vim +/opcode +735 drivers/usb/storage/transport.c

   597	
   598	/*
   599	 * Invoke the transport and basic error-handling/recovery methods
   600	 *
   601	 * This is used by the protocol layers to actually send the message to
   602	 * the device and receive the response.
   603	 */
   604	void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)
   605	{
   606		int need_auto_sense;
   607		int result;
   608	
   609		/* send the command to the transport layer */
   610		scsi_set_resid(srb, 0);
   611		result = us->transport(srb, us);
   612	
   613		/*
   614		 * if the command gets aborted by the higher layers, we need to
   615		 * short-circuit all other processing
   616		 */
   617		if (test_bit(US_FLIDX_TIMED_OUT, &us->dflags)) {
   618			usb_stor_dbg(us, "-- command was aborted\n");
   619			srb->result = DID_ABORT << 16;
   620			goto Handle_Errors;
   621		}
   622	
   623		/* if there is a transport error, reset and don't auto-sense */
   624		if (result == USB_STOR_TRANSPORT_ERROR) {
   625			usb_stor_dbg(us, "-- transport indicates error, resetting\n");
   626			srb->result = DID_ERROR << 16;
   627			goto Handle_Errors;
   628		}
   629	
   630		/* if the transport provided its own sense data, don't auto-sense */
   631		if (result == USB_STOR_TRANSPORT_NO_SENSE) {
   632			srb->result = SAM_STAT_CHECK_CONDITION;
   633			last_sector_hacks(us, srb);
   634			return;
   635		}
   636	
   637		srb->result = SAM_STAT_GOOD;
   638	
   639		/*
   640		 * Determine if we need to auto-sense
   641		 *
   642		 * I normally don't use a flag like this, but it's almost impossible
   643		 * to understand what's going on here if I don't.
   644		 */
   645		need_auto_sense = 0;
   646	
   647		/*
   648		 * If we're running the CB transport, which is incapable
   649		 * of determining status on its own, we will auto-sense
   650		 * unless the operation involved a data-in transfer.  Devices
   651		 * can signal most data-in errors by stalling the bulk-in pipe.
   652		 */
   653		if ((us->protocol == USB_PR_CB || us->protocol == USB_PR_DPCM_USB) &&
   654				srb->sc_data_direction != DMA_FROM_DEVICE) {
   655			usb_stor_dbg(us, "-- CB transport device requiring auto-sense\n");
   656			need_auto_sense = 1;
   657		}
   658	
   659		/* Some devices (Kindle) require another command after SYNC CACHE */
   660		if ((us->fflags & US_FL_SENSE_AFTER_SYNC) &&
   661				srb->cmnd[0] == SYNCHRONIZE_CACHE) {
   662			usb_stor_dbg(us, "-- sense after SYNC CACHE\n");
   663			need_auto_sense = 1;
   664		}
   665	
   666		/*
   667		 * If we have a failure, we're going to do a REQUEST_SENSE 
   668		 * automatically.  Note that we differentiate between a command
   669		 * "failure" and an "error" in the transport mechanism.
   670		 */
   671		if (result == USB_STOR_TRANSPORT_FAILED) {
   672			usb_stor_dbg(us, "-- transport indicates command failure\n");
   673			need_auto_sense = 1;
   674		}
   675	
   676		/*
   677		 * Determine if this device is SAT by seeing if the
   678		 * command executed successfully.  Otherwise we'll have
   679		 * to wait for at least one CHECK_CONDITION to determine
   680		 * SANE_SENSE support
   681		 */
   682		if (unlikely((srb->cmnd[0] == ATA_16 || srb->cmnd[0] == ATA_12) &&
   683		    result == USB_STOR_TRANSPORT_GOOD &&
   684		    !(us->fflags & US_FL_SANE_SENSE) &&
   685		    !(us->fflags & US_FL_BAD_SENSE) &&
   686		    !(srb->cmnd[2] & 0x20))) {
   687			usb_stor_dbg(us, "-- SAT supported, increasing auto-sense\n");
   688			us->fflags |= US_FL_SANE_SENSE;
   689		}
   690	
   691		/*
   692		 * A short transfer on a command where we don't expect it
   693		 * is unusual, but it doesn't mean we need to auto-sense.
   694		 */
   695		if ((scsi_get_resid(srb) > 0) &&
   696		    !((srb->cmnd[0] == REQUEST_SENSE) ||
   697		      (srb->cmnd[0] == INQUIRY) ||
   698		      (srb->cmnd[0] == MODE_SENSE) ||
   699		      (srb->cmnd[0] == LOG_SENSE) ||
   700		      (srb->cmnd[0] == MODE_SENSE_10))) {
   701			usb_stor_dbg(us, "-- unexpectedly short transfer\n");
   702		}
   703	
   704		/* Now, if we need to do the auto-sense, let's do it */
   705		if (need_auto_sense) {
   706			int temp_result;
   707			struct scsi_eh_save ses;
   708			int sense_size = US_SENSE_SIZE;
   709			struct scsi_sense_hdr sshdr;
   710			const u8 *scdd;
   711			u8 fm_ili;
   712	
   713			/* device supports and needs bigger sense buffer */
   714			if (us->fflags & US_FL_SANE_SENSE)
   715				sense_size = ~0;
   716	Retry_Sense:
   717			usb_stor_dbg(us, "Issuing auto-REQUEST_SENSE\n");
   718	
   719			scsi_eh_prep_cmnd(srb, &ses, NULL, 0, sense_size);
   720	
   721			/* Protocol translation per SCSI opcode group */
   722	                switch(us->subclass)
   723	                {
   724	                case USB_SC_UFI:
   725	                case USB_SC_8020:
   726	                case USB_SC_8070:
   727	                case USB_SC_QIC:
   728	                         srb->cmd_len = 12 ;             /* ATAPI/UFI devices always use 12-byte CDBs */
   729	                         break;
   730	
   731	                case USB_SC_RBC:
   732	                case USB_SC_SCSI:
   733	                case USB_SC_CYP_ATACB:                   /* Determine cmd_len based on SCSI opcode group */
   734	
 > 735	                         if(opcode <= 0x1F)             /* Group 0 */
   736	                         {
   737	                                 srb->cmd_len = 6 ;
   738	                         }else if( opcode <= 0x7F)      /* Group 1 & 2 */
   739	                         {
   740	                                 srb->cmd_len = 10;
   741	                         }else if(opcode <= 0x9F )      /* Group 5 */
   742	                         {
   743	                                srb->cmd_len = 16 ;
   744	                         }else if(opcode <=0xBF)        /* Group 6 */
   745	                         {
   746	                                srb->cmd_len = 12 ;
   747	                         }else if( opcode <=0xDF)       /* Group 7 */
   748	                         {
   749	                                srb->cmd_len = 16;
   750	                         }else{
   751	                                ;                       /* Leaving cmd_len  value unchanged  for  0xE0–0xFF vendor-specific*/
   752	
   753	                         }
   754	                        break;
   755	                }
   756	
   757			/* issue the auto-sense command */
   758			scsi_set_resid(srb, 0);
   759			temp_result = us->transport(us->srb, us);
   760	
   761			/* let's clean up right away */
   762			scsi_eh_restore_cmnd(srb, &ses);
   763	
   764			if (test_bit(US_FLIDX_TIMED_OUT, &us->dflags)) {
   765				usb_stor_dbg(us, "-- auto-sense aborted\n");
   766				srb->result = DID_ABORT << 16;
   767	
   768				/* If SANE_SENSE caused this problem, disable it */
   769				if (sense_size != US_SENSE_SIZE) {
   770					us->fflags &= ~US_FL_SANE_SENSE;
   771					us->fflags |= US_FL_BAD_SENSE;
   772				}
   773				goto Handle_Errors;
   774			}
   775	
   776			/*
   777			 * Some devices claim to support larger sense but fail when
   778			 * trying to request it. When a transport failure happens
   779			 * using US_FS_SANE_SENSE, we always retry with a standard
   780			 * (small) sense request. This fixes some USB GSM modems
   781			 */
   782			if (temp_result == USB_STOR_TRANSPORT_FAILED &&
   783					sense_size != US_SENSE_SIZE) {
   784				usb_stor_dbg(us, "-- auto-sense failure, retry small sense\n");
   785				sense_size = US_SENSE_SIZE;
   786				us->fflags &= ~US_FL_SANE_SENSE;
   787				us->fflags |= US_FL_BAD_SENSE;
   788				goto Retry_Sense;
   789			}
   790	
   791			/* Other failures */
   792			if (temp_result != USB_STOR_TRANSPORT_GOOD) {
   793				usb_stor_dbg(us, "-- auto-sense failure\n");
   794	
   795				/*
   796				 * we skip the reset if this happens to be a
   797				 * multi-target device, since failure of an
   798				 * auto-sense is perfectly valid
   799				 */
   800				srb->result = DID_ERROR << 16;
   801				if (!(us->fflags & US_FL_SCM_MULT_TARG))
   802					goto Handle_Errors;
   803				return;
   804			}
   805	
   806			/*
   807			 * If the sense data returned is larger than 18-bytes then we
   808			 * assume this device supports requesting more in the future.
   809			 * The response code must be 70h through 73h inclusive.
   810			 */
   811			if (srb->sense_buffer[7] > (US_SENSE_SIZE - 8) &&
   812			    !(us->fflags & US_FL_SANE_SENSE) &&
   813			    !(us->fflags & US_FL_BAD_SENSE) &&
   814			    (srb->sense_buffer[0] & 0x7C) == 0x70) {
   815				usb_stor_dbg(us, "-- SANE_SENSE support enabled\n");
   816				us->fflags |= US_FL_SANE_SENSE;
   817	
   818				/*
   819				 * Indicate to the user that we truncated their sense
   820				 * because we didn't know it supported larger sense.
   821				 */
   822				usb_stor_dbg(us, "-- Sense data truncated to %i from %i\n",
   823					     US_SENSE_SIZE,
   824					     srb->sense_buffer[7] + 8);
   825				srb->sense_buffer[7] = (US_SENSE_SIZE - 8);
   826			}
   827	
   828			scsi_normalize_sense(srb->sense_buffer, SCSI_SENSE_BUFFERSIZE,
   829					     &sshdr);
   830	
   831			usb_stor_dbg(us, "-- Result from auto-sense is %d\n",
   832				     temp_result);
   833			usb_stor_dbg(us, "-- code: 0x%x, key: 0x%x, ASC: 0x%x, ASCQ: 0x%x\n",
   834				     sshdr.response_code, sshdr.sense_key,
   835				     sshdr.asc, sshdr.ascq);
   836	#ifdef CONFIG_USB_STORAGE_DEBUG
   837			usb_stor_show_sense(us, sshdr.sense_key, sshdr.asc, sshdr.ascq);
   838	#endif
   839	
   840			/* set the result so the higher layers expect this data */
   841			srb->result = SAM_STAT_CHECK_CONDITION;
   842	
   843			scdd = scsi_sense_desc_find(srb->sense_buffer,
   844						    SCSI_SENSE_BUFFERSIZE, 4);
   845			fm_ili = (scdd ? scdd[3] : srb->sense_buffer[2]) & 0xA0;
   846	
   847			/*
   848			 * We often get empty sense data.  This could indicate that
   849			 * everything worked or that there was an unspecified
   850			 * problem.  We have to decide which.
   851			 */
   852			if (sshdr.sense_key == 0 && sshdr.asc == 0 && sshdr.ascq == 0 &&
   853			    fm_ili == 0) {
   854				/*
   855				 * If things are really okay, then let's show that.
   856				 * Zero out the sense buffer so the higher layers
   857				 * won't realize we did an unsolicited auto-sense.
   858				 */
   859				if (result == USB_STOR_TRANSPORT_GOOD) {
   860					srb->result = SAM_STAT_GOOD;
   861					srb->sense_buffer[0] = 0x0;
   862				}
   863	
   864				/*
   865				 * ATA-passthru commands use sense data to report
   866				 * the command completion status, and often devices
   867				 * return Check Condition status when nothing is
   868				 * wrong.
   869				 */
   870				else if (srb->cmnd[0] == ATA_16 ||
   871						srb->cmnd[0] == ATA_12) {
   872					/* leave the data alone */
   873				}
   874	
   875				/*
   876				 * If there was a problem, report an unspecified
   877				 * hardware error to prevent the higher layers from
   878				 * entering an infinite retry loop.
   879				 */
   880				else {
   881					srb->result = DID_ERROR << 16;
   882					if ((sshdr.response_code & 0x72) == 0x72)
   883						srb->sense_buffer[1] = HARDWARE_ERROR;
   884					else
   885						srb->sense_buffer[2] = HARDWARE_ERROR;
   886				}
   887			}
   888		}
   889	
   890		/*
   891		 * Some devices don't work or return incorrect data the first
   892		 * time they get a READ(10) command, or for the first READ(10)
   893		 * after a media change.  If the INITIAL_READ10 flag is set,
   894		 * keep track of whether READ(10) commands succeed.  If the
   895		 * previous one succeeded and this one failed, set the REDO_READ10
   896		 * flag to force a retry.
   897		 */
   898		if (unlikely((us->fflags & US_FL_INITIAL_READ10) &&
   899				srb->cmnd[0] == READ_10)) {
   900			if (srb->result == SAM_STAT_GOOD) {
   901				set_bit(US_FLIDX_READ10_WORKED, &us->dflags);
   902			} else if (test_bit(US_FLIDX_READ10_WORKED, &us->dflags)) {
   903				clear_bit(US_FLIDX_READ10_WORKED, &us->dflags);
   904				set_bit(US_FLIDX_REDO_READ10, &us->dflags);
   905			}
   906	
   907			/*
   908			 * Next, if the REDO_READ10 flag is set, return a result
   909			 * code that will cause the SCSI core to retry the READ(10)
   910			 * command immediately.
   911			 */
   912			if (test_bit(US_FLIDX_REDO_READ10, &us->dflags)) {
   913				clear_bit(US_FLIDX_REDO_READ10, &us->dflags);
   914				srb->result = DID_IMM_RETRY << 16;
   915				srb->sense_buffer[0] = 0;
   916			}
   917		}
   918	
   919		/* Did we transfer less than the minimum amount required? */
   920		if ((srb->result == SAM_STAT_GOOD || srb->sense_buffer[2] == 0) &&
   921				scsi_bufflen(srb) - scsi_get_resid(srb) < srb->underflow)
   922			srb->result = DID_ERROR << 16;
   923	
   924		last_sector_hacks(us, srb);
   925		return;
   926	
   927		/*
   928		 * Error and abort processing: try to resynchronize with the device
   929		 * by issuing a port reset.  If that fails, try a class-specific
   930		 * device reset.
   931		 */
   932	  Handle_Errors:
   933	
   934		/*
   935		 * Set the RESETTING bit, and clear the ABORTING bit so that
   936		 * the reset may proceed.
   937		 */
   938		scsi_lock(us_to_host(us));
   939		set_bit(US_FLIDX_RESETTING, &us->dflags);
   940		clear_bit(US_FLIDX_ABORTING, &us->dflags);
   941		scsi_unlock(us_to_host(us));
   942	
   943		/*
   944		 * We must release the device lock because the pre_reset routine
   945		 * will want to acquire it.
   946		 */
   947		mutex_unlock(&us->dev_mutex);
   948		result = usb_stor_port_reset(us);
   949		mutex_lock(&us->dev_mutex);
   950	
   951		if (result < 0) {
   952			scsi_lock(us_to_host(us));
   953			usb_stor_report_device_reset(us);
   954			scsi_unlock(us_to_host(us));
   955			us->transport_reset(us);
   956		}
   957		clear_bit(US_FLIDX_RESETTING, &us->dflags);
   958		last_sector_hacks(us, srb);
   959	}
   960	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-10-09 21:15 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-03 10:46 [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c Shi Hao
2025-10-03 11:39 ` Greg KH
2025-10-03 11:40 ` Greg KH
2025-10-06 12:31 ` [PATCH v3] usb-storage: Protocol translation for scsi opcode Shi Hao
2025-10-06 13:47   ` Alan Stern
2025-10-07 11:37     ` [PATCH v4] usb-storage v4: Simplify protocol translation Shi Hao
2025-10-07 14:35       ` Alan Stern
2025-10-08  7:04         ` [PATCH v5] usb-storage: " Shi Hao
2025-10-08  7:31           ` Greg KH
2025-10-08  9:18             ` ShiHao
2025-10-08 10:04               ` Greg KH
2025-10-08 11:09                 ` [PATCH v6] usb-storage: Fix comment indentation level Shi Hao
2025-10-08 11:21                   ` Greg KH
2025-10-09 21:15 ` [PATCH] usb: fix protocol translation in drivers/usb/storage/transport.c kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).