Re: [PATCH] platform/x86: intel-hid: Wake up Dell Latitude 7275 from suspend-to-idle

[Andy Shevchenko]

On Tue, Aug 1, 2017 at 12:46 AM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> On Friday, July 28, 2017 02:06:36 AM Rafael J. Wysocki wrote:
>> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>>
>> On Dell Latitude 7275 the 5-button array is not exposed in the
>> ACPI tables, but still notifies are sent to the Intel HID device
>> object (device ID INT33D5) in response to power button actions while
>> suspended to idle.  However, they are currently ignored as the
>> intel-hid driver is not prepared to take care of them.
>>
>> As a result, power button wakeup from suspend-to-idle doesn't work
>> on this platform, but suspend-to-idle is the only reliable suspend
>> variant on it (the S3 implementation in the platform firmware turns
>> out to be broken), so it would be good to handle it properly.
>>
>> For this reason, add an upfront check against the power button press
>> event (0xCE) to notify_handler() in the wakeup mode which allows it
>> to catch the power button wakeup notification on the affected
>> platform (even though priv->array is NULL on it) and should not
>> change the behavior on platforms with priv->array present (because
>> priv->array contains the event in question in those cases).
>>
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196115
>> Tested-by: JÃrÃme de Bretagne <jerome.debretagne@xxxxxxxxx>
>> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> Please note that this change is requisite for
>
> https://patchwork.kernel.org/patch/9873159/
>
> so are there any objections or concerns?

Not from my side,

Acked-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx>

>
>> ---
>>  drivers/platform/x86/intel-hid.c |   17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> Index: linux-pm/drivers/platform/x86/intel-hid.c
>> ===================================================================
>> --- linux-pm.orig/drivers/platform/x86/intel-hid.c
>> +++ linux-pm/drivers/platform/x86/intel-hid.c
>> @@ -203,15 +203,26 @@ static void notify_handler(acpi_handle h
>>       acpi_status status;
>>
>>       if (priv->wakeup_mode) {
>> +             /*
>> +              * Needed for wakeup from suspend-to-idle to work on some
>> +              * platforms that don't expose the 5-button array, but still
>> +              * send notifies with the power button event code to this
>> +              * device object on power button actions while suspended.
>> +              */
>> +             if (event == 0xce)
>> +                     goto wakeup;
>> +
>>               /* Wake up on 5-button array events only. */
>>               if (event == 0xc0 || !priv->array)
>>                       return;
>>
>> -             if (sparse_keymap_entry_from_scancode(priv->array, event))
>> -                     pm_wakeup_hard_event(&device->dev);
>> -             else
>> +             if (!sparse_keymap_entry_from_scancode(priv->array, event)) {
>>                       dev_info(&device->dev, "unknown event 0x%x\n", event);
>> +                     return;
>> +             }
>>
>> +wakeup:
>> +             pm_wakeup_hard_event(&device->dev);
>>               return;
>>       }
>>
>>
>



-- 
With Best Regards,
Andy Shevchenko