* [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).