Hello.
Bartlomiej Zolnierkiewicz wrote:
* Merge idedisk_{read_native,set}_max_address_ext() into
idedisk_{read_native,set}_max_address().
There should be no functionality changes caused by this patch.
This is unfortunately not achieved...
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
No cookie this time. :-)
"Oh, cookie cookie cookie starts with C!" :)
I thought it all started with web. 8-)
Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
[...]
@@ -350,31 +353,12 @@ static unsigned long idedisk_read_native
/* if OK, compute maximum address value */
if ((tf->command & 0x01) == 0) {
- addr = ((tf->device & 0xf) << 24) |
- (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
- addr++; /* since the return value is (maxlba - 1), we add 1 */
Actually, the return value is just maxlba (which is *not* the same as
capacity).
Indeed, moreover idedisk_{read_native,set}_max_address() comments look
also incorrect, ditto for the function names.
Probably if we handle 'addr++'/'addr_req--'/'addr_set++' in
idedisk_check_hpa() all issues will fix themselves.
Care to look into it?
Still no time...
[...]
- high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;
+ if (lba48)
+ high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) |
+ tf->hob_lbal;
+ else
+ high = tf->device & 0xf;
low = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
addr_set = ((__u64)high << 24) | low;
addr_set++;
BTW, haven't you noticed that LBA readback code is duplicate? It just asks
to be put into a separate function, like:
static u64 ide_read_lba(struct ide_taskfile *tf)
{
if (!(tf->command & 0x01)) {
u32 high, low;
if (lba48)
high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) |
tf->hob_lbal;
else
high = tf->device & 0xf;
low = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
return (((__u64)high << 24) | low) + 1;
} else
return 0;
}
"back in the days" I even had a patch doing something similar but since
it is still lost somewhere in the TODO folder + requires update for the
recent changes (which makes it useless a starting point) it would be
great if you could cook a patch adding the above helper.
I'll think about it... :-)
Thanks for review (especially for catching problem with 'high' variable),
revised patch:
[PATCH] ide-disk: merge LBA28 and LBA48 Host Protected Area support code (take 2)
* Merge idedisk_{read_native,set}_max_address_ext() into
idedisk_{read_native,set}_max_address().
v2:
* Remove LBA48 code leftover from idedisk_read_native_max_address()
('high' variable initialization). (Noticed by Sergei).
There should be no functionality changes caused by this patch.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Acked-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
MBR, Sergei
-
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
