Suspend support for IDE
Hi!
This adds driver support to ide-disk.c. Does it look good to you? [If so, applying it will not hurt, altrough it might change in near future.]
Pavel
drivers/ide/ide-disk.c +27 -1
@@ -123,6 +123,8 @@*/ static ide_startstop_t do_rw_disk (ide_drive_t *drive,struct request *rq,unsigned long block) { +if (drive->blocked) +panic ("ide: Request while drive blocked? You don't like your data intact?" );if (!(rq->flags &REQ_CMD )) {blk_dump_rq_flags (rq,"do_rw_disk, bad command" );ide_end_request (drive,0 );
@@ -910,13 +912,36 @@ide_add_setting (drive,"max_failures" ,SETTING_RW ,-1 ,-1 ,TYPE_INT ,0 ,65535 ,1 ,1 , &drive->max_failures,NULL ); } +static int idedisk_suspend (struct device *dev,u32 state,u32 level) +{ +int i; +ide_drive_t *drive = dev->driver_data; + +printk ("ide_disk_suspend()\n" ); +while (HWGROUP (drive)->handler) +schedule (); + drive->blocked =1 ; +} + +static int idedisk_resume (struct device *dev,u32 level) +{ +ide_drive_t *drive = dev->driver_data; +if (!drive->blocked) +panic ("ide: Resume but not suspended?\n" ); + drive->blocked =0 ; +} + +/* This is just a hook for the overall driver tree. * * FIXME: This is soon goig to replace the custom linked list games played up * to great extend between the different components of the IDE drivers. */ -static struct device_driver idedisk_devdrv = {}; +static struct device_driver idedisk_devdrv = { + suspend: idedisk_suspend, + resume: idedisk_resume, +};static void idedisk_setup (ide_drive_t *drive) {
@@ -963,6 +988,7 @@sprintf (drive->device.name,"ide-disk" ); drive->device.driver = &idedisk_devdrv; drive->device.parent = &HWIF (drive)->device; + drive->device.driver_data = drive;device_register (&drive->device); }
include/linux/ide.h +1 -0
@@ -410,6 +410,7 @@ unsigned autotune : 2; /* 1=autotune, 2=noautotune, 0=default */ unsigned remap_0_to_1 : 2; /* 0=remap if ezdrive, 1=remap, 2=noremap */ unsigned ata_flash : 1; /* 1=present, 0=default */ + unsigned blocked : 1; /* 1=powermanagment told us not to do anything, so sleep nicely */unsigned addressing;/* : 2; 0=28-bit, 1=48-bit, 2=64-bit */ byte scsi;/* 0=default, 1=skip current ide-subdriver for ide-scsi emulation */ select_t select;/* basic drive/head select reg value */
Signature · 2 lines
(about SSSCA) "I don't say this lightly. However, I really think that the U.S. no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
drivers/ide/ide-disk.c line 922
Quoted 2 lines
drivers/ide/ide-disk.c +2
-0+while (HWGROUP (drive)->handler) +schedule ();
You need to yield. Remember the process might be hard real time and blocking your real work from occuring. while(foo) schedule() is always a bug
drivers/ide/ide-disk.c line 932
Quoted 7 lines
drivers/ide/ide-disk.c +7
-0+static int idedisk_resume (struct device *dev,u32 level) +{ +ide_drive_t *drive = dev->driver_data; +if (!drive->blocked) +panic ("ide: Resume but not suspended?\n" ); + drive->blocked =0 ; +}
Also remember you must perform the sequences to wake up the drive and restore the controller logic (and of course in the right order). Newer disks won't just wake up when fed a random command (eg ibm microdrives)
The suspend order similarly is important - finish the current command, then flush the disk cache, then when it completes you can tell the drive to power down. On some systems you want to drop it back to PIO0 non DMA before the powerdown or S4BIOS restore from disk will fail.
APM generally does all this for you, ACPI won't.
Quoted 6 lines
drivers/ide/ide-disk.c line 922
Quoted 2 lines
drivers/ide/ide-disk.c +2
-0+while (HWGROUP (drive)->handler) +schedule ();You need to yield. Remember the process might be hard real time and blocking your real work from occuring. while(foo)
schedule()is always a bug
Quoted 7 lines
drivers/ide/ide-disk.c line 932
Quoted 7 lines
drivers/ide/ide-disk.c +7
-0+static int idedisk_resume (struct device *dev,u32 level) +{ +ide_drive_t *drive = dev->driver_data; +if (!drive->blocked) +panic ("ide: Resume but not suspended?\n" ); + drive->blocked =0 ; +}
I wouldn't do it this way in fact. The IDE disk is a device on the IDE bus, so it's up to the IDE bus to get the PM requests, and pass them down appropriately to the low level driver. The IDE common code has to make sure any further request is properly blocked etc... Then, the controller itself can eventually be suspended as well.
You can see code that blocks the IDE requests in ide-pmac sleep code, it isn't perfect (I have a few known issues when waking up from sleep with mounted CD/DVDs for example) but except from these it appear to work well. It is currently specific to the pmac PM scheme, but I hope to find time to port this over to the new macanism and make it more generic in the future. That code is appropriate for what I need, you may, as Alan states, need additional care to please your BIOS, like switching back to PIO0, which means also updating your controller timings.
Ben.
Quoted 5 lines · Pavel Machek
Hi!
This adds driver support to
ide-disk.c. Does it look good to you? [If so, applying it will not hurt, altrough it might change in near future.]
Since I'm open for change anyway and it apprently doesn't hurt my eyes ;-). I'm gald to include it. (God would I love to delete the silly suspen partition on my noebook and suspend to file instead!).
(I will just replace the panic check by a BUG_ON check...)
Tank you very much for your efforts to implement this!
Quoted 69 lines
drivers/ide/ide-disk.c +27
-1@@ -123,6 +123,8 @@*/ static ide_startstop_t do_rw_disk (ide_drive_t *drive,struct request *rq,unsigned long block) { +if (drive->blocked) +panic ("ide: Request while drive blocked? You don't like your data intact?" );if (!(rq->flags &REQ_CMD )) {blk_dump_rq_flags (rq,"do_rw_disk, bad command" );ide_end_request (drive,0 );@@ -910,13 +912,36 @@ide_add_setting (drive,"max_failures" ,SETTING_RW ,-1 ,-1 ,TYPE_INT ,0 ,65535 ,1 ,1 , &drive->max_failures,NULL ); } +static int idedisk_suspend (struct device *dev,u32 state,u32 level) +{ +int i; +ide_drive_t *drive = dev->driver_data; + +printk ("ide_disk_suspend()\n" ); +while (HWGROUP (drive)->handler) +schedule (); + drive->blocked =1 ; +} + +static int idedisk_resume (struct device *dev,u32 level) +{ +ide_drive_t *drive = dev->driver_data; +if (!drive->blocked) +panic ("ide: Resume but not suspended?\n" ); + drive->blocked =0 ; +} + +/* This is just a hook for the overall driver tree. * * FIXME: This is soon goig to replace the custom linked list games played up * to great extend between the different components of the IDE drivers. */ -static struct device_driver idedisk_devdrv = {}; +static struct device_driver idedisk_devdrv = { + suspend: idedisk_suspend, + resume: idedisk_resume, +};static void idedisk_setup (ide_drive_t *drive) {@@ -963,6 +988,7 @@sprintf (drive->device.name,"ide-disk" ); drive->device.driver = &idedisk_devdrv; drive->device.parent = &HWIF (drive)->device; + drive->device.driver_data = drive;device_register (&drive->device); }include/linux/ide.h +1
-0@@ -410,6 +410,7 @@ unsigned autotune : 2; /* 1=autotune, 2=noautotune, 0=default */ unsigned remap_0_to_1 : 2; /* 0=remap if ezdrive, 1=remap, 2=noremap */ unsigned ata_flash : 1; /* 1=present, 0=default */ + unsigned blocked : 1; /* 1=powermanagment told us not to do anything, so sleep nicely */unsigned addressing;/* : 2; 0=28-bit, 1=48-bit, 2=64-bit */ byte scsi;/* 0=default, 1=skip current ide-subdriver for ide-scsi emulation */ select_t select;/* basic drive/head select reg value */
Signature · 3 lines
- phone: +49 214 8656 283 - job: eVision-Ventures AG, LEV .de (MY OPINIONS ARE MY OWN!) - langs: de_DE.ISO8859-1, en_US, pl_PL.ISO8859-2, last ressort: ru_RU.KOI8-R
Hi!
Quoted 5 lines
drivers/ide/ide-disk.c line 922
Quoted 2 lines
drivers/ide/ide-disk.c +2
-0+while (HWGROUP (drive)->handler) +schedule ();You need to yield. Remember the process might be hard real time and blocking your real work from occuring. while(foo)
schedule()is always a bug
The process calling this is kernel thread doing powermangment; we have it under full control. yield() is probably more intuitive, through.
Quoted 4 lines
Quoted 4 lines
drivers/ide/ide-disk.c +4
-0+static int idedisk_resume (struct device *dev,u32 level) +{ +ide_drive_t *drive = dev->driver_data; +if (!drive->blocked)
Quoted 8 lines
drivers/ide/ide-disk.c line 932
Quoted 3 lines
drivers/ide/ide-disk.c +3
-0+panic ("ide: Resume but not suspended?\n" ); + drive->blocked =0 ; +}Also remember you must perform the sequences to wake up the drive and restore the controller logic (and of course in the right order). Newer disks won't just wake up when fed a random command (eg ibm microdrives)
Wake from S3 or S4 should look like power-up from disks perspective. I should need no commands to do that.
Restoring right UDMA mode... Well, I'll need to do that, probably. (What I have there is just enough to prevent disk corruption. I'm still likely to see some bus resets, but no longer data loose, I believe.)
Quoted 2 lines
The suspend order similarly is important - finish the current command,
The while loop above should make sure no command is happening just now, right?
Quoted 2 lines
then flush the disk cache, then when it completes you can tell the drive
Disks that need cache flush are broken, anyway -- they lied us on command completion -- right?
Quoted 1 line
to power down.
Why should I tell the drive to power down? It is going to loose its power, anyway (I believe in both S3 and S4).
Quoted 2 lines
On some systems you want to drop it back to PIO0 non DMA before the powerdown or S4BIOS restore from disk will fail.
S4BIOS is not on my list just now; agreed it would be better.
Signature · 2 lines
(about SSSCA) "I don't say this lightly. However, I really think that the U.S. no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
Quoted 2 lines
Wake from S3 or S4 should look like power-up from disks perspective. I should need no commands to do that.
Assuming the BIOS didn't do the work you need to bring the disks up as if you were the BIOS. That means doing the controller configuration, waking the drive as per ATA6. What does ACPI guarantee here ? If ACPI brough the drive back into its "happy bios state" then I agree you are right. If its like an APM resume from suspend to ram then its not always so clear cut.
Also there is the some fun about buggy drives and power up happenings. On no account can you issue any command that might touch the platter unless you know the drive is at full running speed when spinning up certain old drives because the firmware in some cases forgets to check the drive is at speed and you physically destroy the disk over time. Thankfully thats old old drives (540Mb quantum if I remember rightly)
Quoted 5 lines
Quoted 2 lines
then flush the disk cache, then when it completes you can tell the drive
Disks that need cache flush are broken, anyway -- they lied us on command completion -- right?
Wrong. Please read the spec. If you are going to be the IDE maintainer you are wasting your time until you read and understand the specifications. If you don't do the cache flush you will lose data on some drives. An IDE drive is permitted to keep a write back cache. Forced writes to the media are in an upcoming IDE command set draft (although in general you can half fake that with a read and verify).
Quoted 2 lines
Why should I tell the drive to power down? It is going to loose its power, anyway (I believe in both S3 and S4).
So it can shut itself down nicely and do any housework it wants to do (like flushing the cache if the cache flush command isnt supported.. its
optional in older ATA standards)
Quoted 4 lines
Quoted 2 lines
On some systems you want to drop it back to PIO0 non DMA before the powerdown or S4BIOS restore from disk will fail.
S4BIOS is not on my list just now; agreed it would be better.
Alan
Quoted 6 lines · Alan Cox
Also there is the some fun about buggy drives and power up happenings. On no account can you issue any command that might touch the platter unless you know the drive is at full running speed when spinning up certain old drives because the firmware in some cases forgets to check the drive is at speed and you physically destroy the disk over time. Thankfully thats old old drives (540Mb quantum if I remember rightly)
That could be 1.6G or 2.5G WD drives. AC31600, AC32500.
Roger.
Signature · 4 lines
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 ** *-- BitWizard writes Linux device drivers for any device you may have! --* * There are old pilots, and there are bold pilots. * There are also old, bald pilots.
Does the IDE blacklist include those drives that malfunction if they're not using the correct connector on the IDE ribbon cable?
I found out a few weeks ago that my Caviar WD36400 is that particular. Upon putting that drive back on the middle connector on secondary, my box was then able to suspend and resume without the drive disconnecting on me.
-- Derek Witt.
----- Original Message -----
From: "Rogier Wolff" <[email protected]>
To: "Alan Cox" <[email protected]>
Cc: "Pavel Machek" <[email protected]>; <[email protected]>; "kernel list" <[email protected]>
Sent: Sunday, March 10, 2002 2:23 AM
Subject: Re: Suspend support for IDE
Quoted 2 lines
Quoted 1 line · Alan Cox
Also there is the some fun about buggy drives and power up happenings.
On no
Quoted 1 line
Quoted 1 line
account can you issue any command that might touch the platter unless
you
Quoted 1 line
Quoted 1 line
know the drive is at full running speed when spinning up certain old
drives
Quoted 1 line
Quoted 1 line
because the firmware in some cases forgets to check the drive is at
speed
Quoted 18 lines
Quoted 2 lines
and you physically destroy the disk over time. Thankfully thats old old drives (540Mb quantum if I remember rightly)
That could be 1.6G or 2.5G WD drives. AC31600, AC32500.
Roger.
--
** [email protected] ** http://www.BitWizard.nl/ ** +31-15-2137555 ** *-- BitWizard writes Linux device drivers for any device you may have! --*
* There are old pilots, and there are bold pilots.
* There are also old, bald pilots.
-
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 2 lines
Does the IDE blacklist include those drives that malfunction if they're not using the correct connector on the IDE ribbon cable?
Thats not something the drive is even aware of unless the cable is using CS (cable select) in which case its a bios/host master-slave issue
Quoted 6 lines
Quoted 2 lines
Why should I tell the drive to power down? It is going to loose its power, anyway (I believe in both S3 and S4).
So it can shut itself down nicely and do any housework it wants to do (like flushing the cache if the cache flush command isnt supported.. its
optional in older ATA standards)
Or, in the case of newer IBM TravelStars, so that the drive can unload its head properly instead of having to do an uncontrolled emergency unload that shortens the drive's life and makes an awful screeching noise.
-Barry K. Nathan <[email protected]>
Quoted 10 lines · Barry K. Nathan
Quoted 6 lines
Quoted 2 lines
Why should I tell the drive to power down? It is going to loose its power, anyway (I believe in both S3 and S4).
So it can shut itself down nicely and do any housework it wants to do (like flushing the cache if the cache flush command isnt supported.. its
optional in older ATA standards)Or, in the case of newer IBM TravelStars, so that the drive can unload its head properly instead of having to do an uncontrolled emergency unload that shortens the drive's life and makes an awful screeching noise.
Eeep. Have we just set the wayback machine for 15 years ago? I don't remember seeing PARK.COM in /sbin recently.
Sorry, couldn't resist.
Reid
_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com
Quoted 8 lines · Reid Hekman
Quoted 7 lines · Barry K. Nathan
Quoted 6 lines
Quoted 2 lines
Why should I tell the drive to power down? It is going to loose its power, anyway (I believe in both S3 and S4).
So it can shut itself down nicely and do any housework it wants to do (like flushing the cache if the cache flush command isnt supported.. its
optional in older ATA standards)
Quoted 8 lines
Quoted 3 lines
Or, in the case of newer IBM TravelStars, so that the drive can unload its head properly instead of having to do an uncontrolled emergency unload that shortens the drive's life and makes an awful screeching noise.
Eeep. Have we just set the wayback machine for 15 years ago? I don't remember seeing PARK.COM in /sbin recently.
Sorry, couldn't resist.
No, I put it in the NOTIFY calls with some help from Tim Hockin! Since the end-user generally does not need to know how to deal w/ these issues, the kernel had better get smart but who knows if it made in in there in 2.4 yet and whether it got broke in 2.5 or not.
Regards,
Andre Hedrick
Linux Disk Certification Project Linux ATA Development
Hi!
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
then flush the disk cache, then when it completes you can tell the drive
Disks that need cache flush are broken, anyway -- they lied us on command completion -- right?
Wrong. Please read the spec. If you are going to be the IDE maintainer you are
I'm *NOT* going to be the IDE maintainer. Here's pavel!
Pavel
Signature · 2 lines
Casualities in World Trade Center: ~3k dead inside the building, cryptography in U.S.A. and free speech in Czech Republic.
Quoted 7 lines · Pavel Machek
Wake from S3 or S4 should look like power-up from disks perspective. I should need no commands to do that.
Restoring right UDMA mode... Well, I'll need to do that, probably. (What I have there is just enough to prevent disk corruption. I'm still likely to see some bus resets, but no longer data loose, I believe.)
Yeep. But doing this will only be possible in few weeks, when the corresponding setup code is really modularized and note the current "ifdef whatever" buch of collected fixes. However for certain your patch is a starting point and we are in odd series anyway...
Quoted 6 lines
Quoted 3 lines
The suspend order similarly is important - finish the current command,
The while loop above should make sure no command is happening just now, right?
Beware of the long houl interrupts found by ide_add_timer
Quoted 6 lines
Quoted 3 lines
then flush the disk cache, then when it completes you can tell the drive
Disks that need cache flush are broken, anyway -- they lied us on command completion -- right?
;-)... And the games they wan't to play on the data registers are silly as well... if you ask me... well personally they are running directly into a "backword compatibility to our own mistakes" trap. The only comparable thing to this was the QIC interfaces abusing the floppy disk stepper line for *serial* command transfer.