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

Groups > linux.kernel > #1201431 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

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

#1201431 — [PATCH] 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 | 203 ++++++++++++++++++++++++++++++
 4 files changed, 214 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..8e47af6
--- /dev/null
+++ b/drivers/platform/x86/surfacepro3_button.c
@@ -0,0 +1,203 @@
+/*
+ * 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;
+};
+
+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) {
+	case SURFACE_BUTTON_NOTIFY_PRESS_POWER:
+		pressed = true;
+		/*go through*/
+	case SURFACE_BUTTON_NOTIFY_RELEASE_POWER:
+		key_code = KEY_POWER;
+		break;
+	case SURFACE_BUTTON_NOTIFY_PRESS_HOME:
+		pressed = true;
+	case SURFACE_BUTTON_NOTIFY_RELEASE_HOME:
+		key_code = KEY_LEFTMETA;
+		break;
+	case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_UP:
+		pressed = true;
+	case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_UP:
+		key_code = KEY_VOLUMEUP;
+		break;
+	case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN:
+		pressed = true;
+	case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN:
+		key_code = KEY_VOLUMEDOWN;
+		break;
+	default:
+		dev_info(&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;
+
+	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]

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

On Thu, 2015-08-06 at 13:16 +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.

style trivia:

> diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
[]
> +static void surface_button_notify(struct acpi_device *device, u32 event)
> +{
[]
> +	switch (event) {
> +	case SURFACE_BUTTON_NOTIFY_PRESS_POWER:
> +		pressed = true;
> +		/*go through*/

/* fall through */ is more common

> +	case SURFACE_BUTTON_NOTIFY_PRESS_HOME:
> +		pressed = true;
> +	case SURFACE_BUTTON_NOTIFY_RELEASE_HOME:
> +		key_code = KEY_LEFTMETA;
> +		break;

It may be better to add a comment about the style or
maybe add a macro like

#define HANDLE_SURFACE_BUTTON_NOTIFY(type, code)	\
	case SURFACE_BUTTON_NOTIFY_PRESS_##type:	\
		pressed = true;	/* and fall-through */	\
	case SURFACE_BUTTON_NOTIFY_RELEASE_##type:	\
		key_code = code;			\
		break;

> +	case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_UP:
> +		pressed = true;
> +	case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_UP:
> +		key_code = KEY_VOLUMEUP;
> +		break;

> +	case SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN:
> +		pressed = true;
> +	case SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN:
> +		key_code = KEY_VOLUMEDOWN;
> +		break;
> +	default:
> +		dev_info(&device->dev,
> +				  "Unsupported event [0x%x]\n", event);

It might be useful to ratelimit this


--
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]

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

VGhhbmtzIEpvZSwNCk9uIFdlZCwgMjAxNS0wOC0wNSBhdCAyMjozMCAtMDcwMCwgSm9lIFBlcmNo
ZXMgd3JvdGU6DQo+IE9uIFRodSwgMjAxNS0wOC0wNiBhdCAxMzoxNiArMDgwMCwgQ2hlbiBZdSB3
cm90ZToNCj4gPiBTaW5jZSBTdXJmYWNlIFBybyAzIGRvZXMgbm90IGZvbGxvdyB0aGUgc3BlY3Mg
b2YgIldpbmRvd3MgQUNQSSBEZXNpZ24NCj4gPiBHdWlkZSBmb3IgU29DIFBsYXRmb3JtIiwgY29k
ZSBpbiBkcml2ZXJzL2lucHV0L21pc2Mvc29jX2FycmF5LmMgY2FuDQo+ID4gbm90IGRldGVjdCB0
aGVzZSBidXR0b25zIG9uIGl0Lg0KPiANCj4gc3R5bGUgdHJpdmlhOg0KPiANCj4gPiBkaWZmIC0t
Z2l0IGEvZHJpdmVycy9wbGF0Zm9ybS94ODYvc3VyZmFjZXBybzNfYnV0dG9uLmMgYi9kcml2ZXJz
L3BsYXRmb3JtL3g4Ni9zdXJmYWNlcHJvM19idXR0b24uYw0KPiBbXQ0KPiA+ICtzdGF0aWMgdm9p
ZCBzdXJmYWNlX2J1dHRvbl9ub3RpZnkoc3RydWN0IGFjcGlfZGV2aWNlICpkZXZpY2UsIHUzMiBl
dmVudCkNCj4gPiArew0KPiBbXQ0KPiA+ICsJc3dpdGNoIChldmVudCkgew0KPiA+ICsJY2FzZSBT
VVJGQUNFX0JVVFRPTl9OT1RJRllfUFJFU1NfUE9XRVI6DQo+ID4gKwkJcHJlc3NlZCA9IHRydWU7
DQo+ID4gKwkJLypnbyB0aHJvdWdoKi8NCj4gDQo+IC8qIGZhbGwgdGhyb3VnaCAqLyBpcyBtb3Jl
IGNvbW1vbg0KPiANCk9LLg0KPiA+ICsJY2FzZSBTVVJGQUNFX0JVVFRPTl9OT1RJRllfUFJFU1Nf
SE9NRToNCj4gPiArCQlwcmVzc2VkID0gdHJ1ZTsNCj4gPiArCWNhc2UgU1VSRkFDRV9CVVRUT05f
Tk9USUZZX1JFTEVBU0VfSE9NRToNCj4gPiArCQlrZXlfY29kZSA9IEtFWV9MRUZUTUVUQTsNCj4g
PiArCQlicmVhazsNCj4gDQo+IEl0IG1heSBiZSBiZXR0ZXIgdG8gYWRkIGEgY29tbWVudCBhYm91
dCB0aGUgc3R5bGUgb3INCj4gbWF5YmUgYWRkIGEgbWFjcm8gbGlrZQ0KPiANCj4gI2RlZmluZSBI
QU5ETEVfU1VSRkFDRV9CVVRUT05fTk9USUZZKHR5cGUsIGNvZGUpCVwNCj4gCWNhc2UgU1VSRkFD
RV9CVVRUT05fTk9USUZZX1BSRVNTXyMjdHlwZToJXA0KPiAJCXByZXNzZWQgPSB0cnVlOwkvKiBh
bmQgZmFsbC10aHJvdWdoICovCVwNCj4gCWNhc2UgU1VSRkFDRV9CVVRUT05fTk9USUZZX1JFTEVB
U0VfIyN0eXBlOglcDQo+IAkJa2V5X2NvZGUgPSBjb2RlOwkJCVwNCj4gCQlicmVhazsNCj4gDQpX
UlQgbWFjcm8gSEFORExFX1NVUkZBQ0VfQlVUVE9OX05PVElGWSwgdGhlIGNoZWNrcGF0Y2gucGwN
CmNvbXBsYWlucyB0aGF0IG11bHRpIGxpbmVzIG9mIGNvZGVzIHNob3VsZCBiZSB3cmFwcGVkIGlu
ICdkbw0Kd2hpbGUnc3RhdGUsIGJ1dCBkb2luZyBsaWtlIHRoaXMgbWlnaHQgbGVhZCB0byBpbmNv
cnJlY3Qgc2VtYW50aWMuDQpJcyBpdCBvayB0byBrZWVwIHRoZXNlIGNvZGVzIGFuZCBhZGQgY29t
bWVudHMgbGlrZToNCi8qDQogKiBXaGVuIGEgYnV0dG9uKHBvd2VyIGJ1dHRvbi92b2x1bWUgYnV0
dG9uL2hvbWUgYnV0dG9uKSBpcyANCiAqIHByZXNzZWQgZG93biBvciByZWxlYXNlZCwgZGlmZmVy
ZW50IEFDUEkgbm90aWZpY2F0aW9uIGNvZGVzIA0KICogd2lsbCBiZSBnZW5lcmF0ZWQuIFdlIGNh
biBkaXN0aW5ndWlzaCBkaWZmZXJlbnQgZXZlbnQgY29kZSANCiAqIGFuZCB2YWx1ZSBvZiBidXR0
b25zIGJ5IHRoZXNlIG5vdGlmaWNhdGlvbiBjb2RlcywgdGhlbiBwYXNzDQogKiAoRVZfS0VZLCBl
dmVudCBjb2RlKGtleV9jb2RlKSwgdmFsdWUocHJlc3NlZCkpIHRvIGlucHV0IGxheWVyLg0KICov
DQoNCj4gPiArCWNhc2UgU1VSRkFDRV9CVVRUT05fTk9USUZZX1BSRVNTX1ZPTFVNRV9VUDoNCj4g
PiArCQlwcmVzc2VkID0gdHJ1ZTsNCj4gPiArCWNhc2UgU1VSRkFDRV9CVVRUT05fTk9USUZZX1JF
TEVBU0VfVk9MVU1FX1VQOg0KPiA+ICsJCWtleV9jb2RlID0gS0VZX1ZPTFVNRVVQOw0KPiA+ICsJ
CWJyZWFrOw0KPiANCj4gPiArCWNhc2UgU1VSRkFDRV9CVVRUT05fTk9USUZZX1BSRVNTX1ZPTFVN
RV9ET1dOOg0KPiA+ICsJCXByZXNzZWQgPSB0cnVlOw0KPiA+ICsJY2FzZSBTVVJGQUNFX0JVVFRP
Tl9OT1RJRllfUkVMRUFTRV9WT0xVTUVfRE9XTjoNCj4gPiArCQlrZXlfY29kZSA9IEtFWV9WT0xV
TUVET1dOOw0KPiA+ICsJCWJyZWFrOw0KPiA+ICsJZGVmYXVsdDoNCj4gPiArCQlkZXZfaW5mbygm
ZGV2aWNlLT5kZXYsDQo+ID4gKwkJCQkgICJVbnN1cHBvcnRlZCBldmVudCBbMHgleF1cbiIsIGV2
ZW50KTsNCj4gDQo+IEl0IG1pZ2h0IGJlIHVzZWZ1bCB0byByYXRlbGltaXQgdGhpcw0KPiANCk9L
LCBjaGFuZ2VkIHRvIGRldl9pbmZvX3JhdGVsaW1pdGVkDQo+IA0KDQpCZXN0IFJlZ2FyZHMsDQpZ
dQ0KDQo=
--
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]

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

On Thu, 2015-08-06 at 11:20 +0000, Chen, Yu C wrote:
> On Wed, 2015-08-05 at 22:30 -0700, Joe Perches wrote:
[]
> > > +	case SURFACE_BUTTON_NOTIFY_PRESS_HOME:
> > > +		pressed = true;
> > > +	case SURFACE_BUTTON_NOTIFY_RELEASE_HOME:
> > > +		key_code = KEY_LEFTMETA;
> > > +		break;
> > 
> > It may be better to add a comment about the style or
> > maybe add a macro like
> > 
> > #define HANDLE_SURFACE_BUTTON_NOTIFY(type, code)	\
> > 	case SURFACE_BUTTON_NOTIFY_PRESS_##type:	\
> > 		pressed = true;	/* and fall-through */	\
> > 	case SURFACE_BUTTON_NOTIFY_RELEASE_##type:	\
> > 		key_code = code;			\
> > 		break;
> > 
> WRT macro HANDLE_SURFACE_BUTTON_NOTIFY, the checkpatch.pl
> complains that multi lines of codes should be wrapped in 'do
> while'state, but doing like this might lead to incorrect semantic.

checkpatch is a brainless tool that should be ignored
whenever you want.

> Is it ok to keep these codes and add comments like:

Up to you.  It'd OK to do nothing too.


--
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]

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

On Thu, Aug 06, 2015 at 11:20:44AM +0000, Chen, Yu C wrote:
> Thanks Joe,
> On Wed, 2015-08-05 at 22:30 -0700, Joe Perches wrote:
> > On Thu, 2015-08-06 at 13:16 +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.
> > 
> > style trivia:
> > 
> > > diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
> > []
> > > +static void surface_button_notify(struct acpi_device *device, u32 event)
> > > +{
> > []
> > > +	switch (event) {
> > > +	case SURFACE_BUTTON_NOTIFY_PRESS_POWER:
> > > +		pressed = true;
> > > +		/*go through*/
> > 
> > /* fall through */ is more common
> > 
> OK.
> > > +	case SURFACE_BUTTON_NOTIFY_PRESS_HOME:
> > > +		pressed = true;
> > > +	case SURFACE_BUTTON_NOTIFY_RELEASE_HOME:
> > > +		key_code = KEY_LEFTMETA;
> > > +		break;
> > 
> > It may be better to add a comment about the style or
> > maybe add a macro like
> > 
> > #define HANDLE_SURFACE_BUTTON_NOTIFY(type, code)	\
> > 	case SURFACE_BUTTON_NOTIFY_PRESS_##type:	\
> > 		pressed = true;	/* and fall-through */	\
> > 	case SURFACE_BUTTON_NOTIFY_RELEASE_##type:	\
> > 		key_code = code;			\
> > 		break;
> > 
> WRT macro HANDLE_SURFACE_BUTTON_NOTIFY, the checkpatch.pl
> complains that multi lines of codes should be wrapped in 'do
> while'state, but doing like this might lead to incorrect semantic.
> Is it ok to keep these codes and add comments like:
> /*
>  * When a button(power button/volume button/home button) is 
>  * pressed down or released, different ACPI notification codes 
>  * will be generated. We can distinguish different event code 
>  * and value of buttons by these notification codes, then pass
>  * (EV_KEY, event code(key_code), value(pressed)) to input layer.
>  */

The commentary is useful regardless. However, I suspect Joe was
referring to the approach pairing the PRESS and RELEASE cases?

-- 
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]

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

On Wed, 2015-08-05 at 16:47 -0700, Darren Hart wrote:
> On Thu, Aug 06, 2015 at 11:20:44AM +0000, Chen, Yu C wrote:
[]
> > Is it ok to keep these codes and add comments like:

It's your code Yu, do whatever you think appropriate.

> > /*
> >  * When a button(power button/volume button/home button) is 
> >  * pressed down or released, different ACPI notification codes 
> >  * will be generated. We can distinguish different event code 
> >  * and value of buttons by these notification codes, then pass
> >  * (EV_KEY, event code(key_code), value(pressed)) to input layer.
> >  */
> 
> The commentary is useful regardless. However, I suspect Joe was
> referring to the approach pairing the PRESS and RELEASE cases?
> 

True.

btw Darren, your computer's email time setting seems off.


--
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]

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

On Thu, Aug 06, 2015 at 11:55:29AM -0700, Joe Perches wrote:
> On Wed, 2015-08-05 at 16:47 -0700, Darren Hart wrote:
> > On Thu, Aug 06, 2015 at 11:20:44AM +0000, Chen, Yu C wrote:
> []
> > > Is it ok to keep these codes and add comments like:
> 
> It's your code Yu, do whatever you think appropriate.
> 
> > > /*
> > >  * When a button(power button/volume button/home button) is 
> > >  * pressed down or released, different ACPI notification codes 
> > >  * will be generated. We can distinguish different event code 
> > >  * and value of buttons by these notification codes, then pass
> > >  * (EV_KEY, event code(key_code), value(pressed)) to input layer.
> > >  */
> > 
> > The commentary is useful regardless. However, I suspect Joe was
> > referring to the approach pairing the PRESS and RELEASE cases?
> > 
> 
> True.
> 
> btw Darren, your computer's email time setting seems off.

Nothing gets past kernel devs! Corporate firewall broke ntp for Linux VM from
where I sent this, didn't notice until too late.

-- 
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]

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

SGksIERhcnJlbiBhbmQgSm9lLA0KT24gV2VkLCAyMDE1LTA4LTA1IGF0IDE2OjQ3IC0wNzAwLCBE
YXJyZW4gSGFydCB3cm90ZToNCj4gT24gVGh1LCBBdWcgMDYsIDIwMTUgYXQgMTE6MjA6NDRBTSAr
MDAwMCwgQ2hlbiwgWXUgQyB3cm90ZToNCj4gDQo+IFRoZSBjb21tZW50YXJ5IGlzIHVzZWZ1bCBy
ZWdhcmRsZXNzLiBIb3dldmVyLCBJIHN1c3BlY3QgSm9lIHdhcw0KPiByZWZlcnJpbmcgdG8gdGhl
IGFwcHJvYWNoIHBhaXJpbmcgdGhlIFBSRVNTIGFuZCBSRUxFQVNFIGNhc2VzPw0KPiANCkkndmUg
d3JvdGUgYW5vdGhlciBwaWVjZSBvZiBNYWNybyB0byBtYWtlIGl0IHBhaXJpbmcgdGhlIFBSRVNT
IGFuZA0KUkVMRUFTRSBjYXNlcywgd291bGQgeW91IHBsZWFzZSBoZWxwIGNoZWNrIGlmIGl0IGlz
IHN1aXRhYmxlLHRoYW5rcw0KZm9yIHlvdXIgdGltZS4NClBTOiBJIHJlc2VuZCBhbm90aGVyIHBh
dGNoIHRpdGxlZCB3aXRoIHYyLg0KDQoNCkJlc3QgUmVnYXJkcywNCll1DQo=
--
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