linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] surface pro 4: Add support for Surface Pro 4 Buttons
@ 2015-12-24 20:28 Weng Xuetian
  2015-12-27 13:36 ` Andy Shevchenko
  2015-12-27 17:29 ` [PATCH v2] " Weng Xuetian
  0 siblings, 2 replies; 17+ messages in thread
From: Weng Xuetian @ 2015-12-24 20:28 UTC (permalink / raw)
  To: Chen Yu; +Cc: Darren Hart, linux-kernel, platform-driver-x86, Weng Xuetian

Surface Pro 4 button is managed by a device with _HID "MSHW0040"
different from Surface Pro 3.

This commit adds MSHW0040 to id list to Support Surface Pro 4, and
rename the driver to surfacepro_button accordingly.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
---
 MAINTAINERS                               |   4 +-
 drivers/platform/x86/Kconfig              |   6 +-
 drivers/platform/x86/Makefile             |   2 +-
 drivers/platform/x86/surfacepro3_button.c | 216 -----------------------------
 drivers/platform/x86/surfacepro_button.c  | 218 ++++++++++++++++++++++++++++++
 5 files changed, 224 insertions(+), 222 deletions(-)
 delete mode 100644 drivers/platform/x86/surfacepro3_button.c
 create mode 100644 drivers/platform/x86/surfacepro_button.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 233f834..1c07436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7019,11 +7019,11 @@ T:	git git://git.monstr.eu/linux-2.6-microblaze.git
 S:	Supported
 F:	arch/microblaze/
 
-MICROSOFT SURFACE PRO 3 BUTTON DRIVER
+MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Supported
-F:	drivers/platform/x86/surfacepro3_button.c
+F:	drivers/platform/x86/surfacepro_button.c
 
 MICROTEK X6 SCANNER
 M:	Oliver Neukum <oliver@neukum.org>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..3358fb0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -939,9 +939,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"
+config SURFACE_PRO_BUTTON
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
 	depends on ACPI && INPUT
 	---help---
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 3ca78a3..b4ece33 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -61,4 +61,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
+obj-$(CONFIG_SURFACE_PRO_BUTTON)	+= surfacepro_button.o
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
deleted file mode 100644
index f7dade3..0000000
--- a/drivers/platform/x86/surfacepro3_button.c
+++ /dev/null
@@ -1,216 +0,0 @@
-/*
- * power/home/volume button support for
- * Microsoft Surface Pro 3 tablet.
- *
- * Copyright (c) 2015 Intel Corporation.
- * All rights reserved.
- *
- * 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);
diff --git a/drivers/platform/x86/surfacepro_button.c b/drivers/platform/x86/surfacepro_button.c
new file mode 100644
index 0000000..cda52b8
--- /dev/null
+++ b/drivers/platform/x86/surfacepro_button.c
@@ -0,0 +1,218 @@
+/*
+ * power/home/volume button support for
+ * Microsoft Surface Pro Series tablet.
+ *
+ * Copyright (c) 2015 Intel Corporation.
+ * All rights reserved.
+ *
+ * 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_PRO3_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO4_BUTTON_HID		"MSHW0040"
+#define SURFACE_BUTTON_OBJ_NAME		"VGBI"
+#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro Series 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 series button");
+
+MODULE_AUTHOR("Chen Yu");
+MODULE_DESCRIPTION("Surface Pro Series 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_PRO3_BUTTON_HID,    0},
+	{SURFACE_PRO4_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_pro_button",
+	.class = "SurfacePro",
+	.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);
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-24 20:28 [PATCH] surface pro 4: Add support for Surface Pro 4 Buttons Weng Xuetian
@ 2015-12-27 13:36 ` Andy Shevchenko
  2015-12-27 17:29 ` [PATCH v2] " Weng Xuetian
  1 sibling, 0 replies; 17+ messages in thread
From: Andy Shevchenko @ 2015-12-27 13:36 UTC (permalink / raw)
  To: Weng Xuetian
  Cc: Chen Yu, Darren Hart, linux-kernel@vger.kernel.org,
	platform-driver-x86

On Thu, Dec 24, 2015 at 10:28 PM, Weng Xuetian <wengxt@gmail.com> wrote:
> Surface Pro 4 button is managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
>
> This commit adds MSHW0040 to id list to Support Surface Pro 4, and
> rename the driver to surfacepro_button accordingly.
>

No review until you resend with a properly formatted patch, i.e. with -M -C.

> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> ---
>  MAINTAINERS                               |   4 +-
>  drivers/platform/x86/Kconfig              |   6 +-
>  drivers/platform/x86/Makefile             |   2 +-
>  drivers/platform/x86/surfacepro3_button.c | 216 -----------------------------
>  drivers/platform/x86/surfacepro_button.c  | 218 ++++++++++++++++++++++++++++++
>  5 files changed, 224 insertions(+), 222 deletions(-)
>  delete mode 100644 drivers/platform/x86/surfacepro3_button.c
>  create mode 100644 drivers/platform/x86/surfacepro_button.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 233f834..1c07436 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7019,11 +7019,11 @@ T:      git git://git.monstr.eu/linux-2.6-microblaze.git
>  S:     Supported
>  F:     arch/microblaze/
>
> -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
>  M:     Chen Yu <yu.c.chen@intel.com>
>  L:     platform-driver-x86@vger.kernel.org
>  S:     Supported
> -F:     drivers/platform/x86/surfacepro3_button.c
> +F:     drivers/platform/x86/surfacepro_button.c
>
>  MICROTEK X6 SCANNER
>  M:     Oliver Neukum <oliver@neukum.org>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 1089eaa..3358fb0 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -939,9 +939,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"
> +config SURFACE_PRO_BUTTON
> +       tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
>         depends on ACPI && INPUT
>         ---help---
> -         This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
> +         This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 3ca78a3..b4ece33 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -61,4 +61,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
> +obj-$(CONFIG_SURFACE_PRO_BUTTON)       += surfacepro_button.o
> diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
> deleted file mode 100644
> index f7dade3..0000000
> --- a/drivers/platform/x86/surfacepro3_button.c
> +++ /dev/null
> @@ -1,216 +0,0 @@
> -/*
> - * power/home/volume button support for
> - * Microsoft Surface Pro 3 tablet.
> - *
> - * Copyright (c) 2015 Intel Corporation.
> - * All rights reserved.
> - *
> - * 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);
> diff --git a/drivers/platform/x86/surfacepro_button.c b/drivers/platform/x86/surfacepro_button.c
> new file mode 100644
> index 0000000..cda52b8
> --- /dev/null
> +++ b/drivers/platform/x86/surfacepro_button.c
> @@ -0,0 +1,218 @@
> +/*
> + * power/home/volume button support for
> + * Microsoft Surface Pro Series tablet.
> + *
> + * Copyright (c) 2015 Intel Corporation.
> + * All rights reserved.
> + *
> + * 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_PRO3_BUTTON_HID                "MSHW0028"
> +#define SURFACE_PRO4_BUTTON_HID                "MSHW0040"
> +#define SURFACE_BUTTON_OBJ_NAME                "VGBI"
> +#define SURFACE_BUTTON_DEVICE_NAME     "Surface Pro Series 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 series button");
> +
> +MODULE_AUTHOR("Chen Yu");
> +MODULE_DESCRIPTION("Surface Pro Series 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_PRO3_BUTTON_HID,    0},
> +       {SURFACE_PRO4_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_pro_button",
> +       .class = "SurfacePro",
> +       .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);
> --
> 2.6.4
>
> --
> 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/



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-24 20:28 [PATCH] surface pro 4: Add support for Surface Pro 4 Buttons Weng Xuetian
  2015-12-27 13:36 ` Andy Shevchenko
@ 2015-12-27 17:29 ` Weng Xuetian
  2015-12-27 19:07   ` Chen, Yu C
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
  1 sibling, 2 replies; 17+ messages in thread
From: Weng Xuetian @ 2015-12-27 17:29 UTC (permalink / raw)
  To: Chen Yu; +Cc: Darren Hart, linux-kernel, platform-driver-x86, Weng Xuetian

Surface Pro 4 button is managed by a device with _HID "MSHW0040"
different from Surface Pro 3.

This commit adds MSHW0040 to id list to Support Surface Pro 4, and
rename the driver to surfacepro_button accordingly.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
---
v2:
 - Reformat patch with -M -C
---
 MAINTAINERS                                            |  4 ++--
 drivers/platform/x86/Kconfig                           |  6 +++---
 drivers/platform/x86/Makefile                          |  2 +-
 .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 ++++++++++--------
 4 files changed, 16 insertions(+), 14 deletions(-)
 rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} (93%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 233f834..1c07436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7019,11 +7019,11 @@ T:	git git://git.monstr.eu/linux-2.6-microblaze.git
 S:	Supported
 F:	arch/microblaze/
 
-MICROSOFT SURFACE PRO 3 BUTTON DRIVER
+MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Supported
-F:	drivers/platform/x86/surfacepro3_button.c
+F:	drivers/platform/x86/surfacepro_button.c
 
 MICROTEK X6 SCANNER
 M:	Oliver Neukum <oliver@neukum.org>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..3358fb0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -939,9 +939,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"
+config SURFACE_PRO_BUTTON
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
 	depends on ACPI && INPUT
 	---help---
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 3ca78a3..b4ece33 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -61,4 +61,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
+obj-$(CONFIG_SURFACE_PRO_BUTTON)	+= surfacepro_button.o
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro_button.c
similarity index 93%
rename from drivers/platform/x86/surfacepro3_button.c
rename to drivers/platform/x86/surfacepro_button.c
index f7dade3..cda52b8 100644
--- a/drivers/platform/x86/surfacepro3_button.c
+++ b/drivers/platform/x86/surfacepro_button.c
@@ -1,6 +1,6 @@
 /*
  * power/home/volume button support for
- * Microsoft Surface Pro 3 tablet.
+ * Microsoft Surface Pro Series tablet.
  *
  * Copyright (c) 2015 Intel Corporation.
  * All rights reserved.
@@ -19,9 +19,10 @@
 #include <linux/acpi.h>
 #include <acpi/button.h>
 
-#define SURFACE_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO3_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO4_BUTTON_HID		"MSHW0040"
 #define SURFACE_BUTTON_OBJ_NAME		"VGBI"
-#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro 3 Buttons"
+#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro Series Buttons"
 
 #define SURFACE_BUTTON_NOTIFY_PRESS_POWER	0xc6
 #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER	0xc7
@@ -35,10 +36,10 @@
 #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN	0xc2
 #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN	0xc3
 
-ACPI_MODULE_NAME("surface pro 3 button");
+ACPI_MODULE_NAME("surface pro series button");
 
 MODULE_AUTHOR("Chen Yu");
-MODULE_DESCRIPTION("Surface Pro3 Button Driver");
+MODULE_DESCRIPTION("Surface Pro Series Button Driver");
 MODULE_LICENSE("GPL v2");
 
 /*
@@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
  * acpi_driver.
  */
 static const struct acpi_device_id surface_button_device_ids[] = {
-	{SURFACE_BUTTON_HID,    0},
+	{SURFACE_PRO3_BUTTON_HID,    0},
+	{SURFACE_PRO4_BUTTON_HID,    0},
 	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
@@ -202,8 +204,8 @@ 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",
+	.name = "surface_pro_button",
+	.class = "SurfacePro",
 	.ids = surface_button_device_ids,
 	.ops = {
 		.add = surface_button_add,
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* RE: [PATCH v2] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 17:29 ` [PATCH v2] " Weng Xuetian
@ 2015-12-27 19:07   ` Chen, Yu C
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
  1 sibling, 0 replies; 17+ messages in thread
From: Chen, Yu C @ 2015-12-27 19:07 UTC (permalink / raw)
  To: Weng Xuetian
  Cc: Darren Hart, linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org, Andy Shevchenko

Hi,

> -----Original Message-----
> From: Weng Xuetian [mailto:wengxt@gmail.com]
> Sent: Monday, December 28, 2015 1:30 AM
> To: Chen, Yu C
> Cc: Darren Hart; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; Weng Xuetian
> Subject: [PATCH v2] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> Surface Pro 4 button is managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
s/button is/buttons are
> This commit adds MSHW0040 to id list to Support Surface Pro 4, and rename
> the driver to surfacepro_button accordingly.
> 
s/Support/support the
s/rename/renames
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> ---
> v2:
>  - Reformat patch with -M -C
others look good to me, 

thanks,
yu



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 17:29 ` [PATCH v2] " Weng Xuetian
  2015-12-27 19:07   ` Chen, Yu C
@ 2015-12-27 19:21   ` Weng Xuetian
  2015-12-27 20:58     ` Andy Shevchenko
                       ` (3 more replies)
  1 sibling, 4 replies; 17+ messages in thread
From: Weng Xuetian @ 2015-12-27 19:21 UTC (permalink / raw)
  To: Chen Yu; +Cc: Darren Hart, linux-kernel, platform-driver-x86, Weng Xuetian

Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
different from Surface Pro 3.

This commit adds MSHW0040 to id list to support the Surface Pro 4, and
renames the driver to surfacepro_button accordingly.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
---
v3:
 - Fix commit message grammar mistakes.
v2:
 - Reformat patch with -M -C
---
 MAINTAINERS                                            |  4 ++--
 drivers/platform/x86/Kconfig                           |  6 +++---
 drivers/platform/x86/Makefile                          |  2 +-
 .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 ++++++++++--------
 4 files changed, 16 insertions(+), 14 deletions(-)
 rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} (93%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 233f834..1c07436 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7019,11 +7019,11 @@ T:	git git://git.monstr.eu/linux-2.6-microblaze.git
 S:	Supported
 F:	arch/microblaze/
 
-MICROSOFT SURFACE PRO 3 BUTTON DRIVER
+MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
 M:	Chen Yu <yu.c.chen@intel.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Supported
-F:	drivers/platform/x86/surfacepro3_button.c
+F:	drivers/platform/x86/surfacepro_button.c
 
 MICROTEK X6 SCANNER
 M:	Oliver Neukum <oliver@neukum.org>
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..3358fb0 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -939,9 +939,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"
+config SURFACE_PRO_BUTTON
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
 	depends on ACPI && INPUT
 	---help---
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 3ca78a3..b4ece33 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -61,4 +61,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
+obj-$(CONFIG_SURFACE_PRO_BUTTON)	+= surfacepro_button.o
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro_button.c
similarity index 93%
rename from drivers/platform/x86/surfacepro3_button.c
rename to drivers/platform/x86/surfacepro_button.c
index f7dade3..cda52b8 100644
--- a/drivers/platform/x86/surfacepro3_button.c
+++ b/drivers/platform/x86/surfacepro_button.c
@@ -1,6 +1,6 @@
 /*
  * power/home/volume button support for
- * Microsoft Surface Pro 3 tablet.
+ * Microsoft Surface Pro Series tablet.
  *
  * Copyright (c) 2015 Intel Corporation.
  * All rights reserved.
@@ -19,9 +19,10 @@
 #include <linux/acpi.h>
 #include <acpi/button.h>
 
-#define SURFACE_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO3_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO4_BUTTON_HID		"MSHW0040"
 #define SURFACE_BUTTON_OBJ_NAME		"VGBI"
-#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro 3 Buttons"
+#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro Series Buttons"
 
 #define SURFACE_BUTTON_NOTIFY_PRESS_POWER	0xc6
 #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER	0xc7
@@ -35,10 +36,10 @@
 #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN	0xc2
 #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN	0xc3
 
-ACPI_MODULE_NAME("surface pro 3 button");
+ACPI_MODULE_NAME("surface pro series button");
 
 MODULE_AUTHOR("Chen Yu");
-MODULE_DESCRIPTION("Surface Pro3 Button Driver");
+MODULE_DESCRIPTION("Surface Pro Series Button Driver");
 MODULE_LICENSE("GPL v2");
 
 /*
@@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
  * acpi_driver.
  */
 static const struct acpi_device_id surface_button_device_ids[] = {
-	{SURFACE_BUTTON_HID,    0},
+	{SURFACE_PRO3_BUTTON_HID,    0},
+	{SURFACE_PRO4_BUTTON_HID,    0},
 	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
@@ -202,8 +204,8 @@ 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",
+	.name = "surface_pro_button",
+	.class = "SurfacePro",
 	.ids = surface_button_device_ids,
 	.ops = {
 		.add = surface_button_add,
-- 
2.6.4


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
@ 2015-12-27 20:58     ` Andy Shevchenko
  2016-01-04 20:35       ` Darren Hart
  2016-01-04 20:11     ` Darren Hart
                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Andy Shevchenko @ 2015-12-27 20:58 UTC (permalink / raw)
  To: Weng Xuetian
  Cc: Chen Yu, Darren Hart, linux-kernel@vger.kernel.org,
	platform-driver-x86

On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wengxt@gmail.com> wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
>
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>

Darren, this one looks fine for me, still couple of question up to you.

First is do we really want to rename driver? (Renaming variables and
stuff like I said if fine to me)

> ---
> v3:
>  - Fix commit message grammar mistakes.
> v2:
>  - Reformat patch with -M -C
> ---
>  MAINTAINERS                                            |  4 ++--
>  drivers/platform/x86/Kconfig                           |  6 +++---
>  drivers/platform/x86/Makefile                          |  2 +-
>  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 ++++++++++--------
>  4 files changed, 16 insertions(+), 14 deletions(-)
>  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} (93%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 233f834..1c07436 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7019,11 +7019,11 @@ T:      git git://git.monstr.eu/linux-2.6-microblaze.git
>  S:     Supported
>  F:     arch/microblaze/
>
> -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
>  M:     Chen Yu <yu.c.chen@intel.com>
>  L:     platform-driver-x86@vger.kernel.org
>  S:     Supported
> -F:     drivers/platform/x86/surfacepro3_button.c
> +F:     drivers/platform/x86/surfacepro_button.c
>
>  MICROTEK X6 SCANNER
>  M:     Oliver Neukum <oliver@neukum.org>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 1089eaa..3358fb0 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -939,9 +939,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"
> +config SURFACE_PRO_BUTTON
> +       tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
>         depends on ACPI && INPUT
>         ---help---
> -         This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
> +         This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
>  endif # X86_PLATFORM_DEVICES
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 3ca78a3..b4ece33 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -61,4 +61,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
> +obj-$(CONFIG_SURFACE_PRO_BUTTON)       += surfacepro_button.o
> diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro_button.c
> similarity index 93%
> rename from drivers/platform/x86/surfacepro3_button.c
> rename to drivers/platform/x86/surfacepro_button.c
> index f7dade3..cda52b8 100644
> --- a/drivers/platform/x86/surfacepro3_button.c
> +++ b/drivers/platform/x86/surfacepro_button.c
> @@ -1,6 +1,6 @@
>  /*
>   * power/home/volume button support for
> - * Microsoft Surface Pro 3 tablet.
> + * Microsoft Surface Pro Series tablet.
>   *
>   * Copyright (c) 2015 Intel Corporation.
>   * All rights reserved.
> @@ -19,9 +19,10 @@
>  #include <linux/acpi.h>
>  #include <acpi/button.h>
>
> -#define SURFACE_BUTTON_HID             "MSHW0028"
> +#define SURFACE_PRO3_BUTTON_HID                "MSHW0028"
> +#define SURFACE_PRO4_BUTTON_HID                "MSHW0040"
>  #define SURFACE_BUTTON_OBJ_NAME                "VGBI"
> -#define SURFACE_BUTTON_DEVICE_NAME     "Surface Pro 3 Buttons"
> +#define SURFACE_BUTTON_DEVICE_NAME     "Surface Pro Series Buttons"
>
>  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER      0xc6
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER    0xc7
> @@ -35,10 +36,10 @@
>  #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN        0xc2
>  #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN      0xc3
>
> -ACPI_MODULE_NAME("surface pro 3 button");
> +ACPI_MODULE_NAME("surface pro series button");
>
>  MODULE_AUTHOR("Chen Yu");
> -MODULE_DESCRIPTION("Surface Pro3 Button Driver");
> +MODULE_DESCRIPTION("Surface Pro Series Button Driver");
>  MODULE_LICENSE("GPL v2");
>
>  /*
> @@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
>   * acpi_driver.
>   */
>  static const struct acpi_device_id surface_button_device_ids[] = {
> -       {SURFACE_BUTTON_HID,    0},
> +       {SURFACE_PRO3_BUTTON_HID,    0},
> +       {SURFACE_PRO4_BUTTON_HID,    0},
>         {"", 0},
>  };
>  MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
> @@ -202,8 +204,8 @@ 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",
> +       .name = "surface_pro_button",
> +       .class = "SurfacePro",

So, beside the driver renaming I don't know the side effect of
renaming .class field here.

>         .ids = surface_button_device_ids,
>         .ops = {
>                 .add = surface_button_add,

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
  2015-12-27 20:58     ` Andy Shevchenko
@ 2016-01-04 20:11     ` Darren Hart
  2016-01-11 18:38     ` Darren Hart
  2016-01-12 19:43     ` [PATCH v4] " Weng Xuetian
  3 siblings, 0 replies; 17+ messages in thread
From: Darren Hart @ 2016-01-04 20:11 UTC (permalink / raw)
  To: Weng Xuetian; +Cc: Chen Yu, linux-kernel, platform-driver-x86

On Sun, Dec 27, 2015 at 11:21:20AM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871

Is there a Surface Pro v1 and v2? If so, and they are not supported, then the 3
makes sense to keep. Or, at the very least, the Kconfig help and the comments in
the driver should make it clear that it supports v3 and later.

Chen, any concerns?

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 20:58     ` Andy Shevchenko
@ 2016-01-04 20:35       ` Darren Hart
  2016-01-05  0:00         ` Chen, Yu C
  0 siblings, 1 reply; 17+ messages in thread
From: Darren Hart @ 2016-01-04 20:35 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Weng Xuetian, Chen Yu, linux-kernel@vger.kernel.org,
	platform-driver-x86

On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wengxt@gmail.com> wrote:
> > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > different from Surface Pro 3.
> >
> > This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> > renames the driver to surfacepro_button accordingly.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> 
> Darren, this one looks fine for me, still couple of question up to you.
> 
> First is do we really want to rename driver? (Renaming variables and
> stuff like I said if fine to me)

Sorry, replied before seeing this.

I agree that renaming the file is probably not necessary (this works on v3+ as I
understand it, so the name isn't really a problem, and dropping the 3 suggests
it works on v1 and v2).

I'd prefer to keep the name changing to a minimum, and add some information to
the Kconfig help and driver comments making it clear that this supports 3+.

--
Darren

> 
> > ---
> > v3:
> >  - Fix commit message grammar mistakes.
> > v2:
> >  - Reformat patch with -M -C
> > ---
> >  MAINTAINERS                                            |  4 ++--
> >  drivers/platform/x86/Kconfig                           |  6 +++---
> >  drivers/platform/x86/Makefile                          |  2 +-
> >  .../x86/{surfacepro3_button.c => surfacepro_button.c}  | 18 ++++++++++--------
> >  4 files changed, 16 insertions(+), 14 deletions(-)
> >  rename drivers/platform/x86/{surfacepro3_button.c => surfacepro_button.c} (93%)
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 233f834..1c07436 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7019,11 +7019,11 @@ T:      git git://git.monstr.eu/linux-2.6-microblaze.git
> >  S:     Supported
> >  F:     arch/microblaze/
> >
> > -MICROSOFT SURFACE PRO 3 BUTTON DRIVER
> > +MICROSOFT SURFACE PRO SERIES BUTTON DRIVER
> >  M:     Chen Yu <yu.c.chen@intel.com>
> >  L:     platform-driver-x86@vger.kernel.org
> >  S:     Supported
> > -F:     drivers/platform/x86/surfacepro3_button.c
> > +F:     drivers/platform/x86/surfacepro_button.c
> >
> >  MICROTEK X6 SCANNER
> >  M:     Oliver Neukum <oliver@neukum.org>
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 1089eaa..3358fb0 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -939,9 +939,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"
> > +config SURFACE_PRO_BUTTON
> > +       tristate "Power/home/volume buttons driver for Microsoft Surface Pro Series tablet"
> >         depends on ACPI && INPUT
> >         ---help---
> > -         This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
> > +         This driver handles the power/home/volume buttons on the Microsoft Surface Pro Series tablet.
> >  endif # X86_PLATFORM_DEVICES
> > diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> > index 3ca78a3..b4ece33 100644
> > --- a/drivers/platform/x86/Makefile
> > +++ b/drivers/platform/x86/Makefile
> > @@ -61,4 +61,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
> > +obj-$(CONFIG_SURFACE_PRO_BUTTON)       += surfacepro_button.o
> > diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro_button.c
> > similarity index 93%
> > rename from drivers/platform/x86/surfacepro3_button.c
> > rename to drivers/platform/x86/surfacepro_button.c
> > index f7dade3..cda52b8 100644
> > --- a/drivers/platform/x86/surfacepro3_button.c
> > +++ b/drivers/platform/x86/surfacepro_button.c
> > @@ -1,6 +1,6 @@
> >  /*
> >   * power/home/volume button support for
> > - * Microsoft Surface Pro 3 tablet.
> > + * Microsoft Surface Pro Series tablet.
> >   *
> >   * Copyright (c) 2015 Intel Corporation.
> >   * All rights reserved.
> > @@ -19,9 +19,10 @@
> >  #include <linux/acpi.h>
> >  #include <acpi/button.h>
> >
> > -#define SURFACE_BUTTON_HID             "MSHW0028"
> > +#define SURFACE_PRO3_BUTTON_HID                "MSHW0028"
> > +#define SURFACE_PRO4_BUTTON_HID                "MSHW0040"
> >  #define SURFACE_BUTTON_OBJ_NAME                "VGBI"
> > -#define SURFACE_BUTTON_DEVICE_NAME     "Surface Pro 3 Buttons"
> > +#define SURFACE_BUTTON_DEVICE_NAME     "Surface Pro Series Buttons"
> >
> >  #define SURFACE_BUTTON_NOTIFY_PRESS_POWER      0xc6
> >  #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER    0xc7
> > @@ -35,10 +36,10 @@
> >  #define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN        0xc2
> >  #define SURFACE_BUTTON_NOTIFY_RELEASE_VOLUME_DOWN      0xc3
> >
> > -ACPI_MODULE_NAME("surface pro 3 button");
> > +ACPI_MODULE_NAME("surface pro series button");
> >
> >  MODULE_AUTHOR("Chen Yu");
> > -MODULE_DESCRIPTION("Surface Pro3 Button Driver");
> > +MODULE_DESCRIPTION("Surface Pro Series Button Driver");
> >  MODULE_LICENSE("GPL v2");
> >
> >  /*
> > @@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
> >   * acpi_driver.
> >   */
> >  static const struct acpi_device_id surface_button_device_ids[] = {
> > -       {SURFACE_BUTTON_HID,    0},
> > +       {SURFACE_PRO3_BUTTON_HID,    0},
> > +       {SURFACE_PRO4_BUTTON_HID,    0},
> >         {"", 0},
> >  };
> >  MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
> > @@ -202,8 +204,8 @@ 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",
> > +       .name = "surface_pro_button",
> > +       .class = "SurfacePro",
> 
> So, beside the driver renaming I don't know the side effect of
> renaming .class field here.
> 
> >         .ids = surface_button_device_ids,
> >         .ops = {
> >                 .add = surface_button_add,
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-04 20:35       ` Darren Hart
@ 2016-01-05  0:00         ` Chen, Yu C
  0 siblings, 0 replies; 17+ messages in thread
From: Chen, Yu C @ 2016-01-05  0:00 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko
  Cc: Weng Xuetian, linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org



Hi Darren,

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Tuesday, January 05, 2016 4:36 AM
> To: Andy Shevchenko
> Cc: Weng Xuetian; Chen, Yu C; linux-kernel@vger.kernel.org; platform-
> driver-x86@vger.kernel.org
> Subject: Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> On Sun, Dec 27, 2015 at 10:58:06PM +0200, Andy Shevchenko wrote:
> > On Sun, Dec 27, 2015 at 9:21 PM, Weng Xuetian <wengxt@gmail.com>
> wrote:
> > > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > > different from Surface Pro 3.
> > >
> > > This commit adds MSHW0040 to id list to support the Surface Pro 4,
> > > and renames the driver to surfacepro_button accordingly.
> > >
> > > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > > Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> >
> > Darren, this one looks fine for me, still couple of question up to you.
> >
> > First is do we really want to rename driver? (Renaming variables and
> > stuff like I said if fine to me)
> 
> Sorry, replied before seeing this.
> 
> I agree that renaming the file is probably not necessary (this works on v3+ as
> I understand it, so the name isn't really a problem, and dropping the 3
> suggests it works on v1 and v2).
> 
> I'd prefer to keep the name changing to a minimum, and add some
> information to the Kconfig help and driver comments making it clear that this
> supports 3+.
> 
[Yu] I agree, since I don't have a surface pro < 3 tested, there is no need to rename it.

thanks,
yu


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v3] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
  2015-12-27 20:58     ` Andy Shevchenko
  2016-01-04 20:11     ` Darren Hart
@ 2016-01-11 18:38     ` Darren Hart
  2016-01-12 19:43     ` [PATCH v4] " Weng Xuetian
  3 siblings, 0 replies; 17+ messages in thread
From: Darren Hart @ 2016-01-11 18:38 UTC (permalink / raw)
  To: Weng Xuetian; +Cc: Chen Yu, linux-kernel, platform-driver-x86

On Sun, Dec 27, 2015 at 11:21:20AM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4, and
> renames the driver to surfacepro_button accordingly.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>

Weng,

Given the feedback on renaming from Andy, Yu, and myself, I'm dropping this
patch and waiting for a v4 without the rename.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
  2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
                       ` (2 preceding siblings ...)
  2016-01-11 18:38     ` Darren Hart
@ 2016-01-12 19:43     ` Weng Xuetian
  2016-01-14 23:01       ` Darren Hart
  2016-01-17 23:10       ` [PATCH v5] " Weng Xuetian
  3 siblings, 2 replies; 17+ messages in thread
From: Weng Xuetian @ 2016-01-12 19:43 UTC (permalink / raw)
  To: Chen Yu; +Cc: Darren Hart, linux-kernel, platform-driver-x86, Weng Xuetian

Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
different from Surface Pro 3.

This commit adds MSHW0040 to id list to support the Surface Pro 4.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
---
v4:
 - Do not rename the driver.
v3:
 - Fix commit message grammar mistakes.
v2:
 - Reformat patch with -M -C
---
 drivers/platform/x86/Kconfig              | 4 ++--
 drivers/platform/x86/surfacepro3_button.c | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..ea76d67 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -940,8 +940,8 @@ config INTEL_PMC_IPC
 	with other entities in the CPU.
 
 config SURFACE_PRO3_BUTTON
-	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3 tablet"
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
 	depends on ACPI && INPUT
 	---help---
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
index f7dade3..2c0e7d7 100644
--- a/drivers/platform/x86/surfacepro3_button.c
+++ b/drivers/platform/x86/surfacepro3_button.c
@@ -1,6 +1,6 @@
 /*
  * power/home/volume button support for
- * Microsoft Surface Pro 3 tablet.
+ * Microsoft Surface Pro 3/4 tablet.
  *
  * Copyright (c) 2015 Intel Corporation.
  * All rights reserved.
@@ -19,7 +19,8 @@
 #include <linux/acpi.h>
 #include <acpi/button.h>
 
-#define SURFACE_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO3_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO4_BUTTON_HID		"MSHW0040"
 #define SURFACE_BUTTON_OBJ_NAME		"VGBI"
 #define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro 3 Buttons"
 
@@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
  * acpi_driver.
  */
 static const struct acpi_device_id surface_button_device_ids[] = {
-	{SURFACE_BUTTON_HID,    0},
+	{SURFACE_PRO3_BUTTON_HID,    0},
+	{SURFACE_PRO4_BUTTON_HID,    0},
 	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
-- 
2.6.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-12 19:43     ` [PATCH v4] " Weng Xuetian
@ 2016-01-14 23:01       ` Darren Hart
  2016-01-15  1:41         ` Chen, Yu C
  2016-01-15  1:50         ` Chen, Yu C
  2016-01-17 23:10       ` [PATCH v5] " Weng Xuetian
  1 sibling, 2 replies; 17+ messages in thread
From: Darren Hart @ 2016-01-14 23:01 UTC (permalink / raw)
  To: Weng Xuetian; +Cc: Chen Yu, linux-kernel, platform-driver-x86

On Tue, Jan 12, 2016 at 11:43:55AM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>

This is so much better, thank you Weng.

Yu, are you happy with this version?

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-14 23:01       ` Darren Hart
@ 2016-01-15  1:41         ` Chen, Yu C
  2016-01-15  1:50         ` Chen, Yu C
  1 sibling, 0 replies; 17+ messages in thread
From: Chen, Yu C @ 2016-01-15  1:41 UTC (permalink / raw)
  To: Darren Hart, Weng Xuetian
  Cc: linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org

Hi Darren,

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, January 15, 2016 7:01 AM
> To: Weng Xuetian
> Cc: Chen, Yu C; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> On Tue, Jan 12, 2016 at 11:43:55AM -0800, Weng Xuetian wrote:
> > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > different from Surface Pro 3.
> >
> > This commit adds MSHW0040 to id list to support the Surface Pro 4.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> 
> This is so much better, thank you Weng.
> 
> Yu, are you happy with this version?
> 
 Looks good to me.
Acked-by: Chen Yu <yu.c.chen@intel.com>

thanks,
Yu

^ permalink raw reply	[flat|nested] 17+ messages in thread

* RE: [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-14 23:01       ` Darren Hart
  2016-01-15  1:41         ` Chen, Yu C
@ 2016-01-15  1:50         ` Chen, Yu C
  1 sibling, 0 replies; 17+ messages in thread
From: Chen, Yu C @ 2016-01-15  1:50 UTC (permalink / raw)
  To: Darren Hart, Weng Xuetian
  Cc: linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org

Hi Darren, Xuetian

> -----Original Message-----
> From: Darren Hart [mailto:dvhart@infradead.org]
> Sent: Friday, January 15, 2016 7:01 AM
> To: Weng Xuetian
> Cc: Chen, Yu C; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org
> Subject: Re: [PATCH v4] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> On Tue, Jan 12, 2016 at 11:43:55AM -0800, Weng Xuetian wrote:
> > Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> > different from Surface Pro 3.
> >
> > This commit adds MSHW0040 to id list to support the Surface Pro 4.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> > Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> 
> This is so much better, thank you Weng.
> 
> Yu, are you happy with this version?
> 
One minor nit below, other than that,
Acked-by: Chen Yu <yu.c.chen@intel.com> 

-- #define SURFACE_BUTTON_DEVICE_NAME      "Surface Pro 3 Buttons"
++ #define SURFACE_BUTTON_DEVICE_NAME      "Surface Pro 3/4 Buttons"

And  

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v5] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-12 19:43     ` [PATCH v4] " Weng Xuetian
  2016-01-14 23:01       ` Darren Hart
@ 2016-01-17 23:10       ` Weng Xuetian
  2016-01-18  3:18         ` Chen, Yu C
  2016-01-19 21:00         ` Darren Hart
  1 sibling, 2 replies; 17+ messages in thread
From: Weng Xuetian @ 2016-01-17 23:10 UTC (permalink / raw)
  To: Chen Yu; +Cc: Darren Hart, linux-kernel, platform-driver-x86, Weng Xuetian

Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
different from Surface Pro 3.

This commit adds MSHW0040 to id list to support the Surface Pro 4.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
---
v5:
 - rename device name macro as required by Chen Yu
v4:
 - Do not rename the driver.
v3:
 - Fix commit message grammar mistakes.
v2:
 - Reformat patch with -M -C
---
 drivers/platform/x86/Kconfig              |  4 ++--
 drivers/platform/x86/surfacepro3_button.c | 10 ++++++----
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 1089eaa..ea76d67 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -940,8 +940,8 @@ config INTEL_PMC_IPC
 	with other entities in the CPU.
 
 config SURFACE_PRO3_BUTTON
-	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3 tablet"
+	tristate "Power/home/volume buttons driver for Microsoft Surface Pro 3/4 tablet"
 	depends on ACPI && INPUT
 	---help---
-	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3 tablet.
+	  This driver handles the power/home/volume buttons on the Microsoft Surface Pro 3/4 tablet.
 endif # X86_PLATFORM_DEVICES
diff --git a/drivers/platform/x86/surfacepro3_button.c b/drivers/platform/x86/surfacepro3_button.c
index f7dade3..b9c38f6 100644
--- a/drivers/platform/x86/surfacepro3_button.c
+++ b/drivers/platform/x86/surfacepro3_button.c
@@ -1,6 +1,6 @@
 /*
  * power/home/volume button support for
- * Microsoft Surface Pro 3 tablet.
+ * Microsoft Surface Pro 3/4 tablet.
  *
  * Copyright (c) 2015 Intel Corporation.
  * All rights reserved.
@@ -19,9 +19,10 @@
 #include <linux/acpi.h>
 #include <acpi/button.h>
 
-#define SURFACE_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO3_BUTTON_HID		"MSHW0028"
+#define SURFACE_PRO4_BUTTON_HID		"MSHW0040"
 #define SURFACE_BUTTON_OBJ_NAME		"VGBI"
-#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro 3 Buttons"
+#define SURFACE_BUTTON_DEVICE_NAME	"Surface Pro 3/4 Buttons"
 
 #define SURFACE_BUTTON_NOTIFY_PRESS_POWER	0xc6
 #define SURFACE_BUTTON_NOTIFY_RELEASE_POWER	0xc7
@@ -54,7 +55,8 @@ MODULE_LICENSE("GPL v2");
  * acpi_driver.
  */
 static const struct acpi_device_id surface_button_device_ids[] = {
-	{SURFACE_BUTTON_HID,    0},
+	{SURFACE_PRO3_BUTTON_HID,    0},
+	{SURFACE_PRO4_BUTTON_HID,    0},
 	{"", 0},
 };
 MODULE_DEVICE_TABLE(acpi, surface_button_device_ids);
-- 
2.7.0

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* RE: [PATCH v5] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-17 23:10       ` [PATCH v5] " Weng Xuetian
@ 2016-01-18  3:18         ` Chen, Yu C
  2016-01-19 21:00         ` Darren Hart
  1 sibling, 0 replies; 17+ messages in thread
From: Chen, Yu C @ 2016-01-18  3:18 UTC (permalink / raw)
  To: Weng Xuetian
  Cc: Darren Hart, linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org

> -----Original Message-----
> From: Weng Xuetian [mailto:wengxt@gmail.com]
> Sent: Monday, January 18, 2016 7:11 AM
> To: Chen, Yu C
> Cc: Darren Hart; linux-kernel@vger.kernel.org; platform-driver-
> x86@vger.kernel.org; Weng Xuetian
> Subject: [PATCH v5] surface pro 4: Add support for Surface Pro 4 Buttons
> 
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>

Acked-by: Chen Yu <yu.c.chen@intel.com>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v5] surface pro 4: Add support for Surface Pro 4 Buttons
  2016-01-17 23:10       ` [PATCH v5] " Weng Xuetian
  2016-01-18  3:18         ` Chen, Yu C
@ 2016-01-19 21:00         ` Darren Hart
  1 sibling, 0 replies; 17+ messages in thread
From: Darren Hart @ 2016-01-19 21:00 UTC (permalink / raw)
  To: Weng Xuetian; +Cc: Chen Yu, linux-kernel, platform-driver-x86

On Sun, Jan 17, 2016 at 03:10:38PM -0800, Weng Xuetian wrote:
> Surface Pro 4 buttons are managed by a device with _HID "MSHW0040"
> different from Surface Pro 3.
> 
> This commit adds MSHW0040 to id list to support the Surface Pro 4.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=109871
> Signed-off-by: Weng Xuetian <wengxt@gmail.com>
> ---
> v5:
>  - rename device name macro as required by Chen Yu

Queued up, thanks!
-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2016-01-19 21:00 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-24 20:28 [PATCH] surface pro 4: Add support for Surface Pro 4 Buttons Weng Xuetian
2015-12-27 13:36 ` Andy Shevchenko
2015-12-27 17:29 ` [PATCH v2] " Weng Xuetian
2015-12-27 19:07   ` Chen, Yu C
2015-12-27 19:21   ` [PATCH v3] " Weng Xuetian
2015-12-27 20:58     ` Andy Shevchenko
2016-01-04 20:35       ` Darren Hart
2016-01-05  0:00         ` Chen, Yu C
2016-01-04 20:11     ` Darren Hart
2016-01-11 18:38     ` Darren Hart
2016-01-12 19:43     ` [PATCH v4] " Weng Xuetian
2016-01-14 23:01       ` Darren Hart
2016-01-15  1:41         ` Chen, Yu C
2016-01-15  1:50         ` Chen, Yu C
2016-01-17 23:10       ` [PATCH v5] " Weng Xuetian
2016-01-18  3:18         ` Chen, Yu C
2016-01-19 21:00         ` Darren Hart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).