| Message ID | 20220929093200.v6.9.I2efb7f551e0aa2dc4c53b5fd5bbea91a1cdd9b32@changeid (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1796f808e4bb2c074824dc32258ed1e719370cb3 |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | acpi: i2c: Use SharedAndWake and ExclusiveAndWake to enable wake irq | expand |
On Thu, Sep 29, 2022 at 10:19:13AM -0600, Raul E Rangel wrote:
> This is now handled by the i2c-core driver.
What happened to this patch? I don't see it in the Linux Next...
On Mon, Nov 21, 2022 at 7:49 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Sep 29, 2022 at 10:19:13AM -0600, Raul E Rangel wrote: > > This is now handled by the i2c-core driver. > > What happened to this patch? I don't see it in the Linux Next... > This was just merged into dtor's next branch a few days ago: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/commit/?id=1796f808e4bb2c074824dc32258ed1e719370cb3 > -- > With Best Regards, > Andy Shevchenko > >
Hi, Am 29.09.22 um 18:19 schrieb Raul E Rangel: > This is now handled by the i2c-core drive > > Signed-off-by: Raul E Rangel <rrangel@chromium.org> > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > > (no changes since v5) > > Changes in v5: > - Added Acked-by: Benjamin Tissoires > > drivers/hid/i2c-hid/i2c-hid-acpi.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c > index b96ae15e0ad917e..375c77c3db74d92 100644 > --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c > +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c > @@ -105,11 +105,6 @@ static int i2c_hid_acpi_probe(struct i2c_client *client) > > acpi_device_fix_up_power(adev); > > - if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) { > - device_set_wakeup_capable(dev, true); > - device_set_wakeup_enable(dev, false); > - } > - > return i2c_hid_core_probe(client, &ihid_acpi->ops, > hid_descriptor_address, 0); > } this patch is causing a regression on the Clevo NL50RU of which the touchpad instantly wakes up the device when going to sleep. That wasn't triggered until this patch by the default settings: Setting wake capable but not enabling it by default. So unless a user enabled it by hand, the device went correctly to sleep. I'm not deep into this subsystem so I don't know what the best approach is to ?work around this firmware bug?/?fix this issue?: - Changing the default back again? - Adding a quirk list for bad devices? - Maybe this isn't a firmware bug, but the touchpad was not meant to wakeup the device and we can somehow detect that? For reference: The debugging issue that lead me here: https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1719789 Kind regards, Werner
On 1/12/2023 13:57, Werner Sembach wrote: > Hi, > > Am 29.09.22 um 18:19 schrieb Raul E Rangel: >> This is now handled by the i2c-core drive > >> Signed-off-by: Raul E Rangel <rrangel@chromium.org> >> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> >> (no changes since v5) >> >> Changes in v5: >> - Added Acked-by: Benjamin Tissoires >> >> drivers/hid/i2c-hid/i2c-hid-acpi.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c >> b/drivers/hid/i2c-hid/i2c-hid-acpi.c >> index b96ae15e0ad917e..375c77c3db74d92 100644 >> --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c >> +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c >> @@ -105,11 +105,6 @@ static int i2c_hid_acpi_probe(struct i2c_client >> *client) >> acpi_device_fix_up_power(adev); >> - if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) { >> - device_set_wakeup_capable(dev, true); >> - device_set_wakeup_enable(dev, false); >> - } >> - >> return i2c_hid_core_probe(client, &ihid_acpi->ops, >> hid_descriptor_address, 0); >> } > > this patch is causing a regression on the Clevo NL50RU of which the > touchpad instantly wakes up the device when going to sleep. That wasn't > triggered until this patch by the default settings: Setting wake capable > but not enabling it by default. So unless a user enabled it by hand, the > device went correctly to sleep. > > I'm not deep into this subsystem so I don't know what the best approach > is to ?work around this firmware bug?/?fix this issue?: > - Changing the default back again? > - Adding a quirk list for bad devices? > - Maybe this isn't a firmware bug, but the touchpad was not meant to > wakeup the device and we can somehow detect that? > "Generally" laptops that support modern standby/s2idle can be woken from the touchpad. By chance can you check Windows? Wait a few minutes after you put it to sleep to ensure it's really gotten down to a hardware sleep state. > For reference: The debugging issue that lead me here: > https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1719789 TLDR - That bug has multiple conflated issues, and you can ignore 95% of it. It's just the last few comments we concluded and bisected down this regression.
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c index b96ae15e0ad917e..375c77c3db74d92 100644 --- a/drivers/hid/i2c-hid/i2c-hid-acpi.c +++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c @@ -105,11 +105,6 @@ static int i2c_hid_acpi_probe(struct i2c_client *client) acpi_device_fix_up_power(adev); - if (acpi_gbl_FADT.flags & ACPI_FADT_LOW_POWER_S0) { - device_set_wakeup_capable(dev, true); - device_set_wakeup_enable(dev, false); - } - return i2c_hid_core_probe(client, &ihid_acpi->ops, hid_descriptor_address, 0); }