Re: [PATCH] i2c: ACPI: Do not check ACPI_FADT_LOW_POWER_S0

I do plan on coming back and updating those patches. I got derailed
with other priorities. But as Hans pointed out, we wanted to use
`ExclusiveAndWake` to make the decision since not all IRQs can be wake
sources while in s0i3.


On Fri, Aug 5, 2022 at 12:54 PM Hans de Goede [off-list ref] wrote:

Hi,

On 8/5/22 19:08, Rafael J. Wysocki wrote:

quoted

On Fri, Aug 5, 2022 at 6:59 PM Limonciello, Mario
[off-list ref] wrote:

quoted

On 8/5/2022 11:51, Rafael J. Wysocki wrote:

quoted

From: Rafael J. Wysocki <redacted>

The ACPI_FADT_LOW_POWER_S0 flag merely means that it is better to
use low-power S0 idle on the given platform than S3 (provided that
the latter is supported) and it doesn't preclude using either of
them (which of them will be used depends on the choices made by user
space).

Because of that, ACPI_FADT_LOW_POWER_S0 is generally not sufficient
for making decisions in device drivers and so i2c_hid_acpi_probe()
should not use it.

Moreover, Linux always supports suspend-to-idle, so if a given
device can wake up the system from suspend-to-idle, then it can be
marked as wakeup capable unconditionally, so make that happen in
i2c_hid_acpi_probe().

Signed-off-by: Rafael J. Wysocki <redacted>

+Raul
+Hans
+KH

Raul had a patch that was actually going to just tear out this code
entirely:
https://lkml.kernel.org/lkml/20211220163823.1.Ie20ca47a26d3ea68124d8197b67bb1344c67f650@changeid/

As part of that patch series discussion another suggestion had
transpired
(https://patchwork.kernel.org/project/linux-input/patch/20211220163823.2.Id022caf53d01112188308520915798f08a33cd3e@changeid/#24681016):

if ((acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) &&
            !adev->flags.power_manageable) {
                 device_set_wakeup_capable(dev, true);
                 device_set_wakeup_enable(dev, false);
         }

If this is being changed, maybe consider that suggestion to
check `adev->flags.power_manageable`.

Fair enough, I'll send a v2 with this check added.

Re-reading the original thread:
https://lkml.kernel.org/lkml/20211220163823.1.Ie20ca47a26d3ea68124d8197b67bb1344c67f650@changeid/T/#u

The conclusion there was that the :

                 device_set_wakeup_capable(dev, true);
                 device_set_wakeup_enable(dev, false);

Calls should be made conditional on the IRQ being
marked ExclusiveAndWake instead of the ACPI_FADT_LOW_POWER_S0
check.

Regards,

Hans