Patchwork libata: fix ata_id_logical_per_physical_sectors

login
register
mail settings
Submitter Christoph Hellwig
Date Jan. 28, 2010, 10:42 a.m.
Message ID <20100128104234.GA25693@lst.de>
Download mbox | patch
Permalink /patch/43856/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Christoph Hellwig - Jan. 28, 2010, 10:42 a.m.
The value we get from the low byte of the ATA_ID_SECTOR_SIZE word is not not
a plain multiple, but the log of it, so fix the helper to give the correct
answer.  Without this we'll get an incorrect minimal I/O size in the block
limits VPD page for 4k sector drives.

Signed-off-by: Christoph Hellwig <hch@lst.de>

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov - Jan. 28, 2010, 10:56 a.m.
Hello.

Christoph Hellwig wrote:

> The value we get from the low byte of the ATA_ID_SECTOR_SIZE word is not not
> a plain multiple, but the log of it, so fix the helper to give the correct
> answer.  Without this we'll get an incorrect minimal I/O size in the block
> limits VPD page for 4k sector drives.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
>
> Index: linux-2.6/include/linux/ata.h
> ===================================================================
> --- linux-2.6.orig/include/linux/ata.h	2010-01-28 11:17:41.968003842 +0100
> +++ linux-2.6/include/linux/ata.h	2010-01-28 11:20:02.355268610 +0100
> @@ -649,7 +649,7 @@ static inline int ata_id_has_large_logic
>  
>  static inline u8 ata_id_logical_per_physical_sectors(const u16 *id)
>  {
> -	return id[ATA_ID_SECTOR_SIZE] & 0xf;
> +	return 1 << (id[ATA_ID_SECTOR_SIZE] & 0xf);
>   

   Will this still fit into u8?

WBR, Sergei


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Christoph Hellwig - Jan. 28, 2010, 10:59 a.m.
On Thu, Jan 28, 2010 at 01:56:21PM +0300, Sergei Shtylyov wrote:
> > static inline u8 ata_id_logical_per_physical_sectors(const u16 *id)
> > {
> >-	return id[ATA_ID_SECTOR_SIZE] & 0xf;
> >+	return 1 << (id[ATA_ID_SECTOR_SIZE] & 0xf);
> >  
> 
>   Will this still fit into u8?

Right now the only value we get in practice is 8 for 4k sector drivers
which still fits fine into an u8.  But it might indeed be safer to use a
large value.

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

Index: linux-2.6/include/linux/ata.h
===================================================================
--- linux-2.6.orig/include/linux/ata.h	2010-01-28 11:17:41.968003842 +0100
+++ linux-2.6/include/linux/ata.h	2010-01-28 11:20:02.355268610 +0100
@@ -649,7 +649,7 @@  static inline int ata_id_has_large_logic
 
 static inline u8 ata_id_logical_per_physical_sectors(const u16 *id)
 {
-	return id[ATA_ID_SECTOR_SIZE] & 0xf;
+	return 1 << (id[ATA_ID_SECTOR_SIZE] & 0xf);
 }
 
 static inline int ata_id_has_lba48(const u16 *id)