Hi David,
Thank you for your patch.
I became aware of this patch because the discussion on the regressions
list. Next time for patches to files under drivers/platform/x86/
at a minimum please Cc: platform-driver-x86@xxxxxxxxxxxxxxx and
preferably also send the patch directly to me and Ilpo as shown
by get_maintainer.pl:
[hans@shalem linux]$ scripts/get_maintainer.pl -f drivers/platform/x86/intel/vbtn.c
AceLan Kao <acelan.kao@xxxxxxxxxxxxx> (maintainer:INTEL VIRTUAL BUTTON DRIVER)
Hans de Goede <hdegoede@xxxxxxxxxx> (maintainer:X86 PLATFORM DRIVERS)
"Ilpo Järvinen" <ilpo.jarvinen@xxxxxxxxxxxxxxx> (maintainer:X86 PLATFORM DRIVERS)
platform-driver-x86@xxxxxxxxxxxxxxx (open list:INTEL VIRTUAL BUTTON DRIVER)
linux-kernel@xxxxxxxxxxxxxxx (open list)
Please make sure you are sending this to the right people for v2.
On 3/18/24 8:11 PM, 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.
>
> 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.
>
As discussed by Thorsten this needs a fixes tag, to help with backporting
it to relevant stable kernels. Also for v2 please don't forget to add
Enrik's Tested-by from elsewhere in thread:
Tested-by: Enrik Berkhan <Enrik.Berkhan@xxxxxxx>
> Signed-off-by: David McFarland <corngood@xxxxxxxxx>
> ---
> 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);
> +
I would prefer for there to be no empty line between the "ke =" assignment
and the "if (!ke)".
Otherwise the patch looks good to me, so for v3
you can add:
Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
After fixing this + adding the Fixes and Tested-by tags.
Regards,
Hans
> + 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);
>
