* mysterious shifts in USB storage drivers.
@ 2003-05-15 3:31 davej
2003-05-15 3:37 ` Linus Torvalds
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: davej @ 2003-05-15 3:31 UTC (permalink / raw)
To: torvalds; +Cc: linux-kernel, gregkh
These went into 2.4 over a year ago. Unfortunatly,
with no comments in the logs.
diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/datafab.c linux-2.5/drivers/usb/storage/datafab.c
--- bk-linus/drivers/usb/storage/datafab.c 2003-04-10 06:01:25.000000000 +0100
+++ linux-2.5/drivers/usb/storage/datafab.c 2003-03-17 23:42:38.000000000 +0000
@@ -670,7 +670,7 @@ int datafab_transport(Scsi_Cmnd * srb, s
srb->result = SUCCESS;
} else {
info->sense_key = UNIT_ATTENTION;
- srb->result = CHECK_CONDITION << 1;
+ srb->result = CHECK_CONDITION;
}
return rc;
}
diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/jumpshot.c linux-2.5/drivers/usb/storage/jumpshot.c
--- bk-linus/drivers/usb/storage/jumpshot.c 2003-04-10 06:01:25.000000000 +0100
+++ linux-2.5/drivers/usb/storage/jumpshot.c 2003-03-17 23:42:38.000000000 +0000
@@ -611,7 +611,7 @@ int jumpshot_transport(Scsi_Cmnd * srb,
srb->result = SUCCESS;
} else {
info->sense_key = UNIT_ATTENTION;
- srb->result = CHECK_CONDITION << 1;
+ srb->result = CHECK_CONDITION;
}
return rc;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mysterious shifts in USB storage drivers.
2003-05-15 3:31 mysterious shifts in USB storage drivers davej
@ 2003-05-15 3:37 ` Linus Torvalds
2003-05-15 4:56 ` Greg KH
2003-05-15 14:31 ` Andries Brouwer
2 siblings, 0 replies; 6+ messages in thread
From: Linus Torvalds @ 2003-05-15 3:37 UTC (permalink / raw)
To: davej; +Cc: linux-kernel, gregkh
On Thu, 15 May 2003 davej@codemonkey.org.uk wrote:
>
> These went into 2.4 over a year ago. Unfortunatly,
> with no comments in the logs.
There's a lot more of these than the two this patch fixes. Just do a grep
for CHECK_COND in storage/*.c.
Patch dropped pending further explanation of why these two cases would be
special.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mysterious shifts in USB storage drivers.
2003-05-15 3:31 mysterious shifts in USB storage drivers davej
2003-05-15 3:37 ` Linus Torvalds
@ 2003-05-15 4:56 ` Greg KH
2003-05-15 7:14 ` Matthew Dharm
2003-05-15 14:31 ` Andries Brouwer
2 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2003-05-15 4:56 UTC (permalink / raw)
To: davej, mdharm-usb; +Cc: torvalds, linux-kernel
On Thu, May 15, 2003 at 04:31:17AM +0100, davej@codemonkey.org.uk wrote:
> These went into 2.4 over a year ago. Unfortunatly,
> with no comments in the logs.
My logs show this went into 2.4 with this comment:
usb-storage: ISD-200 fixes, more unusual devices, and many cleanups
(o) Fix to ISD-200 driver to work on big-endian platforms, including PPC.
This has been in circulation for a while, and seems well-tested.
(o) Add several unusual_devs.h entries
(o) A couple more debugging improvements
(o) A slight improvement to the EXPERIMENTAL HP82xx driver, which should
help with newer units.
(o) A _major_ cleanup of error handling code throughout the driver. Note
that this is in preparation to deploy the new error-handling state
machine (special thanks to Alan Sterm for this work). Right now, the
optimizations are simple and straightforward (elimination of redundant
code paths, etc). Nothing tremendous, but it looks kinda invasive.
So this was a tiny part of a bigger patch. I defer to Matt as to if
this kind of change should be put into 2.5.
thanks,
greg k-h
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/datafab.c linux-2.5/drivers/usb/storage/datafab.c
> --- bk-linus/drivers/usb/storage/datafab.c 2003-04-10 06:01:25.000000000 +0100
> +++ linux-2.5/drivers/usb/storage/datafab.c 2003-03-17 23:42:38.000000000 +0000
> @@ -670,7 +670,7 @@ int datafab_transport(Scsi_Cmnd * srb, s
> srb->result = SUCCESS;
> } else {
> info->sense_key = UNIT_ATTENTION;
> - srb->result = CHECK_CONDITION << 1;
> + srb->result = CHECK_CONDITION;
> }
> return rc;
> }
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/jumpshot.c linux-2.5/drivers/usb/storage/jumpshot.c
> --- bk-linus/drivers/usb/storage/jumpshot.c 2003-04-10 06:01:25.000000000 +0100
> +++ linux-2.5/drivers/usb/storage/jumpshot.c 2003-03-17 23:42:38.000000000 +0000
> @@ -611,7 +611,7 @@ int jumpshot_transport(Scsi_Cmnd * srb,
> srb->result = SUCCESS;
> } else {
> info->sense_key = UNIT_ATTENTION;
> - srb->result = CHECK_CONDITION << 1;
> + srb->result = CHECK_CONDITION;
> }
> return rc;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mysterious shifts in USB storage drivers.
2003-05-15 4:56 ` Greg KH
@ 2003-05-15 7:14 ` Matthew Dharm
2003-05-15 8:07 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Matthew Dharm @ 2003-05-15 7:14 UTC (permalink / raw)
To: Greg KH; +Cc: davej, torvalds, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2932 bytes --]
Hrm... nothing in the logs, but I remember this. Apparently, the
srb->result field actually requires this format. If you look at other LLDD
code in linux/drivers/scsi/ you'll see that the << 1 is common.
This should be in 2.5... I thought it already was.
Matt
On Wed, May 14, 2003 at 09:56:37PM -0700, Greg KH wrote:
> On Thu, May 15, 2003 at 04:31:17AM +0100, davej@codemonkey.org.uk wrote:
> > These went into 2.4 over a year ago. Unfortunatly,
> > with no comments in the logs.
>
> My logs show this went into 2.4 with this comment:
>
> usb-storage: ISD-200 fixes, more unusual devices, and many cleanups
>
> (o) Fix to ISD-200 driver to work on big-endian platforms, including PPC.
> This has been in circulation for a while, and seems well-tested.
> (o) Add several unusual_devs.h entries
> (o) A couple more debugging improvements
> (o) A slight improvement to the EXPERIMENTAL HP82xx driver, which should
> help with newer units.
> (o) A _major_ cleanup of error handling code throughout the driver. Note
> that this is in preparation to deploy the new error-handling state
> machine (special thanks to Alan Sterm for this work). Right now, the
> optimizations are simple and straightforward (elimination of redundant
> code paths, etc). Nothing tremendous, but it looks kinda invasive.
>
> So this was a tiny part of a bigger patch. I defer to Matt as to if
> this kind of change should be put into 2.5.
>
> thanks,
>
> greg k-h
>
> > diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/datafab.c linux-2.5/drivers/usb/storage/datafab.c
> > --- bk-linus/drivers/usb/storage/datafab.c 2003-04-10 06:01:25.000000000 +0100
> > +++ linux-2.5/drivers/usb/storage/datafab.c 2003-03-17 23:42:38.000000000 +0000
> > @@ -670,7 +670,7 @@ int datafab_transport(Scsi_Cmnd * srb, s
> > srb->result = SUCCESS;
> > } else {
> > info->sense_key = UNIT_ATTENTION;
> > - srb->result = CHECK_CONDITION << 1;
> > + srb->result = CHECK_CONDITION;
> > }
> > return rc;
> > }
> > diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/jumpshot.c linux-2.5/drivers/usb/storage/jumpshot.c
> > --- bk-linus/drivers/usb/storage/jumpshot.c 2003-04-10 06:01:25.000000000 +0100
> > +++ linux-2.5/drivers/usb/storage/jumpshot.c 2003-03-17 23:42:38.000000000 +0000
> > @@ -611,7 +611,7 @@ int jumpshot_transport(Scsi_Cmnd * srb,
> > srb->result = SUCCESS;
> > } else {
> > info->sense_key = UNIT_ATTENTION;
> > - srb->result = CHECK_CONDITION << 1;
> > + srb->result = CHECK_CONDITION;
> > }
> > return rc;
> > }
--
Matthew Dharm Home: mdharm-usb@one-eyed-alien.net
Maintainer, Linux USB Mass Storage Driver
Dudes! May the Open Source be with you.
-- Eric S. Raymond
User Friendly, 12/3/1998
[-- Attachment #2: Type: application/pgp-signature, Size: 232 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mysterious shifts in USB storage drivers.
2003-05-15 7:14 ` Matthew Dharm
@ 2003-05-15 8:07 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2003-05-15 8:07 UTC (permalink / raw)
To: mdharm-usb; +Cc: davej, torvalds, linux-kernel
On Thu, May 15, 2003 at 12:14:59AM -0700, Matthew Dharm wrote:
> Hrm... nothing in the logs, but I remember this. Apparently, the
> srb->result field actually requires this format. If you look at other LLDD
> code in linux/drivers/scsi/ you'll see that the << 1 is common.
>
> This should be in 2.5... I thought it already was.
Nope, care to send me a patch that fixes this, and the other usages of
this for 2.5?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: mysterious shifts in USB storage drivers.
2003-05-15 3:31 mysterious shifts in USB storage drivers davej
2003-05-15 3:37 ` Linus Torvalds
2003-05-15 4:56 ` Greg KH
@ 2003-05-15 14:31 ` Andries Brouwer
2 siblings, 0 replies; 6+ messages in thread
From: Andries Brouwer @ 2003-05-15 14:31 UTC (permalink / raw)
To: davej; +Cc: torvalds, linux-kernel, gregkh
On Thu, May 15, 2003 at 04:31:17AM +0100, davej@codemonkey.org.uk wrote:
> These went into 2.4 over a year ago. Unfortunatly,
> with no comments in the logs.
>
>
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/drivers/usb/storage/datafab.c linux-2.5/drivers/usb/storage/datafab.c
> --- bk-linus/drivers/usb/storage/datafab.c 2003-04-10 06:01:25.000000000 +0100
> +++ linux-2.5/drivers/usb/storage/datafab.c 2003-03-17 23:42:38.000000000 +0000
> @@ -670,7 +670,7 @@ int datafab_transport(Scsi_Cmnd * srb, s
> srb->result = SUCCESS;
> } else {
> info->sense_key = UNIT_ATTENTION;
> - srb->result = CHECK_CONDITION << 1;
> + srb->result = CHECK_CONDITION;
> }
> return rc;
> }
And then you say: I have no idea what they do, let us also put them in 2.5?
Very long ago someone noticed that all status codes were even
and decided to save some space in arrays by shifting them right one.
Thus, we find in drivers/scsi/scsi.h:
#define status_byte(result) (((result) >> 1) & 0x1f)
Usually the status byte is returned by the device, but everywhere
where we invent a status ourselves we need the <<1.
This is annoying, and the kernl, both 2.4 and 2.5, is full of
mistakes here - may submit some patches against 2.5.70 in case
that appears soon. These days we also have the explicit codes
that have been shifted already:
/*
* SCSI Architecture Model (SAM) Status codes. Taken from SAM-3 draft
* T10/1561-D Revision 4 Draft dated 7th November 2002.
*/
#define SAM_STAT_GOOD 0x00
#define SAM_STAT_CHECK_CONDITION 0x02
#define SAM_STAT_CONDITION_MET 0x04
#define SAM_STAT_BUSY 0x08
...
/*
* Status codes. These are deprecated as they are shifted 1 bit right
* from those found in the SCSI standards. This causes confusion for
* applications that are ported to several OSes. Prefer SAM Status codes
* above.
*/
#define GOOD 0x00
#define CHECK_CONDITION 0x01
#define CONDITION_GOOD 0x02
#define BUSY 0x04
Andries
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-05-15 14:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-05-15 3:31 mysterious shifts in USB storage drivers davej
2003-05-15 3:37 ` Linus Torvalds
2003-05-15 4:56 ` Greg KH
2003-05-15 7:14 ` Matthew Dharm
2003-05-15 8:07 ` Greg KH
2003-05-15 14:31 ` Andries Brouwer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.