On Sun, 2008-06-22 at 07:31 -0600, 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.
>
> 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:
>
> /*
> * 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;
> }
>
> static unsigned int scsi_ram_get_u16(unsigned char *addr)
> {
> unsigned int data;
> data = addr[0] << 8;
> data |= addr[1];
>
> return data;
> }
>
> 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;
> }
>
> 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;
> }
This last one (but unfortunately that one only) actually exists in
scsi/scsi.h as scsi_to_u32(). If they're generally useful, expanding
them to more data types can be done ... they could even be implemented
in terms of put_unaligned_beX which might hide the confusion that
plethora of macros causes.
James
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
