Re: [PATCH] Cleanup libata HPA support

From: Mark Lord
Date: Tue May 08 2007 - 09:45:04 EST


Ben Collins wrote:
..
static u64 ata_tf_to_lba48(struct ata_taskfile *tf)
{
- u64 sectors = 0;
+ u64 sectors;
+ u32 high, low;
- sectors |= ((u64)(tf->hob_lbah & 0xff)) << 40;
- sectors |= ((u64)(tf->hob_lbam & 0xff)) << 32;
- sectors |= (tf->hob_lbal & 0xff) << 24;
- sectors |= (tf->lbah & 0xff) << 16;
- sectors |= (tf->lbam & 0xff) << 8;
- sectors |= (tf->lbal & 0xff);
+ high = (tf->hob_lbah << 16) |
+ (tf->hob_lbam << 8) |
+ tf->hob_lbal;
+ low = (tf->lbah << 16) |
+ (tf->lbam << 8) |
+ tf->lbal;
- return ++sectors;
+ sectors = ((u64)high << 24) | low;
+
+ return sectors + 1;
}

Ben, I very much prefer the fixed version of this function
as implemented by the patch above.

But.. is the original code actually broken, or just messy?

Cheers

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/