Re: [PATCH 1/8] Input: elan_i2c - Use PM subsystem to manage wake irq
flat view
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: 2022-08-31 18:01:35
Also in:
linux-acpi, lkml
On Wed, Aug 31, 2022 at 1:16 AM Raul E Rangel [off-list ref] wrote:
The Elan I2C touchpad driver is currently manually managing the wake IRQ. This change removes the explicit enable_irq_wake/disable_irq_wake and instead relies on the PM subsystem. This is done by calling dev_pm_set_wake_irq. i2c_device_probe already calls dev_pm_set_wake_irq when using device tree, so it's only required when using ACPI. The net result is that this change should be a no-op. i2c_device_remove also already calls dev_pm_clear_wake_irq, so we don't need to do that in this driver. I tested this on an ACPI system where the touchpad doesn't have _PRW defined. I verified I can still wake the system and that the wake source was the touchpad IRQ GPIO. Signed-off-by: Raul E Rangel <redacted>
I like this a lot, but the assumption in the wakeirq code is that the IRQ in question will be dedicated for signaling wakeup. Does it hold here?
quoted hunk ↗ jump to hunk
--- drivers/input/mouse/elan_i2c_core.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c index e1758d5ffe4218..7d997d2b56436b 100644 --- a/drivers/input/mouse/elan_i2c_core.c +++ b/drivers/input/mouse/elan_i2c_core.c@@ -33,6 +33,7 @@ #include <linux/jiffies.h> #include <linux/completion.h> #include <linux/of.h> +#include <linux/pm_wakeirq.h> #include <linux/property.h> #include <linux/regulator/consumer.h> #include <asm/unaligned.h>@@ -86,8 +87,6 @@ struct elan_tp_data { u16 fw_page_size; u32 fw_signature_address; - bool irq_wake; - u8 min_baseline; u8 max_baseline; bool baseline_ready;@@ -1337,8 +1336,10 @@ static int elan_probe(struct i2c_client *client, * Systems using device tree should set up wakeup via DTS, * the rest will configure device as wakeup source by default. */ - if (!dev->of_node) + if (!dev->of_node) { device_init_wakeup(dev, true); + dev_pm_set_wake_irq(dev, client->irq); + } return 0; }@@ -1362,8 +1363,6 @@ static int __maybe_unused elan_suspend(struct device *dev) if (device_may_wakeup(dev)) { ret = elan_sleep(data); - /* Enable wake from IRQ */ - data->irq_wake = (enable_irq_wake(client->irq) == 0); } else { ret = elan_set_power(data, false); if (ret)@@ -1394,9 +1393,6 @@ static int __maybe_unused elan_resume(struct device *dev) dev_err(dev, "error %d enabling regulator\n", error); goto err; } - } else if (data->irq_wake) { - disable_irq_wake(client->irq); - data->irq_wake = false; } error = elan_set_power(data, true); --2.37.2.672.g94769d06f0-goog