[patch]liberal acceptance of 0x28 asc in sd.c

[patch]liberal acceptance of 0x28 asc in sd.c

[Oliver Neukum]

Hi,

>From reading the SCSI spec it seems that having the valid bit 0 (0x70
checked in scsi_sense_valid) should does not invalidate the ASC or ASQ.
[See page 37 of spc4r02.pdf].  It should only invalidate the INFORMATION
field. Therefore remove the sense_valid check from the USB quirk.

This is needed for a strange USB storage device.

Signed-off-by: Brandon Philips <bphilips@suse.de>
Signed-off-by: Oliver Neukum <oneukum@suse.de>

---

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index e5e7d78..ac119a0 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1204,8 +1204,7 @@ sd_spinup_disk(struct scsi_disk *sdkp)
 		 * Yes, this sense key/ASC combination shouldn't
 		 * occur here.  It's characteristic of these devices.
 		 */
-		} else if (sense_valid &&
-				sshdr.sense_key == UNIT_ATTENTION &&
+		} else if (sshdr.sense_key == UNIT_ATTENTION &&
 				sshdr.asc == 0x28) {
 			if (!spintime) {
 				spintime_expire = jiffies + 5 * HZ;

Re: [patch]liberal acceptance of 0x28 asc in sd.c

[James Bottomley]

On Thu, 2008-10-30 at 13:09 +0100, Oliver Neukum wrote:
> >From reading the SCSI spec it seems that having the valid bit 0 (0x70
> checked in scsi_sense_valid) should does not invalidate the ASC or ASQ.
> [See page 37 of spc4r02.pdf].

That's a correct reading of the spec as regards the valid bit for non
descriptor sense data, but not applicable to this situation. sense_valid
is generated from (byte[0] & 0x70 == 0x70) (see scsi_sense_valid() in
include/scsi/scsi_eh.h) not from the valid bit.  This test is *required*
by the standard otherwise what we have isn't sense data.

>   It should only invalidate the INFORMATION
> field. Therefore remove the sense_valid check from the USB quirk.
> 
> This is needed for a strange USB storage device.

Could you elaborate a bit more?  What's its problem (and what is it
replying as byte 0 to a REQUEST_SENSE)?

Also:

> Signed-off-by: Brandon Philips <bphilips@suse.de>
> Signed-off-by: Oliver Neukum <oneukum@suse.de>

These signoffs don't make sense for what you sent ... this implies that
its Brandon's patch which you altered, in which case it needs a From: at
the beginning to make him the author.

James

Re: [patch]liberal acceptance of 0x28 asc in sd.c

[Oliver Neukum]

Am Donnerstag, 30. Oktober 2008 15:01:13 schrieb James Bottomley:
> On Thu, 2008-10-30 at 13:09 +0100, Oliver Neukum wrote:
> > >From reading the SCSI spec it seems that having the valid bit 0 (0x70
> > checked in scsi_sense_valid) should does not invalidate the ASC or ASQ.
> > [See page 37 of spc4r02.pdf].
> 
> That's a correct reading of the spec as regards the valid bit for non
> descriptor sense data, but not applicable to this situation. sense_valid
> is generated from (byte[0] & 0x70 == 0x70) (see scsi_sense_valid() in
> include/scsi/scsi_eh.h) not from the valid bit.  This test is *required*
> by the standard otherwise what we have isn't sense data.
> 
> >   It should only invalidate the INFORMATION
> > field. Therefore remove the sense_valid check from the USB quirk.
> > 
> > This is needed for a strange USB storage device.
> 
> Could you elaborate a bit more?  What's its problem (and what is it
> replying as byte 0 to a REQUEST_SENSE)?

I got a report about a storage device that reports 0x28 and scsi_sense_valid()
returns false.

> > Signed-off-by: Brandon Philips <bphilips@suse.de>
> > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> 
> These signoffs don't make sense for what you sent ... this implies that
> its Brandon's patch which you altered, in which case it needs a From: at
> the beginning to make him the author.

I wrote it.

	Regards
		Oliver