Messages in this thread | | | Date | Sun, 09 Mar 2008 09:21:40 +0100 | From | matthieu castet <> | Subject | Re: [PATCH] mass storage : emulation of sat scsi_pass_thru with ATACB |
| |
Matthew Dharm wrote: > On Sat, Mar 08, 2008 at 09:08:26PM +0100, matthieu castet wrote: >> Hi Matthew, >> >> thanks for your comments >> >> Matthew Dharm wrote: >>> Why are you using an initializer instead of a new protocol code? >> Because using a new protocol code means I need to patch all the place >> where there is a comparison between us->subclass and US_SC_SCSI. >> After all I am US_SC_SCSI with a special case for ATA12 & ATA16 commands. >> I don't translate all scsi to atacb (that's what does US_SC_ISD200). > > Yet, you call invoke_transport directly, just like any other protocol > handler. > > The proper way to do this is as a separate protocol handler. If you want > to make it clear that you are only intercepting a couple of command types, > then don't call invoke_transport() directly, call the transparent scsi > protocol handler (which, of course, does the same thing but provides > clearer layering). > > Oh, and you should add some "unlikely" tags to these if() conditionals. Hum, may be to avoid confusion with a new protocol handler, I can add my hook in usb_stor_control_thread with a new flag.
Something like : [...] /* Handle those devices which need us to fake * their inquiry data */ else if ((us->srb->cmnd[0] == INQUIRY) && (us->flags & US_FL_FIX_INQUIRY)) { [...] else if (( (us->srb->cmnd[0] == ATA_12) || (us->srb->cmnd[0] == ATA_16)) && (us->flags & US_FL_CYPRESS_ATACB)) { US_DEBUGP("emulating ATA pass thru\n"); call to emulate_pass_thru_with_atacb code }
/* we've got a command, let's do it! */ else { US_DEBUG(usb_stor_show_command(us->srb)); us->proto_handler(us->srb, us); }
Does it sound better ?
Matthieu
| |