[PATCH] libata support for rotation speed

[PATCH] libata support for rotation speed

[Matthew Wilcox]

Add support for VPD page b1 to libata

SCSI VPD page b1 reports the nominal rotation speed of the device.
ATA devices return this information in word 217 of the identify data.
b1 can also report the physical size of the device (5.25", 3.5", etc)
but that doesn't surrently seem to be defined in the ATA specification.

I also added 0x89 to the list of pages that libata supports.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2e6e162..64e20bb 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
 	const u8 pages[] = {
 		0x00,	/* page 0x00, this page */
 		0x80,	/* page 0x80, unit serial no page */
-		0x83	/* page 0x83, device ident page */
+		0x83,	/* page 0x83, device ident page */
+		0x89,	/* page 0x89, ata info page */
+		0xb1,	/* page 0xb1, block device characteristics page */
 	};
 
 	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
@@ -1899,6 +1901,16 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 }
 
+static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
+{
+	rbuf[1] = 0xb1;
+	rbuf[3] = 0x3c;
+	rbuf[4] = args->id[217] >> 8;
+	rbuf[5] = args->id[217];
+
+	return 0;
+}
+
 /**
  *	ata_scsiop_noop - Command handler that simply returns success.
  *	@args: device IDENTIFY data / SCSI command of interest.
@@ -2922,6 +2934,9 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,
 		case 0x89:
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_89);
 			break;
+		case 0xb1:
+			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b1);
+			break;
 		default:
 			ata_scsi_invalid_field(cmd, done);
 			break;
-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] libata support for rotation speed

[Matthew Wilcox]

Version 2, Jeff pointed out I was missing a check for a sufficiently
recent version of the specification.

Add support for VPD page b1 to libata

SCSI VPD page b1 reports the nominal rotation speed of the device.
ATA devices return this information in word 217 of the identify data.
b1 can also report the physical size of the device (5.25", 3.5", etc)
but that doesn't currently seem to be defined in the ATA specification.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2e6e162..07512d4 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
 	const u8 pages[] = {
 		0x00,	/* page 0x00, this page */
 		0x80,	/* page 0x80, unit serial no page */
-		0x83	/* page 0x83, device ident page */
+		0x83,	/* page 0x83, device ident page */
+		0x89,	/* page 0x89, ata info page */
+		0xb1,	/* page 0xb1, block device characteristics page */
 	};
 
 	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
@@ -1899,6 +1901,18 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 }
 
+static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
+{
+	rbuf[1] = 0xb1;
+	rbuf[3] = 0x3c;
+	if (ata_id_major_version(args->id) > 7) {
+		rbuf[4] = args->id[217] >> 8;
+		rbuf[5] = args->id[217];
+	}
+
+	return 0;
+}
+
 /**
  *	ata_scsiop_noop - Command handler that simply returns success.
  *	@args: device IDENTIFY data / SCSI command of interest.
@@ -2922,6 +2936,9 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,
 		case 0x89:
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_89);
 			break;
+		case 0xb1:
+			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b1);
+			break;
 		default:
 			ata_scsi_invalid_field(cmd, done);
 			break;

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] libata support for rotation speed

[James Bottomley]

On Thu, 2008-06-19 at 11:59 -0600, Matthew Wilcox wrote:
> Version 2, Jeff pointed out I was missing a check for a sufficiently
> recent version of the specification.
> 
> Add support for VPD page b1 to libata
> 
> SCSI VPD page b1 reports the nominal rotation speed of the device.
> ATA devices return this information in word 217 of the identify data.
> b1 can also report the physical size of the device (5.25", 3.5", etc)
> but that doesn't currently seem to be defined in the ATA specification.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 2e6e162..07512d4 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
>  	const u8 pages[] = {
>  		0x00,	/* page 0x00, this page */
>  		0x80,	/* page 0x80, unit serial no page */
> -		0x83	/* page 0x83, device ident page */
> +		0x83,	/* page 0x83, device ident page */
> +		0x89,	/* page 0x89, ata info page */
> +		0xb1,	/* page 0xb1, block device characteristics page */
>  	};
>  
>  	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
> @@ -1899,6 +1901,18 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
>  	return 0;
>  }
>  
> +static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
> +{
> +	rbuf[1] = 0xb1;

This looks like it can be called for ATA_DEV_ATAPI as well, in which
case it's an illegal page to ask for.  Also, shouldn't this be added to
VPD page 0 (which shows the list of available VPD pages ... although it
looks like the supported 0x89 is missing too)?

> +	rbuf[3] = 0x3c;
> +	if (ata_id_major_version(args->id) > 7) {
> +		rbuf[4] = args->id[217] >> 8;
> +		rbuf[5] = args->id[217];
> +	}
> +
> +	return 0;

This would currently set device nominal form factor to unreported.  It's
actually available in identify word 168 (bits 0:3) in an identical
format to the one the VPD inquiry is expecting, so why not use it?

James

Re: [PATCH] libata support for rotation speed

[Matthew Wilcox]

On Thu, Jun 19, 2008 at 01:24:55PM -0500, James Bottomley wrote:
> On Thu, 2008-06-19 at 11:59 -0600, Matthew Wilcox wrote:
> > Version 2, Jeff pointed out I was missing a check for a sufficiently
> > recent version of the specification.
> > 
> > Add support for VPD page b1 to libata
> > 
> > SCSI VPD page b1 reports the nominal rotation speed of the device.
> > ATA devices return this information in word 217 of the identify data.
> > b1 can also report the physical size of the device (5.25", 3.5", etc)
> > but that doesn't currently seem to be defined in the ATA specification.
> > 
> > Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> > 
> > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> > index 2e6e162..07512d4 100644
> > --- a/drivers/ata/libata-scsi.c
> > +++ b/drivers/ata/libata-scsi.c
> > @@ -1899,6 +1901,18 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
> >  	return 0;
> >  }
> >  
> > +static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
> > +{
> > +	rbuf[1] = 0xb1;
> 
> This looks like it can be called for ATA_DEV_ATAPI as well, in which
> case it's an illegal page to ask for.

I was under the impression that this entire file is only used for
non-atapi devices.  At least, the VPD is supposed to have PQ/PDT filled
in, which would be 0x5 instead of 0.

> Also, shouldn't this be added to
> VPD page 0 (which shows the list of available VPD pages ... although it
> looks like the supported 0x89 is missing too)?

You even quoted the bit where I did that:

> > @@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
> >  	const u8 pages[] = {
> >  		0x00,	/* page 0x00, this page */
> >  		0x80,	/* page 0x80, unit serial no page */
> > -		0x83	/* page 0x83, device ident page */
> > +		0x83,	/* page 0x83, device ident page */
> > +		0x89,	/* page 0x89, ata info page */
> > +		0xb1,	/* page 0xb1, block device characteristics page */
> >  	};
> >  
> >  	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
> 
> This would currently set device nominal form factor to unreported.  It's
> actually available in identify word 168 (bits 0:3) in an identical
> format to the one the VPD inquiry is expecting, so why not use it?

The version of ATA-ACS 8 I have doesn't have it ;-)

I'll go and download a more recent one and respin with it.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] libata support for rotation speed

[Jeff Garzik]

Matthew Wilcox wrote:
> On Thu, Jun 19, 2008 at 01:24:55PM -0500, James Bottomley wrote:
>> This looks like it can be called for ATA_DEV_ATAPI as well, in which
>> case it's an illegal page to ask for.
> 
> I was under the impression that this entire file is only used for
> non-atapi devices.

Correct.  That code path is not travelled by ATAPI.

	Jeff

Re: [PATCH] libata support for rotation speed

[Matthew Wilcox]

On Thu, Jun 19, 2008 at 03:01:33PM -0400, Jeff Garzik wrote:
> Matthew Wilcox wrote:
> >On Thu, Jun 19, 2008 at 01:24:55PM -0500, James Bottomley wrote:
> >>This looks like it can be called for ATA_DEV_ATAPI as well, in which
> >>case it's an illegal page to ask for.
> >
> >I was under the impression that this entire file is only used for
> >non-atapi devices.
> 
> Correct.  That code path is not travelled by ATAPI.

Thanks, Jeff.  Here's version three, including the form factor from word
168:


Add support for VPD page b1 to libata

SCSI VPD page b1 reports the nominal rotation speed and physical size
of the device.  Devices that conform to ATA-8 can return this information
in words 217 and 168 of the identify data.

Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 2e6e162..c08c3b9 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
 	const u8 pages[] = {
 		0x00,	/* page 0x00, this page */
 		0x80,	/* page 0x80, unit serial no page */
-		0x83	/* page 0x83, device ident page */
+		0x83,	/* page 0x83, device ident page */
+		0x89,	/* page 0x89, ata info page */
+		0xb1,	/* page 0xb1, block device characteristics page */
 	};
 
 	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
@@ -1899,6 +1901,19 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
 	return 0;
 }
 
+static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
+{
+	rbuf[1] = 0xb1;
+	rbuf[3] = 0x3c;
+	if (ata_id_major_version(args->id) > 7) {
+		rbuf[4] = args->id[217] >> 8;
+		rbuf[5] = args->id[217];
+		rbuf[7] = args->id[168] & 0xf;
+	}
+
+	return 0;
+}
+
 /**
  *	ata_scsiop_noop - Command handler that simply returns success.
  *	@args: device IDENTIFY data / SCSI command of interest.
@@ -2922,6 +2937,9 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,
 		case 0x89:
 			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_89);
 			break;
+		case 0xb1:
+			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b1);
+			break;
 		default:
 			ata_scsi_invalid_field(cmd, done);
 			break;

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] libata support for rotation speed

[Boaz Harrosh]

Matthew Wilcox wrote:
> On Thu, Jun 19, 2008 at 03:01:33PM -0400, Jeff Garzik wrote:
>> Matthew Wilcox wrote:
>>> On Thu, Jun 19, 2008 at 01:24:55PM -0500, James Bottomley wrote:
>>>> This looks like it can be called for ATA_DEV_ATAPI as well, in which
>>>> case it's an illegal page to ask for.
>>> I was under the impression that this entire file is only used for
>>> non-atapi devices.
>> Correct.  That code path is not travelled by ATAPI.
> 
> Thanks, Jeff.  Here's version three, including the form factor from word
> 168:
> 
> 
> Add support for VPD page b1 to libata
> 
> SCSI VPD page b1 reports the nominal rotation speed and physical size
> of the device.  Devices that conform to ATA-8 can return this information
> in words 217 and 168 of the identify data.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 2e6e162..c08c3b9 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1778,7 +1778,9 @@ static unsigned int ata_scsiop_inq_00(struct ata_scsi_args *args, u8 *rbuf)
>  	const u8 pages[] = {
>  		0x00,	/* page 0x00, this page */
>  		0x80,	/* page 0x80, unit serial no page */
> -		0x83	/* page 0x83, device ident page */
> +		0x83,	/* page 0x83, device ident page */
> +		0x89,	/* page 0x89, ata info page */
> +		0xb1,	/* page 0xb1, block device characteristics page */
>  	};
>  
>  	rbuf[3] = sizeof(pages);	/* number of supported VPD pages */
> @@ -1899,6 +1901,19 @@ static unsigned int ata_scsiop_inq_89(struct ata_scsi_args *args, u8 *rbuf)
>  	return 0;
>  }
>  
> +static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
> +{
> +	rbuf[1] = 0xb1;
> +	rbuf[3] = 0x3c;
> +	if (ata_id_major_version(args->id) > 7) {
> +		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?

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]);

> +		rbuf[7] = args->id[168] & 0xf;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   *	ata_scsiop_noop - Command handler that simply returns success.
>   *	@args: device IDENTIFY data / SCSI command of interest.
> @@ -2922,6 +2937,9 @@ void ata_scsi_simulate(struct ata_device *dev, struct scsi_cmnd *cmd,
>  		case 0x89:
>  			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_89);
>  			break;
> +		case 0xb1:
> +			ata_scsi_rbuf_fill(&args, ata_scsiop_inq_b1);
> +			break;
>  		default:
>  			ata_scsi_invalid_field(cmd, done);
>  			break;
> 

Just my $0.017
Boaz

Re: [PATCH] libata support for rotation speed

[Matthew Wilcox]

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;
}

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.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] libata support for rotation speed

[James Bottomley]

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

Re: [PATCH] libata support for rotation speed

[Boaz Harrosh]

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

Re: [PATCH] libata support for rotation speed

[Jeff Garzik]

Matthew Wilcox wrote:
> On Thu, Jun 19, 2008 at 03:01:33PM -0400, Jeff Garzik wrote:
>> Matthew Wilcox wrote:
>>> On Thu, Jun 19, 2008 at 01:24:55PM -0500, James Bottomley wrote:
>>>> This looks like it can be called for ATA_DEV_ATAPI as well, in which
>>>> case it's an illegal page to ask for.
>>> I was under the impression that this entire file is only used for
>>> non-atapi devices.
>> Correct.  That code path is not travelled by ATAPI.
> 
> Thanks, Jeff.  Here's version three, including the form factor from word
> 168:
> 
> 
> Add support for VPD page b1 to libata
> 
> SCSI VPD page b1 reports the nominal rotation speed and physical size
> of the device.  Devices that conform to ATA-8 can return this information
> in words 217 and 168 of the identify data.
> 
> Signed-off-by: Matthew Wilcox <willy@linux.intel.com>

applied (2.6.27)