From:   Raul E Rangel <rrangel@chromium.org>
To:     linux-kernel@vger.kernel.org
Cc:     mario.limonciello@amd.com, linux-input@vger.kernel.org,
        dianders@chromium.org, Raul E Rangel <rrangel@chromium.org>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Benjamin Tissoires <benjamin.tissoires@redhat.com>,
        Benson Leung <bleung@chromium.org>,
        Dmitry Torokhov <dmitry.torokhov@gmail.com>,
        Enric Balletbo i Serra <enric.balletbo@collabora.com>,
        Guenter Roeck <groeck@chromium.org>,
        Gwendal Grignou <gwendal@chromium.org>,
        Hans de Goede <hdegoede@redhat.com>,
        Jiri Kosina <jikos@kernel.org>,
        Prashant Malani <pmalani@chromium.org>,
        "jingle.wu" <jingle.wu@emc.com.tw>
Subject: [PATCH 0/3] Fix spurious wakes on ACPI platforms
Date:   Mon, 20 Dec 2021 16:43:43 -0700
Message-Id: <20211220234346.2798027-1-rrangel@chromium.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

d62bd5ce12d7 ("pinctrl: amd: Implement irq_set_wake") gave the kernel
the ability to control the GPIO wake bit on AMD platforms. This
uncovered various drivers that were manually managing their wake
capabilities. This is problematic on ACPI systems because the ACPI
subsystem is responsible for managing the wake capabilities for the
device. ACPI devices need to define the `_PRW` property that defines the
GPE or GPIO that will be used to wake the system, and also the power
resources that must be enabled for the device to issue a wake.

The following real world example shows the problem:
* We have an ACPI HID device that has a power resource defined in `_PR0`
  and `_PR3`. It doesn't have `_PRW` so that means the device can't wake
  the system.
* The IRQ line is active level low for this device and is pulled up by the
  power resource defined in `_PR0`/`_PR3`.

The i2c-hid driver manually sets the device as wake capable, and the wake
flag gets set on the IRQ GPIO when entering suspend. As part of suspend,
ACPI will turn off the devices power resources since the device isn't a
wake source. This immediately triggers a wake because the IRQ line is
now low.

For devices that are marked as being wake capable (via `_PRW`), they
might use GPEs for wakes, while others might use the GPIO controller
(via _AEI). We need to respect the firmware configuration so we don't
have both the GPE and GPIO triggering a wake.

So when using ACPI, the correct thing to do is remove the driver from
managing the wake configuration. The ACPI subsystem has more knowledge of
the platform topology than the driver does.

This patch series fixes a few drivers that I noticed were incorrectly
setting the GPIO wake bit on my Guybrush Chromebook. I'm sure there will
be more.

I will be OOO until the New Year, so my responses will be delayed.


Raul E Rangel (3):
  HID: i2c-hid-acpi: Remove explicit device_set_wakeup_capable
  Input: elan_i2c - Use PM subsystem to manage wake irq
  platform/chrome: cros_ec: Don't enable wake pin if ACPI managed

 drivers/hid/i2c-hid/i2c-hid-acpi.c          |  5 -----
 drivers/input/mouse/elan_i2c_core.c         | 21 +++++++--------------
 drivers/platform/chrome/cros_ec.c           |  8 ++++++--
 include/linux/platform_data/cros_ec_proto.h |  1 -
 4 files changed, 13 insertions(+), 22 deletions(-)

-- 
2.34.1.307.g9b7440fafd-goog