All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <bhelgaas@google.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Len Brown <lenb@kernel.org>,
	ACPI Devel Mailing List <linux-acpi@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linux-pci@vger.kernel.org, Alan Stern <stern@rowland.harvard.edu>,
	"Oleksij Rempel (fishor)" <bug-track@fisher-privat.net>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ACPI / PCI: Make _SxD/_SxW check follow ACPI 4.0a spec
Date: Mon, 30 Apr 2012 15:38:55 -0600	[thread overview]
Message-ID: <CAErSpo57jEuoKLnqVp6V-fkb8TBj+cUv3kgEbfpvFErE315pCA@mail.gmail.com> (raw)
In-Reply-To: <201204302341.12528.rjw@sisk.pl>

On Mon, Apr 30, 2012 at 3:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Monday, April 30, 2012, Bjorn Helgaas wrote:
>> On Sun, Apr 29, 2012 at 2:44 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > From: Oleksij Rempel <bug-track@fisher-privat.net>
>> >
>> > This patch makes _SxD/_SxW check follow the ACPI 4.0a specification
>> > more closely and fixes suspend bug found on ASUS Zenbook UX31E.
>> >
>> > Some OEM use _SxD fileds do blacklist brocken Dx states.
>> > If _SxD/_SxW return values are check before suspend as appropriate,
>> > some nasty suspend/resume issues may be avoided.
>> >
>> > References: https://bugzilla.kernel.org/show_bug.cgi?id=42728
>> > Signed-off-by: Oleksij Rempel <bug-track@fisher-privat.net>
>> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> > ---
>> >
>> > Bjorn, Len,
>> >
>> > This is -stable material and therefore v3.4 as well, IMO.  Please let me
>> > know if one of you can take it or whether you want me to handle it all the
>> > way to Linus.
>>
>> I'm OK with this from a PCI perspective.  Most of the change is in
>> ACPI, so I propose that either you or Len take care of it.
>>
>> The second paragraph of the changelog has several typos
>> (fileds/fields, do/to, brocken/broken, etc).
>
> Thanks for spotting that, the version below should be better.
>
> I'll wait for Len to respond till Friday and will take care of the patch myself
> if he doesn't say anything.
>
> May I assume an ACK from you?

Yep, sorry :)

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> From: Oleksij Rempel <bug-track@fisher-privat.net>
> Subject: ACPI / PCI: Make _SxD/_SxW check follow ACPI 4.0a spec
>
> This patch makes _SxD/_SxW check follow the ACPI 4.0a specification
> more closely and fixes suspend bug found on ASUS Zenbook UX31E.
>
> Some OEM use _SxD fields to blacklist broken device Dx states, so
> if _SxD/_SxW return values are checked before suspend as appropriate,
> some nasty suspend/resume issues may be avoided.
>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=42728
> Signed-off-by: Oleksij Rempel <bug-track@fisher-privat.net>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/sleep.c |   22 +++++++++++++++++++++-
>  drivers/pci/pci.c    |   12 +++++++++++-
>  2 files changed, 32 insertions(+), 2 deletions(-)
>
> Index: linux/drivers/acpi/sleep.c
> ===================================================================
> --- linux.orig/drivers/acpi/sleep.c
> +++ linux/drivers/acpi/sleep.c
> @@ -720,10 +720,30 @@ int acpi_pm_device_sleep_state(struct de
>         *
>         * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
>         * provided -- that's our fault recovery, we ignore retval.
> +        *
> +        * According to ACPI 4.0a (April 5, 2010), page 294,
> +        * Table 7-7 S3 Action / Result Table, we need to evaluate _SxW in
> +        * addition to _SxD if the device is configured for wakeup:
> +        * Desired Action    | _S3D | _PRW | _S3W | Resultant D-state
> +        * Enter S3          |  N/A |  D/C |  N/A | OSPM decides
> +        * Enter S3, No Wake |   2  |  D/C |  D/C | Enter D2 or D3
> +        * Enter S3, Wake    |   2  |   3  |  N/A | Enter D2
> +        * Enter S3, Wake    |   2  |   3  |   3  | Enter D2 or D3
> +        * Enter S3, Wake    |  N/A |   3  |   2  | Enter D0, D1 or D2
>         */
> -       if (acpi_target_sleep_state > ACPI_STATE_S0)
> +       if (acpi_target_sleep_state > ACPI_STATE_S0) {
> +               acpi_status status;
> +
>                acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
>
> +               if (device_may_wakeup(dev)) {
> +                       acpi_method[3] = 'W';
> +                       status = acpi_evaluate_integer(handle, acpi_method,
> +                                                      NULL, &d_max);
> +                       if (ACPI_FAILURE(status))
> +                               d_max = d_min;
> +               }
> +       }
>        /*
>         * If _PRW says we can wake up the system from the target sleep state,
>         * the D-state returned by _SxD is sufficient for that (we assume a
> Index: linux/drivers/pci/pci.c
> ===================================================================
> --- linux.orig/drivers/pci/pci.c
> +++ linux/drivers/pci/pci.c
> @@ -1691,10 +1691,20 @@ pci_power_t pci_target_state(struct pci_
>  {
>        pci_power_t target_state = PCI_D3hot;
>
> -       if (platform_pci_power_manageable(dev)) {
> +       /*
> +        * According to ACPI 4.0a,7.2 Device Power Management Objects, device
> +        * with wake capability should have _PRW or _PSW object and can have
> +        * _SxD or _SxW object.
> +        * It looks like some OEMs use this fields to avoid buggy Dx states
> +        * of devices, so we need to check for _PRW or _PSW and see if _SxD or
> +        * _SxW indicate to overwrite Dx.
> +        */
> +       if (platform_pci_power_manageable(dev)
> +           || platform_pci_can_wakeup(dev)) {
>                /*
>                 * Call the platform to choose the target state of the device
>                 * and enable wake-up from this state if supported.
> +                * (Check _SxD and _SxW)
>                 */
>                pci_power_t state = platform_pci_choose_state(dev);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: Bjorn Helgaas <bhelgaas@google.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: Len Brown <lenb@kernel.org>,
	ACPI Devel Mailing List <linux-acpi@vger.kernel.org>,
	Linux PM list <linux-pm@vger.kernel.org>,
	linux-pci@vger.kernel.org, Alan Stern <stern@rowland.harvard.edu>,
	"Oleksij Rempel (fishor)" <bug-track@fisher-privat.net>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ACPI / PCI: Make _SxD/_SxW check follow ACPI 4.0a spec
Date: Mon, 30 Apr 2012 15:38:55 -0600	[thread overview]
Message-ID: <CAErSpo57jEuoKLnqVp6V-fkb8TBj+cUv3kgEbfpvFErE315pCA@mail.gmail.com> (raw)
In-Reply-To: <201204302341.12528.rjw@sisk.pl>

On Mon, Apr 30, 2012 at 3:41 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Monday, April 30, 2012, Bjorn Helgaas wrote:
>> On Sun, Apr 29, 2012 at 2:44 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > From: Oleksij Rempel <bug-track@fisher-privat.net>
>> >
>> > This patch makes _SxD/_SxW check follow the ACPI 4.0a specification
>> > more closely and fixes suspend bug found on ASUS Zenbook UX31E.
>> >
>> > Some OEM use _SxD fileds do blacklist brocken Dx states.
>> > If _SxD/_SxW return values are check before suspend as appropriate,
>> > some nasty suspend/resume issues may be avoided.
>> >
>> > References: https://bugzilla.kernel.org/show_bug.cgi?id=42728
>> > Signed-off-by: Oleksij Rempel <bug-track@fisher-privat.net>
>> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
>> > ---
>> >
>> > Bjorn, Len,
>> >
>> > This is -stable material and therefore v3.4 as well, IMO.  Please let me
>> > know if one of you can take it or whether you want me to handle it all the
>> > way to Linus.
>>
>> I'm OK with this from a PCI perspective.  Most of the change is in
>> ACPI, so I propose that either you or Len take care of it.
>>
>> The second paragraph of the changelog has several typos
>> (fileds/fields, do/to, brocken/broken, etc).
>
> Thanks for spotting that, the version below should be better.
>
> I'll wait for Len to respond till Friday and will take care of the patch myself
> if he doesn't say anything.
>
> May I assume an ACK from you?

Yep, sorry :)

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
> From: Oleksij Rempel <bug-track@fisher-privat.net>
> Subject: ACPI / PCI: Make _SxD/_SxW check follow ACPI 4.0a spec
>
> This patch makes _SxD/_SxW check follow the ACPI 4.0a specification
> more closely and fixes suspend bug found on ASUS Zenbook UX31E.
>
> Some OEM use _SxD fields to blacklist broken device Dx states, so
> if _SxD/_SxW return values are checked before suspend as appropriate,
> some nasty suspend/resume issues may be avoided.
>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=42728
> Signed-off-by: Oleksij Rempel <bug-track@fisher-privat.net>
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/sleep.c |   22 +++++++++++++++++++++-
>  drivers/pci/pci.c    |   12 +++++++++++-
>  2 files changed, 32 insertions(+), 2 deletions(-)
>
> Index: linux/drivers/acpi/sleep.c
> ===================================================================
> --- linux.orig/drivers/acpi/sleep.c
> +++ linux/drivers/acpi/sleep.c
> @@ -720,10 +720,30 @@ int acpi_pm_device_sleep_state(struct de
>         *
>         * NOTE: We rely on acpi_evaluate_integer() not clobbering the integer
>         * provided -- that's our fault recovery, we ignore retval.
> +        *
> +        * According to ACPI 4.0a (April 5, 2010), page 294,
> +        * Table 7-7 S3 Action / Result Table, we need to evaluate _SxW in
> +        * addition to _SxD if the device is configured for wakeup:
> +        * Desired Action    | _S3D | _PRW | _S3W | Resultant D-state
> +        * Enter S3          |  N/A |  D/C |  N/A | OSPM decides
> +        * Enter S3, No Wake |   2  |  D/C |  D/C | Enter D2 or D3
> +        * Enter S3, Wake    |   2  |   3  |  N/A | Enter D2
> +        * Enter S3, Wake    |   2  |   3  |   3  | Enter D2 or D3
> +        * Enter S3, Wake    |  N/A |   3  |   2  | Enter D0, D1 or D2
>         */
> -       if (acpi_target_sleep_state > ACPI_STATE_S0)
> +       if (acpi_target_sleep_state > ACPI_STATE_S0) {
> +               acpi_status status;
> +
>                acpi_evaluate_integer(handle, acpi_method, NULL, &d_min);
>
> +               if (device_may_wakeup(dev)) {
> +                       acpi_method[3] = 'W';
> +                       status = acpi_evaluate_integer(handle, acpi_method,
> +                                                      NULL, &d_max);
> +                       if (ACPI_FAILURE(status))
> +                               d_max = d_min;
> +               }
> +       }
>        /*
>         * If _PRW says we can wake up the system from the target sleep state,
>         * the D-state returned by _SxD is sufficient for that (we assume a
> Index: linux/drivers/pci/pci.c
> ===================================================================
> --- linux.orig/drivers/pci/pci.c
> +++ linux/drivers/pci/pci.c
> @@ -1691,10 +1691,20 @@ pci_power_t pci_target_state(struct pci_
>  {
>        pci_power_t target_state = PCI_D3hot;
>
> -       if (platform_pci_power_manageable(dev)) {
> +       /*
> +        * According to ACPI 4.0a,7.2 Device Power Management Objects, device
> +        * with wake capability should have _PRW or _PSW object and can have
> +        * _SxD or _SxW object.
> +        * It looks like some OEMs use this fields to avoid buggy Dx states
> +        * of devices, so we need to check for _PRW or _PSW and see if _SxD or
> +        * _SxW indicate to overwrite Dx.
> +        */
> +       if (platform_pci_power_manageable(dev)
> +           || platform_pci_can_wakeup(dev)) {
>                /*
>                 * Call the platform to choose the target state of the device
>                 * and enable wake-up from this state if supported.
> +                * (Check _SxD and _SxW)
>                 */
>                pci_power_t state = platform_pci_choose_state(dev);
>

  reply	other threads:[~2012-04-30 21:39 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-29 20:44 [PATCH] ACPI / PCI: Make _SxD/_SxW check follow ACPI 4.0a spec Rafael J. Wysocki
2012-04-30 16:03 ` Bjorn Helgaas
2012-04-30 17:53   ` Alan Stern
2012-04-30 17:53     ` Alan Stern
2012-04-30 21:30     ` Oleksij Rempel
2012-04-30 21:43       ` Rafael J. Wysocki
2012-05-01  6:38         ` Oleksij Rempel (fishor)
2012-05-01 14:04           ` Rafael J. Wysocki
2012-05-01 14:04             ` Rafael J. Wysocki
2012-05-01 14:11             ` Alan Stern
2012-05-01 14:11               ` Alan Stern
2012-05-01 16:27               ` Oleksij Rempel (fishor)
2012-05-01 16:59                 ` Alan Stern
2012-05-01 16:59                   ` Alan Stern
2012-05-02  4:10                   ` Oleksij Rempel (fishor)
2012-05-25 19:15                     ` Alan Stern
2012-05-25 20:00                       ` Rafael J. Wysocki
2012-05-26  6:03                         ` Oleksij Rempel (fishor)
2012-05-26  8:15                           ` Oleksij Rempel
2012-05-26 20:21                             ` Rafael J. Wysocki
2012-04-30 21:32     ` Rafael J. Wysocki
2012-04-30 21:32       ` Rafael J. Wysocki
2012-04-30 21:41   ` Rafael J. Wysocki
2012-04-30 21:38     ` Bjorn Helgaas [this message]
2012-04-30 21:38       ` Bjorn Helgaas
2012-05-10 20:14     ` Rafael J. Wysocki
2012-05-10 20:14       ` Rafael J. Wysocki
2012-07-11  0:08 ` Greg KH
2012-07-11  9:07   ` Rafael J. Wysocki
2012-07-11 13:53     ` Greg KH
2012-07-11 19:04       ` Rafael J. Wysocki
2012-07-11 19:20         ` Greg KH
2012-07-11 19:29           ` Rafael J. Wysocki
2012-07-12  4:30         ` Ben Hutchings

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAErSpo57jEuoKLnqVp6V-fkb8TBj+cUv3kgEbfpvFErE315pCA@mail.gmail.com \
    --to=bhelgaas@google.com \
    --cc=bug-track@fisher-privat.net \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=rjw@sisk.pl \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.