Use normalized sense when emulating autosense
Subsystems (3)
PATCH 1/1 Use normalized sense when emulating autosense
This patch solves two things:
1) Enables autosense emulation code to correctly interpret descriptor format sense data, and
2) Fixes a bug whereby the autosense emulation code would overwrite descriptor format sense data with SENSE KEY HARDWARE ERROR in fixed format, to incorrectly look like this:
Oct 21 14:11:07 localhost kernel: sd 7:0:0:0: [sdc] Sense Key : Recovered Error [current] [descriptor] Oct 21 14:11:07 localhost kernel: Descriptor sense data with sense descriptors (in hex): Oct 21 14:11:07 localhost kernel: 72 01 04 1d 00 00 00 0e 09 0c 00 00 00 00 00 00 Oct 21 14:11:07 localhost kernel: 00 4f 00 c2 00 50 Oct 21 14:11:07 localhost kernel: sd 7:0:0:0: [sdc] ASC=0x4 ASCQ=0x1d
--- drivers/usb/storage/transport.c | 27 ++++++++++++--------------- 1 files changed, 12 insertions(+), 15 deletions(-)
drivers/usb/storage/transport.c +12 -15
@@ -691,6 +691,7 @@ void usb_stor_invoke_transport(struct scsi_cmnd *srb, struct us_data *us)int temp_result;struct scsi_eh_save ses;int sense_size =US_SENSE_SIZE ; +struct scsi_sense_hdr sshdr;/* device supports and needs bigger sense buffer */ if (us->fflags &US_FL_SANE_SENSE )
@@ -774,17 +775,15 @@ Retry_Sense: srb->sense_buffer[7 ] = (US_SENSE_SIZE -8 ); } +scsi_normalize_sense (srb->sense_buffer,SCSI_SENSE_BUFFERSIZE , + &sshdr); +US_DEBUGP ("-- Result from auto-sense is %d\n" , temp_result);US_DEBUGP ("-- code: 0x%x, key: 0x%x, ASC: 0x%x, ASCQ: 0x%x\n" , - srb->sense_buffer[0 ], - srb->sense_buffer[2 ] &0xf , - srb->sense_buffer[12 ], - srb->sense_buffer[13 ]); + sshdr.response_code, sshdr.sense_key, + sshdr.asc, sshdr.ascq);#ifdef CONFIG_USB_STORAGE_DEBUG -usb_stor_show_sense ( - srb->sense_buffer[2 ] &0xf , - srb->sense_buffer[12 ], - srb->sense_buffer[13 ]); +usb_stor_show_sense (sshdr.sense_key, sshdr.asc, sshdr.ascq);#endif /* set the result so the higher layers expect this data */
@@ -794,12 +793,7 @@ Retry_Sense:* everything worked or that there was an unspecified * problem. We have to decide which. */ -if (/* Filemark 0, ignore EOM, ILI 0, no sense */ - (srb->sense_buffer[2 ] &0xaf ) ==0 && -/* No ASC or ASCQ */ - srb->sense_buffer[12 ] ==0 && - srb->sense_buffer[13 ] ==0 ) { - +if (sshdr.sense_key ==0 && sshdr.asc ==0 && sshdr.ascq ==0 ) {/* If things are really okay, then let's show that. * Zero out the sense buffer so the higher layers * won't realize we did an unsolicited auto-sense.
@@ -814,7 +808,10 @@ Retry_Sense:*/ }else { srb->result =DID_ERROR <<16 ; - srb->sense_buffer[2 ] =HARDWARE_ERROR ; +if ((sshdr.response_code &0x72 ) ==0x72 ) + srb->sense_buffer[1 ] =HARDWARE_ERROR ; +else + srb->sense_buffer[2 ] =HARDWARE_ERROR ; } } }
Signature · 1 line
1.7.0.1
Alan Stern
maintainerQuoted 7 lines · Luben Tuikov
This patch solves two things:
1) Enables autosense emulation code to correctly interpret descriptor format sense data, and
2) Fixes a bug whereby the autosense emulation code would overwrite descriptor format sense data with SENSE KEY HARDWARE ERROR in fixed format, to incorrectly look like this:
...
Quoted 4 lines
drivers/usb/storage/transport.c +0
-0@@ -794,12 +793,7 @@ Retry_Sense:* everything worked or that there was an unspecified * problem. We have to decide which. */
drivers/usb/storage/transport.c Retry_Sense: · line 799
Quoted 10 lines
drivers/usb/storage/transport.c +1
-6-if (/* Filemark 0, ignore EOM, ILI 0, no sense */ - (srb->sense_buffer[2 ] &0xaf ) ==0 && -/* No ASC or ASCQ */ - srb->sense_buffer[12 ] ==0 && - srb->sense_buffer[13 ] ==0 ) { - +if (sshdr.sense_key ==0 && sshdr.asc ==0 && sshdr.ascq ==0 ) {/* If things are really okay, then let's show that. * Zero out the sense buffer so the higher layers * won't realize we did an unsolicited auto-sense.
What about the Filemark and ILI tests? You mustn't get rid of them; they are needed for tape drives.
Alan Stern
Quoted 3 lines · Alan Stern
What about the Filemark and ILI tests? You mustn't get rid of them;
they are needed for tape drives.
I've resubmitted the patch.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Alan Stern
maintainerQuoted 6 lines · Luben Tuikov
Quoted 3 lines · Alan Stern
What about the Filemark and ILI tests? You mustn't get rid of them;
they are needed for tape drives.I've resubmitted the patch.
Yeah, the new one is better. I would have preferred to see fm_ili explained in a comment, but this is okay.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 3 lines · Alan Stern
Yeah, the new one is better. I would have preferred to see
fm_ili
explained in a comment, but this is okay.
Thanks. I tried to preserve the logic and the comments as much as possible and tried not to introduce neither additional comment nor additional logic. Is this an implicit ACK then? Or does Greg needs an explicit one? Will he or I have to ask for an explicit ACK? From whom else?
Alan Stern
maintainerQuoted 6 lines · Luben Tuikov
Quoted 3 lines · Alan Stern
Yeah, the new one is better. I would have preferred to see
fm_ili
explained in a comment, but this is okay.Thanks. I tried to preserve the logic and the comments as much as possible and tried not to introduce neither additional comment nor additional logic. Is this an implicit ACK then? Or does Greg needs an explicit one? Will he or I have to ask for an explicit ACK? From whom else?
You may consider it an ACK from me. However this needs to be approved by the usb-storage maintainer.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html