Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Daniel J Blueman]

Hi Luben, Greg, Matthew,

Any plan for reviewing and accepting this patch [1] yet? I'm still
seeing the symptoms on USB 3 SATA bridges with linux-3.0+.

Thanks,
  Daniel

--- [1]

http://www.kerneltrap.com/mailarchive/linux-kernel/2010/11/11/4644097/thread

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

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/usb/storage/transport.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 64ec073..cb04664 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -691,6 +691,9 @@ 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;
+		const u8 *scdd;
+		u8 fm_ili;

 		/* device supports and needs bigger sense buffer */
 		if (us->fflags & US_FL_SANE_SENSE)
@@ -774,32 +777,30 @@ 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 */
 		srb->result = SAM_STAT_CHECK_CONDITION;

+		scdd = scsi_sense_desc_find(srb->sense_buffer,
+					    SCSI_SENSE_BUFFERSIZE, 4);
+		fm_ili = (scdd ? scdd[3] : srb->sense_buffer[2]) & 0xA0;
+
 		/* We often get empty sense data.  This could indicate that
 		 * 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 &&
+		    fm_ili == 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 +815,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;
 			}
 		}
 	}
-- 
Daniel J Blueman

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Fri, Sep 02, 2011 at 02:24:36PM +0800, Daniel J Blueman wrote:
> Hi Luben, Greg, Matthew,
> 
> Any plan for reviewing and accepting this patch [1] yet? I'm still
> seeing the symptoms on USB 3 SATA bridges with linux-3.0+.

I needed an ack from Matthew before I could take it.

Matthew?


> --- [1]
> 
> http://www.kerneltrap.com/mailarchive/linux-kernel/2010/11/11/4644097/thread
> 
> 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
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> ---
>  drivers/usb/storage/transport.c |   34 +++++++++++++++++++---------------
>  1 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
> index 64ec073..cb04664 100644
> --- a/drivers/usb/storage/transport.c
> +++ b/drivers/usb/storage/transport.c
> @@ -691,6 +691,9 @@ 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;
> +		const u8 *scdd;
> +		u8 fm_ili;
> 
>  		/* device supports and needs bigger sense buffer */
>  		if (us->fflags & US_FL_SANE_SENSE)
> @@ -774,32 +777,30 @@ 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 */
>  		srb->result = SAM_STAT_CHECK_CONDITION;
> 
> +		scdd = scsi_sense_desc_find(srb->sense_buffer,
> +					    SCSI_SENSE_BUFFERSIZE, 4);
> +		fm_ili = (scdd ? scdd[3] : srb->sense_buffer[2]) & 0xA0;
> +
>  		/* We often get empty sense data.  This could indicate that
>  		 * 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 &&
> +		    fm_ili == 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 +815,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;
>  			}
>  		}
>  	}
> -- 
> Daniel J Blueman

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

On Sep 2, 2011, at 11:18, Greg KH <greg@kroah.com> wrote:

> On Fri, Sep 02, 2011 at 02:24:36PM +0800, Daniel J Blueman wrote:
>> Hi Luben, Greg, Matthew,
>> 
>> Any plan for reviewing and accepting this patch [1] yet? I'm still
>> seeing the symptoms on USB 3 SATA bridges with linux-3.0+.
> 
> I needed an ack from Matthew before I could take it.
> 
> Matthew?


He acked it already: http://marc.info/?l=linux-kernel&m=128950140420081&w=2

But interestingly enough, it never made it into your tree. 

> 
> 
>> --- [1]
>> 
>> http://www.kerneltrap.com/mailarchive/linux-kernel/2010/11/11/4644097/thread
>> 
>> 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
>> 
>> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
>> ---
>> drivers/usb/storage/transport.c |   34 +++++++++++++++++++---------------
>> 1 files changed, 19 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
>> index 64ec073..cb04664 100644
>> --- a/drivers/usb/storage/transport.c
>> +++ b/drivers/usb/storage/transport.c
>> @@ -691,6 +691,9 @@ 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;
>> +        const u8 *scdd;
>> +        u8 fm_ili;
>> 
>>        /* device supports and needs bigger sense buffer */
>>        if (us->fflags & US_FL_SANE_SENSE)
>> @@ -774,32 +777,30 @@ 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 */
>>        srb->result = SAM_STAT_CHECK_CONDITION;
>> 
>> +        scdd = scsi_sense_desc_find(srb->sense_buffer,
>> +                        SCSI_SENSE_BUFFERSIZE, 4);
>> +        fm_ili = (scdd ? scdd[3] : srb->sense_buffer[2]) & 0xA0;
>> +
>>        /* We often get empty sense data.  This could indicate that
>>         * 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 &&
>> +            fm_ili == 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 +815,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;
>>            }
>>        }
>>    }
>> -- 
>> Daniel J Blueman

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Fri, Sep 02, 2011 at 12:08:44PM -0700, Luben Tuikov wrote:
> On Sep 2, 2011, at 11:18, Greg KH <greg@kroah.com> wrote:
> 
> > On Fri, Sep 02, 2011 at 02:24:36PM +0800, Daniel J Blueman wrote:
> >> Hi Luben, Greg, Matthew,
> >> 
> >> Any plan for reviewing and accepting this patch [1] yet? I'm still
> >> seeing the symptoms on USB 3 SATA bridges with linux-3.0+.
> > 
> > I needed an ack from Matthew before I could take it.
> > 
> > Matthew?
> 
> 
> He acked it already: http://marc.info/?l=linux-kernel&m=128950140420081&w=2
> 
> But interestingly enough, it never made it into your tree. 

Odd, you did, and I found the resend I asked for in my archives, but I
don't see it in the tree, I don't know what happened here, sorry.

I'll go queue it up when master.kernel.org comes back up, sorry about
that.

greg k-h

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

On Sep 2, 2011, at 12:22, Greg KH <greg@kroah.com> wrote:

> On Fri, Sep 02, 2011 at 12:08:44PM -0700, Luben Tuikov wrote:
>> On Sep 2, 2011, at 11:18, Greg KH <greg@kroah.com> wrote:
>> 
>>> On Fri, Sep 02, 2011 at 02:24:36PM +0800, Daniel J Blueman wrote:
>>>> Hi Luben, Greg, Matthew,
>>>> 
>>>> Any plan for reviewing and accepting this patch [1] yet? I'm still
>>>> seeing the symptoms on USB 3 SATA bridges with linux-3.0+.
>>> 
>>> I needed an ack from Matthew before I could take it.
>>> 
>>> Matthew?
>> 
>> 
>> He acked it already: http://marc.info/?l=linux-kernel&m=128950140420081&w=2
>> 
>> But interestingly enough, it never made it into your tree. 
> 
> Odd, you did, and I found the resend I asked for in my archives, but I
> don't see it in the tree, I don't know what happened here, sorry.
> 
> I'll go queue it up when master.kernel.org comes back up, sorry about
> that.

Are you sure it's mine. I don't remember resending it other than the "take 2" thread (link above).

        Luben

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Fri, Sep 02, 2011 at 02:28:50PM -0700, Luben Tuikov wrote:
> On Sep 2, 2011, at 12:22, Greg KH <greg@kroah.com> wrote:
> 
> > On Fri, Sep 02, 2011 at 12:08:44PM -0700, Luben Tuikov wrote:
> >> On Sep 2, 2011, at 11:18, Greg KH <greg@kroah.com> wrote:
> >> 
> >>> On Fri, Sep 02, 2011 at 02:24:36PM +0800, Daniel J Blueman wrote:
> >>>> Hi Luben, Greg, Matthew,
> >>>> 
> >>>> Any plan for reviewing and accepting this patch [1] yet? I'm still
> >>>> seeing the symptoms on USB 3 SATA bridges with linux-3.0+.
> >>> 
> >>> I needed an ack from Matthew before I could take it.
> >>> 
> >>> Matthew?
> >> 
> >> 
> >> He acked it already: http://marc.info/?l=linux-kernel&m=128950140420081&w=2
> >> 
> >> But interestingly enough, it never made it into your tree. 
> > 
> > Odd, you did, and I found the resend I asked for in my archives, but I
> > don't see it in the tree, I don't know what happened here, sorry.
> > 
> > I'll go queue it up when master.kernel.org comes back up, sorry about
> > that.
> 
> Are you sure it's mine. I don't remember resending it other than the
> "take 2" thread (link above).

I have a Message-ID: <929551.13749.qm@web31805.mail.mud.yahoo.com>
in my inbox that looks like it was the message needed, did I just edit
that one by hand with the needed acks?

I'll bounce it to you for verification.

greg k-h

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

On Nov 11, 2010, at 11:11, Greg KH <greg@kroah.com> wrote:

> On Thu, Nov 11, 2010 at 10:49:46AM -0800, Matthew Dharm wrote:
>> On Thu, Nov 11, 2010 at 10:27:11AM -0800, Luben Tuikov wrote:
>>> The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.
>> 
>> Looks reasonable.  The underlying SCSI functions seem to all have the tests
>> to distinguish between sense format types.  Let's just hope there are no
>> devices out there which can confuse those tests, or we'll be buried in
>> regressions.
>> 
>> ACKed-by: Matthew Dharm <mdharm-usb@one-eyed-alien.net>
> 
> Luben, can you resend me the original patch with Matthew's ack on it?  I
> no longer have it in my mboxes.
> 

No prob. 

> thanks,
> 
> greg k-h

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

On Nov 11, 2010, at 11:11, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:

> On Thu, Nov 11, 2010 at 10:49:46AM -0800, Matthew Dharm wrote:
>> On Thu, Nov 11, 2010 at 10:27:11AM -0800, Luben Tuikov wrote:
>>> The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.
>> 
>> Looks reasonable.  The underlying SCSI functions seem to all have the tests
>> to distinguish between sense format types.  Let's just hope there are no
>> devices out there which can confuse those tests, or we'll be buried in
>> regressions.
>> 
>> ACKed-by: Matthew Dharm <mdharm-usb-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>
> 
> Luben, can you resend me the original patch with Matthew's ack on it?  I
> no longer have it in my mboxes.
> 

No prob. 

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

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Thu, Nov 11, 2010 at 10:49:46AM -0800, Matthew Dharm wrote:
> On Thu, Nov 11, 2010 at 10:27:11AM -0800, Luben Tuikov wrote:
> > The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.
> 
> Looks reasonable.  The underlying SCSI functions seem to all have the tests
> to distinguish between sense format types.  Let's just hope there are no
> devices out there which can confuse those tests, or we'll be buried in
> regressions.
> 
> ACKed-by: Matthew Dharm <mdharm-usb@one-eyed-alien.net>

Luben, can you resend me the original patch with Matthew's ack on it?  I
no longer have it in my mboxes.

thanks,

greg k-h

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

On Thu, Nov 11, 2010 at 10:27:11AM -0800, Luben Tuikov wrote:
> The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.

Looks reasonable.  The underlying SCSI functions seem to all have the tests
to distinguish between sense format types.  Let's just hope there are no
devices out there which can confuse those tests, or we'll be buried in
regressions.

ACKed-by: Matthew Dharm <mdharm-usb@one-eyed-alien.net>

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

My mother not mind to die for stoppink Windows NT!  She is rememberink 
Stalin!
					-- Pitr
User Friendly, 9/6/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 771 bytes --]

On Thu, Nov 11, 2010 at 10:27:11AM -0800, Luben Tuikov wrote:
> The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.

Looks reasonable.  The underlying SCSI functions seem to all have the tests
to distinguish between sense format types.  Let's just hope there are no
devices out there which can confuse those tests, or we'll be buried in
regressions.

ACKed-by: Matthew Dharm <mdharm-usb-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

My mother not mind to die for stoppink Windows NT!  She is rememberink 
Stalin!
					-- Pitr
User Friendly, 9/6/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> On Thu, Nov 11, 2010 at 09:07:53AM -0800, Luben Tuikov wrote:
> > 
> > Oh, I see. I'll re-post the patch then to Matthew and
> the people and lists you listed above.
> 
> Now that I'm on the CC, I see this.
> 
> What's the 10k-foot level summary here?

I'm not sure either. It's apparently the procedure of getting a patch into the linux kernel nowadays. Last time I did this was 5 years ago and in linux-scsi--I remember it being easier, again that was 5 years ago.

> Is there an alternate sense-data format that uses some sort of "descriptor" structure?

Yes. Some devices return this format of sense data for some commands.

> I'm assuming this is
> a new SCSI-III thing, or did I just miss it in SCSI-II?
> This *looks* like you're using functions provided by the
> SCSI core to
> access and modify the sense data... so I'm assuming that's
> because there
> are alternate formats...

The patch doesn't add code that modifies the sense data. That modification code is already there, the patch fixes the modification which assumed only one type of format of sense data.

The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.

> 
> Matt
> 
> -- 
> Matthew Dharm           
>                
>   Home: mdharm-usb@one-eyed-alien.net
> 
> Maintainer, Linux USB Mass Storage Driver
> 
> It was a new hope.
>            
>         -- Dust Puppy
> User Friendly, 12/25/1998
> 

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Matthew Dharm <mdharm-kernel@one-eyed-alien.net> wrote:
> On Thu, Nov 11, 2010 at 09:07:53AM -0800, Luben Tuikov wrote:
> > 
> > Oh, I see. I'll re-post the patch then to Matthew and
> the people and lists you listed above.
> 
> Now that I'm on the CC, I see this.
> 
> What's the 10k-foot level summary here?

I'm not sure either. It's apparently the procedure of getting a patch into the linux kernel nowadays. Last time I did this was 5 years ago and in linux-scsi--I remember it being easier, again that was 5 years ago.

> Is there an alternate sense-data format that uses some sort of "descriptor" structure?

Yes. Some devices return this format of sense data for some commands.

> I'm assuming this is
> a new SCSI-III thing, or did I just miss it in SCSI-II?
> This *looks* like you're using functions provided by the
> SCSI core to
> access and modify the sense data... so I'm assuming that's
> because there
> are alternate formats...

The patch doesn't add code that modifies the sense data. That modification code is already there, the patch fixes the modification which assumed only one type of format of sense data.

The patch uses SCSI functions to abstract the sense information irrespective of the type of format of sense data.

> 
> Matt
> 
> -- 
> Matthew Dharm           
>                
>   Home: mdharm-usb@one-eyed-alien.net
> 
> Maintainer, Linux USB Mass Storage Driver
> 
> It was a new hope.
>            
>         -- Dust Puppy
> User Friendly, 12/25/1998
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Matthew Dharm]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

On Thu, Nov 11, 2010 at 09:07:53AM -0800, Luben Tuikov wrote:
> 
> Oh, I see. I'll re-post the patch then to Matthew and the people and lists you listed above.

Now that I'm on the CC, I see this.

What's the 10k-foot level summary here?  Is there an alternate sense-data
format that uses some sort of "descriptor" structure?  I'm assuming this is
a new SCSI-III thing, or did I just miss it in SCSI-II?

This *looks* like you're using functions provided by the SCSI core to
access and modify the sense data... so I'm assuming that's because there
are alternate formats...

Matt

-- 
Matthew Dharm                              Home: mdharm-usb@one-eyed-alien.net 
Maintainer, Linux USB Mass Storage Driver

It was a new hope.
					-- Dust Puppy
User Friendly, 12/25/1998

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Greg KH <greg@kroah.com> wrote:
> On Thu, Nov 11, 2010 at 08:49:51AM
> -0800, Luben Tuikov wrote:
> > --- On Thu, 11/11/10, Greg KH <greg@kroah.com>
> wrote:
> > > On Fri, Oct 22, 2010 at 02:12:51PM
> > > -0700, Luben Tuikov wrote:
> > > > 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
> > > > 
> > > > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> > > 
> > > I need to get the ack from the usb-storage
> > > maintainer.? Can you get that
> > > so that I can apply this?
> > 
> > Hi Greg,
> > 
> > I don't know who the usb-storage maintainer is, as
> I've mostly
> > contributed to linux-scsi. Do you?
> 
> $ ./scripts/get_maintainer.pl --roles --file
> drivers/usb/storage/usb.c
> Matthew Dharm <mdharm-usb@one-eyed-alien.net>
> (maintainer:USB MASS STORAGE...)
> Greg Kroah-Hartman <gregkh@suse.de>
> (supporter:USB SUBSYSTEM)
> linux-usb@vger.kernel.org
> (open list:USB MASS STORAGE...)
> usb-storage@lists.one-eyed-alien.net
> (open list:USB MASS STORAGE...)
> linux-kernel@vger.kernel.org
> (open list)
> 
> Or you can just look in the MAINTAINERS file :)

Oh, and I was looking for the MAINTAINERS file in Documentation/.

Wow! There is actually a "get_maintainer" script!? Unbelievable!

> > I know Alan acked this patch. Is it him? Any reason
> you didn't CC the
> > usb-storage maintainer in this email?  Can you
> reply back to this
> > thread CC-ing him? The only email I have in my address
> book is yours.
> 
> Please resend your patch to Matthew, and the people and
> lists listed
> above.  I need his ack before I can take it.

Oh, I see. I'll re-post the patch then to Matthew and the people and lists you listed above.

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> wrote:
> On Thu, Nov 11, 2010 at 08:49:51AM
> -0800, Luben Tuikov wrote:
> > --- On Thu, 11/11/10, Greg KH <greg-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
> wrote:
> > > On Fri, Oct 22, 2010 at 02:12:51PM
> > > -0700, Luben Tuikov wrote:
> > > > 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
> > > > 
> > > > Signed-off-by: Luben Tuikov <ltuikov-/E1597aS9LQAvxtiuMwx3w@public.gmane.org>
> > > 
> > > I need to get the ack from the usb-storage
> > > maintainer.? Can you get that
> > > so that I can apply this?
> > 
> > Hi Greg,
> > 
> > I don't know who the usb-storage maintainer is, as
> I've mostly
> > contributed to linux-scsi. Do you?
> 
> $ ./scripts/get_maintainer.pl --roles --file
> drivers/usb/storage/usb.c
> Matthew Dharm <mdharm-usb-JGfshJpz5UybPZpvUQj5UqxOck334EZe@public.gmane.org>
> (maintainer:USB MASS STORAGE...)
> Greg Kroah-Hartman <gregkh-l3A5Bk7waGM@public.gmane.org>
> (supporter:USB SUBSYSTEM)
> linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> (open list:USB MASS STORAGE...)
> usb-storage-ijkIwGHArpdIPJnuZ7Njw4oP9KaGy4wf@public.gmane.org
> (open list:USB MASS STORAGE...)
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> (open list)
> 
> Or you can just look in the MAINTAINERS file :)

Oh, and I was looking for the MAINTAINERS file in Documentation/.

Wow! There is actually a "get_maintainer" script!? Unbelievable!

> > I know Alan acked this patch. Is it him? Any reason
> you didn't CC the
> > usb-storage maintainer in this email?  Can you
> reply back to this
> > thread CC-ing him? The only email I have in my address
> book is yours.
> 
> Please resend your patch to Matthew, and the people and
> lists listed
> above.  I need his ack before I can take it.

Oh, I see. I'll re-post the patch then to Matthew and the people and lists you listed above.

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

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Thu, Nov 11, 2010 at 08:49:51AM -0800, Luben Tuikov wrote:
> --- On Thu, 11/11/10, Greg KH <greg@kroah.com> wrote:
> > On Fri, Oct 22, 2010 at 02:12:51PM
> > -0700, Luben Tuikov wrote:
> > > 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
> > > 
> > > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> > 
> > I need to get the ack from the usb-storage
> > maintainer.? Can you get that
> > so that I can apply this?
> 
> Hi Greg,
> 
> I don't know who the usb-storage maintainer is, as I've mostly
> contributed to linux-scsi. Do you?

$ ./scripts/get_maintainer.pl --roles --file drivers/usb/storage/usb.c
Matthew Dharm <mdharm-usb@one-eyed-alien.net> (maintainer:USB MASS STORAGE...)
Greg Kroah-Hartman <gregkh@suse.de> (supporter:USB SUBSYSTEM)
linux-usb@vger.kernel.org (open list:USB MASS STORAGE...)
usb-storage@lists.one-eyed-alien.net (open list:USB MASS STORAGE...)
linux-kernel@vger.kernel.org (open list)

Or you can just look in the MAINTAINERS file :)

> I know Alan acked this patch. Is it him? Any reason you didn't CC the
> usb-storage maintainer in this email?  Can you reply back to this
> thread CC-ing him? The only email I have in my address book is yours.

Please resend your patch to Matthew, and the people and lists listed
above.  I need his ack before I can take it.

thanks,

greg k-h

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Greg KH <greg@kroah.com> wrote:
> On Fri, Oct 22, 2010 at 02:12:51PM
> -0700, Luben Tuikov wrote:
> > 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
> > 
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> I need to get the ack from the usb-storage
> maintainer.  Can you get that
> so that I can apply this?

Hi Greg,

I don't know who the usb-storage maintainer is, as I've mostly contributed to linux-scsi. Do you? I know Alan acked this patch. Is it him? Any reason you didn't CC the usb-storage maintainer in this email?  Can you reply back to this thread CC-ing him? The only email I have in my address book is yours.

Thank you for being so nice and patient in this long, difficult, arduous and intimidating[1] process.

   Luben
[1]linux-2.6/Documentation/development-process/6.Followthrough:22:
"Working with reviewers can be, for many developers, the most intimidating part of the kernel development process."

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Luben Tuikov]

--- On Thu, 11/11/10, Greg KH <greg@kroah.com> wrote:
> On Fri, Oct 22, 2010 at 02:12:51PM
> -0700, Luben Tuikov wrote:
> > 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
> > 
> > Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> I need to get the ack from the usb-storage
> maintainer.  Can you get that
> so that I can apply this?

Hi Greg,

I don't know who the usb-storage maintainer is, as I've mostly contributed to linux-scsi. Do you? I know Alan acked this patch. Is it him? Any reason you didn't CC the usb-storage maintainer in this email?  Can you reply back to this thread CC-ing him? The only email I have in my address book is yours.

Thank you for being so nice and patient in this long, difficult, arduous and intimidating[1] process.

   Luben
[1]linux-2.6/Documentation/development-process/6.Followthrough:22:
"Working with reviewers can be, for many developers, the most intimidating part of the kernel development process."

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

Re: [PATCH take 2] [USB] Use normalized sense when emulating autosense

[Greg KH]

On Fri, Oct 22, 2010 at 02:12:51PM -0700, Luben Tuikov wrote:
> 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
> 
> Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>

I need to get the ack from the usb-storage maintainer.  Can you get that
so that I can apply this?

thanks,

greg k-h

[PATCH take 2] [USB] Use normalized sense when emulating autosense

[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:

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

Signed-off-by: Luben Tuikov <ltuikov@yahoo.com>
---
 drivers/usb/storage/transport.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/storage/transport.c b/drivers/usb/storage/transport.c
index 64ec073..cb04664 100644
--- a/drivers/usb/storage/transport.c
+++ b/drivers/usb/storage/transport.c
@@ -691,6 +691,9 @@ 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;
+		const u8 *scdd;
+		u8 fm_ili;
 
 		/* device supports and needs bigger sense buffer */
 		if (us->fflags & US_FL_SANE_SENSE)
@@ -774,32 +777,30 @@ 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 */
 		srb->result = SAM_STAT_CHECK_CONDITION;
 
+		scdd = scsi_sense_desc_find(srb->sense_buffer,
+					    SCSI_SENSE_BUFFERSIZE, 4);
+		fm_ili = (scdd ? scdd[3] : srb->sense_buffer[2]) & 0xA0;
+
 		/* We often get empty sense data.  This could indicate that
 		 * 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 &&
+		    fm_ili == 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 +815,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;
 			}
 		}
 	}
-- 
1.7.0.1