[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 ----------
> From: Sergei Trofimovich <[email protected]>
> 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)
> To: [email protected], Matthew Garrett <[email protected]>, Holger Macht <[email protected]>, Lin Ming <[email protected]>, Jeff Garzik <[email protected]>
> Cc: [email protected]
>
>
> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Holger Macht <[email protected]>
> Signed-off-by: Lin Ming <[email protected]>
> Signed-off-by: Jeff Garzik <[email protected]>
>
> :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
On Tue, 14 Aug 2012 16:09:52 +0800
Aaron Lu <[email protected]> wrote:
> [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.
> 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 ----------
> > From: Sergei Trofimovich <[email protected]>
> > 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)
> > To: [email protected], Matthew Garrett <[email protected]>, Holger Macht <[email protected]>, Lin Ming <[email protected]>, Jeff Garzik <[email protected]>
> > Cc: [email protected]
> >
> >
> > 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 <[email protected]>
> > 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 <[email protected]>
> > Signed-off-by: Holger Macht <[email protected]>
> > Signed-off-by: Lin Ming <[email protected]>
> > Signed-off-by: Jeff Garzik <[email protected]>
> >
> > :040000 040000 6a659a9d4a92b2085f6d0b58484cb2f82cd12cfa
> > 125fe5d5fa8b208a08792b03e752571d825465d2 M drivers
> > :040000 040000 b7c3819be4e82ae6e2ac9688055aeb2bc1bc4ebd
> > 9cbc8fd15b147a178f723bcdb9e7c34f9868400f M include
--
Sergei
On Tue, Aug 14, 2012 at 11:44:20AM +0300, Sergei Trofimovich wrote:
> > 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?
No, I don't think so.
> 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)
I guess the above errors are related to video, not ata.
> ata1: ACPI get timing mode failed (AE 0x1001)
This means the _GTM control method failed, I suppose this control method
on your platform always failed, even with a working kernel. So this
shouldn't be the cause.
> ACPI: Invalid Power Resource to register!
Not related to the hang.
I've tried here with two systems, the sata controller are set to IDE
mode using ata_piix driver and no problem with S3. Don't have a clue at
the moment :-(
Thanks,
Aaron
>
> They are not new errors.
>
> > 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
> >
On Tue, Aug 14, 2012 at 11:44:20AM +0300, Sergei Trofimovich wrote:
> > 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)
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
On Tue, 14 Aug 2012 22:48:08 +0800
Aaron Lu <[email protected]> wrote:
> On Tue, Aug 14, 2012 at 11:44:20AM +0300, Sergei Trofimovich wrote:
> > > 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)
>
>
> And here is another try:
This one did the trick! Survived suspend/resume cycle.
Whole dmesg is attached.
> > 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!
>
> 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