Skip to contentSkip to conversation

input: misc: Add driver for Intel Bay Trail GPIO buttons

PATCH Zhu, Lejun 3 participants 3 messages Mar 31, 2014 – Apr 20, 2014 1 message not in the archive

Subsystems (2)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Zhu, Lejun

author
#1 lore raw

Hi Dmitry,

Quoted 15 lines · Dmitry Torokhov

Hi Lejun,

Review

Quoted 7 lines · Lejun
(quoted diff) +7 -0
+
+static struct soc_button_info soc_button_tbl[] = {
+	{"power", 0, KEY_POWER, 0, 1, -1},
+	{"home", 1, KEY_HOME, 0, 1, -1},
+	{"volume_up", 2, KEY_VOLUMEUP, 1, 0, -1},
+	{"volume_down", 3, KEY_VOLUMEDOWN, 1, 0, -1},
+	{"rotation_lock", 4, KEY_RO, 0, 0, -1},

KEY_RO is not for rotating but rather for switching one of Japanese layouts. I think we shoudl allow using switchws and use SW_ROTATE_LOCK for that.

Quoted 5 lines

Also I do not quite like so many static structures, what do you think about the version below?

Thanks!

Your version looks fine. I have tested it and fixed a couple glitches (+1 vs ++ at line 87, and check gpio < 0 at line 94).

Also, yu-sheng Chen suggests that we'd better look for PNP0C40, which is the compatible ID defined in the spec and will probably match a broader range of devices.

I attached the updated version below. What do you think?

Thanks.
Lejun
--

Input: misc - Add driver for Intel Bay Trail GPIO buttons

From: Lejun Zhu <[email protected]>

This patch adds support for the GPIO buttons on some Intel Bay Trail tablets originally running Windows 8. The ACPI description of these buttons follows "Windows ACPI Design Guide for SoC Platforms".

---
 drivers/input/misc/Kconfig            |  10 ++
 drivers/input/misc/Makefile           |   1 +
 drivers/input/misc/soc_button_array.c | 218
++++++++++++++++++++++++++++++++++
 3 files changed, 229 insertions(+)
 create mode 100644 drivers/input/misc/soc_button_array.c
drivers/input/misc/Kconfig +10 -0
index 7904ab0..c1298af 100644
@@ -666,4 +666,14 @@ config INPUT_IDEAPAD_SLIDEBAR
 	  To compile this driver as a module, choose M here: the
 	  module will be called ideapad_slidebar.
 
+config INPUT_SOC_BUTTON_ARRAY
+	tristate "Windows-compatible SoC Button Array"
+	depends on KEYBOARD_GPIO
+	help
+	  Say Y here if you have a SoC-based tablet that originally
+	  runs Windows 8.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called soc_button_array.
+
 endif
drivers/input/misc/Makefile +1 -0
index cda71fc..4955ad3 100644
@@ -53,6 +53,7 @@ obj-$(CONFIG_INPUT_RETU_PWRBUTTON)	+= retu-pwrbutton.o
 obj-$(CONFIG_INPUT_GPIO_ROTARY_ENCODER)	+= rotary_encoder.o
 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_TWL4030_PWRBUTTON)	+= twl4030-pwrbutton.o
 obj-$(CONFIG_INPUT_TWL4030_VIBRA)	+= twl4030-vibra.o
(quoted diff) +0 -0

b/drivers/input/misc/soc_button_array.c
new file mode 100644
index 0000000..08ead2a

drivers/input/misc/soc_button_array.c new file +218 -0

218 lines not on this page · raw message

#2 lore raw

Hi!

Quoted 8 lines

Input: misc - Add driver for Intel Bay Trail GPIO buttons

From: Lejun Zhu <[email protected]>

This patch adds support for the GPIO buttons on some Intel Bay Trail tablets originally running Windows 8. The ACPI description of these buttons follows "Windows ACPI Design Guide for SoC Platforms".

Hmm. Is it time for x86 to adopt device tree? Because this is 200 lines of C code which should really have been 10 lines of .dts...

drivers/input/misc/soc_button_array.c line 49

Quoted 11 lines
drivers/input/misc/soc_button_array.c new file +11 -0
+
+/*
+ * Some of the buttons like volume up/down are auto repeat, while others
+ * are not. To support both, we register two platform devices, and put
+ * buttons into them based on whether the key should be auto repeat.
+ */
+#define BUTTON_TYPES	2
+
+struct soc_button_data {
+	struct platform_device *children[BUTTON_TYPES];
+};

Would it be possible to extend device description so that this hack is not needed?
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
maintainer
#3 lore raw
Quoted 13 lines · Pavel Machek

Hi!

Quoted 8 lines

Input: misc - Add driver for Intel Bay Trail GPIO buttons

From: Lejun Zhu <[email protected]>

This patch adds support for the GPIO buttons on some Intel Bay Trail tablets originally running Windows 8. The ACPI description of these buttons follows "Windows ACPI Design Guide for SoC Platforms".

Hmm. Is it time for x86 to adopt device tree? Because this is 200 lines of C code which should really have been 10 lines of .dts...

Quoted 14 lines

drivers/input/misc/soc_button_array.c line 49

Quoted 11 lines
drivers/input/misc/soc_button_array.c new file +11 -0
+
+/*
+ * Some of the buttons like volume up/down are auto repeat, while others
+ * are not. To support both, we register two platform devices, and put
+ * buttons into them based on whether the key should be auto repeat.
+ */
+#define BUTTON_TYPES	2
+
+struct soc_button_data {
+	struct platform_device *children[BUTTON_TYPES];
+};

Would it be possible to extend device description so that this hack is not needed?

It would be not that easy as that would mean we need to control autorepeat (and thus control repeat delay/repeat rate and also report them) on a per-key basis.

Having 2 separate devices, one offering autorepeat and another not using it, is much simpler.

Thanks.

Signature · 1 line
Dmitry