| Message ID | 1439911825-23536-1-git-send-email-yu.c.chen@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
On Tue, Aug 18, 2015 at 11:30:25PM +0800, Chen Yu wrote: > Since Surface Pro 3 does not follow the specs of "Windows ACPI Design > Guide for SoC Platform", code in drivers/input/misc/soc_array.c can > not detect these buttons on it. According to bios implementation, > Surface Pro 3 encapsulates these buttons in a device named "VGBI", > with _HID "MSHW0028". When any of the buttons is pressed, a specify > ACPI notification code for this button will be delivered to "VGBI". For > example, if power button is pressed down, ACPI notification code of 0xc6 > will be sent by Notify(VGBI, 0xc6). > > This patch leverages "VGBI" to distinguish different ACPI notification > code from Power button, Home button, Volume button, then dispatches these > code to input layer. Lid is already covered by acpi button driver, so > there's no need to rewrite. > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=84651 > Tested-by: Ethan Schoonover <es@ethanschoonover.com> > Tested-by: Peter Amidon <psa.pub.0@picnicpark.org> > Tested-by: Donavan Lance <tusklahoma@gmail.com> > Tested-by: Stephen Just <stephenjust@gmail.com> > Signed-off-by: Chen Yu <yu.c.chen@intel.com> Joe, you provided a lot of review, are you happy with this version? Rafael, any concerns over the justification to use ACPI instead of platform_driver/i2c_driver as described in the comment block below? Chen, a couple more nitpics below. No need to resend if Joe and Rafael have no objections. I'll correct and queue. For now, queued to testing. Thanks! > --- > v4: > - Add following code in driver's probe callback: > if (strncmp(acpi_device_bid(device), SURFACE_BUTTON_OBJ_NAME, > strlen(SURFACE_BUTTON_OBJ_NAME))) > return -ENODEV; > to make sure only device object name of 'VGBI' will load this driver. > Because it is reported that, Surface 3(no Pro) also has a device with > hid MSHW0028, but it is not a button device. > > v3: > - Revert handle_surface_button_notify and keep original > 'switch/case' in surface_button_notify. Add/fix some > comments for surface_button_notify. > > v2: > - Introduce MACRO handle_surface_button_notify to make > it pairing the PRESS and RELEASE cases, convert dev_info > to dev_info_ratelimited when in error condition. > > --- > MAINTAINERS | 5 + > drivers/platform/x86/Kconfig | 5 + > drivers/platform/x86/Makefile | 1 + > drivers/platform/x86/surfacepro3_button.c | 215 ++++++++++++++++++++++++++++++ > 4 files changed, 226 insertions(+) > create mode 100644 drivers/platform/x86/surfacepro3_button.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 569568f..eacaa41 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -6721,6 +6721,11 @@ T: git git://git.monstr.eu/linux-2.6-microblaze.git > S: Supported > F: arch/microblaze/ > > +MICROSOFT SURFACE PRO 3 BUTTON DRIVER > +M: Chen Yu <yu.c.chen@intel.com> It's typical to include the list here as well: L: platform-driver-x86@vger.kernel.org > +S: Supported > +F: drivers/platform/x86/surfacepro3_button.c Also, spaces should have been tabs to be consistent with existing whitespace usage in MAINTAINERS. Consider displaying whitespace in your editor if you don't already. > + > MICROTEK X6 SCANNER > M: Oliver Neukum <oliver@neukum.org> > S: Maintained > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig > index 6dc13e4..c69bb70 100644 > --- a/drivers/platform/x86/Kconfig > +++ b/drivers/platform/x86/Kconfig > @@ -919,4 +919,9 @@ config INTEL_PMC_IPC > The PMC is an ARC processor which defines IPC commands for communication > with other entities in the CPU. > > +config SURFACE_PRO3_BUTTON > + tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3 tablet" > + depends on ACPI && INPUT > + ---help--- > + This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet. > endif # X86_PLATFORM_DEVICES > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile > index dda95a9..ada5128 100644 > --- a/drivers/platform/x86/Makefile > +++ b/drivers/platform/x86/Makefile > @@ -60,3 +60,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o > obj-$(CONFIG_PVPANIC) += pvpanic.o > obj-$(CONFIG_ALIENWARE_WMI) += alienware-wmi.o > obj-$(CONFIG_INTEL_PMC_IPC) += intel_pmc_ipc.o > +obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o > diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c > new file mode 100644 > index 0000000..6c6f11c > --- /dev/null > +++ b/drivers/platform/x86/surfacepro3_button.c > @@ -0,0 +1,215 @@ > +/* > + * power/home/volume button support for > + * Microsoft Surface Pro 3 tablet. > + * > + * (C) Copyright 2015 Intel Corporation Intel standard copyright notice: Copyright (c) 2015, Intel Corporation. All rights reserved. But the (c) generally always goes after Copyright
On Wed, 2015-08-26 at 00:22 -0700, Darren Hart wrote: > On Tue, Aug 18, 2015 at 11:30:25PM +0800, Chen Yu wrote: > > Since Surface Pro 3 does not follow the specs of "Windows ACPI Design > > Guide for SoC Platform", code in drivers/input/misc/soc_array.c can > > not detect these buttons on it. According to bios implementation, > > Surface Pro 3 encapsulates these buttons in a device named "VGBI", > > with _HID "MSHW0028". When any of the buttons is pressed, a specify > > ACPI notification code for this button will be delivered to "VGBI". For > > example, if power button is pressed down, ACPI notification code of 0xc6 > > will be sent by Notify(VGBI, 0xc6). [] > Joe, you provided a lot of review, are you happy with this version? It looks fine. Thanks for picking the other little nits too Darren. -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Aug 26, 2015 at 05:04:01AM -0700, Joe Perches wrote: > On Wed, 2015-08-26 at 00:22 -0700, Darren Hart wrote: > > On Tue, Aug 18, 2015 at 11:30:25PM +0800, Chen Yu wrote: > > > Since Surface Pro 3 does not follow the specs of "Windows ACPI Design > > > Guide for SoC Platform", code in drivers/input/misc/soc_array.c can > > > not detect these buttons on it. According to bios implementation, > > > Surface Pro 3 encapsulates these buttons in a device named "VGBI", > > > with _HID "MSHW0028". When any of the buttons is pressed, a specify > > > ACPI notification code for this button will be delivered to "VGBI". For > > > example, if power button is pressed down, ACPI notification code of 0xc6 > > > will be sent by Notify(VGBI, 0xc6). > [] > > Joe, you provided a lot of review, are you happy with this version? > > It looks fine. > > Thanks for picking the other little nits too Darren. This is not queued for next.
On Fri, Aug 28, 2015 at 1:56 PM, Darren Hart <dvhart@infradead.org> wrote: > On Wed, Aug 26, 2015 at 05:04:01AM -0700, Joe Perches wrote: >> On Wed, 2015-08-26 at 00:22 -0700, Darren Hart wrote: >> > On Tue, Aug 18, 2015 at 11:30:25PM +0800, Chen Yu wrote: >> > > Since Surface Pro 3 does not follow the specs of "Windows ACPI Design >> > > Guide for SoC Platform", code in drivers/input/misc/soc_array.c can >> > > not detect these buttons on it. According to bios implementation, >> > > Surface Pro 3 encapsulates these buttons in a device named "VGBI", >> > > with _HID "MSHW0028". When any of the buttons is pressed, a specify >> > > ACPI notification code for this button will be delivered to "VGBI". For >> > > example, if power button is pressed down, ACPI notification code of 0xc6 >> > > will be sent by Notify(VGBI, 0xc6). >> [] >> > Joe, you provided a lot of review, are you happy with this version? >> >> It looks fine. >> >> Thanks for picking the other little nits too Darren. > > This is not queued for next. Not or now? If not, why not? josh -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Sep 01, 2015 at 01:30:27PM -0400, Josh Boyer wrote: > On Fri, Aug 28, 2015 at 1:56 PM, Darren Hart <dvhart@infradead.org> wrote: > > On Wed, Aug 26, 2015 at 05:04:01AM -0700, Joe Perches wrote: > >> On Wed, 2015-08-26 at 00:22 -0700, Darren Hart wrote: > >> > On Tue, Aug 18, 2015 at 11:30:25PM +0800, Chen Yu wrote: > >> > > Since Surface Pro 3 does not follow the specs of "Windows ACPI Design > >> > > Guide for SoC Platform", code in drivers/input/misc/soc_array.c can > >> > > not detect these buttons on it. According to bios implementation, > >> > > Surface Pro 3 encapsulates these buttons in a device named "VGBI", > >> > > with _HID "MSHW0028". When any of the buttons is pressed, a specify > >> > > ACPI notification code for this button will be delivered to "VGBI". For > >> > > example, if power button is pressed down, ACPI notification code of 0xc6 > >> > > will be sent by Notify(VGBI, 0xc6). > >> [] > >> > Joe, you provided a lot of review, are you happy with this version? > >> > >> It looks fine. > >> > >> Thanks for picking the other little nits too Darren. > > > > This is not queued for next. > > Not or now? If not, why not? Sorry, typo :-) It is queued for next, you'll find it in my "for-next" branch as well as in linux-next currently.
diff --git a/MAINTAINERS b/MAINTAINERS index 569568f..eacaa41 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6721,6 +6721,11 @@ T: git git://git.monstr.eu/linux-2.6-microblaze.git S: Supported F: arch/microblaze/ +MICROSOFT SURFACE PRO 3 BUTTON DRIVER +M: Chen Yu <yu.c.chen@intel.com> +S: Supported +F: drivers/platform/x86/surfacepro3_button.c + MICROTEK X6 SCANNER M: Oliver Neukum <oliver@neukum.org> S: Maintained diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 6dc13e4..c69bb70 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -919,4 +919,9 @@ config INTEL_PMC_IPC The PMC is an ARC processor which defines IPC commands for communication with other entities in the CPU. +config SURFACE_PRO3_BUTTON + tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3 tablet" + depends on ACPI && INPUT + ---help--- + This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet. endif # X86_PLATFORM_DEVICES diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile index dda95a9..ada5128 100644 --- a/drivers/platform/x86/Makefile +++ b/drivers/platform/x86/Makefile @@ -60,3 +60,4 @@ obj-$(CONFIG_INTEL_SMARTCONNECT) += intel-smartconnect.o obj-$(CONFIG_PVPANIC) += pvpanic.o obj-$(CONFIG_ALIENWARE_WMI) += alienware-wmi.o obj-$(CONFIG_INTEL_PMC_IPC) += intel_pmc_ipc.o +obj-$(CONFIG_SURFACE_PRO3_BUTTON) += surfacepro3_button.o diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c new file mode 100644 index 0000000..6c6f11c --- /dev/null +++ b/drivers/platform/x86/surfacepro3_button.c @@ -0,0 +1,215 @@ +/* + * power/home/volume button support for + * Microsoft Surface Pro 3 tablet. + * + * (C) Copyright 2015 Intel Corporation + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; version 2 + * of the License. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/types.h> +#include <linux/input.h> +#include <linux/acpi.h> +#include <acpi/button.h> + +#define SURFACE_BUTTON_HID "MSHW0028" +#define SURFACE_BUTTON_OBJ_NAME "VGBI" +#define SURFACE_BUTTON_DEVICE_NAME "Surface Pro 3 Buttons" + +#define SURFACE_BUTTON_NOTIFY_PRESS_POWER 0xc6 +#define SURFACE_BUTTON_NOTIFY_RELEASE_POWER 0xc7 + +#define SURFACE_BUTTON_NOTIFY_PRESS_HOME 0xc4 +#define SURFACE_BUTTON_NOTIFY_RELEASE_HOME 0xc5 + +#define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_UP 0xc0 +#define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_UP 0xc1 + +#define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN 0xc2 +#define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN 0xc3 + +ACPI_MODULE_NAME("surface pro 3 button"); + +MODULE_AUTHOR("Chen Yu"); +MODULE_DESCRIPTION("Surface Pro3 Button Driver"); +MODULE_LICENSE("GPL v2"); + +/* + * Power button, Home button, Volume buttons support is supposed to + * be covered by drivers/input/misc/soc_button_array.c, which is implemented + * according to "Windows ACPI Design Guide for SoC Platforms". + * However surface pro3 seems not to obey the specs, instead it uses + * device VGBI(MSHW0028) for dispatching the events. + * We choose acpi_driver rather than platform_driver/i2c_driver because + * although VGBI has an i2c resource connected to i2c controller, it + * is not embedded in any i2c controller's scope, thus neither platform_device + * will be created, nor i2c_client will be enumerated, we have to use + * acpi_driver. + */ +static const struct acpi_device_id surface_button_device_ids[] = { + {SURFACE_BUTTON_HID, 0}, + {"", 0}, +}; +MODULE_DEVICE_TABLE(acpi, surface_button_device_ids); + +struct surface_button { + unsigned int type; + struct input_dev *input; + char phys[32]; /* for input device */ + unsigned long pushed; + bool suspended; +}; + +static void surface_button_notify(struct acpi_device *device, u32 event) +{ + struct surface_button *button = acpi_driver_data(device); + struct input_dev *input; + int key_code = KEY_RESERVED; + bool pressed = false; + + switch (event) { + /* Power button press,release handle */ + case SURFACE_BUTTON_NOTIFY_PRESS_POWER: + pressed = true; + /*fall through*/ + case SURFACE_BUTTON_NOTIFY_RELEASE_POWER: + key_code = KEY_POWER; + break; + /* Home button press,release handle */ + case SURFACE_BUTTON_NOTIFY_PRESS_HOME: + pressed = true; + /*fall through*/ + case SURFACE_BUTTON_NOTIFY_RELEASE_HOME: + key_code = KEY_LEFTMETA; + break; + /* Volume up button press,release handle */ + case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_UP: + pressed = true; + /*fall through*/ + case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_UP: + key_code = KEY_VOLUMEUP; + break; + /* Volume down button press,release handle */ + case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN: + pressed = true; + /*fall through*/ + case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN: + key_code = KEY_VOLUMEDOWN; + break; + default: + dev_info_ratelimited(&device->dev, + "Unsupported event [0x%x]\n", event); + break; + } + input = button->input; + if (KEY_RESERVED == key_code) + return; + if (pressed) + pm_wakeup_event(&device->dev, 0); + if (button->suspended) + return; + input_report_key(input, key_code, pressed?1:0); + input_sync(input); +} + +#ifdef CONFIG_PM_SLEEP +static int surface_button_suspend(struct device *dev) +{ + struct acpi_device *device = to_acpi_device(dev); + struct surface_button *button = acpi_driver_data(device); + + button->suspended = true; + return 0; +} + +static int surface_button_resume(struct device *dev) +{ + struct acpi_device *device = to_acpi_device(dev); + struct surface_button *button = acpi_driver_data(device); + + button->suspended = false; + return 0; +} +#endif + +static int surface_button_add(struct acpi_device *device) +{ + struct surface_button *button; + struct input_dev *input; + const char *hid = acpi_device_hid(device); + char *name; + int error; + + if (strncmp(acpi_device_bid(device), SURFACE_BUTTON_OBJ_NAME, + strlen(SURFACE_BUTTON_OBJ_NAME))) + return -ENODEV; + + button = kzalloc(sizeof(struct surface_button), GFP_KERNEL); + if (!button) + return -ENOMEM; + + device->driver_data = button; + button->input = input = input_allocate_device(); + if (!input) { + error = -ENOMEM; + goto err_free_button; + } + + name = acpi_device_name(device); + strcpy(name, SURFACE_BUTTON_DEVICE_NAME); + snprintf(button->phys, sizeof(button->phys), "%s/buttons", hid); + + input->name = name; + input->phys = button->phys; + input->id.bustype = BUS_HOST; + input->dev.parent = &device->dev; + input_set_capability(input, EV_KEY, KEY_POWER); + input_set_capability(input, EV_KEY, KEY_LEFTMETA); + input_set_capability(input, EV_KEY, KEY_VOLUMEUP); + input_set_capability(input, EV_KEY, KEY_VOLUMEDOWN); + + error = input_register_device(input); + if (error) + goto err_free_input; + dev_info(&device->dev, + "%s [%s]\n", name, acpi_device_bid(device)); + return 0; + + err_free_input: + input_free_device(input); + err_free_button: + kfree(button); + return error; +} + +static int surface_button_remove(struct acpi_device *device) +{ + struct surface_button *button = acpi_driver_data(device); + + input_unregister_device(button->input); + kfree(button); + return 0; +} + +static SIMPLE_DEV_PM_OPS(surface_button_pm, + surface_button_suspend, surface_button_resume); + +static struct acpi_driver surface_button_driver = { + .name = "surface_pro3_button", + .class = "SurfacePro3", + .ids = surface_button_device_ids, + .ops = { + .add = surface_button_add, + .remove = surface_button_remove, + .notify = surface_button_notify, + }, + .drv.pm = &surface_button_pm, +}; + +module_acpi_driver(surface_button_driver);