Re: [PATCH] Make power-button key report the button-up event when the 5-button array does not exist

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 23, 2018 at 7:36 AM, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
> On Sun, Apr 22, 2018 at 1:25 AM, Tristian Celestin
> <tristiancelestin@xxxxxxxxxxxx> wrote:
>
> Thanks for the patch.
>
> First of all, please, include all PDx86 maintainers to the discussion as well.
> Second, please, use `git send-email` tool to send patches, it avoids
> attachments.
>
>> I am running Fedora 28 and Android-x86 on a Dell Latitude 5175 tablet. The
>> power button functionality is driven by the intel-hid driver. I am using
>> kernel version 4.16.
>>
>> Currently, the intel-hid driver does not supply a KEY_POWER up event in cases
>> where the platform doesn't expose the 5-button array. Without this patch, the
>> power button can't reliably respond when the platform is running Android.
>>
>> When running Fedora, I can use the power button to suspend and resume the
>> tablet. I can initiate this suspend by short-pressing the power button for a
>> second, and can resume it using another short-press.
>>
>> When running Android-x86, I can only short-press the power button once. After
>> the press, the button seems to no longer respond. This is problematic when
>> using a short-press to initiate a suspend, since a subsequent short press will
>> not wake the tablet.
>>
>> I used getevent to display the KeyEvents[1] detected by Android, and a
>> combination of 'cat /proc/kmsg' and debug statements in the intel-hid driver
>> to display the events generated by the driver. I found the block in the intel-
>> hid driver that generates power button events for my device. On line 253 of
>> intel-hid.c:
>>
>>         if (!priv->array) {
>>                 if (event == 0xce) {
>>                         input_report_key(priv->input_dev, KEY_POWER, 1);
>>                         input_sync(priv->input_dev);
>>                         return;
>>                 }
>>
>>                 if (event == 0xcf)
>>                         return;
>> }

Thanks for the work. This somehow sounds similar to Wacom MobileStudio
Pro that we worked on before. A quirk was added to enable 5 button
array, and the commit is c454a99d4ce1cebb.

Could you please try to add a DMI entry in button_array_table[] and
verify the power button again? If this works, we can use the DMI quirk
instead.

>>
>> When I short-press the power button, intel-hid produces a KEY_POWER down
>> event, but doesn't produce a KEY_POWER up event when I release the power
>> button. Suppose intel-hid has been mapped to the input device /dev/input/
>> event19. Then, on Android-x86, the command "getevent -lt" produces the
>> following output:
>>
>> /dev/input/event19: EV_KEY       KEY_POWER            DOWN
>> /dev/input/event19: EV_SYN       SYN_REPORT           00000000
>>
>> Subsequent presses produced no output for that input device.
>>
>> When I added a call to input_report_key(...) and input_sync(...) on the
>> KEY_POWER up event in the intel-hid driver, I could repeatedly short-press the
>> power button and have Android respond appropriately, including resuming the
>> device from suspend. My hunch as to why this is the case is that Android needs
>> a paired KEY_POWER DOWN and UP event before it will handle the press.
>
> WRT, patch contents:
> - please, do a proper commit message
> - while it has crucial semantic mistake (missing {}) it suddenly works
> because nothing behind the condition you had touched
> - I would rather unify conditionals, though I would like to hear from
> Alex and Dmitry if it's fine to do what you are trying to do in the
> patch
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
Cheers,
Alex Hung



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux