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