On Wed, Aug 31, 2022 at 12:12:41PM -0700, Dmitry Torokhov wrote: > On Wed, Aug 31, 2022 at 08:01:12PM +0200, Rafael J. Wysocki wrote: > > On Wed, Aug 31, 2022 at 1:16 AM Raul E Rangel <rrangel@xxxxxxxxxxxx> 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 <rrangel@xxxxxxxxxxxx> > > > > I like this a lot [...] > > I also like this a lot, but this assumes that firmware has correct > settings for the interrupt... Unfortunately it is not always the case > and I see that at least Chrome OS devices, such as glados line (cave, chell, sentry, > ect) do not mark interrupt as wakeup: > > src/mainboard/google/glados/variants/chell/overridetree.cb > > chip drivers/i2c/generic > register "hid" = ""ELAN0000"" > register "desc" = ""ELAN Touchpad"" > register "irq" = "ACPI_IRQ_LEVEL_LOW(GPP_B3_IRQ)" > register "wake" = "GPE0_DW0_05" > device i2c 15 on end > > I assume it should have been ACPI_IRQ_WAKE_LEVEL_LOW for the interrupt > to be marked as wakeup. > > (we do correctly mark GPE as wakeup). > > So we need to do something about older devices.... After re-reading the patch I believe this comment is more applicable to the followup patch to elan_i2c, not this one, which is fine on its own. Acked-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> Thanks. -- Dmitry
