On Tue, Feb 07, 2023 at 09:25:40AM +0200, Mika Westerberg wrote:
> After commit b38f2d5d9615 ("i2c: acpi: Use ACPI wake capability bit to
> set wake_irq") the I2C core has been setting I2C_CLIENT_WAKE for ACPI
> devices if they announce to be wake capable in their device description.
> However, on certain systems where audio codec has been connected through
> I2C this causes system suspend to wake up immediately because power to
> the codec is turned off which pulls the interrupt line "low" triggering
> wake up.
>
> Possible reason why the interrupt is marked as wake capable is that some
> codecs apparently support "Wake on Voice" or similar functionality.
>
> In any case, I don't think we should be enabling wakeup by default on
> all I2C devices that are wake capable. According to device_init_wakeup()
> documentation most devices should leave it disabled, with exceptions on
> devices such as keyboards, power buttons etc. Userspace can enable
> wakeup as needed by writing to device "power/wakeup" attribute.
I agree on the reasoning.
Should we have a Fixes tag?
Otherwise
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
In any case it would be nice if the initial contributors may have a chance
to test this and see how their setup is supposed to work.
> Reported-by: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> ---
> Hi,
>
> Sending this as RFC because I'm not too familiar with the usage of
> I2C_CLIENT_WAKE and whether this is something that is expected behaviour
> in users of I2C devices. On ACPI side I think this is the correct thing
> to do at least.
>
> drivers/i2c/i2c-core-base.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 087e480b624c..7046549bdae7 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -527,7 +527,7 @@ static int i2c_device_probe(struct device *dev)
> goto put_sync_adapter;
> }
>
> - device_init_wakeup(&client->dev, true);
> + device_init_wakeup(&client->dev, false);
>
> if (wakeirq > 0 && wakeirq != client->irq)
> status = dev_pm_set_dedicated_wake_irq(dev, wakeirq);
> --
> 2.39.1
>
--
With Best Regards,
Andy Shevchenko
