[CCing Chris, who authored the culprit; also CCing the platform folks
and a few lists]
On 18.03.24 20:11, David McFarland wrote:
> If, for example, the power button is configured to suspend, holding it
> and releasing it after the machine has suspended, will wake the machine.
David, from here is looks like this is stalled for ten days now. Or was
there some progress and I just missed it?
> Also on some machines, power button release events are sent during
> hibernation, even if the button wasn't used to hibernate the machine.
> This causes hibernation to be aborted.
From the cover letter[1] is sounds a lot like a "Fixes: 0c4cae1bc00d31
("PM: hibernate: Avoid missing wakeup events during hibernation")" would
be appropriate here.
Regarding the patch itself: hopefully this mail will get things moving.
[1]
https://lore.kernel.org/linux-pm/[email protected]/
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
> Signed-off-by: David McFarland <[email protected]>
>> ---
> drivers/platform/x86/intel/hid.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
> index 7457ca2b27a6..707de9895965 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -504,6 +504,7 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
> struct platform_device *device = context;
> struct intel_hid_priv *priv = dev_get_drvdata(&device->dev);
> unsigned long long ev_index;
> + struct key_entry *ke;
> int err;
>
> /*
> @@ -545,11 +546,16 @@ static void notify_handler(acpi_handle handle, u32 event, void *context)
> if (event == 0xc0 || !priv->array)
> return;
>
> - if (!sparse_keymap_entry_from_scancode(priv->array, event)) {
> + ke = sparse_keymap_entry_from_scancode(priv->array, event);
> +
> + if (!ke) {
> dev_info(&device->dev, "unknown event 0x%x\n", event);
> return;
> }
>
> + if (ke->type == KE_IGNORE)
> + return;
> +
> wakeup:
> pm_wakeup_hard_event(&device->dev);
>
"Linux regression tracking (Thorsten Leemhuis)"
<[email protected]> writes:
> David, from here is looks like this is stalled for ten days now. Or was
> there some progress and I just missed it?
No, I've not seen any emails since your last.
> From the cover letter[1] is sounds a lot like a "Fixes: 0c4cae1bc00d31
> ("PM: hibernate: Avoid missing wakeup events during hibernation")" would
> be appropriate here.
The specific behaviour I encountered (failure to hibernate) started with
that commit, but I think it just exposed the underlying behaviour (wake
on button release), which probably dates to when the driver was
introduced.
I believe it would have been possible to reproduce the other behaviour I
mentioned (long hold of button to suspend causes the machine to wake on
release), even before 0c4cae1bc00d31.
That's why I left it off, but I'm happy to revise.
On 29.03.24 19:06, David McFarland wrote:
> "Linux regression tracking (Thorsten Leemhuis)"
> <[email protected]> writes:
>
>> David, from here is looks like this is stalled for ten days now. Or was
>> there some progress and I just missed it?
> No, I've not seen any emails since your last.
Thx for confirming.
>> From the cover letter[1] is sounds a lot like a "Fixes: 0c4cae1bc00d31
>> ("PM: hibernate: Avoid missing wakeup events during hibernation")" would
>> be appropriate here.
>
> The specific behaviour I encountered (failure to hibernate) started with
> that commit, but I think it just exposed the underlying behaviour (wake
> on button release), which probably dates to when the driver was
> introduced.
Well, it depends on the maintainer in question (so you might better want
to ignore this advice!), but I'd say: mention that in the patch
description and add Fixes: tag, to ensure people pick it up when the
change that exposed the problem is backported.
This is hinted at in submitting-patches: "This tag also assists the
stable kernel team in determining which stable kernel versions should
receive your fix.". Maybe that text should mention scenario.
Ciao, Thorsten
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
On Fri, 2024-03-29 at 14:51 +0100, Linux regression tracking (Thorsten
Leemhuis) wrote:
> [CCing Chris, who authored the culprit; also CCing the platform folks
> and a few lists]
(Intentionally replying to the "wrong" e-mail to get the enlarged
recipient list for free ...)
>
> Regarding the patch itself: hopefully this mail will get things
> moving.
>
> [1]
> https://lore.kernel.org/linux-pm/[email protected]/
>
> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker'
> hat)
> --
> Everything you wanna know about Linux kernel regression tracking:
> https://linux-regtracking.leemhuis.info/about/#tldr
> If I did something stupid, please tell me, as explained on that page.
>
> #regzbot poke
>
> > Signed-off-by: David McFarland <[email protected]>
Tested-by: Enrik Berkhan <[email protected]>
> > > ---
> > drivers/platform/x86/intel/hid.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/x86/intel/hid.c
> > b/drivers/platform/x86/intel/hid.c
> > index 7457ca2b27a6..707de9895965 100644
> > --- a/drivers/platform/x86/intel/hid.c
> > +++ b/drivers/platform/x86/intel/hid.c
> > @@ -504,6 +504,7 @@ static void notify_handler(acpi_handle handle,
> > u32 event, void *context)
> > struct platform_device *device = context;
> > struct intel_hid_priv *priv = dev_get_drvdata(&device-
> > >dev);
> > unsigned long long ev_index;
> > + struct key_entry *ke;
> > int err;
> >
> > /*
> > @@ -545,11 +546,16 @@ static void notify_handler(acpi_handle
> > handle, u32 event, void *context)
> > if (event == 0xc0 || !priv->array)
> > return;
> >
> > - if (!sparse_keymap_entry_from_scancode(priv-
> > >array, event)) {
> > + ke = sparse_keymap_entry_from_scancode(priv-
> > >array, event);
> > +
> > + if (!ke) {
> > dev_info(&device->dev, "unknown event
> > 0x%x\n", event);
> > return;
> > }
> >
> > + if (ke->type == KE_IGNORE)
> > + return;
> > +
> > wakeup:
> > pm_wakeup_hard_event(&device->dev);
> >
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCAAdFiEEbLyQBMI0qAWiF6l31AjqSkbLPKQFAmYK1IQACgkQ1AjqSkbL
PKSUXwv/YWiU334Osus+niffw9gN94vCQid6OWuTIbX2zMNIunHmpHouMtfo6dpb
7Qsyaz0sxaUEclRPLf/bSAV8zWromM/4Xoa/DoBq+GPOLle5wTJAlislhZVfDZFX
FZ0EfUg+MvThqlJD8yhGNJWGVnSn92hDid7yy5Y/BSWTPCNhRxEsqy+d0pN82EnD
0mqWnpfG28uqK6dtOLnvbiOuWQhgKYerKFYHXw8FLxZV0eKb3+RqBFf6ZdsAjUEC
+d7G9Oq4pc9jxZNvOMop+z3Eskd/bD8Wi2aDtR1qcRENMaK0ucevTuOjGL8Gh80c
SbgKu2eU/qVlbnVrcaPgpB0CXXza7SYUynw5XWB8jBWOj6w2nxpnQPm5a0Z5aNYw
/kLuabhSommEmUIJtoOA2vgO8D0ACfF2n95cNXaJRiOhuag3SRoair50TOzmf09Z
4uco5iaWeIQwEpVFGdy+qh0V18kYclnNMZHC9bhO6heBDLB9Wy/iXDxko9tMXm/v
XnfAKKw9
=udJV
-----END PGP SIGNATURE-----