Re: [RFC] i2c: core: Do not enable wakeup by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/8/2023 10:29 AM, Mika Westerberg wrote:
Hi,

On Wed, Feb 08, 2023 at 09:28:14AM +0100, Amadeusz Sławiński wrote:
On 2/8/2023 7:57 AM, Mika Westerberg wrote:
Hi,

On Tue, Feb 07, 2023 at 09:33:55AM -0700, Raul Rangel wrote:
Sorry, resending in plain text mode.

On Tue, Feb 7, 2023 at 12:25 AM Mika Westerberg
<mika.westerberg@xxxxxxxxxxxxxxx> 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.

That's generally a bug in the ACPI tables. The wake bit shouldn't be
set if the power domain for the device is powered off on suspend. The
best thing is to fix the ACPI tables, but if you can't, then you can
set the ignore_wake flag for the device:
https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L31.
If that works we can add a quirk for the device:
https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1633.

I've seen this one already and also tried to use it, but it didn't work.
Also when I was reading code I wasn't really convinced that it is linked to
i2c in any straightforward way. I mean i2c decides in different places that
it has wake support (I even added some prints to make sure ;). The code you
pointed out decides in https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L387
but i2c code seems to decide in https://github.com/torvalds/linux/blob/master/drivers/i2c/i2c-core-acpi.c#L176
where it just checks if irq flags has wake_capable flag set. When I looked
at it previously I was pretty sure it comes straight from BIOS and passes
the quirk code you mentioned, still I may have missed something.


I think (hope) these systems are not yet available for public so there
is a chance that the tables can still be fixed, without need to add any
quirks.

@Amadeusz, @Cezary, if that's the case I suggest filing a bug against
the BIOS.


Well, I tried custom DSDT and had problems, but I just remembered that I
probably need to pass "revision+1" in file, so kernel sees it as a newer
version, let me try again. Is it enough to replace "ExclusiveAndWake" with
"Exclusive"?

Yes, I think that should be enough.


And yes, it seems to work when I bump revision. So will use it as workaround for now and see about fixing BIOS.




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux