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

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

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@linux.intel.com> 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.