[PATCH v2 0/6] Input: soc_button_array fixes and support of the Surface 3

[PATCH v2 0/6] Input: soc_button_array fixes and support of the Surface 3

[Benjamin Tissoires]

Hi,

This is the v2 of the series with 3 extra patches to add support of the buttons
found on the Surface 3.

Cheers,
Benjamin


Benjamin Tissoires (6):
  Input - soc_button_array: use gpio_is_valid()
  Input - soc_button_array: bail out earlier if gpiod_count is null
  Input - soc_button_array: make sure one GPIO is not assigned twice
  Input - soc_button_array: allow to specify active_low
  Input - soc_button_array: export part of the internals
  Input - surface3_button_array: Introduce button support for the
    Surface 3

 drivers/input/misc/Kconfig                 |   9 ++
 drivers/input/misc/Makefile                |   1 +
 drivers/input/misc/soc_button_array.c      | 128 ++++++++++++++++++-----------
 drivers/input/misc/surface3_button_array.c | 115 ++++++++++++++++++++++++++
 include/linux/input/soc_button_array.h     |  37 +++++++++
 5 files changed, 240 insertions(+), 50 deletions(-)
 create mode 100644 drivers/input/misc/surface3_button_array.c
 create mode 100644 include/linux/input/soc_button_array.h

-- 
2.5.0

[PATCH v2 1/6] Input - soc_button_array: use gpio_is_valid()

[Benjamin Tissoires]

gpio_keys will later use gpio_is_valid(). To match the actual
behavior, we should use it here too.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v2:
- fixed gpio_is_valid(gpio) -> !gpio_is_valid(gpio)

 drivers/input/misc/soc_button_array.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index c14b827..bbd433c 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -17,6 +17,7 @@
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio_keys.h>
+#include <linux/gpio.h>
 #include <linux/platform_device.h>
 
 /*
@@ -92,7 +93,7 @@ soc_button_device_create(struct platform_device *pdev,
 			continue;
 
 		gpio = soc_button_lookup_gpio(&pdev->dev, info->acpi_index);
-		if (gpio < 0)
+		if (!gpio_is_valid(gpio))
 			continue;
 
 		gpio_keys[n_buttons].type = info->event_type;
-- 
2.5.0

[PATCH v2 2/6] Input - soc_button_array: bail out earlier if gpiod_count is null

[Benjamin Tissoires]

The PNP0C40 device of the Surface 3 doesn't have any GPIO attached to it.
Instead of trying to access the GPIO, request the count beforehand and
bail out if it is null or if an error is returned.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v2:
- hunk moved down for better integration with later patches

 drivers/input/misc/soc_button_array.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index bbd433c..5467d04 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -167,6 +167,11 @@ static int soc_button_probe(struct platform_device *pdev)
 
 	button_info = (struct soc_button_info *)id->driver_data;
 
+	if (gpiod_count(&pdev->dev, KBUILD_MODNAME) <= 0) {
+		dev_info(&pdev->dev, "no GPIO attached, ignoring...\n");
+		return -ENODEV;
+	}
+
 	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
-- 
2.5.0

[PATCH v2 3/6] Input - soc_button_array: make sure one GPIO is not assigned twice

[Benjamin Tissoires]

The Surface 3 declares twice the GPIO as GpioInt and GpioIo in its
ACPI table. Given that we do not keep the gpiod around, but the actual
number associated to, there is a chance while enumerating the GPIOs that
one gets assigned twice. Make sure a previous button has not been mapped
already to the current button to prevent such failure.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
changes in v2:
- no changes

 drivers/input/misc/soc_button_array.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5467d04..cb5cce3 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -77,7 +77,7 @@ soc_button_device_create(struct platform_device *pdev,
 	struct gpio_keys_platform_data *gpio_keys_pdata;
 	int n_buttons = 0;
 	int gpio;
-	int error;
+	int i, error;
 
 	gpio_keys_pdata = devm_kzalloc(&pdev->dev,
 				       sizeof(*gpio_keys_pdata) +
@@ -96,6 +96,13 @@ soc_button_device_create(struct platform_device *pdev,
 		if (!gpio_is_valid(gpio))
 			continue;
 
+		for (i = 0; i < n_buttons; i++) {
+			if (gpio_keys[i].gpio == gpio)
+				break;
+		}
+		if (i < n_buttons)
+			continue; /* the GPIO has already been assigned */
+
 		gpio_keys[n_buttons].type = info->event_type;
 		gpio_keys[n_buttons].code = info->event_code;
 		gpio_keys[n_buttons].gpio = gpio;
-- 
2.5.0

[PATCH v2 4/6] Input - soc_button_array: allow to specify active_low

[Benjamin Tissoires]

The Surface 3 has the "Windows" key active high, and not low. We need
a way to specify it in the description.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
new in v2

 drivers/input/misc/soc_button_array.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index cb5cce3..077e06e4 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -34,6 +34,7 @@ struct soc_button_info {
 	unsigned int event_code;
 	bool autorepeat;
 	bool wakeup;
+	bool active_low;
 };
 
 /*
@@ -106,7 +107,7 @@ soc_button_device_create(struct platform_device *pdev,
 		gpio_keys[n_buttons].type = info->event_type;
 		gpio_keys[n_buttons].code = info->event_code;
 		gpio_keys[n_buttons].gpio = gpio;
-		gpio_keys[n_buttons].active_low = 1;
+		gpio_keys[n_buttons].active_low = info->active_low;
 		gpio_keys[n_buttons].desc = info->name;
 		gpio_keys[n_buttons].wakeup = info->wakeup;
 		n_buttons++;
@@ -206,11 +207,11 @@ static int soc_button_probe(struct platform_device *pdev)
 }
 
 static struct soc_button_info soc_button_PNP0C40[] = {
-	{ "power", 0, EV_KEY, KEY_POWER, false, true },
-	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true },
-	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false },
-	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false },
-	{ "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false },
+	{ "power", 0, EV_KEY, KEY_POWER, false, true, true },
+	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true, true },
+	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false, true },
+	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false, true },
+	{ "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false, true },
 	{ }
 };
 
-- 
2.5.0

[PATCH v2 5/6] Input - soc_button_array: export part of the internals

[Benjamin Tissoires]

The MS Surfaces are not directly using the PNP0C40 device for the buttons
but instead a MSHW0028. This one is bound on a I2C bus and so we need to
use a separate driver for it.

One noticeable change is that the original soc_button_array fails
to rmmod and then modprobe when using KBUILD_MODNAME as the gpiod con_id
with the Surface 3. Add this as a parameter of the new API.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
new in v2

 drivers/input/misc/soc_button_array.c  | 108 +++++++++++++++++++--------------
 include/linux/input/soc_button_array.h |  37 +++++++++++
 2 files changed, 98 insertions(+), 47 deletions(-)
 create mode 100644 include/linux/input/soc_button_array.h

diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 077e06e4..96012f9 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -19,23 +19,7 @@
 #include <linux/gpio_keys.h>
 #include <linux/gpio.h>
 #include <linux/platform_device.h>
-
-/*
- * Definition of buttons on the tablet. The ACPI index of each button
- * is defined in section 2.8.7.2 of "Windows ACPI Design Guide for SoC
- * Platforms"
- */
-#define MAX_NBUTTONS	5
-
-struct soc_button_info {
-	const char *name;
-	int acpi_index;
-	unsigned int event_type;
-	unsigned int event_code;
-	bool autorepeat;
-	bool wakeup;
-	bool active_low;
-};
+#include <linux/input/soc_button_array.h>
 
 /*
  * Some of the buttons like volume up/down are auto repeat, while others
@@ -51,12 +35,13 @@ struct soc_button_data {
 /*
  * Get the Nth GPIO number from the ACPI object.
  */
-static int soc_button_lookup_gpio(struct device *dev, int acpi_index)
+static int soc_button_lookup_gpio(struct device *dev, const char *con_id,
+				  int acpi_index)
 {
 	struct gpio_desc *desc;
 	int gpio;
 
-	desc = gpiod_get_index(dev, KBUILD_MODNAME, acpi_index, GPIOD_ASIS);
+	desc = gpiod_get_index(dev, con_id, acpi_index, GPIOD_ASIS);
 	if (IS_ERR(desc))
 		return PTR_ERR(desc);
 
@@ -68,7 +53,8 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index)
 }
 
 static struct platform_device *
-soc_button_device_create(struct platform_device *pdev,
+soc_button_device_create(struct device *dev,
+			 const char *gpiod_con_id,
 			 const struct soc_button_info *button_info,
 			 bool autorepeat)
 {
@@ -80,7 +66,7 @@ soc_button_device_create(struct platform_device *pdev,
 	int gpio;
 	int i, error;
 
-	gpio_keys_pdata = devm_kzalloc(&pdev->dev,
+	gpio_keys_pdata = devm_kzalloc(dev,
 				       sizeof(*gpio_keys_pdata) +
 					sizeof(*gpio_keys) * MAX_NBUTTONS,
 				       GFP_KERNEL);
@@ -93,7 +79,9 @@ soc_button_device_create(struct platform_device *pdev,
 		if (info->autorepeat != autorepeat)
 			continue;
 
-		gpio = soc_button_lookup_gpio(&pdev->dev, info->acpi_index);
+		gpio = soc_button_lookup_gpio(dev,
+					      gpiod_con_id,
+					      info->acpi_index);
 		if (!gpio_is_valid(gpio))
 			continue;
 
@@ -142,14 +130,12 @@ soc_button_device_create(struct platform_device *pdev,
 err_free_pdev:
 	platform_device_put(pd);
 err_free_mem:
-	devm_kfree(&pdev->dev, gpio_keys_pdata);
+	devm_kfree(dev, gpio_keys_pdata);
 	return ERR_PTR(error);
 }
 
-static int soc_button_remove(struct platform_device *pdev)
+int soc_dev_button_remove(struct soc_button_data *priv)
 {
-	struct soc_button_data *priv = platform_get_drvdata(pdev);
-
 	int i;
 
 	for (i = 0; i < BUTTON_TYPES; i++)
@@ -158,40 +144,28 @@ static int soc_button_remove(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(soc_dev_button_remove);
 
-static int soc_button_probe(struct platform_device *pdev)
+int soc_dev_button_enumerate(struct device *dev, struct soc_button_data *priv,
+			     const char *gpiod_con_id,
+			     struct soc_button_info *button_info)
 {
-	struct device *dev = &pdev->dev;
-	const struct acpi_device_id *id;
-	struct soc_button_info *button_info;
-	struct soc_button_data *priv;
 	struct platform_device *pd;
 	int i;
 	int error;
 
-	id = acpi_match_device(dev->driver->acpi_match_table, dev);
-	if (!id)
-		return -ENODEV;
-
-	button_info = (struct soc_button_info *)id->driver_data;
-
-	if (gpiod_count(&pdev->dev, KBUILD_MODNAME) <= 0) {
-		dev_info(&pdev->dev, "no GPIO attached, ignoring...\n");
+	if (gpiod_count(dev, gpiod_con_id) <= 0) {
+		dev_info(dev, "no GPIO attached, ignoring...\n");
 		return -ENODEV;
 	}
 
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
-	platform_set_drvdata(pdev, priv);
-
 	for (i = 0; i < BUTTON_TYPES; i++) {
-		pd = soc_button_device_create(pdev, button_info, i == 0);
+		pd = soc_button_device_create(dev, gpiod_con_id, button_info,
+					      i == 0);
 		if (IS_ERR(pd)) {
 			error = PTR_ERR(pd);
 			if (error != -ENODEV) {
-				soc_button_remove(pdev);
+				soc_dev_button_remove(priv);
 				return error;
 			}
 			continue;
@@ -205,6 +179,46 @@ static int soc_button_probe(struct platform_device *pdev)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(soc_dev_button_enumerate);
+
+struct soc_button_data *soc_dev_button_data_allocate(struct device *dev)
+{
+	return devm_kzalloc(dev, sizeof(struct soc_button_data), GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(soc_dev_button_data_allocate);
+
+static int soc_button_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct acpi_device_id *id;
+	struct soc_button_info *button_info;
+	struct soc_button_data *priv;
+	int error;
+
+	id = acpi_match_device(dev->driver->acpi_match_table, dev);
+	if (!id)
+		return -ENODEV;
+
+	button_info = (struct soc_button_info *)id->driver_data;
+
+	priv = soc_dev_button_data_allocate(dev);
+	if (!priv)
+		return -ENOMEM;
+
+	error = soc_dev_button_enumerate(dev, priv, KBUILD_MODNAME,
+					 button_info);
+	if (error)
+		return error;
+
+	platform_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static int soc_button_remove(struct platform_device *pdev)
+{
+	return soc_dev_button_remove(platform_get_drvdata(pdev));
+}
 
 static struct soc_button_info soc_button_PNP0C40[] = {
 	{ "power", 0, EV_KEY, KEY_POWER, false, true, true },
diff --git a/include/linux/input/soc_button_array.h b/include/linux/input/soc_button_array.h
new file mode 100644
index 0000000..634ce90
--- /dev/null
+++ b/include/linux/input/soc_button_array.h
@@ -0,0 +1,37 @@
+/*
+ * Supports for the button array on SoC tablets originally running
+ * Windows 8.
+ *
+ * (C) Copyright 2014 Intel Corporation
+ * (C) Copyright 2016 Red Hat, Inc
+ *
+ * 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.
+ */
+
+/*
+ * Definition of buttons on the tablet. The ACPI index of each button
+ * is defined in section 2.8.7.2 of "Windows ACPI Design Guide for SoC
+ * Platforms"
+ */
+#define MAX_NBUTTONS	5
+
+struct soc_button_info {
+	const char *name;
+	int acpi_index;
+	unsigned int event_type;
+	unsigned int event_code;
+	bool autorepeat;
+	bool wakeup;
+	bool active_low;
+};
+
+struct soc_button_data;
+
+struct soc_button_data *soc_dev_button_data_allocate(struct device *dev);
+int soc_dev_button_remove(struct soc_button_data *priv);
+int soc_dev_button_enumerate(struct device *dev, struct soc_button_data *priv,
+			     const char *gpiod_con_id,
+			     struct soc_button_info *button_info);
-- 
2.5.0

[PATCH v2 6/6] Input - surface3_button_array: Introduce button support for the Surface 3

[Benjamin Tissoires]

The Surface 3 is not following the ACPI spec for PNP0C40, but nearly.
The device is connected to a I2C device that might have some magic
but we don't know about.
Just create the device after the enumeration and use the declared GPIOs
to provide button support.

The Surface Pro 3 is using an ACPI driver and matches against the bid
of the device ("VGBI") but I think it also could use this. To prevent
this driver to be used on the Surface Pro, we add a match on the
Surface 3 bid "TEV2".

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
new in v2

 drivers/input/misc/Kconfig                 |   9 +++
 drivers/input/misc/Makefile                |   1 +
 drivers/input/misc/surface3_button_array.c | 115 +++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/input/misc/surface3_button_array.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 1f2337a..17c2205 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -762,6 +762,15 @@ config INPUT_SOC_BUTTON_ARRAY
 	  To compile this driver as a module, choose M here: the
 	  module will be called soc_button_array.
 
+config INPUT_SURFACE3_BUTTON_ARRAY
+	tristate "Microsoft Surface 3 SoC Button Array"
+	depends on INPUT_SOC_BUTTON_ARRAY
+	help
+	  Say Y here if you have a MS Surface tablet.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called surface3_button_array.
+
 config INPUT_DRV260X_HAPTICS
 	tristate "TI DRV260X haptics support"
 	depends on INPUT && I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 0357a08..1624a23 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_INPUT_SGI_BTNS)		+= sgi_btns.o
 obj-$(CONFIG_INPUT_SIRFSOC_ONKEY)	+= sirfsoc-onkey.o
 obj-$(CONFIG_INPUT_SOC_BUTTON_ARRAY)	+= soc_button_array.o
 obj-$(CONFIG_INPUT_SPARCSPKR)		+= sparcspkr.o
+obj-$(CONFIG_INPUT_SURFACE3_BUTTON_ARRAY)	+= surface3_button_array.o
 obj-$(CONFIG_INPUT_TPS65218_PWRBUTTON)	+= tps65218-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_VIBRA)	+= twl4030-vibra.o
diff --git a/drivers/input/misc/surface3_button_array.c b/drivers/input/misc/surface3_button_array.c
new file mode 100644
index 0000000..b48307c
--- /dev/null
+++ b/drivers/input/misc/surface3_button_array.c
@@ -0,0 +1,115 @@
+/*
+ * Supports for the button array on the Surface tablets.
+ *
+ * (C) Copyright 2016 Red Hat, Inc
+ *
+ * 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/i2c.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <linux/input/soc_button_array.h>
+#include <uapi/linux/input-event-codes.h>
+
+struct soc_device_info {
+	const char * const *obj_names;
+	struct soc_button_info *buttons;
+};
+
+static int surface3_probe(struct i2c_client *client,
+			   const struct i2c_device_id *id)
+{
+	const char *bid, *obj_name;
+	struct soc_device_info *device_info;
+	struct soc_button_data *priv;
+	int error;
+	int i;
+
+	if (!id->driver_data)
+		return -EINVAL;
+
+	device_info = (struct soc_device_info *)id->driver_data;
+
+	if (device_info->obj_names) {
+		bid = acpi_device_bid(ACPI_COMPANION(&client->dev));
+		i = 0;
+		do {
+			obj_name = device_info->obj_names[i++];
+			if (obj_name && !strcmp(bid, obj_name))
+				break;
+		} while (obj_name);
+		/* no acpi_device_bid match, bail out */
+		if (!obj_name)
+			return -ENODEV;
+	}
+
+	priv = soc_dev_button_data_allocate(&client->dev);
+	if (!priv)
+		return -ENOMEM;
+
+	error = soc_dev_button_enumerate(&client->dev, priv, NULL,
+					 device_info->buttons);
+	if (error)
+		return error;
+
+	i2c_set_clientdata(client, priv);
+
+	return 0;
+}
+
+static int surface3_remove(struct i2c_client *client)
+{
+	return soc_dev_button_remove(i2c_get_clientdata(client));
+}
+
+static struct soc_button_info soc_button_surface3[] = {
+	{ "power", 0, EV_KEY, KEY_POWER, false, true, true },
+	{ "home", 1, EV_KEY, KEY_LEFTMETA, false, true, false },
+	{ "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false, true },
+	{ "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false, true },
+	{ }
+};
+
+static const char * const soc_device_obj_names_surface3[] = {
+	"TEV2",
+	0,
+};
+
+static const struct soc_device_info soc_device_surface3 = {
+	.obj_names = soc_device_obj_names_surface3,
+	.buttons = soc_button_surface3,
+};
+
+static const struct i2c_device_id surface3_id[] = {
+	{ "MSHW0028:00", (unsigned long)&soc_device_surface3 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, surface3_id);
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id surface3_acpi_match[] = {
+	{ "MSHW0028", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, surface3_acpi_match);
+#endif
+
+static struct i2c_driver surface3_driver = {
+	.probe = surface3_probe,
+	.remove = surface3_remove,
+	.id_table = surface3_id,
+	.driver = {
+		.name = "surface3",
+		.acpi_match_table = ACPI_PTR(surface3_acpi_match),
+	},
+};
+module_i2c_driver(surface3_driver);
+
+MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
+MODULE_DESCRIPTION("surface3 button array driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

Re: [PATCH v2 0/6] Input: soc_button_array fixes and support of the Surface 3

[Bastien Nocera]

On Fri, 2016-05-13 at 18:05 +0200, Benjamin Tissoires wrote:
> Hi,
> 
> This is the v2 of the series with 3 extra patches to add support of
> the buttons
> found on the Surface 3.
> 
> Cheers,
> Benjamin
> 
> 
> Benjamin Tissoires (6):
>   Input - soc_button_array: use gpio_is_valid()
>   Input - soc_button_array: bail out earlier if gpiod_count is null
>   Input - soc_button_array: make sure one GPIO is not assigned twice
>   Input - soc_button_array: allow to specify active_low
>   Input - soc_button_array: export part of the internals
>   Input - surface3_button_array: Introduce button support for the
>     Surface 3

Works on my Surface 3, the volume buttons, and the Windows button work
as expected. Note that the power button does not send out any events on
my machine, but that might be due to firmware differences.

My BIOS is listed as version 1.51116.78 released 03/09/2015.

As there's no errors generated, it would be fine to look at it as a bug
fix later on (or for me to updated my system).

Tested-by: Bastien Nocera <hadess@hadess.net>

Re: [PATCH v2 0/6] Input: soc_button_array fixes and support of the Surface 3

[Benjamin Tissoires]

On May 17 2016 or thereabouts, Bastien Nocera wrote:
> On Fri, 2016-05-13 at 18:05 +0200, Benjamin Tissoires wrote:
> > Hi,
> > 
> > This is the v2 of the series with 3 extra patches to add support of
> > the buttons
> > found on the Surface 3.
> > 
> > Cheers,
> > Benjamin
> > 
> > 
> > Benjamin Tissoires (6):
> >   Input - soc_button_array: use gpio_is_valid()
> >   Input - soc_button_array: bail out earlier if gpiod_count is null
> >   Input - soc_button_array: make sure one GPIO is not assigned twice
> >   Input - soc_button_array: allow to specify active_low
> >   Input - soc_button_array: export part of the internals
> >   Input - surface3_button_array: Introduce button support for the
> >     Surface 3
> 
> Works on my Surface 3, the volume buttons, and the Windows button work
> as expected. Note that the power button does not send out any events on
> my machine, but that might be due to firmware differences.
> 
> My BIOS is listed as version 1.51116.78 released 03/09/2015.
> 
> As there's no errors generated, it would be fine to look at it as a bug
> fix later on (or for me to updated my system).
> 
> Tested-by: Bastien Nocera <hadess@hadess.net>

Bastien told me in a different thread that the power button is not
working only after a reboot from Windows. After a cold start, it works
fine. There might be something that needs to be set at the I2C level,
but I think while we don't know much about the I2C protocol there, it
should be safe to still get this upstream.

Cheers,
Benjamin

Re: [PATCH v2 0/6] Input: soc_button_array fixes and support of the Surface 3

[Bastien Nocera]

On Fri, 2016-05-20 at 09:53 +0200, Benjamin Tissoires wrote:
> 
<snip>
> Bastien told me in a different thread that the power button is not
> working only after a reboot from Windows. After a cold start, it
> works
> fine.

Not quite. It worked after a hot reboot from Windows another time.

>  There might be something that needs to be set at the I2C level,
> but I think while we don't know much about the I2C protocol there, it
> should be safe to still get this upstream.

I don't know what the problem could be, but it seems safe enough for
now.

Cheers