Matthew Wilcox wrote: > On Sun, Jun 22, 2008 at 03:41:56PM +0300, Boaz Harrosh wrote: >>> + rbuf[4] = args->id[217] >> 8; >>> + rbuf[5] = args->id[217]; >> args->id of struct ata_scsi_args are defined as u16. >> Are they actually SWABed at this point? Are they LE? BE? > > They're cpu-endian at this point, I believe. See ata_dev_read_id() in > libata-core.c where it calls swap_buf_le16(). > >> if args->id are actually __be16 then above should be >> + put_unaligned(args->id[217], &rbuf[4]); >> else >> + put_unaligned_be16(args->id[217], &rbuf[4]); > > But the buf isn't unaligned, nor the offset within it. And I > get confused by all these macros anyway. If we had something like > store_scsi_u16(unsigned char *, u16), I'd use that, but put_unaligned_be16 > just doesn't make sense. > If you look at the code _unaligned here means to fetch a byte pointer char by char and swab by shifts, as opposed to SWABX instruction by CPU. So what I proposed is code equivalent to what you did, and optimized for BE systems. If you actually calculated that they are aligned and want the extra CPU help you will need a typecast as *((__be16 *)&rbuf[4]) = cpu_to_be16(args->id[217]); > I actually wrote my own accessors for scsi_ram. If they were to be more > generic, they ought to be renamed, but this is what I found useful: > But all this work is done. No need to reinvent the wheel, which is my point. Just change below scsi_xxx to beXX_ and your set. > /* > * SCSI requires quantities to be written MSB. They're frequently misaligned, > * so don't mess about with cpu_to_beN, just access it byte-wise > */ > static void scsi_ram_put_u32(unsigned char *addr, unsigned int data) > { > addr[0] = data >> 24; > addr[1] = data >> 16; > addr[2] = data >> 8; > addr[3] = data; > } > exactly same code as put_unaligned_be32 on LE, not optimized for BE. (actually suboptimal for some LE systems as well) > static unsigned int scsi_ram_get_u16(unsigned char *addr) > { > unsigned int data; > data = addr[0] << 8; > data |= addr[1]; > > return data; > } > dito get_unaligned_be16 > static unsigned int scsi_ram_get_u24(unsigned char *addr) > { > unsigned int data; > data = addr[0] << 16; > data |= addr[1] << 8; > data |= addr[2]; > > return data; > } > OK You got me here > static unsigned int scsi_ram_get_u32(unsigned char *addr) > { > unsigned int data; > data = addr[0] << 24; > data |= addr[1] << 16; > data |= addr[2] << 8; > data |= addr[3]; > > return data; > } > dito get_unaligned_be32 > I'm sure a more fully-featured set would include put_u24 and put_u16 too. > For the curious, u24 is useful for implementing read6/write6. > Kernel's is much nicer then those "ntoul..." which make my headache. All we need to remember is that SCSI is __bexx, and they are pretty intuitive, I think. Boaz -- 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