Re: ST340823A disk size issue

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

Hi,

On Wednesday 01 August 2007, Mikko Rapeli wrote:
> Hello, 
> 
> (More details at http://bugzilla.kernel.org/show_bug.cgi?id=8816 )
> 
> My home server disk doesn't work with latest kernels since its size is
> reported or probed correctly. With 2.6.22.1 I get:
> 
> Probing IDE interface ide1...
> hdd: ST340823A, ATA DISK drive
> hdd: selected mode 0x42
> ...
> hdd: max request size: 128KiB
> hdd: Host Protected Area detected.
>         current capacity is 78165360 sectors (40020 MB)
>         native  capacity is 78165361 sectors (40020 MB)
> hdd: Host Protected Area disabled.
> hdd: 78165361 sectors (40020 MB) w/1024KiB Cache, CHS=65535/16/63,
> UDMA(33)
> hdd: cache flushes not supported
>  hdd: hdd1 hdd2 hdd3
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, 
> sector=78165360
> ide: failed opcode was: unknown
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
> sector=78165360
> ide: failed opcode was: unknown
> hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
> hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360,
> sector=78165360
> ...
> 
> And as said shows in bugzilla, badblocks, fsck and smartctl think the
> drive is not broken or going to die too soon.
> 
> What's the difference between the size formats of
> WIN_READ_NATIVE_MAX_EXT read by
> ide-disk.c/idedisk_read_native_max_address_ext() and (hdreg.h) 
> hd_driveid->lba_capacity_2 read by, if I read drivers/ide/* code
> correctly, ide-probe.c/do_identify()?
> 
> If this disk is reporting one of these wrong, which fix could be
> applied? 
> 
> Both disabling 'stroke' and removing addr++ in
> ide-disk.c/idedisk_read_native_max_address*() fix the problem, but if
> kernel code is correct and standard compliant, these fixes are out of
> the question.
> 
> Perhaps a boot parameter to kernel could force the disk sector count to
> some value for buggy drives?

Could you try attached patch?

[PATCH] ide-disk: workaround for buggy HPA support on ST340823A

This disk reports total number of sectors instead of maximum sector address
in response to READ_NATIVE_MAX_ADDRESS command and also happily accepts
SET_MAX_ADDRESS command with the bogus value.  This results in +1 sector
capacity being used and errors on attempts to use the last sector.

...
hdd: Host Protected Area detected.
        current capacity is 78165360 sectors (40020 MB)
        native  capacity is 78165361 sectors (40020 MB)
hdd: Host Protected Area disabled.
...
hdd: dma_intr: status=0x51 { DriveReady SeekComplete Error }
hdd: dma_intr: error=0x10 { SectorIdNotFound }, LBAsect=78165360, sector=78165360
...

Add hpa_list[] table and workaround the issue in idedisk_check_hpa().

Fixes kernel bugzilla bug #8816.

Thanks to Mikko for investigating the issue and testing this patch.

Cc: Mikko Rapeli <mikko.rapeli@iki.fi>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-disk.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Index: b/drivers/ide/ide-disk.c
===================================================================
--- a/drivers/ide/ide-disk.c
+++ b/drivers/ide/ide-disk.c
@@ -481,6 +481,14 @@ static inline int idedisk_supports_lba48
 	       && id->lba_capacity_2;
 }
 
+/*
+ * Some disks report total number of sectors instead of
+ * maximum sector address.  We list them here.
+ */
+static const struct drive_list_entry hpa_list[] = {
+	{ "ST340823A",	NULL },
+};
+
 static void idedisk_check_hpa(ide_drive_t *drive)
 {
 	unsigned long long capacity, set_max;
@@ -492,6 +500,15 @@ static void idedisk_check_hpa(ide_drive_
 	else
 		set_max = idedisk_read_native_max_address(drive);
 
+	if (ide_in_drive_list(drive->id, hpa_list)) {
+		/*
+		 * Since we are inclusive wrt to firmware revisions do this
+		 * extra check and apply the workaround only when needed.
+		 */
+		if (set_max == capacity + 1)
+			set_max--;
+	}
+
 	if (set_max <= capacity)
 		return;