hello,
kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
I tried also 2.6.31-rc1 doesn't work either
I bisected it to :
etienne@etienne-desktop:~/linux-2.6$ git bisect bad
2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
commit 2f0d0fd2a605666d38e290c5c0d2907484352dc4
Author: Bartlomiej Zolnierkiewicz <[email protected]>
Date: Tue Mar 24 23:22:42 2009 +0100
ide-acpi: cleanup do_drive_get_GTF()
* ide_noacpi is already checked by ide_acpi_exec_tfs()
which is the only user of do_drive_get_GTF().
* ide_acpi_exec_tfs() prints sufficient debug info about the
device so no need to have excessive data about port/host.
* It is sufficient to check for drive->acpidata->obj_handle
as it will be NULL if dev == NULL or hwif->acpidata == NULL
or device is not present.
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
If i revert this patch on top of 2.6.31-rc1, STR Works OK
(i think the only device handled by the legacy IDE subsystem on my computer is my CDROM drive)
thanks,
Etienne
etienne@etienne-desktop:~/linux-2.6$ lspci
00:00.0 Host bridge: Intel Corporation 82915G/P/GV/GL/PL/910GL Memory Controller Hub (rev 04)
00:01.0 PCI bridge: Intel Corporation 82915G/P/GV/GL/PL/910GL PCI Express Root Port (rev 04)
00:1c.0 PCI bridge: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) PCI Express Port 1 (rev 03)
00:1d.0 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #1 (rev 03)
00:1d.1 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #2 (rev 03)
00:1d.2 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #3 (rev 03)
00:1d.3 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB UHCI #4 (rev 03)
00:1d.7 USB Controller: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) USB2 EHCI Controller (rev 03)
00:1e.0 PCI bridge: Intel Corporation 82801 PCI Bridge (rev d3)
00:1f.0 ISA bridge: Intel Corporation 82801FB/FR (ICH6/ICH6R) LPC Interface Bridge (rev 03)
00:1f.1 IDE interface: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) IDE Controller (rev 03)
00:1f.2 IDE interface: Intel Corporation 82801FB/FW (ICH6/ICH6W) SATA Controller (rev 03)
00:1f.3 SMBus: Intel Corporation 82801FB/FBM/FR/FW/FRW (ICH6 Family) SMBus Controller (rev 03)
01:00.0 VGA compatible controller: ATI Technologies Inc RV370 5B60 [Radeon X300 (PCIE)]
01:00.1 Display controller: ATI Technologies Inc RV370 [Radeon X300SE]
03:00.0 Multimedia audio controller: Creative Labs SB Audigy (rev 04)
03:00.2 FireWire (IEEE 1394): Creative Labs SB Audigy FireWire Port (rev 04)
03:08.0 Ethernet controller: Intel Corporation 82562ET/EZ/GT/GZ - PRO/100 VE (LOM) Ethernet Controller (rev 03)
etienne@etienne-desktop:~/linux-2.6$ cat .config|grep IDE
CONFIG_HAVE_IDE=y
CONFIG_IDE=y
# Please see Documentation/ide/ide.txt for help/info on IDE drives
CONFIG_IDE_XFER_MODE=y
CONFIG_IDE_ATAPI=y
# CONFIG_BLK_DEV_IDE_SATA is not set
# CONFIG_IDE_GD is not set
CONFIG_BLK_DEV_IDECD=y
CONFIG_BLK_DEV_IDECD_VERBOSE_ERRORS=y
# CONFIG_BLK_DEV_IDETAPE is not set
CONFIG_BLK_DEV_IDEACPI=y
# CONFIG_IDE_TASK_IOCTL is not set
CONFIG_IDE_PROC_FS=y
# IDE chipset support/bugfixes
CONFIG_IDE_GENERIC=y
# CONFIG_BLK_DEV_IDEPNP is not set
CONFIG_BLK_DEV_IDEDMA_SFF=y
# PCI IDE chipsets support
CONFIG_BLK_DEV_IDEPCI=y
CONFIG_IDEPCI_PCIBUS_ORDER=y
CONFIG_BLK_DEV_IDEDMA_PCI=y
CONFIG_BLK_DEV_IDEDMA=y
Hi,
On Friday 26 June 2009 17:40:30 Etienne Basset wrote:
> hello,
>
> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
> I tried also 2.6.31-rc1 doesn't work either
> I bisected it to :
>
> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
Does the following patch fix it?
---
drivers/ide/ide-acpi.c | 37 +++++++------------------------------
drivers/ide/ide-pm.c | 30 ++++++++++++++++++------------
include/linux/ide.h | 2 ++
3 files changed, 27 insertions(+), 42 deletions(-)
Index: b/drivers/ide/ide-acpi.c
===================================================================
--- a/drivers/ide/ide-acpi.c
+++ b/drivers/ide/ide-acpi.c
@@ -92,6 +92,11 @@ int ide_acpi_init(void)
return 0;
}
+bool ide_port_acpi(ide_hwif_t *hwif)
+{
+ return ide_noacpi == 0 && hwif->acpidata;
+}
+
/**
* ide_get_dev_handle - finds acpi_handle and PCI device.function
* @dev: device to locate
@@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive
unsigned long gtf_address;
unsigned long obj_loc;
- if (ide_noacpi)
- return 0;
-
DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn);
ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc);
@@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwi
struct acpi_buffer output;
union acpi_object *out_obj;
- if (ide_noacpi)
- return;
-
- DEBPRINT("ENTER:\n");
-
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* Setting up output buffer for _GTM */
output.length = ACPI_ALLOCATE_BUFFER;
output.pointer = NULL; /* ACPI-CA sets this; save/free it later */
@@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hw
struct ide_acpi_drive_link *master = &hwif->acpidata->master;
struct ide_acpi_drive_link *slave = &hwif->acpidata->slave;
- if (ide_noacpi)
- return;
-
- DEBPRINT("ENTER:\n");
-
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* Give the GTM buffer + drive Identify data to the channel via the
* _STM method: */
/* setup input parameters buffer for _STM */
@@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif
ide_drive_t *drive;
int i;
- if (ide_noacpi || ide_noacpi_psx)
+ if (ide_noacpi_psx)
return;
DEBPRINT("ENTER:\n");
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* channel first and then drives for power on and verse versa for power off */
if (on)
acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0);
@@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif
drive->name, err);
}
- if (!ide_acpionboot) {
+ if (ide_noacpi || ide_acpionboot == 0) {
DEBPRINT("ACPI methods disabled on boot\n");
return;
}
Index: b/drivers/ide/ide-pm.c
===================================================================
--- a/drivers/ide/ide-pm.c
+++ b/drivers/ide/ide-pm.c
@@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *d
struct request_pm_state rqpm;
int ret;
- /* call ACPI _GTM only once */
- if ((drive->dn & 1) == 0 || pair == NULL)
- ide_acpi_get_timing(hwif);
+ if (ide_port_acpi(hwif)) {
+ /* call ACPI _GTM only once */
+ if ((drive->dn & 1) == 0 || pair == NULL)
+ ide_acpi_get_timing(hwif);
+ }
memset(&rqpm, 0, sizeof(rqpm));
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
@@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *d
ret = blk_execute_rq(drive->queue, NULL, rq, 0);
blk_put_request(rq);
- /* call ACPI _PS3 only after both devices are suspended */
- if (ret == 0 && ((drive->dn & 1) || pair == NULL))
- ide_acpi_set_state(hwif, 0);
+ if (ret == 0 && ide_port_acpi(hwif)) {
+ /* call ACPI _PS3 only after both devices are suspended */
+ if ((drive->dn & 1) || pair == NULL)
+ ide_acpi_set_state(hwif, 0);
+ }
return ret;
}
@@ -42,13 +46,15 @@ int generic_ide_resume(struct device *de
struct request_pm_state rqpm;
int err;
- /* call ACPI _PS0 / _STM only once */
- if ((drive->dn & 1) == 0 || pair == NULL) {
- ide_acpi_set_state(hwif, 1);
- ide_acpi_push_timing(hwif);
- }
+ if (ide_port_acpi(hwif)) {
+ /* call ACPI _PS0 / _STM only once */
+ if ((drive->dn & 1) == 0 || pair == NULL) {
+ ide_acpi_set_state(hwif, 1);
+ ide_acpi_push_timing(hwif);
+ }
- ide_acpi_exec_tfs(drive);
+ ide_acpi_exec_tfs(drive);
+ }
memset(&rqpm, 0, sizeof(rqpm));
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1420,6 +1420,7 @@ static inline void ide_dma_unmap_sg(ide_
#ifdef CONFIG_BLK_DEV_IDEACPI
int ide_acpi_init(void);
+bool ide_port_acpi(ide_hwif_t *hwif);
extern int ide_acpi_exec_tfs(ide_drive_t *drive);
extern void ide_acpi_get_timing(ide_hwif_t *hwif);
extern void ide_acpi_push_timing(ide_hwif_t *hwif);
@@ -1428,6 +1429,7 @@ void ide_acpi_port_init_devices(ide_hwif
extern void ide_acpi_set_state(ide_hwif_t *hwif, int on);
#else
static inline int ide_acpi_init(void) { return 0; }
+static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; }
static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; }
static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; }
static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; }
Hi,
Bartlomiej Zolnierkiewicz wrote:
> Hi,
>
> On Friday 26 June 2009 17:40:30 Etienne Basset wrote:
>> hello,
>>
>> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
>> I tried also 2.6.31-rc1 doesn't work either
>> I bisected it to :
>>
>> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
>> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
>
> Does the following patch fix it?
>
Yes, it works now
thanks!
Etienne
> ---
> drivers/ide/ide-acpi.c | 37 +++++++------------------------------
> drivers/ide/ide-pm.c | 30 ++++++++++++++++++------------
> include/linux/ide.h | 2 ++
> 3 files changed, 27 insertions(+), 42 deletions(-)
>
> Index: b/drivers/ide/ide-acpi.c
> ===================================================================
> --- a/drivers/ide/ide-acpi.c
> +++ b/drivers/ide/ide-acpi.c
> @@ -92,6 +92,11 @@ int ide_acpi_init(void)
> return 0;
> }
>
> +bool ide_port_acpi(ide_hwif_t *hwif)
> +{
> + return ide_noacpi == 0 && hwif->acpidata;
> +}
> +
> /**
> * ide_get_dev_handle - finds acpi_handle and PCI device.function
> * @dev: device to locate
> @@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive
> unsigned long gtf_address;
> unsigned long obj_loc;
>
> - if (ide_noacpi)
> - return 0;
> -
> DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn);
>
> ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc);
> @@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwi
> struct acpi_buffer output;
> union acpi_object *out_obj;
>
> - if (ide_noacpi)
> - return;
> -
> - DEBPRINT("ENTER:\n");
> -
> - if (!hwif->acpidata) {
> - DEBPRINT("no ACPI data for %s\n", hwif->name);
> - return;
> - }
> -
> /* Setting up output buffer for _GTM */
> output.length = ACPI_ALLOCATE_BUFFER;
> output.pointer = NULL; /* ACPI-CA sets this; save/free it later */
> @@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hw
> struct ide_acpi_drive_link *master = &hwif->acpidata->master;
> struct ide_acpi_drive_link *slave = &hwif->acpidata->slave;
>
> - if (ide_noacpi)
> - return;
> -
> - DEBPRINT("ENTER:\n");
> -
> - if (!hwif->acpidata) {
> - DEBPRINT("no ACPI data for %s\n", hwif->name);
> - return;
> - }
> -
> /* Give the GTM buffer + drive Identify data to the channel via the
> * _STM method: */
> /* setup input parameters buffer for _STM */
> @@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif
> ide_drive_t *drive;
> int i;
>
> - if (ide_noacpi || ide_noacpi_psx)
> + if (ide_noacpi_psx)
> return;
>
> DEBPRINT("ENTER:\n");
>
> - if (!hwif->acpidata) {
> - DEBPRINT("no ACPI data for %s\n", hwif->name);
> - return;
> - }
> -
> /* channel first and then drives for power on and verse versa for power off */
> if (on)
> acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0);
> @@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif
> drive->name, err);
> }
>
> - if (!ide_acpionboot) {
> + if (ide_noacpi || ide_acpionboot == 0) {
> DEBPRINT("ACPI methods disabled on boot\n");
> return;
> }
> Index: b/drivers/ide/ide-pm.c
> ===================================================================
> --- a/drivers/ide/ide-pm.c
> +++ b/drivers/ide/ide-pm.c
> @@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *d
> struct request_pm_state rqpm;
> int ret;
>
> - /* call ACPI _GTM only once */
> - if ((drive->dn & 1) == 0 || pair == NULL)
> - ide_acpi_get_timing(hwif);
> + if (ide_port_acpi(hwif)) {
> + /* call ACPI _GTM only once */
> + if ((drive->dn & 1) == 0 || pair == NULL)
> + ide_acpi_get_timing(hwif);
> + }
>
> memset(&rqpm, 0, sizeof(rqpm));
> rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
> @@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *d
> ret = blk_execute_rq(drive->queue, NULL, rq, 0);
> blk_put_request(rq);
>
> - /* call ACPI _PS3 only after both devices are suspended */
> - if (ret == 0 && ((drive->dn & 1) || pair == NULL))
> - ide_acpi_set_state(hwif, 0);
> + if (ret == 0 && ide_port_acpi(hwif)) {
> + /* call ACPI _PS3 only after both devices are suspended */
> + if ((drive->dn & 1) || pair == NULL)
> + ide_acpi_set_state(hwif, 0);
> + }
>
> return ret;
> }
> @@ -42,13 +46,15 @@ int generic_ide_resume(struct device *de
> struct request_pm_state rqpm;
> int err;
>
> - /* call ACPI _PS0 / _STM only once */
> - if ((drive->dn & 1) == 0 || pair == NULL) {
> - ide_acpi_set_state(hwif, 1);
> - ide_acpi_push_timing(hwif);
> - }
> + if (ide_port_acpi(hwif)) {
> + /* call ACPI _PS0 / _STM only once */
> + if ((drive->dn & 1) == 0 || pair == NULL) {
> + ide_acpi_set_state(hwif, 1);
> + ide_acpi_push_timing(hwif);
> + }
>
> - ide_acpi_exec_tfs(drive);
> + ide_acpi_exec_tfs(drive);
> + }
>
> memset(&rqpm, 0, sizeof(rqpm));
> rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
> Index: b/include/linux/ide.h
> ===================================================================
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1420,6 +1420,7 @@ static inline void ide_dma_unmap_sg(ide_
>
> #ifdef CONFIG_BLK_DEV_IDEACPI
> int ide_acpi_init(void);
> +bool ide_port_acpi(ide_hwif_t *hwif);
> extern int ide_acpi_exec_tfs(ide_drive_t *drive);
> extern void ide_acpi_get_timing(ide_hwif_t *hwif);
> extern void ide_acpi_push_timing(ide_hwif_t *hwif);
> @@ -1428,6 +1429,7 @@ void ide_acpi_port_init_devices(ide_hwif
> extern void ide_acpi_set_state(ide_hwif_t *hwif, int on);
> #else
> static inline int ide_acpi_init(void) { return 0; }
> +static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; }
> static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; }
> static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; }
> static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; }
>
On Sat, Jun 27, 2009 at 5:04 PM, Etienne
Basset<[email protected]> wrote:
>>> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
Same problem on Thinkpad X61. STD STR suspended ok. Upon resume,
everything "seems" fine for about 100 seconds. Then mouse, keyboard
frozen.
>>> I tried also 2.6.31-rc1 doesn't work either
>>> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
>>> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
>>
>> Does the following patch fix it?
> Yes, it works now
Works for me too with the patch.
Thanks,
Jeff.
>> ---
>> ?drivers/ide/ide-acpi.c | ? 37 +++++++------------------------------
>> ?drivers/ide/ide-pm.c ? | ? 30 ++++++++++++++++++------------
>> ?include/linux/ide.h ? ?| ? ?2 ++
>> ?3 files changed, 27 insertions(+), 42 deletions(-)
>>
>> Index: b/drivers/ide/ide-acpi.c
>> ===================================================================
>> --- a/drivers/ide/ide-acpi.ch
>> +++ b/drivers/ide/ide-acpi.c
>> @@ -92,6 +92,11 @@ int ide_acpi_init(void)
>> ? ? ? return 0;
>> ?}
>>
>> +bool ide_port_acpi(ide_hwif_t *hwif)
>> +{
>> + ? ? return ide_noacpi == 0 && hwif->acpidata;
>> +}
>> +
>> ?/**
>> ? * ide_get_dev_handle - finds acpi_handle and PCI device.function
>> ? * @dev: device to locate
>> @@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive
>> ? ? ? unsigned long ? gtf_address;
>> ? ? ? unsigned long ? obj_loc;
>>
>> - ? ? if (ide_noacpi)
>> - ? ? ? ? ? ? return 0;
>> -
>> ? ? ? DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn);
>>
>> ? ? ? ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc);
>> @@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwi
>> ? ? ? struct acpi_buffer ? ? ?output;
>> ? ? ? union acpi_object ? ? ? *out_obj;
>>
>> - ? ? if (ide_noacpi)
>> - ? ? ? ? ? ? return;
>> -
>> - ? ? DEBPRINT("ENTER:\n");
>> -
>> - ? ? if (!hwif->acpidata) {
>> - ? ? ? ? ? ? DEBPRINT("no ACPI data for %s\n", hwif->name);
>> - ? ? ? ? ? ? return;
>> - ? ? }
>> -
>> ? ? ? /* Setting up output buffer for _GTM */
>> ? ? ? output.length = ACPI_ALLOCATE_BUFFER;
>> ? ? ? output.pointer = NULL; ?/* ACPI-CA sets this; save/free it later */
>> @@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hw
>> ? ? ? struct ide_acpi_drive_link ? ? ?*master = &hwif->acpidata->master;
>> ? ? ? struct ide_acpi_drive_link ? ? ?*slave = &hwif->acpidata->slave;
>>
>> - ? ? if (ide_noacpi)
>> - ? ? ? ? ? ? return;
>> -
>> - ? ? DEBPRINT("ENTER:\n");
>> -
>> - ? ? if (!hwif->acpidata) {
>> - ? ? ? ? ? ? DEBPRINT("no ACPI data for %s\n", hwif->name);
>> - ? ? ? ? ? ? return;
>> - ? ? }
>> -
>> ? ? ? /* Give the GTM buffer + drive Identify data to the channel via the
>> ? ? ? ?* _STM method: */
>> ? ? ? /* setup input parameters buffer for _STM */
>> @@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif
>> ? ? ? ide_drive_t *drive;
>> ? ? ? int i;
>>
>> - ? ? if (ide_noacpi || ide_noacpi_psx)
>> + ? ? if (ide_noacpi_psx)
>> ? ? ? ? ? ? ? return;
>>
>> ? ? ? DEBPRINT("ENTER:\n");
>>
>> - ? ? if (!hwif->acpidata) {
>> - ? ? ? ? ? ? DEBPRINT("no ACPI data for %s\n", hwif->name);
>> - ? ? ? ? ? ? return;
>> - ? ? }
>> -
>> ? ? ? /* channel first and then drives for power on and verse versa for power off */
>> ? ? ? if (on)
>> ? ? ? ? ? ? ? acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0);
>> @@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?drive->name, err);
>> ? ? ? }
>>
>> - ? ? if (!ide_acpionboot) {
>> + ? ? if (ide_noacpi || ide_acpionboot == 0) {
>> ? ? ? ? ? ? ? DEBPRINT("ACPI methods disabled on boot\n");
>> ? ? ? ? ? ? ? return;
>> ? ? ? }
>> Index: b/drivers/ide/ide-pm.c
>> ===================================================================
>> --- a/drivers/ide/ide-pm.c
>> +++ b/drivers/ide/ide-pm.c
>> @@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *d
>> ? ? ? struct request_pm_state rqpm;
>> ? ? ? int ret;
>>
>> - ? ? /* call ACPI _GTM only once */
>> - ? ? if ((drive->dn & 1) == 0 || pair == NULL)
>> - ? ? ? ? ? ? ide_acpi_get_timing(hwif);
>> + ? ? if (ide_port_acpi(hwif)) {
>> + ? ? ? ? ? ? /* call ACPI _GTM only once */
>> + ? ? ? ? ? ? if ((drive->dn & 1) == 0 || pair == NULL)
>> + ? ? ? ? ? ? ? ? ? ? ide_acpi_get_timing(hwif);
>> + ? ? }
>>
>> ? ? ? memset(&rqpm, 0, sizeof(rqpm));
>> ? ? ? rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
>> @@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *d
>> ? ? ? ret = blk_execute_rq(drive->queue, NULL, rq, 0);
>> ? ? ? blk_put_request(rq);
>>
>> - ? ? /* call ACPI _PS3 only after both devices are suspended */
>> - ? ? if (ret == 0 && ((drive->dn & 1) || pair == NULL))
>> - ? ? ? ? ? ? ide_acpi_set_state(hwif, 0);
>> + ? ? if (ret == 0 && ide_port_acpi(hwif)) {
>> + ? ? ? ? ? ? /* call ACPI _PS3 only after both devices are suspended */
>> + ? ? ? ? ? ? if ((drive->dn & 1) || pair == NULL)
>> + ? ? ? ? ? ? ? ? ? ? ide_acpi_set_state(hwif, 0);
>> + ? ? }
>>
>> ? ? ? return ret;
>> ?}
>> @@ -42,13 +46,15 @@ int generic_ide_resume(struct device *de
>> ? ? ? struct request_pm_state rqpm;
>> ? ? ? int err;
>>
>> - ? ? /* call ACPI _PS0 / _STM only once */
>> - ? ? if ((drive->dn & 1) == 0 || pair == NULL) {
>> - ? ? ? ? ? ? ide_acpi_set_state(hwif, 1);
>> - ? ? ? ? ? ? ide_acpi_push_timing(hwif);
>> - ? ? }
>> + ? ? if (ide_port_acpi(hwif)) {
>> + ? ? ? ? ? ? /* call ACPI _PS0 / _STM only once */
>> + ? ? ? ? ? ? if ((drive->dn & 1) == 0 || pair == NULL) {
>> + ? ? ? ? ? ? ? ? ? ? ide_acpi_set_state(hwif, 1);
>> + ? ? ? ? ? ? ? ? ? ? ide_acpi_push_timing(hwif);
>> + ? ? ? ? ? ? }
>>
>> - ? ? ide_acpi_exec_tfs(drive);
>> + ? ? ? ? ? ? ide_acpi_exec_tfs(drive);
>> + ? ? }
>>
>> ? ? ? memset(&rqpm, 0, sizeof(rqpm));
>> ? ? ? rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
>> Index: b/include/linux/ide.h
>> ===================================================================
>> --- a/include/linux/ide.h
>> +++ b/include/linux/ide.h
>> @@ -1420,6 +1420,7 @@ static inline void ide_dma_unmap_sg(ide_
>>
>> ?#ifdef CONFIG_BLK_DEV_IDEACPI
>> ?int ide_acpi_init(void);
>> +bool ide_port_acpi(ide_hwif_t *hwif);
>> ?extern int ide_acpi_exec_tfs(ide_drive_t *drive);
>> ?extern void ide_acpi_get_timing(ide_hwif_t *hwif);
>> ?extern void ide_acpi_push_timing(ide_hwif_t *hwif);
>> @@ -1428,6 +1429,7 @@ void ide_acpi_port_init_devices(ide_hwif
>> ?extern void ide_acpi_set_state(ide_hwif_t *hwif, int on);
>> ?#else
>> ?static inline int ide_acpi_init(void) { return 0; }
>> +static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; }
>> ?static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; }
>> ?static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; }
>> ?static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; }
On Sun, Jun 28, 2009 at 2:09 AM, Jeff Chua<[email protected]> wrote:
> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
> Basset<[email protected]> wrote:
>>>> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
>
> Same problem on Thinkpad X61. STD STR suspended ok. Upon resume,
> everything "seems" fine for about 100 seconds. Then mouse, keyboard
> frozen.
>
>>>> I tried also 2.6.31-rc1 doesn't work either
>>>> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
>>>> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
>>>
>>> Does the following patch fix it?
>> Yes, it works now
>
> Works for me too with the patch.
UPDATE ...
STR able to resume and works after >100 seconds
STD able to resume but failed after 100 seconds (could be 300 seconds)
but eventually failed. Everything seems ok, but mouse and keyboard
just freezed suddenly. Just haven't really time it enough.
Jeff.
Jeff Chua wrote:
> On Sun, Jun 28, 2009 at 2:09 AM, Jeff Chua<[email protected]> wrote:
>> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
>> Basset<[email protected]> wrote:
>>>>> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
>> Same problem on Thinkpad X61. STD STR suspended ok. Upon resume,
>> everything "seems" fine for about 100 seconds. Then mouse, keyboard
>> frozen.
>>
>>>>> I tried also 2.6.31-rc1 doesn't work either
>>>>> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
>>>>> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
>>>> Does the following patch fix it?
>>> Yes, it works now
>> Works for me too with the patch.
>
> UPDATE ...
>
> STR able to resume and works after >100 seconds
>
> STD able to resume but failed after 100 seconds (could be 300 seconds)
> but eventually failed. Everything seems ok, but mouse and keyboard
> just freezed suddenly. Just haven't really time it enough.
>
> Jeff.
>
Hello,
same here ; after resume, computer hangs after 2minutes (cannot ping from outside)
and STR/resume doesn't work if I try from console not from X
there must be at least one another bug lurking
If someone hasn't a better idea I'll try another bisection, applying bart's patch at each step
Etienne
On Sun, Jun 28, 2009 at 4:06 PM, Etienne
Basset<[email protected]> wrote:
> same here ; after resume, computer hangs after 2minutes (cannot ping from outside)
> and STR/resume doesn't work if I try from console not from X
> there must be at least one another bug lurking
> If someone hasn't a better idea I'll try another bisection, applying bart's patch at each step
The patch seems to help, but I got a few hangs from STR still.
Jeff.
On Sunday 28 June 2009 10:06:02 Etienne Basset wrote:
> Jeff Chua wrote:
> > On Sun, Jun 28, 2009 at 2:09 AM, Jeff Chua<[email protected]> wrote:
> >> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
> >> Basset<[email protected]> wrote:
> >>>>> kernel v2.6.29 suspends to RAM reliably on my computer; v2.6.30 doesn't resume after STR
> >> Same problem on Thinkpad X61. STD STR suspended ok. Upon resume,
> >> everything "seems" fine for about 100 seconds. Then mouse, keyboard
> >> frozen.
> >>
> >>>>> I tried also 2.6.31-rc1 doesn't work either
> >>>>> etienne@etienne-desktop:~/linux-2.6$ git bisect bad
> >>>>> 2f0d0fd2a605666d38e290c5c0d2907484352dc4 is first bad commit
> >>>> Does the following patch fix it?
> >>> Yes, it works now
> >> Works for me too with the patch.
Thanks for testing (also sorry for the problem, hopefully the fix will
prevent any similar issues in the future).
> > UPDATE ...
> >
> > STR able to resume and works after >100 seconds
> >
> > STD able to resume but failed after 100 seconds (could be 300 seconds)
> > but eventually failed. Everything seems ok, but mouse and keyboard
> > just freezed suddenly. Just haven't really time it enough.
> >
> > Jeff.
> >
> Hello,
>
> same here ; after resume, computer hangs after 2minutes (cannot ping from outside)
> and STR/resume doesn't work if I try from console not from X
> there must be at least one another bug lurking
> If someone hasn't a better idea I'll try another bisection, applying bart's patch at each step
I think that you're right. It seems that the underlying issue could be
a non-working/broken ACPI support (please note that the IDE ACPI bug won't
be triggered otherwise).
David, please apply:
From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: fix resume for CONFIG_BLK_DEV_IDEACPI=y
commit 2f0d0fd2a605666d38e290c5c0d2907484352dc4 ("ide-acpi: cleanup
do_drive_get_GTF()") didn't account for the lack of hwif->acpidata
check in generic_ide_suspend() [ indirect user of do_drive_get_GTF()
through ide_acpi_exec_tfs() ] resulting in broken resume when ACPI
support is enabled but ACPI data is unavailable.
Fix it by adding ide_port_acpi() helper for checking if port needs
ACPI handling and cleaning generic_ide_{suspend,resume}() to use it
instead of hiding hwif->acpidata and ide_noacpi checks in IDE ACPI
helpers (this should help in preventing similar bugs in the future).
While at it:
- kill superfluous debugging printks in ide_acpi_{get,push}_timing()
Reported-and-tested-by: Etienne Basset <[email protected]>
Also-reported-and-tested-by: Jeff Chua <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: [email protected]
Signed-off-by: Bartlomiej Zolnierkiewicz <[email protected]>
---
Added patch description, the patch itself remains unchanged.
drivers/ide/ide-acpi.c | 37 +++++++------------------------------
drivers/ide/ide-pm.c | 30 ++++++++++++++++++------------
include/linux/ide.h | 2 ++
3 files changed, 27 insertions(+), 42 deletions(-)
Index: b/drivers/ide/ide-acpi.c
===================================================================
--- a/drivers/ide/ide-acpi.c
+++ b/drivers/ide/ide-acpi.c
@@ -92,6 +92,11 @@ int ide_acpi_init(void)
return 0;
}
+bool ide_port_acpi(ide_hwif_t *hwif)
+{
+ return ide_noacpi == 0 && hwif->acpidata;
+}
+
/**
* ide_get_dev_handle - finds acpi_handle and PCI device.function
* @dev: device to locate
@@ -352,9 +357,6 @@ int ide_acpi_exec_tfs(ide_drive_t *drive
unsigned long gtf_address;
unsigned long obj_loc;
- if (ide_noacpi)
- return 0;
-
DEBPRINT("call get_GTF, drive=%s port=%d\n", drive->name, drive->dn);
ret = do_drive_get_GTF(drive, >f_length, >f_address, &obj_loc);
@@ -389,16 +391,6 @@ void ide_acpi_get_timing(ide_hwif_t *hwi
struct acpi_buffer output;
union acpi_object *out_obj;
- if (ide_noacpi)
- return;
-
- DEBPRINT("ENTER:\n");
-
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* Setting up output buffer for _GTM */
output.length = ACPI_ALLOCATE_BUFFER;
output.pointer = NULL; /* ACPI-CA sets this; save/free it later */
@@ -479,16 +471,6 @@ void ide_acpi_push_timing(ide_hwif_t *hw
struct ide_acpi_drive_link *master = &hwif->acpidata->master;
struct ide_acpi_drive_link *slave = &hwif->acpidata->slave;
- if (ide_noacpi)
- return;
-
- DEBPRINT("ENTER:\n");
-
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* Give the GTM buffer + drive Identify data to the channel via the
* _STM method: */
/* setup input parameters buffer for _STM */
@@ -527,16 +509,11 @@ void ide_acpi_set_state(ide_hwif_t *hwif
ide_drive_t *drive;
int i;
- if (ide_noacpi || ide_noacpi_psx)
+ if (ide_noacpi_psx)
return;
DEBPRINT("ENTER:\n");
- if (!hwif->acpidata) {
- DEBPRINT("no ACPI data for %s\n", hwif->name);
- return;
- }
-
/* channel first and then drives for power on and verse versa for power off */
if (on)
acpi_bus_set_power(hwif->acpidata->obj_handle, ACPI_STATE_D0);
@@ -616,7 +593,7 @@ void ide_acpi_port_init_devices(ide_hwif
drive->name, err);
}
- if (!ide_acpionboot) {
+ if (ide_noacpi || ide_acpionboot == 0) {
DEBPRINT("ACPI methods disabled on boot\n");
return;
}
Index: b/drivers/ide/ide-pm.c
===================================================================
--- a/drivers/ide/ide-pm.c
+++ b/drivers/ide/ide-pm.c
@@ -10,9 +10,11 @@ int generic_ide_suspend(struct device *d
struct request_pm_state rqpm;
int ret;
- /* call ACPI _GTM only once */
- if ((drive->dn & 1) == 0 || pair == NULL)
- ide_acpi_get_timing(hwif);
+ if (ide_port_acpi(hwif)) {
+ /* call ACPI _GTM only once */
+ if ((drive->dn & 1) == 0 || pair == NULL)
+ ide_acpi_get_timing(hwif);
+ }
memset(&rqpm, 0, sizeof(rqpm));
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
@@ -26,9 +28,11 @@ int generic_ide_suspend(struct device *d
ret = blk_execute_rq(drive->queue, NULL, rq, 0);
blk_put_request(rq);
- /* call ACPI _PS3 only after both devices are suspended */
- if (ret == 0 && ((drive->dn & 1) || pair == NULL))
- ide_acpi_set_state(hwif, 0);
+ if (ret == 0 && ide_port_acpi(hwif)) {
+ /* call ACPI _PS3 only after both devices are suspended */
+ if ((drive->dn & 1) || pair == NULL)
+ ide_acpi_set_state(hwif, 0);
+ }
return ret;
}
@@ -42,13 +46,15 @@ int generic_ide_resume(struct device *de
struct request_pm_state rqpm;
int err;
- /* call ACPI _PS0 / _STM only once */
- if ((drive->dn & 1) == 0 || pair == NULL) {
- ide_acpi_set_state(hwif, 1);
- ide_acpi_push_timing(hwif);
- }
+ if (ide_port_acpi(hwif)) {
+ /* call ACPI _PS0 / _STM only once */
+ if ((drive->dn & 1) == 0 || pair == NULL) {
+ ide_acpi_set_state(hwif, 1);
+ ide_acpi_push_timing(hwif);
+ }
- ide_acpi_exec_tfs(drive);
+ ide_acpi_exec_tfs(drive);
+ }
memset(&rqpm, 0, sizeof(rqpm));
rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1420,6 +1420,7 @@ static inline void ide_dma_unmap_sg(ide_
#ifdef CONFIG_BLK_DEV_IDEACPI
int ide_acpi_init(void);
+bool ide_port_acpi(ide_hwif_t *hwif);
extern int ide_acpi_exec_tfs(ide_drive_t *drive);
extern void ide_acpi_get_timing(ide_hwif_t *hwif);
extern void ide_acpi_push_timing(ide_hwif_t *hwif);
@@ -1428,6 +1429,7 @@ void ide_acpi_port_init_devices(ide_hwif
extern void ide_acpi_set_state(ide_hwif_t *hwif, int on);
#else
static inline int ide_acpi_init(void) { return 0; }
+static inline bool ide_port_acpi(ide_hwif_t *hwif) { return 0; }
static inline int ide_acpi_exec_tfs(ide_drive_t *drive) { return 0; }
static inline void ide_acpi_get_timing(ide_hwif_t *hwif) { ; }
static inline void ide_acpi_push_timing(ide_hwif_t *hwif) { ; }