| Message ID | 1512618023-30209-1-git-send-email-alex.hung@canonical.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Thu, Dec 07, 2017 at 11:40:23AM +0800, Alex Hung wrote: > HEBC method reports capabilities of 5 button array but Wacom > MobileStudio Pro does not have this control method. A DMI quirk > was created to enable 5 button array for this system. Jason, have you been able to verify this patch with testing? > [V2] Update DMI strings from shipping systems Please place patch versioning detail below the --- in the future > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197991 > > Reported-by: Jason Gerecke <jason.gerecke@wacom.com> > Signed-off-by: Alex Hung <alex.hung@canonical.com> > --- here (see submitting patches docs for details)
On Fri, Dec 8, 2017 at 4:04 PM, Darren Hart <dvhart@infradead.org> wrote: > On Thu, Dec 07, 2017 at 11:40:23AM +0800, Alex Hung wrote: >> HEBC method reports capabilities of 5 button array but Wacom >> MobileStudio Pro does not have this control method. A DMI quirk >> was created to enable 5 button array for this system. > > Jason, have you been able to verify this patch with testing? > I tested the attachment at [1], which produces an identical copy of intel-hid.c as applying this patch to the platform-drivers-x86/review-dvhart branch of git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git. Assuming that's the correct code, feel free to attach the following: Tested-by: Jason Gerecke <jason.gerecke@wacom.com> [1]: https://bugzilla.kernel.org/attachment.cgi?id=261033 Jason --- Now instead of four in the eights place / you’ve got three, ‘Cause you added one / (That is to say, eight) to the two, / But you can’t take seven from three, / So you look at the sixty-fours.... >> [V2] Update DMI strings from shipping systems > > Please place patch versioning detail below the --- in the future > >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197991 >> >> Reported-by: Jason Gerecke <jason.gerecke@wacom.com> >> Signed-off-by: Alex Hung <alex.hung@canonical.com> >> --- > > here (see submitting patches docs for details) > > > -- > Darren Hart > VMware Open Source Technology Center
On Mon, Dec 11, 2017 at 02:13:40PM -0800, Jason Gerecke wrote: > On Fri, Dec 8, 2017 at 4:04 PM, Darren Hart <dvhart@infradead.org> wrote: > > On Thu, Dec 07, 2017 at 11:40:23AM +0800, Alex Hung wrote: > >> HEBC method reports capabilities of 5 button array but Wacom > >> MobileStudio Pro does not have this control method. A DMI quirk > >> was created to enable 5 button array for this system. > > > > Jason, have you been able to verify this patch with testing? > > > > I tested the attachment at [1], which produces an identical copy of > intel-hid.c as applying this patch to the > platform-drivers-x86/review-dvhart branch of > git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git. > > Assuming that's the correct code, feel free to attach the following: > Tested-by: Jason Gerecke <jason.gerecke@wacom.com> Great, thanks. Queued up for testing.
On Tue, Dec 12, 2017 at 2:51 AM, Darren Hart <dvhart@infradead.org> wrote: > On Mon, Dec 11, 2017 at 02:13:40PM -0800, Jason Gerecke wrote: >> On Fri, Dec 8, 2017 at 4:04 PM, Darren Hart <dvhart@infradead.org> wrote: >> > On Thu, Dec 07, 2017 at 11:40:23AM +0800, Alex Hung wrote: >> >> HEBC method reports capabilities of 5 button array but Wacom >> >> MobileStudio Pro does not have this control method. A DMI quirk >> >> was created to enable 5 button array for this system. >> > >> > Jason, have you been able to verify this patch with testing? >> > >> >> I tested the attachment at [1], which produces an identical copy of >> intel-hid.c as applying this patch to the >> platform-drivers-x86/review-dvhart branch of >> git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git. >> >> Assuming that's the correct code, feel free to attach the following: >> Tested-by: Jason Gerecke <jason.gerecke@wacom.com> > > Great, thanks. Queued up for testing. Just in case you didn't promote it yet, could you move inclusion to be more ordered alphabetically? (After linux/acpi.h I suppose)
On Tue, Dec 12, 2017 at 10:02:36AM +0200, Andy Shevchenko wrote: > On Tue, Dec 12, 2017 at 2:51 AM, Darren Hart <dvhart@infradead.org> wrote: > > On Mon, Dec 11, 2017 at 02:13:40PM -0800, Jason Gerecke wrote: > >> On Fri, Dec 8, 2017 at 4:04 PM, Darren Hart <dvhart@infradead.org> wrote: > >> > On Thu, Dec 07, 2017 at 11:40:23AM +0800, Alex Hung wrote: > >> >> HEBC method reports capabilities of 5 button array but Wacom > >> >> MobileStudio Pro does not have this control method. A DMI quirk > >> >> was created to enable 5 button array for this system. > >> > > >> > Jason, have you been able to verify this patch with testing? > >> > > >> > >> I tested the attachment at [1], which produces an identical copy of > >> intel-hid.c as applying this patch to the > >> platform-drivers-x86/review-dvhart branch of > >> git://git.infradead.org/users/dvhart/linux-platform-drivers-x86.git. > >> > >> Assuming that's the correct code, feel free to attach the following: > >> Tested-by: Jason Gerecke <jason.gerecke@wacom.com> > > > > Great, thanks. Queued up for testing. > > Just in case you didn't promote it yet, could you move inclusion to be > more ordered alphabetically? (After linux/acpi.h I suppose) I had done that initially, but realized the entire list is in no apparent order and it wasn't just moving the one line he added. So it'll need to be a cleanup patch since all the test builds have run already.
On Tue, Dec 12, 2017 at 6:00 PM, Darren Hart <dvhart@infradead.org> wrote: > On Tue, Dec 12, 2017 at 10:02:36AM +0200, Andy Shevchenko wrote: >> Just in case you didn't promote it yet, could you move inclusion to be >> more ordered alphabetically? (After linux/acpi.h I suppose) > > I had done that initially, but realized the entire list is in no apparent order > and it wasn't just moving the one line he added. So it'll need to be a cleanup > patch since all the test builds have run already. Ah, fair enough.
On Wed, Dec 13, 2017 at 12:13 AM, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Dec 12, 2017 at 6:00 PM, Darren Hart <dvhart@infradead.org> wrote: >> On Tue, Dec 12, 2017 at 10:02:36AM +0200, Andy Shevchenko wrote: > >>> Just in case you didn't promote it yet, could you move inclusion to be >>> more ordered alphabetically? (After linux/acpi.h I suppose) >> >> I had done that initially, but realized the entire list is in no apparent order >> and it wasn't just moving the one line he added. So it'll need to be a cleanup >> patch since all the test builds have run already. > > Ah, fair enough. Do you mean the includes listed alphabetically such as below? I can send a patch to clean it up. #include <linux/acpi.h> #include <acpi/acpi_bus.h> #include <linux/dmi.h> #include <linux/init.h> #include <linux/input.h> #include <linux/input/sparse-keymap.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/suspend.h> > > -- > With Best Regards, > Andy Shevchenko
On Thu, Dec 14, 2017 at 9:25 AM, Alex Hung <alex.hung@canonical.com> wrote: > On Wed, Dec 13, 2017 at 12:13 AM, Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: >> On Tue, Dec 12, 2017 at 6:00 PM, Darren Hart <dvhart@infradead.org> wrote: >>> On Tue, Dec 12, 2017 at 10:02:36AM +0200, Andy Shevchenko wrote: >> >>>> Just in case you didn't promote it yet, could you move inclusion to be >>>> more ordered alphabetically? (After linux/acpi.h I suppose) >>> >>> I had done that initially, but realized the entire list is in no apparent order >>> and it wasn't just moving the one line he added. So it'll need to be a cleanup >>> patch since all the test builds have run already. >> >> Ah, fair enough. > > Do you mean the includes listed alphabetically such as below? Yes. > I can > send a patch to clean it up. As a standalone change it would not be required, but if you are going to send some behaviour changing or bigger clean up series, feel free to add. > #include <linux/acpi.h> > #include <acpi/acpi_bus.h> > #include <linux/dmi.h> > #include <linux/init.h> > #include <linux/input.h> > #include <linux/input/sparse-keymap.h> > #include <linux/kernel.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/suspend.h>
diff --git a/drivers/platform/x86/intel-hid.c b/drivers/platform/x86/intel-hid.c index f470279..d1a0131 100644 --- a/drivers/platform/x86/intel-hid.c +++ b/drivers/platform/x86/intel-hid.c @@ -25,6 +25,7 @@ #include <linux/acpi.h> #include <linux/suspend.h> #include <acpi/acpi_bus.h> +#include <linux/dmi.h> MODULE_LICENSE("GPL"); MODULE_AUTHOR("Alex Hung"); @@ -73,6 +74,24 @@ static const struct key_entry intel_array_keymap[] = { { KE_END }, }; +static const struct dmi_system_id button_array_table[] = { + { + .ident = "Wacom MobileStudio Pro 13", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Wacom Co.,Ltd"), + DMI_MATCH(DMI_PRODUCT_NAME, "Wacom MobileStudio Pro 13"), + }, + }, + { + .ident = "Wacom MobileStudio Pro 16", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "Wacom Co.,Ltd"), + DMI_MATCH(DMI_PRODUCT_NAME, "Wacom MobileStudio Pro 16"), + }, + }, + { } +}; + struct intel_hid_priv { struct input_dev *input_dev; struct input_dev *array; @@ -263,10 +282,27 @@ static void notify_handler(acpi_handle handle, u32 event, void *context) ev_index); } +static bool button_array_present(struct platform_device *device) +{ + acpi_handle handle = ACPI_HANDLE(&device->dev); + unsigned long long event_cap; + acpi_status status; + bool supported = false; + + status = acpi_evaluate_integer(handle, "HEBC", NULL, &event_cap); + if (ACPI_SUCCESS(status) && (event_cap & 0x20000)) + supported = true; + + if (dmi_check_system(button_array_table)) + supported = true; + + return supported; +} + static int intel_hid_probe(struct platform_device *device) { acpi_handle handle = ACPI_HANDLE(&device->dev); - unsigned long long event_cap, mode; + unsigned long long mode; struct intel_hid_priv *priv; acpi_status status; int err; @@ -299,8 +335,7 @@ static int intel_hid_probe(struct platform_device *device) } /* Setup 5 button array */ - status = acpi_evaluate_integer(handle, "HEBC", NULL, &event_cap); - if (ACPI_SUCCESS(status) && (event_cap & 0x20000)) { + if (button_array_present(device)) { dev_info(&device->dev, "platform supports 5 button array\n"); err = intel_button_array_input_setup(device); if (err)
HEBC method reports capabilities of 5 button array but Wacom MobileStudio Pro does not have this control method. A DMI quirk was created to enable 5 button array for this system. [V2] Update DMI strings from shipping systems Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197991 Reported-by: Jason Gerecke <jason.gerecke@wacom.com> Signed-off-by: Alex Hung <alex.hung@canonical.com> --- drivers/platform/x86/intel-hid.c | 41 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-)