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>,
        Benson Leung <bleung@chromium.org>,
        Enric Balletbo i Serra <enric.balletbo@collabora.com>,
        Guenter Roeck <groeck@chromium.org>,
        Gwendal Grignou <gwendal@chromium.org>,
        Prashant Malani <pmalani@chromium.org>
Subject: [PATCH 3/3] platform/chrome: cros_ec: Don't enable wake pin if ACPI managed
Date:   Mon, 20 Dec 2021 16:43:46 -0700
Message-Id: <20211220163823.3.I2339b40dd2ed02569ba204734dc335866a1c5b98@changeid>
In-Reply-To: <20211220234346.2798027-1-rrangel@chromium.org>
References: <20211220234346.2798027-1-rrangel@chromium.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

In ACPI managed systems, the `_PRW` method defines the wake source for
the device. This could be a GPE or a GPIO not related to the IRQ. The
way the cros_ec_lpc driver works is that the irq field is optional. The
IRQ defined in the `_CRS` is only used to speed up sensor event
processing. Before this change, the SYNC_IRQ GPIO would have its wake
bit enabled. This means that we now have two wake sources defined for
the EC. This change makes the CrOS EC driver leave wake configuration
alone if the device is ACPI managed.

I tested this on guybrush and no longer see the EC SYNC IRQ enabled as a
wake source when suspending.

Signed-off-by: Raul E Rangel <rrangel@chromium.org>
---

 drivers/platform/chrome/cros_ec.c           | 8 ++++++--
 include/linux/platform_data/cros_ec_proto.h | 1 -
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c
index fc5aa1525d13..81e334157338 100644
--- a/drivers/platform/chrome/cros_ec.c
+++ b/drivers/platform/chrome/cros_ec.c
@@ -9,6 +9,7 @@
  * battery charging and regulator control, firmware update.
  */
 
+#include <linux/acpi.h>
 #include <linux/of_platform.h>
 #include <linux/interrupt.h>
 #include <linux/slab.h>
@@ -336,11 +337,14 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
 		dev_dbg(ec_dev->dev, "Error %d sending suspend event to ec",
 			ret);
 
-	if (device_may_wakeup(dev))
+	/*
+	 * For non-ACPI subsystems we need to explicitly enable the wake source.
+	 * For ACPI systems, the ACPI subsystem will handle all the details.
+	 */
+	if (device_may_wakeup(dev) && !ACPI_COMPANION(ec_dev->dev))
 		ec_dev->wake_enabled = !enable_irq_wake(ec_dev->irq);
 
 	disable_irq(ec_dev->irq);
-	ec_dev->was_wake_device = ec_dev->wake_enabled;
 	ec_dev->suspended = true;
 
 	return 0;
diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h
index 992796e40cd6..054d28ddb4c1 100644
--- a/include/linux/platform_data/cros_ec_proto.h
+++ b/include/linux/platform_data/cros_ec_proto.h
@@ -139,7 +139,6 @@ struct cros_ec_device {
 	/* These are used by other drivers that want to talk to the EC */
 	const char *phys_name;
 	struct device *dev;
-	bool was_wake_device;
 	struct class *cros_class;
 	int (*cmd_readmem)(struct cros_ec_device *ec, unsigned int offset,
 			   unsigned int bytes, void *dest);
-- 
2.34.1.307.g9b7440fafd-goog