Re: [PATCH 7/12] ide-disk: merge LBA28 and LBA48 Host Protected Area support code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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. :-)

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).

-	}
-	return addr;
-}
-
-static unsigned long long idedisk_read_native_max_address_ext(ide_drive_t *drive)
-{
-	ide_task_t args;
-	struct ide_taskfile *tf = &args.tf;
-	unsigned long long addr = 0;
-
-	/* Create IDE/ATA command request structure */
-	memset(&args, 0, sizeof(ide_task_t));
-	tf->device  = ATA_LBA;
-	tf->command = WIN_READ_NATIVE_MAX_EXT;
-	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
-	args.handler				= &task_no_data_intr;
-        /* submit command request */
-        ide_raw_taskfile(drive, &args, NULL);
-
-	/* if OK, compute maximum address value */
-	if ((tf->command & 0x01) == 0) {
 		u32 high, low;

No newline after declaration block. Well, I see that it's been this way before the patch but it doesn't have to when this code has been already touched...

+		if (lba48)
+			high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) |
+				tf->hob_lbal;
+		else
+			high = tf->device & 0xf;
 		high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;

Wait, you've just properly calculated 'high'! And now LBA28 variant is borken. :-|

 		low  = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
 		addr = ((__u64)high << 24) | low;
@@ -387,38 +371,11 @@ static unsigned long long idedisk_read_n

[...]

@@ -438,7 +400,11 @@ static unsigned long long idedisk_set_ma
 	/* if OK, compute maximum address value */
 	if ((tf->command & 0x01) == 0) {
 		u32 high, low;

  Again no newline after declaration...

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

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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux