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