linux-kernel - Re: [PATCH] ACPI: do not fail suspend if unable to configure wakeup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Oct 2014 10:10:20 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	Dmitry Torokhov <dtor@...omium.org>
Cc:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
	Benson Leung <bleung@...omium.org>,
	ACPI Devel Maling List <linux-acpi@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ACPI: do not fail suspend if unable to configure wakeup

Hi,

On Thu, Oct 16, 2014 at 12:56 AM, Dmitry Torokhov <dtor@...omium.org> wrote:
> Newer kernels put i2c devices with ACPI companion in ACPI power domain and
> then ACPI will try to configure them for wakeup (if requested).
> Unfortunately on some Chromebooks firmware separates wakeup GPIO into a
> completely separate device (which is handled by the kernel as a sleep
> button), leaving the touchpads themselves not wakeup capable (as far as
> ACPI is concerned). This causes ACPI late suspend code to fail to configure
> them as wakeup sources and aborts entire suspend.
>
> To work around this issues let's not abort entire suspend process if
> driver asked to be a wakeup source but ACPI can not satisfy that
> request.
>
> Note that originally I tried to simply change the driver to not mark
> device as wakeup source, unfortunately then we do not know that we
> should not be powering down the device completely, otherwise we can't
> wake up.
>
> Verified by making sure that "echo mem > /sys/power/state" works on
> Squawks.
>
> Reviewed-by: Benson Leung <bleung@...omium.org>
> Signed-off-by: Dmitry Torokhov <dtor@...omium.org>
> ---
>  drivers/acpi/device_pm.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index 67075f8..440bc3d 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -871,6 +871,7 @@ int acpi_dev_suspend_late(struct device *dev)
>         struct acpi_device *adev = ACPI_COMPANION(dev);
>         u32 target_state;
>         bool wakeup;
> +       bool can_wakeup;
>         int error;
>
>         if (!adev)
> @@ -878,12 +879,19 @@ int acpi_dev_suspend_late(struct device *dev)
>
>         target_state = acpi_target_system_state();
>         wakeup = device_may_wakeup(dev);
> -       error = acpi_device_wakeup(adev, target_state, wakeup);
> -       if (wakeup && error)
> -               return error;
> +       can_wakeup = acpi_device_can_wakeup(adev);
> +
> +       if (can_wakeup) {
> +               error = acpi_device_wakeup(adev, target_state, wakeup);
> +               if (wakeup && error)
> +                       return error;
> +       } else if (wakeup) {

I think we just need to return an error code in that case, because otherwise
this is potentially dangerous (worst case, it may be impossible to wake up
the machine at all after that).

> +               dev_warn(dev,
> +                        "device is not wakeup-capable, not enabling wakeup\n");
> +       }
>
>         error = acpi_dev_pm_low_power(dev, adev, target_state);
> -       if (error)
> +       if (error && can_wakeup)
>                 acpi_device_wakeup(adev, ACPI_STATE_UNKNOWN, false);
>
>         return error;
> --

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