HPA Resume patch

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel. If I made a mistake please be gentle and correct me ;)

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

If I made a mistake please be gentle and correct me ;)

diff -Naur linux-2.6.18-rc4-old/include/linux/ide.h linux-2.6.18-rc4/include/linux/ide.h

include/linux/ide.h +11 -0

@@ -1201,6 +1201,17 @@
 void ide_register_subdriver(ide_drive_t *, ide_driver_t *);
 void ide_unregister_subdriver(ide_drive_t *, ide_driver_t *);
 
+/* Bits 10 of command_set_1 and cfs_enable_1 must be equal,
+ * so on non-buggy drives we need test only one.
+ * However, we should also check whether these fields are valid.
+*/
+static inline int idedisk_supports_hpa(const struct hd_driveid *id)
+{
+        return (id->command_set_1 & 0x0400) && (id->cfs_enable_1 & 0x0400);
+}
+
+extern void init_idedisk_capacity (ide_drive_t  *drive);
+
 #define ON_BOARD		1
 #define NEVER_BOARD		0
 

diff -Naur linux-2.6.18-rc4-old/drivers/ide/ide-disk.c linux-2.6.18-rc4/drivers/ide/ide-disk.c

drivers/ide/ide-disk.c +3 -11

@@ -464,16 +464,6 @@
 }
 
 /*
- * Bits 10 of command_set_1 and cfs_enable_1 must be equal,
- * so on non-buggy drives we need test only one.
- * However, we should also check whether these fields are valid.
- */
-static inline int idedisk_supports_hpa(const struct hd_driveid *id)
-{
-	return (id->command_set_1 & 0x0400) && (id->cfs_enable_1 & 0x0400);
-}
-
-/*
  * The same here.
  */
 static inline int idedisk_supports_lba48(const struct hd_driveid *id)

@@ -528,7 +518,7 @@
  * in above order (i.e., if value of higher priority is available,
  * reset will be ignored).
  */
-static void init_idedisk_capacity (ide_drive_t  *drive)
+void init_idedisk_capacity (ide_drive_t  *drive)
 {
 	struct hd_driveid *id = drive->id;
 	/*

@@ -555,6 +545,8 @@
 	}
 }
 
+EXPORT_SYMBOL(init_idedisk_capacity);
+
 static sector_t idedisk_capacity (ide_drive_t *drive)
 {
 	return drive->capacity64 - drive->sect0;

diff -Naur linux-2.6.18-rc4-old/drivers/ide/ide.c linux-2.6.18-rc4/drivers/ide/ide.c

drivers/ide/ide.c +10 -1

@@ -1232,6 +1232,7 @@
 	struct request rq;
 	struct request_pm_state rqpm;
 	ide_task_t args;
+	int ide_cmd;
 
 	memset(&rq, 0, sizeof(rq));
 	memset(&rqpm, 0, sizeof(rqpm));

@@ -1242,7 +1243,15 @@
 	rqpm.pm_step = ide_pm_state_start_resume;
 	rqpm.pm_state = PM_EVENT_ON;
 
-	return ide_do_drive_cmd(drive, &rq, ide_head_wait);
+	ide_cmd = ide_do_drive_cmd(drive, &rq, ide_head_wait);
+
+	/* check to see if this is a hard drive
+	 * if it is then checkhpa needs to be
+	 * disabled */
+	if(drive->media == ide_disk && idedisk_supports_hpa(drive->id))
+		init_idedisk_capacity(drive);
+
+	return ide_cmd;
 }
 
 int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,

Hi!

Quoted 6 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 1 line

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

Quoted 13 lines · Pavel Machek

Hi!

Quoted 7 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 7 lines

Quoted 2 lines

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

It has design issues, at someone else already noticed. hpa restore needs to be a driver private step, included in the resume state machine. The current patch is a gross layering violation.

But thanks to Lee for taking a stab at this, I hope he'll continue and get it polished :-)

Quoted 23 lines · Jens Axboe

Quoted 13 lines · Pavel Machek

Hi!

Quoted 7 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 7 lines

Quoted 2 lines

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

Quoted 8 lines

It has design issues, at someone else already noticed. hpa restore needs to be a driver private step, included in the resume state machine. The current patch is a gross layering violation.

But thanks to Lee for taking a stab at this, I hope he'll continue and get it polished :-)

Ok I redid the patch following exactly what Sergey and Randy said. This problem happens on any computer that has HPA on their drive when they come back from resume so I don't think you have to only test this with Thinkpad users. Anyway my only question is how to I get my patched signed off by someone?

Thanks for all your help!

include/linux/ide.h +1 -0

@@ -987,6 +987,7 @@ typedef struct ide_driver_s {
 	int		(*probe)(ide_drive_t *);
 	void		(*remove)(ide_drive_t *);
 	void		(*shutdown)(ide_drive_t *);
+	void		(*resume)(ide_drive_t *);
 } ide_driver_t;
 
 #define to_ide_driver(drv) container_of(drv, ide_driver_t, gen_driver)

drivers/ide/ide.c +8 -1

@@ -1229,9 +1229,11 @@ static int generic_ide_suspend(struct de
 static int generic_ide_resume(struct device *dev)
 {
 	ide_drive_t *drive = dev->driver_data;
+	ide_driver_t *drv = to_ide_driver(dev->driver);
 	struct request rq;
 	struct request_pm_state rqpm;
 	ide_task_t args;
+	int err;
 
 	memset(&rq, 0, sizeof(rq));
 	memset(&rqpm, 0, sizeof(rqpm));

@@ -1242,7 +1244,12 @@ static int generic_ide_resume(struct dev
 	rqpm.pm_step = ide_pm_state_start_resume;
 	rqpm.pm_state = PM_EVENT_ON;
 
-	return ide_do_drive_cmd(drive, &rq, ide_head_wait);
+	err = ide_do_drive_cmd(drive, &rq, ide_head_wait);
+
+	if (err == 0 && drv->resume)
+		drv->resume(drive);
+
+	return err;
 }
 
 int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,

drivers/ide/ide-disk.c +11 -0

@@ -1024,6 +1024,17 @@ static void ide_disk_release(struct kref
 
 static int ide_disk_probe(ide_drive_t *drive);
 
+/*
+ * On HPA drives the capacity needs to be
+ * reinitilized on resume otherwise the disk
+ * can not be used and a hard reset is required
+ */
+static void ide_disk_resume(ide_drive_t *drive)
+{
+	if (idedisk_supports_hpa(drive->id))
+		init_idedisk_capacity(drive);
+}
+
 static void ide_device_shutdown(ide_drive_t *drive)
 {
 #ifdef	CONFIG_ALPHA

Quoted 39 lines · On Mon, 28 Aug 2006 22:14:34 -0400 Lee Trager wrote:

Quoted 28 lines · Jens Axboe

Quoted 16 lines · Pavel Machek

Hi!

Quoted 8 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 9 lines

Quoted 3 lines

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

Quoted 9 lines

It has design issues, at someone else already noticed. hpa restore needs to be a driver private step, included in the resume state machine. The current patch is a gross layering violation.

But thanks to Lee for taking a stab at this, I hope he'll continue and get it polished :-)

Quoted 6 lines

Ok I redid the patch following exactly what Sergey and Randy said. This problem happens on any computer that has HPA on their drive when they come back from resume so I don't think you have to only test this with Thinkpad users. Anyway my only question is how to I get my patched signed off by someone?

You do that :) after you read Documentation/SubmittingPatches and can "sign off" on the patch.

Quoted 67 lines

Thanks for all your help!

include/linux/ide.h +1 -0

@@ -987,6 +987,7 @@ typedef struct ide_driver_s {
 	int		(*probe)(ide_drive_t *);
 	void		(*remove)(ide_drive_t *);
 	void		(*shutdown)(ide_drive_t *);
+	void		(*resume)(ide_drive_t *);
 } ide_driver_t;
 
 #define to_ide_driver(drv) container_of(drv, ide_driver_t, gen_driver)

drivers/ide/ide.c +8 -1

@@ -1229,9 +1229,11 @@ static int generic_ide_suspend(struct de
 static int generic_ide_resume(struct device *dev)
 {
 	ide_drive_t *drive = dev->driver_data;
+	ide_driver_t *drv = to_ide_driver(dev->driver);
 	struct request rq;
 	struct request_pm_state rqpm;
 	ide_task_t args;
+	int err;
 
 	memset(&rq, 0, sizeof(rq));
 	memset(&rqpm, 0, sizeof(rqpm));

@@ -1242,7 +1244,12 @@ static int generic_ide_resume(struct dev
 	rqpm.pm_step = ide_pm_state_start_resume;
 	rqpm.pm_state = PM_EVENT_ON;
 
-	return ide_do_drive_cmd(drive, &rq, ide_head_wait);
+	err = ide_do_drive_cmd(drive, &rq, ide_head_wait);
+
+	if (err == 0 && drv->resume)
+		drv->resume(drive);
+
+	return err;
 }
 
 int generic_ide_ioctl(ide_drive_t *drive, struct file *file, struct block_device *bdev,

drivers/ide/ide-disk.c +12 -0

@@ -1024,6 +1024,17 @@ static void ide_disk_release(struct kref
 
 static int ide_disk_probe(ide_drive_t *drive);
 
+/*
+ * On HPA drives the capacity needs to be
+ * reinitilized on resume otherwise the disk
+ * can not be used and a hard reset is required
+ */
+static void ide_disk_resume(ide_drive_t *drive)
+{
+	if (idedisk_supports_hpa(drive->id))
+		init_idedisk_capacity(drive);
+}
+
 static void ide_device_shutdown(ide_drive_t *drive)
 {
 #ifdef	CONFIG_ALPHA

@@ -1067,6 +1078,7 @@ static ide_driver_t idedisk_driver = {
 	.error			= __ide_error,
 	.abort			= __ide_abort,
 	.proc			= idedisk_proc,
+	.resume			= ide_disk_resume,
 };
 

drivers/ide/ide-disk.c static ide_driver_t idedisk_driver = { · line 1084

Quoted 2 lines

drivers/ide/ide-disk.c +0 -0

 static int idedisk_open(struct inode *inode, struct file *filp)

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

Quoted 23 lines · Jens Axboe

Quoted 13 lines · Pavel Machek

Hi!

Quoted 7 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 7 lines

Quoted 2 lines

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

Quoted 8 lines

It has design issues, at someone else already noticed. hpa restore needs to be a driver private step, included in the resume state machine. The current patch is a gross layering violation.

But thanks to Lee for taking a stab at this, I hope he'll continue and get it polished :-)

Ok I redid the patch following exactly what Sergey and Randy said. This problem happens on any computer that has HPA on their drive when they come back from resume so I don't think you have to only test this with Thinkpad users. Anyway my only question is how to I get my patched signed off by someone?

Thanks for all your help!

Quoted 39 lines · Lee Trager

Quoted 28 lines · Jens Axboe

Quoted 16 lines · Pavel Machek

Hi!

Quoted 8 lines

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel.

Congratulations for a first patch.

Quoted 9 lines

Quoted 3 lines

If I made a mistake please be gentle and correct me ;)

We'll need signed-off-by: line next time.

Stefan, can we get this some testing? Or anyone else with thinkpad with host-protected area still enabled?

Quoted 9 lines

It has design issues, at someone else already noticed. hpa restore needs to be a driver private step, included in the resume state machine. The current patch is a gross layering violation.

But thanks to Lee for taking a stab at this, I hope he'll continue and get it polished :-)

Quoted 8 lines

Ok I redid the patch following exactly what Sergey and Randy said. This problem happens on any computer that has HPA on their drive when they come back from resume so I don't think you have to only test this with Thinkpad users. Anyway my only question is how to I get my patched signed off by someone?

Thanks for all your help!

While this is a _lot_ better than your previous patch, I don't think you quite understood my suggestion. Which is probably fault, so I'll try to be a little more verbose.

If you look at the suspend state machine, it goes through a (small) sequence of steps (see drivers/ide/ide-io.c:ide_complete_power_step()) and get the device suspended. Same thing for resume, just not that many steps there. My suggestion was to continue using this infrastructure and just add the HPA restore as a resume state. Right now it does:

        ide_pm_state_start_resume (== idedisk_pm_idle)
                complete that
        ide_pm_restore_dma
                complete that

and we are done. Your patch basically puts more actions into a single resume state switch, not ideal. What you want to do is have the HPA restore as an additional state.

Is that clearer? If not, let me know...

Thinkpad users. Anyway my only question is how to I get my patched signed off by someone?

This patch fixes a problem with computers that have HPA on their hard drive and not being able to come out of resume from RAM or disk. I've tested this patch on 2.6.17.x and 2.6.18-rc4 and it works great on both of these. This patch also fixes the bug #6840. This is my first patch to the kernel and I was told to e-mail the above people to get my patch into the kernel. If I made a mistake please be gentle and correct me ;)