Re: [PATCH] mass storage : emulation of sat scsi_pass_thru withATACB

[matthieu castet]

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/