login
Search
Hi, It appears that the "double spin-off" problem described in http://bugzilla.kernel.org/show_bug.cgi?id=8855 has recently started to appear during hibernation as well as during shutdown. It didn't appear during hibernation on my hp nx6325 with 2.6.26, so this is a recent regression. I have prepared the appended patch that fixes the problem for me, based on the earlier Tejun's patch at http://bugzilla.kernel.org/attachment.cgi?id=15441&action=view but I'm not really sure if this is an acceptable solution. Please advise. Thanks, Rafael not-yet-signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/ata/libata-scsi.c | 16 +++++++++++++--- drivers/ata/sata_sil.c | 30 +++++++++++++++++++++++++++++- include/linux/libata.h | 1 + kernel/power/disk.c | 2 ++ 4 files changed, 45 insertions(+), 4 deletions(-) Index: linux-2.6/drivers/ata/libata-scsi.c =================================================================== --- linux-2.6.orig/drivers/ata/libata-scsi.c +++ linux-2.6/drivers/ata/libata-scsi.c @@ -1181,6 +1181,14 @@ static unsigned int ata_scsi_start_stop_ tf->command = ATA_CMD_VERIFY; /* READ VERIFY */ } else { + /* Some odd clown BIOSen issue spindown on shutdown + * causing some drives to spin up and down again. + */ + if ((qc->ap->flags & ATA_FLAG_NO_SHUTDOWN_SPINDOWN) && + (system_state == SYSTEM_POWER_OFF || + system_state == SYSTEM_SUSPEND_DISK)) + goto skip; + /* XXX: This is for backward compatibility, will be * removed. Read Documentation/feature-removal-schedule.txt * for more info. @@ -1204,8 +1212,7 @@ static unsigned int ata_scsi_start_stop_ scmd->scsi_done = qc->scsidone; qc->scsidone = ata_delayed_done; } - scmd->result = SAM_STAT_GOOD; - return 1; + goto skip; } /* Issue ATA STANDBY IMMEDIATE command */ @@ -1221,10 +1228,13 @@ static unsigned int ata_scsi_start_stop_ return 0; -invalid_fld: + invalid_fld: ...
Hello, The libata part looks fine to me but what are those SYSTEM_SUSPEND_DISK stuff? They don't look like belonging to this patch. Thanks. -- tejun --
Actaully, they do belong to it. This is the part "fixing" the hibernation code path, in which the disk is also powered off unnecessarily. Well, probably I should use SYSTEM_HIBERNATE_ENTER or something similar instead of SYSTEM_SUSPEND_DISK. In short, the idea is to change system_state to something specific to the last phase of hibernation (after saving the image) and check that in ata_scsi_start_stop_xlat(). In fact that's completely analogous to what's done for SYSTEM_POWER_OFF in there. Thanks, Rafael --
Ah.. right, missed the added check for SUSPEND_DISK in libata-scsi.c. Maybe it's a good idea to note it in the commit message later? Thanks. -- tejun --
After some more debugging and refinements I've obtained the patch below. Thanks, Rafael --- From: Rafael J. Wysocki <rjw@sisk.pl> SATA: Blacklist systems that spin off disks during ACPI power off Some notebooks from HP have the problem that their BIOSes attempt to spin down hard drives before entering ACPI system states S4 and S5. This leads to a yo-yo effect during system power-off shutdown and the last phase of hibernation when the disk is first spun down by the kernel and then almost immediately turned on and off by the BIOS. This, in turn, may result in shortening the disk's life times. To prevent this from happening we can blacklist the affected systems using DMI information. However, only the on-board controlles should be blacklisted and their PCI slot numbers can be used for this purpose. Unfortunately the existing interface for checking DMI information of the system is not very convenient for this purpose, because to use it, we would have to define special callback functions or create a separate struct dmi_system_id table for each blacklisted system. To overcome this difficulty introduce a new function dmi_first_match() returning a pointer to the first entry in an array of struct dmi_system_id elements that matches the system DMI information. Then, we can use this pointer to access the entry's .driver_data field containing the additional information, such as the PCI slot number, allowing us to do the desired blacklisting. Introduce a new libata flag ATA_FLAG_NO_POWEROFF_SPINDOWN that, if set, will prevent disks from being spun off during system power off and hibernation (to handle the hibernation case we need a new system state SYSTEM_HIBERNATE_ENTER that can be checked against by libata, in analogy with SYSTEM_POWER_OFF). Use dmi_first_match() to set this flag for some systems affected by the problem described above (HP nx6325, HP nx6310, HP 2510p). Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/ata/ahci.c | 32 ...
libata part looks good to me but I think it would be better to separate out DMI changes into a separate patch. Thanks. -- tejun --
Did these changes ever get separated out? --
I only have the 'combo' patch if that's what you're asking about. [The latest version is at http://bugzilla.kernel.org/attachment.cgi?id=17702&action=view] Still, I can easily split the patch, although in that case the reason for the DMI changes won't be very clear without a reference to the SATA changes IMO. Thanks, Rafael --
That's the nature of every single API change -- you have the change, and then you have the users. Respectfully, please split up the patch as requested, into DMI subsystem and ata subsystem pieces. Re-reviewing the patch, I would even think that you should split out the kernel/power/disk and linux/kernel.h changes as well. Jeff --
Here you go (sorry for the delay). I have also split the SATA changes so that the driver patches are droppable individually if there are problems with them (not anticipated). Thanks, Rafael --- SATA: Blacklist systems that spin off disks during ACPI power off Some notebooks from HP have the problem that their BIOSes attempt to spin down hard drives before entering ACPI system states S4 and S5. This leads to a yo-yo effect during system power-off shutdown and the last phase of hibernation when the disk is first spun down by the kernel and then almost immediately turned on and off by the BIOS. This, in turn, may result in shortening the disk's life times. To prevent this from happening we can blacklist the affected systems using DMI information, which is implemented by the following series of patches. --
From: Rafael J. Wysocki <rjw@sisk.pl>
SATA Sil: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP nx6325 that uses the sata_sil driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/ata/sata_sil.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/ata/sata_sil.c
===================================================================
--- linux-2.6.orig/drivers/ata/sata_sil.c
+++ linux-2.6/drivers/ata/sata_sil.c
@@ -603,11 +603,38 @@ static void sil_init_controller(struct a
}
}
+static bool sil_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq nx6325",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6325"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x12UL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
static int sil_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
static int printed_version;
int board_id = ent->driver_data;
- const struct ata_port_info *ppi[] = { &sil_port_info[board_id], NULL ...From: Rafael J. Wysocki <rjw@sisk.pl> DMI: Introduce dmi_first_match to make the interface more flexible Some notebooks from HP have the problem that their BIOSes attempt to spin down hard drives before entering ACPI system states S4 and S5. This leads to a yo-yo effect during system power-off shutdown and the last phase of hibernation when the disk is first spun down by the kernel and then almost immediately turned on and off by the BIOS. This, in turn, may result in shortening the disk's life times. To prevent this from happening we can blacklist the affected systems using DMI information. However, only the on-board controlles should be blacklisted and their PCI slot numbers can be used for this purpose. Unfortunately the existing interface for checking DMI information of the system is not very convenient for this purpose, because to use it, we would have to define special callback functions or create a separate struct dmi_system_id table for each blacklisted system. To overcome this difficulty introduce a new function dmi_first_match() returning a pointer to the first entry in an array of struct dmi_system_id elements that matches the system DMI information. Then, we can use this pointer to access the entry's .driver_data field containing the additional information, such as the PCI slot number, allowing us to do the desired blacklisting. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/firmware/dmi_scan.c | 68 +++++++++++++++++++++++++++++++++----------- include/linux/dmi.h | 1 2 files changed, 53 insertions(+), 16 deletions(-) Index: linux-2.6/drivers/firmware/dmi_scan.c =================================================================== --- linux-2.6.orig/drivers/firmware/dmi_scan.c +++ linux-2.6/drivers/firmware/dmi_scan.c @@ -407,6 +407,27 @@ void __init dmi_scan_machine(void) } /** + * dmi_match - check if dmi_system_id structure matches system DMI data + * @dmi: pointer to the dmi_system_id structure to check + ...
Acked-by: Tejun Heo <tj@kernel.org> (cc'ing recent committers to dmi) If no one objects, I'd like to push this through libata-dev#upstream. Any objections? Thanks. -- tejun --
From: Rafael J. Wysocki <rjw@sisk.pl>
Hibernation: Introduce new system state for the last phase of hibernation
Replace unused SYSTEM_SUSPEND_DISK with a new system_state value
SYSTEM_HIBERNATE_ENTER that can be used by device drivers to check if
the system is in the last phase of hibernation.
In particular, some SATA drivers are going to use it for blacklisting
systems in which the disks should not be spun down during the last
phase of hibernation (the BIOS will do that anyway).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/kernel.h | 2 +-
kernel/power/disk.c | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -416,6 +416,7 @@ int hibernation_platform_enter(void)
if (error)
goto Close;
+ system_state = SYSTEM_HIBERNATE_ENTER;
suspend_console();
ftrace_save = __ftrace_enabled_save();
error = device_suspend(PMSG_HIBERNATE);
@@ -451,6 +452,7 @@ int hibernation_platform_enter(void)
Finish:
hibernation_ops->finish();
Resume_devices:
+ system_state = SYSTEM_RUNNING;
device_resume(PMSG_RESTORE);
__ftrace_enabled_restore(ftrace_save);
resume_console();
Index: linux-2.6/include/linux/kernel.h
===================================================================
--- linux-2.6.orig/include/linux/kernel.h
+++ linux-2.6/include/linux/kernel.h
@@ -247,7 +247,7 @@ extern enum system_states {
SYSTEM_HALT,
SYSTEM_POWER_OFF,
SYSTEM_RESTART,
- SYSTEM_SUSPEND_DISK,
+ SYSTEM_HIBERNATE_ENTER,
} system_state;
#define TAINT_PROPRIETARY_MODULE (1<<0)
--
Rafael, may I push this through libata-dev? Thanks. -- tejun --
Sure, please go ahead. Thanks, Rafael --
Violent objections. We just don't know what this change will do. It potentially affects every code site in the kernel which looks at system_state. We've had problems in the past with this thing and the more complex we make it, the worse any future problems will be. It's Just Bad. Can we just create a new global? extern bool system_entering_hibernation_or_whatever; ? It'll add four bytes of bss, it'll save *more* than four bytes of text and it is plainly obviously non-injurious to present and future code. hmm? --
Yes, we can, but what about: extern bool system_entering_hibernation(void); that will become a static inline in case of !CONFIG_HIBERNATION, and using a static variable? --
OK, I'll respin the entire series, then. --
As promised: --- SATA: Blacklist systems that spin off disks during ACPI power off (rev. 2) Some notebooks from HP have the problem that their BIOSes attempt to spin down hard drives before entering ACPI system states S4 and S5. This leads to a yo-yo effect during system power-off shutdown and the last phase of hibernation when the disk is first spun down by the kernel and then almost immediately turned on and off by the BIOS. This, in turn, may result in shortening the disk's life times. To prevent this from happening we can blacklist the affected systems using DMI information, which is implemented by the following series of patches. --
From: Rafael J. Wysocki <rjw@sisk.pl>
SATA AHCI: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP nx6310 that uses the AHCI driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/ahci.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
Index: linux-2.6/drivers/ata/ahci.c
===================================================================
--- linux-2.6.orig/drivers/ata/ahci.c
+++ linux-2.6/drivers/ata/ahci.c
@@ -2528,6 +2528,32 @@ static void ahci_p5wdh_workaround(struct
}
}
+static bool ahci_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq nx6310",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6310"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x1FUL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
static int printed_version;
@@ -2623,6 +2649,12 @@ static int ahci_init_one(struct pci_dev
}
}
+ if ...From: Rafael J. Wysocki <rjw@sisk.pl>
SATA: Blacklisting of systems that spin off disks during ACPI power off (rev. 2)
Introduce new libata flags ATA_FLAG_NO_POWEROFF_SPINDOWN and
ATA_FLAG_NO_HIBERNATE_SPINDOWN that, if set, will prevent disks from
being spun off during system power off and hibernation, respectively
(to handle the hibernation case we need the new system state
SYSTEM_HIBERNATE_ENTER that can be checked against by libata, in
analogy with SYSTEM_POWER_OFF).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/libata-scsi.c | 20 +++++++++++++++++---
include/linux/libata.h | 2 ++
2 files changed, 19 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/ata/libata-scsi.c
===================================================================
--- linux-2.6.orig/drivers/ata/libata-scsi.c
+++ linux-2.6/drivers/ata/libata-scsi.c
@@ -46,6 +46,7 @@
#include <linux/libata.h>
#include <linux/hdreg.h>
#include <linux/uaccess.h>
+#include <linux/suspend.h>
#include "libata.h"
@@ -1181,6 +1182,17 @@ static unsigned int ata_scsi_start_stop_
tf->command = ATA_CMD_VERIFY; /* READ VERIFY */
} else {
+ /* Some odd clown BIOSen issue spindown on power off (ACPI S4
+ * or S5) causing some drives to spin up and down again.
+ */
+ if ((qc->ap->flags & ATA_FLAG_NO_POWEROFF_SPINDOWN) &&
+ system_state == SYSTEM_POWER_OFF)
+ goto skip;
+
+ if ((qc->ap->flags & ATA_FLAG_NO_HIBERNATE_SPINDOWN) &&
+ system_entering_hibernation())
+ goto skip;
+
/* XXX: This is for backward compatibility, will be
* removed. Read Documentation/feature-removal-schedule.txt
* for more info.
@@ -1204,8 +1216,7 @@ static unsigned int ata_scsi_start_stop_
scmd->scsi_done = qc->scsidone;
qc->scsidone = ata_delayed_done;
}
- scmd->result = SAM_STAT_GOOD;
- return 1;
+ goto skip;
}
/* Issue ATA STANDBY IMMEDIATE command */
@@ -1221,10 +1232,13 @@ static ...From: Rafael J. Wysocki <rjw@sisk.pl>
SATA PIIX: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP 2510p that uses the ata_piix driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/ata_piix.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
Index: linux-2.6/drivers/ata/ata_piix.c
===================================================================
--- linux-2.6.orig/drivers/ata/ata_piix.c
+++ linux-2.6/drivers/ata/ata_piix.c
@@ -1449,6 +1449,32 @@ static void piix_iocfg_bit18_quirk(struc
}
}
+static bool piix_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq 2510p",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq 2510p"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x1FUL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
/**
* piix_init_one - Register PIIX ATA PCI device with kernel services
* @pdev: PCI device to register
@@ -1484,6 +1510,14 @@ static int __devinit piix_init_one(struc
if (!in_module_init)
...From: Rafael J. Wysocki <rjw@sisk.pl>
SATA Sil: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP nx6325 that uses the sata_sil driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Tejun Heo <htejun@gmail.com>
---
drivers/ata/sata_sil.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
Index: linux-2.6/drivers/ata/sata_sil.c
===================================================================
--- linux-2.6.orig/drivers/ata/sata_sil.c
+++ linux-2.6/drivers/ata/sata_sil.c
@@ -603,11 +603,38 @@ static void sil_init_controller(struct a
}
}
+static bool sil_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq nx6325",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6325"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x12UL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
static int sil_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
static int printed_version;
int board_id = ent->driver_data;
- const struct ata_port_info *ppi[] = { ...From: Rafael J. Wysocki <rjw@sisk.pl> DMI: Introduce dmi_first_match to make the interface more flexible Some notebooks from HP have the problem that their BIOSes attempt to spin down hard drives before entering ACPI system states S4 and S5. This leads to a yo-yo effect during system power-off shutdown and the last phase of hibernation when the disk is first spun down by the kernel and then almost immediately turned on and off by the BIOS. This, in turn, may result in shortening the disk's life times. To prevent this from happening we can blacklist the affected systems using DMI information. However, only the on-board controlles should be blacklisted and their PCI slot numbers can be used for this purpose. Unfortunately the existing interface for checking DMI information of the system is not very convenient for this purpose, because to use it, we would have to define special callback functions or create a separate struct dmi_system_id table for each blacklisted system. To overcome this difficulty introduce a new function dmi_first_match() returning a pointer to the first entry in an array of struct dmi_system_id elements that matches the system DMI information. Then, we can use this pointer to access the entry's .driver_data field containing the additional information, such as the PCI slot number, allowing us to do the desired blacklisting. Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/firmware/dmi_scan.c | 68 +++++++++++++++++++++++++++++++++----------- include/linux/dmi.h | 1 2 files changed, 53 insertions(+), 16 deletions(-) Index: linux-2.6/drivers/firmware/dmi_scan.c =================================================================== --- linux-2.6.orig/drivers/firmware/dmi_scan.c +++ linux-2.6/drivers/firmware/dmi_scan.c @@ -407,6 +407,27 @@ void __init dmi_scan_machine(void) } /** + * dmi_match - check if dmi_system_id structure matches system DMI data + * @dmi: pointer to the dmi_system_id structure to check + ...
From: Rafael J. Wysocki <rjw@sisk.pl>
Hibernation: Introduce system_entering_hibernation
Introduce boolean function system_entering_hibernation() returning
'true' during the last phase of hibernation, in which devices are
being put into low power states and the sleep state (for example,
ACPI S4) is finally entered.
Some device drivers need such a function to check if the system is
in the final phase of hibernation. In particular, some SATA drivers
are going to use it for blacklisting systems in which the disks
should not be spun down during the last phase of hibernation (the
BIOS will do that anyway).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
include/linux/suspend.h | 2 ++
kernel/power/disk.c | 9 +++++++++
2 files changed, 11 insertions(+)
Index: linux-2.6/include/linux/suspend.h
===================================================================
--- linux-2.6.orig/include/linux/suspend.h
+++ linux-2.6/include/linux/suspend.h
@@ -232,6 +232,7 @@ extern unsigned long get_safe_page(gfp_t
extern void hibernation_set_ops(struct platform_hibernation_ops *ops);
extern int hibernate(void);
+extern bool system_entering_hibernation(void);
#else /* CONFIG_HIBERNATION */
static inline int swsusp_page_is_forbidden(struct page *p) { return 0; }
static inline void swsusp_set_page_free(struct page *p) {}
@@ -239,6 +240,7 @@ static inline void swsusp_unset_page_fre
static inline void hibernation_set_ops(struct platform_hibernation_ops *ops) {}
static inline int hibernate(void) { return -ENOSYS; }
+static inline bool system_entering_hibernation(void) { return false; }
#endif /* CONFIG_HIBERNATION */
#ifdef CONFIG_PM_SLEEP
Index: linux-2.6/kernel/power/disk.c
===================================================================
--- linux-2.6.orig/kernel/power/disk.c
+++ linux-2.6/kernel/power/disk.c
@@ -72,6 +72,13 @@ void hibernation_set_ops(struct platform
mutex_unlock(&pm_mutex);
}
+static bool ...If you added the PCI ids for the controller to your tables you could implement this series once instead of in multiple drivers --
Hm, I'm not sure what you mean exactly, but the point is to blacklist the on-board controllers only and a devfn of the controller is needed for that, or a part of it (like the slot number I'm using). Could that be done above the driver level? --
It can be done in one place via the libata core code I think rather than in each driver. Might need to be a helper function but you'd at least remove all the duplication. Alan --
I think it's better to keep the list in each LLD but having a helper in libata-core would be nice. Thanks. -- tejun --
Well, I can try to introduce one in a future patch. Will that be OK? Rafael --
applied 1-6 to tj-upstream git://git.kernel.org/pub/scm/linux/kernel/git/tj/libata-dev.git tj-upstream http://git.kernel.org/?p=linux/kernel/git/tj/libata-dev.git;a=shortlog;h=tj-upstream Thanks. -- tejun --
On Fri, 3 Oct 2008 13:27:18 +0200
I think Andrew also means things looking for system_state > something
like
arch/powerpc/platforms/cell/smp.c: if (system_state < SYSTEM_RUNNING &&
arch/powerpc/kernel/smp.c: if (system_state < SYSTEM_RUNNING)
arch/powerpc/kernel/smp.c: if (system_state > SYSTEM_BOOTING)
drivers/xen/xenbus/xenbus_probe.c: if (system_state > SYSTEM_RUNNING) {
--
Arjan van de Ven Intel Open Source Technology Centre
For development, discussion and tips for power savings,
visit http://www.lesswatts.org
--
These particular ones shouldn't be affected AFAICS. Anyway, I'm going to respin the patchset to take the Andrew's comment into account. --
From: Rafael J. Wysocki <rjw@sisk.pl>
SATA: Blacklisting of systems that spin off disks during ACPI power off
Introduce new libata flags ATA_FLAG_NO_POWEROFF_SPINDOWN and
ATA_FLAG_NO_HIBERNATE_SPINDOWN that, if set, will prevent disks from
being spun off during system power off and hibernation, respectively
(to handle the hibernation case we need the new system state
SYSTEM_HIBERNATE_ENTER that can be checked against by libata, in
analogy with SYSTEM_POWER_OFF).
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/ata/libata-scsi.c | 19 ++++++++++++++++---
include/linux/libata.h | 2 ++
2 files changed, 18 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/ata/libata-scsi.c
===================================================================
--- linux-2.6.orig/drivers/ata/libata-scsi.c
+++ linux-2.6/drivers/ata/libata-scsi.c
@@ -1181,6 +1181,17 @@ static unsigned int ata_scsi_start_stop_
tf->command = ATA_CMD_VERIFY; /* READ VERIFY */
} else {
+ /* Some odd clown BIOSen issue spindown on power off (ACPI S4
+ * or S5) causing some drives to spin up and down again.
+ */
+ if ((qc->ap->flags & ATA_FLAG_NO_POWEROFF_SPINDOWN) &&
+ system_state == SYSTEM_POWER_OFF)
+ goto skip;
+
+ if ((qc->ap->flags & ATA_FLAG_NO_HIBERNATE_SPINDOWN) &&
+ system_state == SYSTEM_HIBERNATE_ENTER)
+ goto skip;
+
/* XXX: This is for backward compatibility, will be
* removed. Read Documentation/feature-removal-schedule.txt
* for more info.
@@ -1204,8 +1215,7 @@ static unsigned int ata_scsi_start_stop_
scmd->scsi_done = qc->scsidone;
qc->scsidone = ata_delayed_done;
}
- scmd->result = SAM_STAT_GOOD;
- return 1;
+ goto skip;
}
/* Issue ATA STANDBY IMMEDIATE command */
@@ -1221,10 +1231,13 @@ static unsigned int ata_scsi_start_stop_
return 0;
-invalid_fld:
+ invalid_fld:
ata_scsi_set_sense(scmd, ILLEGAL_REQUEST, 0x24, 0x0);
/* "Invalid field in cbd" */
return 1;
+ ...ACK 3-6. I'll push these through libata-dev after the first two prerequsite patches are cleared. Thanks. -- tejun --
From: Rafael J. Wysocki <rjw@sisk.pl>
SATA AHCI: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP nx6310 that uses the AHCI driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/ata/ahci.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
Index: linux-2.6/drivers/ata/ahci.c
===================================================================
--- linux-2.6.orig/drivers/ata/ahci.c
+++ linux-2.6/drivers/ata/ahci.c
@@ -2528,6 +2528,32 @@ static void ahci_p5wdh_workaround(struct
}
}
+static bool ahci_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq nx6310",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq nx6310"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x1FUL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
{
static int printed_version;
@@ -2623,6 +2649,12 @@ static int ahci_init_one(struct pci_dev
}
}
+ if (ahci_broken_system_poweroff(pdev)) {
+ pi.flags |= ...From: Rafael J. Wysocki <rjw@sisk.pl>
SATA PIIX: Blacklist system that spins off disks during ACPI power off
Some notebooks from HP have the problem that their BIOSes attempt to
spin down hard drives before entering ACPI system states S4 and S5.
This leads to a yo-yo effect during system power-off shutdown and the
last phase of hibernation when the disk is first spun down by the
kernel and then almost immediately turned on and off by the BIOS.
This, in turn, may result in shortening the disk's life times.
To prevent this from happening we can blacklist the affected systems
using DMI information.
Blacklist HP 2510p that uses the ata_piix driver.
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
drivers/ata/ata_piix.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
Index: linux-2.6/drivers/ata/ata_piix.c
===================================================================
--- linux-2.6.orig/drivers/ata/ata_piix.c
+++ linux-2.6/drivers/ata/ata_piix.c
@@ -1449,6 +1449,32 @@ static void piix_iocfg_bit18_quirk(struc
}
}
+static bool piix_broken_system_poweroff(struct pci_dev *pdev)
+{
+ static const struct dmi_system_id broken_systems[] = {
+ {
+ .ident = "HP Compaq 2510p",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq 2510p"),
+ },
+ /* PCI slot number of the controller */
+ .driver_data = (void *)0x1FUL,
+ },
+
+ { } /* terminate list */
+ };
+ const struct dmi_system_id *dmi = dmi_first_match(broken_systems);
+
+ if (dmi) {
+ unsigned long slot = (unsigned long)dmi->driver_data;
+ /* apply the quirk only to on-board controllers */
+ return slot == PCI_SLOT(pdev->devfn);
+ }
+
+ return false;
+}
+
/**
* piix_init_one - Register PIIX ATA PCI device with kernel services
* @pdev: PCI device to register
@@ -1484,6 +1510,14 @@ static int __devinit piix_init_one(struc
if (!in_module_init)
return -ENODEV;
+ if ...
