On Sun, Jun 22, 2003 at 03:37:28PM -0500, James Bottomley wrote: > On Sun, 2003-06-22 at 14:56, Matthew Dharm wrote: > > > But (as I read it -- remember I'm not an expert), the old sr.c code didn't > > > set the DBD bit, just like the new code. So whatever formula applied to > > > the old code should apply to the new code, yes? > > Hmm, the diff I sent was an older one which had the dbd meaning > inverted. The attached is the one I'm actually testing.
Ah.. I'll review this.
> > The code James sent sets DBD to 0 -- I like that, as many usb-storage
> > devices choke when DBD is set to 1. I believe in avoiding the DBD bit as
> > much as possible, and James seems to have eliminated it.
> >
> > However, DBD==0 means that a block descriptor is likely to be returned --
> > so we need to add in the size of the block descriptor header.
>
> OK, now I'm confused. the write caching determination code that you and
> Andries spent a while getting to work has DBD==1, and I thought we'd
> eliminated all the USB problems with that code...
Yeah, it's confusing.
Andries and I spent a great deal of time getting the use_10_for_ms worked
out -- and that's helped greatly. The DBD=1 for cache determination was
there before that, and is still there, and I would still like it removed.
However, with the use_10_for_ms code in place, many more devices don't
-fatally- choke when DBD=1. Lots still do, however.
Up until now, I really didn't understand what DBD did... now that I do, I
really don't understand why we use it at all. It's a matter of
convenience to parse the data, I guess, but it comes at a great expense of
incompatiblity.
> Since the BD skip code is quite a wodge of complexity, I'd like to know
> what devices fail on DBD==1 before trying to add it all in, especially
> as we (fortunatly) have nothing in the kernel that actually wants to see
> the BDs.
The problem as I see it is ATAPI devices. That means that sbp2, ide-scsi,
and usb-storage (all of which bridge ATAPI devices to the SCSI layer) will
be affected. These devices don't include the DBD bit in their command
specification -- the bit is reserved.
Unfortunately, more data is difficult to come by -- the cache-detect code
is still relatively new, and the use_10_for_ms is even newer (read: last
week). We haven't had the opportunity to get the bug reports back from
everywhere to find out how bad the problem is. The devices I have avoid
the fatal condition by rejecting all commands with the DBD bit set, but
it is still an error condition I'd like to avoid.
And I'm pretty certain that there are devices for which setting DBD is
fatal still. I just don't have any hard evidence to prove I'm right. Tho
I have yet to see a USB device answer the cache-detect query with a
meaningful answer.
And, I don't see DBD as a required element. It's a convenience item, which
will create a great deal of headache for us.
I don't see how the BD code is complex. If we change it so that DBD==0
always, then this problem goes away completely. Just change the calculation
of header_offset to include the DBD size (taken from the header) and we're
done -- the callers still get an offset into the data which shows them
where the mode page starts.
Matt
--
Matthew Dharm Home: [EMAIL PROTECTED]
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
pgp00000.pgp
Description: PGP signature
