[REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Etienne Basset]

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 <bzolnier@gmail.com>      
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 <bzolnier@gmail.com>

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

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Bartlomiej Zolnierkiewicz]

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, &gtf_length, &gtf_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) { ; }

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Etienne Basset]

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, &gtf_length, &gtf_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) { ; }
> 

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Jeff Chua]

On Sat, Jun 27, 2009 at 5:04 PM, Etienne
Basset<etienne.basset@numericable.fr> 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, &gtf_length, &gtf_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) { ; }

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Jeff Chua]

On Sun, Jun 28, 2009 at 2:09 AM, Jeff Chua<jeff.chua.linux@gmail.com> wrote:
> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
> Basset<etienne.basset@numericable.fr> 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.

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Etienne Basset]

Jeff Chua wrote:
> On Sun, Jun 28, 2009 at 2:09 AM, Jeff Chua<jeff.chua.linux@gmail.com> wrote:
>> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
>> Basset<etienne.basset@numericable.fr> 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

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Jeff Chua]

On Sun, Jun 28, 2009 at 4:06 PM, Etienne
Basset<etienne.basset@numericable.fr> 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.

Re: [REGRESSION] 2.6.29=>2.6.30 suspend to ram regression (IDE related)

[Bartlomiej Zolnierkiewicz]

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<jeff.chua.linux@gmail.com> wrote:
> >> On Sat, Jun 27, 2009 at 5:04 PM, Etienne
> >> Basset<etienne.basset@numericable.fr> 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 <bzolnier@gmail.com>
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 <etienne.basset@numericable.fr>
Also-reported-and-tested-by: Jeff Chua <jeff.chua.linux@gmail.com>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: stable@kernel.org
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
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, &gtf_length, &gtf_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) { ; }