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 +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] surface pro 3: Add support driver for Surface Pro 3 buttons
Hi, Darren and Joe, On Wed, 2015-08-05 at 16:47 -0700, Darren Hart wrote: > On Thu, Aug 06, 2015 at 11:20:44AM +, Chen, Yu C wrote: > > The commentary is useful regardless. However, I suspect Joe was > referring to the approach pairing the PRESS and RELEASE cases? > I've wrote another piece of Macro to make it pairing the PRESS and RELEASE cases, would you please help check if it is suitable,thanks for your time. PS: I resend another patch titled with v2. Best Regards, Yu
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 +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] surface pro 3: Add support driver for Surface Pro 3 buttons
On Thu, Aug 06, 2015 at 11:20:44AM +, 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] surface pro 3: Add support driver for Surface Pro 3 buttons
On Thu, 2015-08-06 at 11:20 +, 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] surface pro 3: Add support driver for Surface Pro 3 buttons
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.
*/
> > + 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
>
OK, changed to dev_info_ratelimited
>
Best Regards,
Yu
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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[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
Tested-by: Peter Amidon
Tested-by: Donavan Lance
Signed-off-by: Chen Yu
---
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
+S:Supported
+F:drivers/platform/x86/surfacepro3_button.c
+
MICROTEK X6 SCANNER
M: Oliver Neukum
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 000..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
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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_POWER0xc7
+
+#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_UP0xc1
+
+#define SURFACE_BUTTON_NOTIFY_PRESS_VOLUME_DOWN0xc2
+#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);
+
+
