Re: [example PATCH - not for applying] exclude certain commands

[James Bottomley <James.Bottomley@steeleye.com>]

On Sat, 2003-04-26 at 16:44, Andries.Brouwer@cwi.nl wrote:
> [[problem solved, we understand the hardware, it seems,
> now wish to find an elegant way of doing what is required
> (and only what is required) in the scsi and/or usb storage code]]
> 
> (1) usb-storage is broken in the sense that it uses sr_bufflen
> for the transfer size instead of the buffer length, then fudges
> commands to make them transfer a different length, but without
> updating sr_bufflen.

definitely agreed.

> (1A)
> Of course it is bad when variables have a function that differs
> from what is suggested by their name. So, sr_bufflen must be
> the length of the buffer and nothing else. If the length of
> the transfer is needed, there must be a field sr_xferlen or so.

This looks fine to me.  Originally, sr_bufflen was exactly that (because
all SCSI LLDs have to be prepared to transfer fewer than the specified
number of bytes anyway, since that's required by the spec).  I believe
only USB needs this to be set to the actual command length (and further
require that we ensure that the device *will* transfer exactly
this...that's to some extent the origin of the INQUIRY problems), so the
change should be small.

[...]

> I dislike the choice of identifier "ten" - it is difficult to grep for,

agreed.

> and I changed it into use_10_for_rw, with the same function as before.
> Added a field use_10_for_ms. Thus, in scsi.h:
> 
> +       unsigned use_10_for_rw:1; /* first try 10-byte read / write */
> +       unsigned use_10_for_ms:1; /* first try 10-byte mode sense/select */
> 
> in the declaration of struct scsi_device.
> 
> Now sd_do_mode_sense6() becomes sd_do_mode_sense() and does

This looks good (probably this should go in scsi_lib.c and sr should
start using it for consistency).

> All this is nice and well. Remains the question how usb-storage
> makes sure that the use_10_for_rw and use_10_for_ms flags are set
> for its devices.

Well, this should be done in the USB storage slave_configure...that's
almost exactly what the interface is designed for (altering scsi_device
parameters before the SCSI subsystem starts using it).

> Tonight I kludged this by setting this (in protocol.c) at the moment
> the first INQUIRY is done. But that is terribly ugly.
> Did I overlook some obvious means of communication?
> 
> Comments?

This sounds to be exactly the right approach (other than the missing
slave_configure).

James