Thank you Christoph, Randy, Alen for your comments. Here is ver 2 incorporating all your suggestions. On Mon, Sep 10 2007 at 18:12 +0300, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > I think just struct "struct scsi_eh_save *save" is descriptive enough and > almost fits on a line as well.. Also continuation of the prototype > is indented by two tabs normally. > I think you can kill the old prefixes in the struct, they're saved > per defintion. Done On Mon, Sep 10 2007 at 18:58 +0300, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > A trivial problem is that quilt complains about patch 3, which leaves > extra whitespace at the end of line 591 in transport.c. > Thanks > 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? > I hope you like my solution > The line initializing the sense buffer to zero should be inside the > copy_sense conditional block. > Thanks > 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. > Thanks, good catch > 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. > I was afraid of that at the beginning. But testing both with a Seagate USB sata hard-disk and Sandisk 2Gb usb-stick. They both returned a short read of 56 but other wise were happy. So I thought it is leftovers from The time the sense buffer was driver allocated. Also the standard recommends a short-read behavior. But I have taken your advise to hart and changed the code accordingly. Please check me out. ---------------------------------------------------------------- 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 [patch 1/5] scsi_error: code cleanup before refactoring of scsi_send_eh_cmnd() Move some code around and add new fixtures here. So next patch is left a Mechanical breakup of code. [patch 2/5] scsi_error: Refactoring scsi_error to facilitate in synchronous REQUEST_SENSE Here new API is introduced in scsi/scsi_eh.h and mechanical move of code to the new functions. [PATCH 3/5] usb: transport.c use scsi_eh API in REQUEST_SENSE execution Use above API for drivers/usb/storage/transport.c. Now that last User of use_sg==0, we can do the USB storage cleanups. [PATCH 4/5] NCR5380: Use scsi_eh API for REQUEST_SENSE invocation All NCR5380 family of drivers used to send a REQUEST_SENSE command. [PATCH 5/5] arm: fas216 Use scsi_eh API for REQUEST_SENSE invocation drivers/scsi/arm/fas216.c need change also. Please Test as much as possible, and report any problems/differences. Boaz Harrosh - 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
