Messages in this thread | | | Date | Sun, 9 Mar 2008 18:20:14 -0700 | From | Matthew Dharm <> | Subject | Re: [PATCH] mass storage : emulation of sat scsi_pass_thru with ATACB |
| |
On Sun, Mar 09, 2008 at 10:42:50PM +0100, matthieu castet wrote: > Hi > > here an update version of the patch
We're getting very close here.
> + /* this value can change, but most(all ?) manufacturers keep the > cypress > + * default : 0x24 > + */
Line wrap problem. This happens a few times in your patch, but this was the first one I noticed.
> + srb->cmnd[0] = 0x24; > + srb->cmnd[1] = 0x24; > + srb->cmnd[2] = 0; > + > + srb->cmnd[3] = 0xff - 1; > + srb->cmnd[4] = 1;
What are these magic constants? Symbolic names or comments would be apropriate here.
> + if (srb->cmnd[12] == ATA_CMD_ID_ATA || srb->cmnd[12] == > ATA_CMD_ID_ATAPI) > + srb->cmnd[2] |= (1<<7);
What does this block do? In general, you could use some more comments about the code flow...
> + /* if the device doesn't support ATACB > + * abort and register usb_stor_transparent_scsi_command > + * callback > + */ > + if (srb->result == SAM_STAT_CHECK_CONDITION && > + memcmp(srb->sense_buffer, usb_stor_sense_invalidCDB, > + sizeof(usb_stor_sense_invalidCDB)) == 0) { > + us->subclass = US_SC_SCSI; > + us->proto_handler = usb_stor_transparent_scsi_command; > + goto end; > + }
This is an interesting section of code. Do you have reason to believe that some of the units you've claimed via unusual_devs.h do not support this protocol, or are you just being forward-looking?
Either way, some debug printout would be good here...
> --- linux-2.6.24.2.orig/include/linux/usb_usual.h 2008-03-09 > 21:15:01.000000000 +0100 > +++ linux-2.6.24.2/include/linux/usb_usual.h 2008-03-09 > 21:24:12.000000000 +0100 > @@ -81,8 +81,9 @@ > #define US_SC_8070 0x05 /* Removable media */ > #define US_SC_SCSI 0x06 /* Transparent */ > #define US_SC_ISD200 0x07 /* ISD200 ATA */ > +#define US_SC_CYP_ATACB 0x08 /* Cypress ATACB */ > #define US_SC_MIN US_SC_RBC > -#define US_SC_MAX US_SC_ISD200 > +#define US_SC_MAX US_SC_CYP_ATACB > > #define US_SC_DEVICE 0xff /* Use device's value */
I thought we changed this so that the non-spec protocols started from 0xfe and worked their way down?
Matt
-- Matthew Dharm Home: mdharm-usb@one-eyed-alien.net Maintainer, Linux USB Mass Storage Driver
C: They kicked your ass, didn't they? S: They were cheating! -- The Chief and Stef User Friendly, 11/19/1997 [unhandled content-type:application/pgp-signature] | |