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 30, 2018 at 8:20 AM Alex Hung <alex.hung@xxxxxxxxxxxxx> wrote:
>
> On Sun, Apr 29, 2018 at 9:35 PM, Tristian Celestin
> <tristiancelestin@xxxxxxxxxxxx> wrote:
> > I have a patch ready, but I don't know the underlying cause of the problem, and this is preventing from writing a meaningful commit message.
>
> According to my understanding, the original intel-hid spec supported
> 0xC0 notification and events are reported by HDEM method, and an
> update of "5 button array" added other notification numbers such as
> 0xce for power button; however, some BIOS failed to report 5 button
> array is supported via HEBC method, and the DMI quirk was a workaround
> to always enable 5 button array.
>
> I personally think a commit message similar to c454a99d4ce1cebb is
> good enough, but Andy or Darren will provide more feedbacks if they
> think a refinement is necessary.
>

This is still in the patchwork queue by some reason.
Can you elaborate if this needed or not? If so, would it be
anticipated a new version?

> >
> >
> > On Sun, Apr 29, 2018, at 8:45 PM, Tristian Celestin wrote:
> >>
> >>
> >>
> >> On Mon, Apr 23, 2018, at 8:55 PM, Alex Hung wrote:
> >>> 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.
> >>
> >> Thank you for the guidance. Will do.
> >>
> >>>>> 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.
> >>
> >> Thank you for the guidance. I added a DMI entry to button_array_table[] for the Latitude 5175, and the
> >> tablet now also responds to short presses while suspended.
> >>
> >>>>>
> >>>>> 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
>
>
>
> --
> Cheers,
> Alex Hung



-- 
With Best Regards,
Andy Shevchenko



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

  Powered by Linux