Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1202398 > unrolled thread

[PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

Back to article view | Back to linux.kernel

Contents

  [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons Chen Yu <yu.c.chen@intel.com> - 2015-08-07 10:00 +0200
    Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro  3 buttons Joe Perches <joe@perches.com> - 2015-08-07 10:30 +0200
      Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro  3 buttons "Chen, Yu C" <yu.c.chen@intel.com> - 2015-08-07 10:50 +0200
        Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3  buttons Darren Hart <dvhart@infradead.org> - 2015-08-11 04:50 +0200
          Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro  3 buttons Yu Chen <yu.c.chen@intel.com> - 2015-08-18 03:40 +0200

#1202398 — [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

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>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 MAINTAINERS                               |   5 +
 drivers/platform/x86/Kconfig              |   5 +
 drivers/platform/x86/Makefile             |   1 +
 drivers/platform/x86/surfacepro3_button.c | 197 ++++++++++++++++++++++++++++++
 4 files changed, 208 insertions(+)
 create mode 100644 drivers/platform/x86/surfacepro3_button.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a9ae6c1..687b0dd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6712,6 +6712,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..fd0cc31
--- /dev/null
+++ b/drivers/platform/x86/surfacepro3_button.c
@@ -0,0 +1,197 @@
+/*
+ * 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_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;
+};
+
+#define handle_surface_button_notify(type, code)		\
+({								\
+	int ret = 0;						\
+	if (SURFACE_BUTTON_NOTIFY_PRESS_##type == event)	\
+		pressed = true;					\
+	if (pressed ||						\
+		 (SURFACE_BUTTON_NOTIFY_RELEASE_##type == event)) {	\
+		key_code = code;				\
+		ret = 1;					\
+	} else							\
+		ret = 0;					\
+	ret;							\
+})
+
+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;
+
+	if (!handle_surface_button_notify(POWER, KEY_POWER) &&
+		!handle_surface_button_notify(HOME, KEY_LEFTMETA) &&
+		!handle_surface_button_notify(VOLUME_UP, KEY_VOLUMEUP) &&
+		!handle_surface_button_notify(VOLUME_DOWN, KEY_VOLUMEDOWN))
+		dev_info_ratelimited(&device->dev,
+			"Unsupported event [0x%x]\n", event);
+
+	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;
+
+	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);
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1202442 — Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

On Fri, 2015-08-07 at 15:57 +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.

The comments below are all just style trivia
and can be ignored entirely.

> diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
[]
> +#define handle_surface_button_notify(type, code)		\
> +({								\
> +	int ret = 0;						\
> +	if (SURFACE_BUTTON_NOTIFY_PRESS_##type == event)	\
> +		pressed = true;					\
> +	if (pressed ||						\
> +		 (SURFACE_BUTTON_NOTIFY_RELEASE_##type == event)) {	\
> +		key_code = code;				\
> +		ret = 1;					\
> +	} else							\
> +		ret = 0;					\
> +	ret;							\
> +})

This seems a bit complicated.

The else ret = 0 isn't necessary as it's initialized to 0.
bool might be better than int.

> +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;
> +
> +	if (!handle_surface_button_notify(POWER, KEY_POWER) &&
> +		!handle_surface_button_notify(HOME, KEY_LEFTMETA) &&
> +		!handle_surface_button_notify(VOLUME_UP, KEY_VOLUMEUP) &&
> +		!handle_surface_button_notify(VOLUME_DOWN, KEY_VOLUMEDOWN))
> +		dev_info_ratelimited(&device->dev,
> +			"Unsupported event [0x%x]\n", event);

Some might prefer alignment to the open parenthesis:

	if (!handle_surface_button_notify(POWER, KEY_POWER) &&
	    !handle_surface_button_notify(HOME, KEY_LEFTMETA) &&
	    !handle_surface_button_notify(VOLUME_UP, KEY_VOLUMEUP) &&
	    !handle_surface_button_notify(VOLUME_DOWN, KEY_VOLUMEDOWN))

I think the older switch/case was easier to understand.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202460 — Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

T24gRnJpLCAyMDE1LTA4LTA3IGF0IDAxOjI3IC0wNzAwLCBKb2UgUGVyY2hlcyB3cm90ZToNCj4g
T24gRnJpLCAyMDE1LTA4LTA3IGF0IDE1OjU3ICswODAwLCBDaGVuIFl1IHdyb3RlOgkJCQkJXA0K
PiA+ICsJcmV0OwkJCQkJCQlcDQo+ID4gK30pDQo+IA0KPiBUaGlzIHNlZW1zIGEgYml0IGNvbXBs
aWNhdGVkLg0KPiANCj4gVGhlIGVsc2UgcmV0ID0gMCBpc24ndCBuZWNlc3NhcnkgYXMgaXQncyBp
bml0aWFsaXplZCB0byAwLg0KPiBib29sIG1pZ2h0IGJlIGJldHRlciB0aGFuIGludC4NCj4gDQpH
b3QuDQo+ID4gK3N0YXRpYyB2b2lkIHN1cmZhY2VfYnV0dG9uX25vdGlmeShzdHJ1Y3QgYWNwaV9k
ZXZpY2UgKmRldmljZSwgdTMyIGV2ZW50KQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3Qgc3VyZmFjZV9i
dXR0b24gKmJ1dHRvbiA9IGFjcGlfZHJpdmVyX2RhdGEoZGV2aWNlKTsNCj4gPiArCXN0cnVjdCBp
bnB1dF9kZXYgKmlucHV0Ow0KPiA+ICsJaW50IGtleV9jb2RlID0gS0VZX1JFU0VSVkVEOw0KPiA+
ICsJYm9vbCBwcmVzc2VkID0gZmFsc2U7DQo+ID4gKw0KPiA+ICsJaWYgKCFoYW5kbGVfc3VyZmFj
ZV9idXR0b25fbm90aWZ5KFBPV0VSLCBLRVlfUE9XRVIpICYmDQo+ID4gKwkJIWhhbmRsZV9zdXJm
YWNlX2J1dHRvbl9ub3RpZnkoSE9NRSwgS0VZX0xFRlRNRVRBKSAmJg0KPiA+ICsJCSFoYW5kbGVf
c3VyZmFjZV9idXR0b25fbm90aWZ5KFZPTFVNRV9VUCwgS0VZX1ZPTFVNRVVQKSAmJg0KPiA+ICsJ
CSFoYW5kbGVfc3VyZmFjZV9idXR0b25fbm90aWZ5KFZPTFVNRV9ET1dOLCBLRVlfVk9MVU1FRE9X
TikpDQo+ID4gKwkJZGV2X2luZm9fcmF0ZWxpbWl0ZWQoJmRldmljZS0+ZGV2LA0KPiA+ICsJCQki
VW5zdXBwb3J0ZWQgZXZlbnQgWzB4JXhdXG4iLCBldmVudCk7DQo+IA0KPiBTb21lIG1pZ2h0IHBy
ZWZlciBhbGlnbm1lbnQgdG8gdGhlIG9wZW4gcGFyZW50aGVzaXM6DQo+IA0KPiAJaWYgKCFoYW5k
bGVfc3VyZmFjZV9idXR0b25fbm90aWZ5KFBPV0VSLCBLRVlfUE9XRVIpICYmDQo+IAkgICAgIWhh
bmRsZV9zdXJmYWNlX2J1dHRvbl9ub3RpZnkoSE9NRSwgS0VZX0xFRlRNRVRBKSAmJg0KPiAJICAg
ICFoYW5kbGVfc3VyZmFjZV9idXR0b25fbm90aWZ5KFZPTFVNRV9VUCwgS0VZX1ZPTFVNRVVQKSAm
Jg0KPiAJICAgICFoYW5kbGVfc3VyZmFjZV9idXR0b25fbm90aWZ5KFZPTFVNRV9ET1dOLCBLRVlf
Vk9MVU1FRE9XTikpDQo+IA0KPiBJIHRoaW5rIHRoZSBvbGRlciBzd2l0Y2gvY2FzZSB3YXMgZWFz
aWVyIHRvIHVuZGVyc3RhbmQuDQo+IA0KT0ssIHRoZW4gbGV0J3Mga2VlcCB0aGUgc3dpdGNoL2Nh
c2UuDQp0aGFua3MuDQoNCkJlc3QgUmVnYXJkcywNCll1DQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1204710 — Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

On Fri, Aug 07, 2015 at 08:48:18AM +0000, Chen, Yu C wrote:
> On Fri, 2015-08-07 at 01:27 -0700, Joe Perches wrote:
> > On Fri, 2015-08-07 at 15:57 +0800, Chen Yu wrote:					\
> > > +	ret;							\
> > > +})
> > 
> > This seems a bit complicated.
> > 
> > The else ret = 0 isn't necessary as it's initialized to 0.
> > bool might be better than int.
> > 
> Got.
> > > +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;
> > > +
> > > +	if (!handle_surface_button_notify(POWER, KEY_POWER) &&
> > > +		!handle_surface_button_notify(HOME, KEY_LEFTMETA) &&
> > > +		!handle_surface_button_notify(VOLUME_UP, KEY_VOLUMEUP) &&
> > > +		!handle_surface_button_notify(VOLUME_DOWN, KEY_VOLUMEDOWN))
> > > +		dev_info_ratelimited(&device->dev,
> > > +			"Unsupported event [0x%x]\n", event);
> > 
> > Some might prefer alignment to the open parenthesis:
> > 
> > 	if (!handle_surface_button_notify(POWER, KEY_POWER) &&
> > 	    !handle_surface_button_notify(HOME, KEY_LEFTMETA) &&
> > 	    !handle_surface_button_notify(VOLUME_UP, KEY_VOLUMEUP) &&
> > 	    !handle_surface_button_notify(VOLUME_DOWN, KEY_VOLUMEDOWN))
> > 
> > I think the older switch/case was easier to understand.
> > 
> OK, then let's keep the switch/case.
> thanks.

Agreed.


-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1208994 — Re: [PATCH] [v2] surface pro 3: Add support driver for Surface Pro 3 buttons

On 08/11/2015 10:45 AM, Darren Hart wrote:
> On Fri, Aug 07, 2015 at 08:48:18AM +0000, Chen, Yu C wrote:
>> On Fri, 2015-08-07 at 01:27 -0700, Joe Perches wrote:
>>> On Fri, 2015-08-07 at 15:57 +0800, Chen Yu wrote:					\
>>>
>>> I think the older switch/case was easier to understand.
>>>
>> OK, then let's keep the switch/case.
>> thanks.
>
> Agreed.
>
Hi, Darren and Joe, I'll send a V4 of this patch to
add a method to exclude the load of Surface 3(no Pro),
since Surface 3 also has a device name of MSHW0028, but
it is not used for buttons.
Thanks

Best Regards,
Yu
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web