On Mon, 10 Sep 2007, Boaz Harrosh wrote: > In motivation to abstract scsi_cmnd members and insulate > drivers/transports from scsi_cmnd internals. The last > place left was the REQUEST_SENSE sequence when done > synchronous, by drivers. > > Also all these drivers would do a use_sg==0 command > invocation, preventing from doing cleanups on these > drivers/transports. So this patchset is left-overs > from Christoph's 2.6.18 cleanups. > > In this patchset I have re-factored scsi_error to > make it possible for drivers to use an abstract > (easy to use, I hope) API for invoking a REQUEST_SENSE > command. The API is declared in scsi/scsi_eh.h This is a good idea, but the implementation has a few problems. A trivial problem is that quilt complains about patch 3, which leaves extra whitespace at the end of line 591 in transport.c. More seriously, why make the caller define a static generic_sense array? Why not put the array in scsi_eh_prep_cmnd(), where it can be used whenever copy_sense is nonzero? The line initializing the sense buffer to zero should be inside the copy_sense conditional block. The code setting the LUN bits in scmd[1] is wrong. It should be like the code in scsi_dispatch_cmd(); i.e., those bits should not be set if the scsi_level is SCSI_UNKNOWN. Finally, a really serious problem. The code sets the buffer length for the REQUEST SENSE command to be the length of scmd->sense_buffer, which is 96. But many USB devices won't work properly unless the requesed sense data length is 18. The appropriate adjustment must be added to transport.c in patch 3. Alan Stern - To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
