bisected regression: v3.6-rc1: resume from s2ram does not restore ata

Discussion:

bisected regression: v3.6-rc1: resume from s2ram does not restore ata_piix (v3.5 worked)

(too old to reply)

Sergei Trofimovich

2012-08-13 09:15:32 UTC

It's a laptop compaq 2510p (~5 years old core2 laptop) with a
single SATA drive:

00:1f.1 IDE interface: Intel Corporation 82801HM/HEM (ICH8M/ICH8M-E) IDE Controller (rev 03)
Subsystem: Hewlett-Packard Company Device 30c9
Kernel driver in use: ata_piix

kernel v3.5 worked fine. 3.6-rc1 resumes, but disk stays inaccessble.
Seems to be 100% reproducible. Bisection gave sane result[1].

I was not able to revert it as-is, thus couldn't verify the revert
helps on top of master.

Do you need more info?

Thanks!

[1]
commit 30dcf76acc695cbd2fa919e294670fe9552e16e7
Author: Matthew Garrett <***@redhat.com>
Date: Mon Jun 25 16:13:04 2012 +0800

libata: migrate ACPI code over to new bindings

Now that we have the ability to directly glue the ACPI namespace to the
driver model in libata, we don't need the custom code to handle the same
thing. Remove it and migrate the functions over to the new code.

Signed-off-by: Matthew Garrett <***@redhat.com>
Signed-off-by: Holger Macht <***@homac.de>
Signed-off-by: Lin Ming <***@intel.com>
Signed-off-by: Jeff Garzik <***@redhat.com>

:040000 040000 6a659a9d4a92b2085f6d0b58484cb2f82cd12cfa 125fe5d5fa8b208a08792b03e752571d825465d2 M drivers
:040000 040000 b7c3819be4e82ae6e2ac9688055aeb2bc1bc4ebd 9cbc8fd15b147a178f723bcdb9e7c34f9868400f M include

git bisect good 492d542273a4859f8bf8cc7744cdf71ef50b39ea
# bad: [354b2eac3848bddbcb111079138b907ccca70ae8] libata-acpi: fix up for acpi_pm_device_sleep_state API
git bisect bad 354b2eac3848bddbcb111079138b907ccca70ae8
# bad: [dc7f71f486f4f5fa96f6dcf86833da020cde8a11] sata_dwc_460ex: device tree may specify dma_channel
git bisect bad dc7f71f486f4f5fa96f6dcf86833da020cde8a11
# bad: [91e4d5a1d7d11ca0b08803a11cb8dc866d2d611f] drivers/acpi/glue: revert accidental license-related 6b66d95895c bits
git bisect bad 91e4d5a1d7d11ca0b08803a11cb8dc866d2d611f
# bad: [3bd46600a7a7e938c54df8cdbac9910668c7dfb0] libata-acpi: add ata port runtime D3Cold support
git bisect bad 3bd46600a7a7e938c54df8cdbac9910668c7dfb0
# bad: [30dcf76acc695cbd2fa919e294670fe9552e16e7] libata: migrate ACPI code over to new bindings
git bisect bad 30dcf76acc695cbd2fa919e294670fe9552e16e7
# good: [6b66d95895c149cbc04d4fac5a2f5477c543a8ae] libata: bind the Linux device tree to the ACPI device tree
git bisect good 6b66d95895c149cbc04d4fac5a2f5477c543a8ae
# good: [6b66d95895c149cbc04d4fac5a2f5477c543a8ae] libata: bind the Linux device tree to the ACPI device tree
git bisect good 6b66d95895c149cbc04d4fac5a2f5477c543a8ae
30dcf76acc695cbd2fa919e294670fe9552e16e7 is the first bad commit

--
Sergei

Aaron Lu

2012-08-14 08:10:33 UTC

Permalink

[Re-send due to the last email is not plain text.]

Hi Sergei,

The only problem I can see is the offending commit didn't do a gtm for
IDE channel during init. It was used to be done in
ata_acpi_associate_ide_port.

So can you please test if the following code fix your problem? Thanks.

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..0f338bb 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1101,6 +1101,9 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
if (!*handle)
return -ENODEV;

+ if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+ ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
+
return 0;
}

Thanks,
Aaron

---------- Forwarded message ----------
Date: Mon, Aug 13, 2012 at 5:20 PM
Subject: bisected regression: v3.6-rc1: resume from s2ram does not restore ata_piix (v3.5 worked)
00:1f.1 IDE interface: Intel Corporation 82801HM/HEM (ICH8M/ICH8M-E) IDE Controller (rev 03)
Subsystem: Hewlett-Packard Company Device 30c9
Kernel driver in use: ata_piix
kernel v3.5 worked fine. 3.6-rc1 resumes, but disk stays inaccessble.
Seems to be 100% reproducible. Bisection gave sane result[1].
I was not able to revert it as-is, thus couldn't verify the revert helps on top of master.
Do you need more info?
Thanks!
[1]
commit 30dcf76acc695cbd2fa919e294670fe9552e16e7
Date: Mon Jun 25 16:13:04 2012 +0800
libata: migrate ACPI code over to new bindings
Now that we have the ability to directly glue the ACPI namespace to the
driver model in libata, we don't need the custom code to handle the same
thing. Remove it and migrate the functions over to the new code.
:040000 040000 6a659a9d4a92b2085f6d0b58484cb2f82cd12cfa
125fe5d5fa8b208a08792b03e752571d825465d2 M drivers
:040000 040000 b7c3819be4e82ae6e2ac9688055aeb2bc1bc4ebd
9cbc8fd15b147a178f723bcdb9e7c34f9868400f M include
git bisect good 492d542273a4859f8bf8cc7744cdf71ef50b39ea
# bad: [354b2eac3848bddbcb111079138b907ccca70ae8] libata-acpi: fix up for acpi_pm_device_sleep_state API git bisect bad 354b2eac3848bddbcb111079138b907ccca70ae8
device tree may specify dma_channel
git bisect bad dc7f71f486f4f5fa96f6dcf86833da020cde8a11
revert accidental license-related 6b66d95895c bits git bisect bad 91e4d5a1d7d11ca0b08803a11cb8dc866d2d611f
# bad: [3bd46600a7a7e938c54df8cdbac9910668c7dfb0] libata-acpi: add ata port runtime D3Cold support git bisect bad 3bd46600a7a7e938c54df8cdbac9910668c7dfb0
# bad: [30dcf76acc695cbd2fa919e294670fe9552e16e7] libata: migrate ACPI code over to new bindings git bisect bad 30dcf76acc695cbd2fa919e294670fe9552e16e7
# good: [6b66d95895c149cbc04d4fac5a2f5477c543a8ae] libata: bind the Linux device tree to the ACPI device tree git bisect good 6b66d95895c149cbc04d4fac5a2f5477c543a8ae
# good: [6b66d95895c149cbc04d4fac5a2f5477c543a8ae] libata: bind the Linux device tree to the ACPI device tree git bisect good 6b66d95895c149cbc04d4fac5a2f5477c543a8ae
30dcf76acc695cbd2fa919e294670fe9552e16e7 is the first bad commit
--
Sergei

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

Sergei Trofimovich

2012-08-14 08:39:22 UTC

Permalink

On Tue, 14 Aug 2012 16:09:52 +0800

Post by Aaron Lu
[Re-send due to the last email is not plain text.]
Hi Sergei,
The only problem I can see is the offending commit didn't do a gtm for
IDE channel during init. It was used to be done in
ata_acpi_associate_ide_port.
So can you please test if the following code fix your problem? Thanks.

Unfortunately, nothing changed. The same hangup after resume.
Did the bisected patch change the way kernel relies on ACPI
information? I have some complains in dmesg output about it
(attached whole dmesg) like that:

ACPI Exception: AE_AML_PACKAGE_LIMIT, Index (0x0000000000000004) is beyond end of object (20120711/exoparg2-418)
ACPI Error: Method parse/execution failed [\_SB_.C003.C09A._DOD] (Node ffff88007b82c988), AE_AML_PACKAGE_LIMIT (20120711/psparse-536)
ACPI Exception: AE_AML_PACKAGE_LIMIT, Evaluating _DOD (20120711/video-1149)
ata1: ACPI get timing mode failed (AE 0x1001)
ACPI: Invalid Power Resource to register!

They are not new errors.

Post by Aaron Lu
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..0f338bb 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -1101,6 +1101,9 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
if (!*handle)
return -ENODEV;
+ if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+ ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
+
return 0;
}
Thanks,
Aaron

--
Sergei

Aaron Lu

2012-08-14 09:14:57 UTC

Permalink

This post might be inappropriate. Click to display it.

Aaron Lu

2012-08-14 14:48:26 UTC

Permalink

Post by Sergei Trofimovich

Post by Aaron Lu
The only problem I can see is the offending commit didn't do a gtm for
IDE channel during init. It was used to be done in
ata_acpi_associate_ide_port.
So can you please test if the following code fix your problem? Thanks.

Unfortunately, nothing changed. The same hangup after resume.
Did the bisected patch change the way kernel relies on ACPI
information? I have some complains in dmesg output about it
ACPI Exception: AE_AML_PACKAGE_LIMIT, Index (0x0000000000000004) is beyond end of object (20120711/exoparg2-418)
ACPI Error: Method parse/execution failed [\_SB_.C003.C09A._DOD] (Node ffff88007b82c988), AE_AML_PACKAGE_LIMIT (20120711/psparse-536)
ACPI Exception: AE_AML_PACKAGE_LIMIT, Evaluating _DOD (20120711/video-1149)
ata1: ACPI get timing mode failed (AE 0x1001)

Do you see this error message when using a working kernel?

And here is another try:

diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..fd9ecf7 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -60,17 +60,7 @@ acpi_handle ata_ap_acpi_handle(struct ata_port *ap)
if (ap->flags & ATA_FLAG_ACPI_SATA)
return NULL;

- /*
- * If acpi bind operation has already happened, we can get the handle
- * for the port by checking the corresponding scsi_host device's
- * firmware node, otherwise we will need to find out the handle from
- * its parent's acpi node.
- */
- if (ap->scsi_host)
- return DEVICE_ACPI_HANDLE(&ap->scsi_host->shost_gendev);
- else
- return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
- ap->port_no);
+ return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), ap->port_no);
}
EXPORT_SYMBOL(ata_ap_acpi_handle);

@@ -1101,6 +1091,9 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
if (!*handle)
return -ENODEV;

+ if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+ ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
+
return 0;
}

Thanks,
Aaron
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Sergei Trofimovich

2012-08-14 17:22:03 UTC

Permalink

On Tue, 14 Aug 2012 22:48:08 +0800

Post by Aaron Lu

Post by Sergei Trofimovich

Unfortunately, nothing changed. The same hangup after resume.
Did the bisected patch change the way kernel relies on ACPI
information? I have some complains in dmesg output about it
ACPI Exception: AE_AML_PACKAGE_LIMIT, Index (0x0000000000000004) is beyond end of object (20120711/exoparg2-418)
ACPI Error: Method parse/execution failed [\_SB_.C003.C09A._DOD] (Node ffff88007b82c988), AE_AML_PACKAGE_LIMIT (20120711/psparse-536)
ACPI Exception: AE_AML_PACKAGE_LIMIT, Evaluating _DOD (20120711/video-1149)

This one did the trick! Survived suspend/resume cycle.
Whole dmesg is attached.

Post by Aaron Lu

Post by Sergei Trofimovich
ata1: ACPI get timing mode failed (AE 0x1001)

Do you see this error message when using a working kernel?

Now with patched kernel I see

ata1: ACPI set timing mode failed (status=0x300b)

after resume. No 'get timing' errors. Attached dmesg
with suspend/resume log.

I can boot to older unpatched kernels right before and after
offending commit and send you dmesg diff if you still need/like
to look at it.

Thanks for the fix!

Post by Aaron Lu
diff --git a/drivers/ata/libata-acpi.c b/drivers/ata/libata-acpi.c
index 902b5a4..fd9ecf7 100644
--- a/drivers/ata/libata-acpi.c
+++ b/drivers/ata/libata-acpi.c
@@ -60,17 +60,7 @@ acpi_handle ata_ap_acpi_handle(struct ata_port *ap)
if (ap->flags & ATA_FLAG_ACPI_SATA)
return NULL;
- /*
- * If acpi bind operation has already happened, we can get the handle
- * for the port by checking the corresponding scsi_host device's
- * firmware node, otherwise we will need to find out the handle from
- * its parent's acpi node.
- */
- if (ap->scsi_host)
- return DEVICE_ACPI_HANDLE(&ap->scsi_host->shost_gendev);
- else
- return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev),
- ap->port_no);
+ return acpi_get_child(DEVICE_ACPI_HANDLE(ap->host->dev), ap->port_no);
}
EXPORT_SYMBOL(ata_ap_acpi_handle);
@@ -1101,6 +1091,9 @@ static int ata_acpi_bind_host(struct ata_port *ap, acpi_handle *handle)
if (!*handle)
return -ENODEV;
+ if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+ ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
+
return 0;
}
Thanks,
Aaron

--
Sergei