* [PATCH 0/5] Add ST Keyscan driver
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Gabriel Fernandez
The goal of this series is to add ST Keyscan support to ST SoCs.
The DT definition is added for STiH415 and STiH416 SoCs on
B2000 board.
Gabriel Fernandez (4):
drivers: input: keyboard: st-keyscan: add keyscan driver
ARM: STi: DT: add keyscan for stih416
ARM: STi: DT: add keyscan for stih41x-b2000
ARM: multi_v7_defconfig: add st-keyscan driver
Giuseppe CONDORELLI (1):
ARM: STi: DT: add keyscan for stih415
.../devicetree/bindings/input/st-keypad.txt | 50 ++++
arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 +
arch/arm/boot/dts/stih415.dtsi | 10 +
arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 +
arch/arm/boot/dts/stih416.dtsi | 10 +
arch/arm/boot/dts/stih41x-b2000.dtsi | 12 +
arch/arm/configs/multi_v7_defconfig | 1 +
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
10 files changed, 451 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
create mode 100644 drivers/input/keyboard/st-keyscan.c
--
1.9.0
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 0/5] Add ST Keyscan driver
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
The goal of this series is to add ST Keyscan support to ST SoCs.
The DT definition is added for STiH415 and STiH416 SoCs on
B2000 board.
Gabriel Fernandez (4):
drivers: input: keyboard: st-keyscan: add keyscan driver
ARM: STi: DT: add keyscan for stih416
ARM: STi: DT: add keyscan for stih41x-b2000
ARM: multi_v7_defconfig: add st-keyscan driver
Giuseppe CONDORELLI (1):
ARM: STi: DT: add keyscan for stih415
.../devicetree/bindings/input/st-keypad.txt | 50 ++++
arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 +
arch/arm/boot/dts/stih415.dtsi | 10 +
arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 +
arch/arm/boot/dts/stih416.dtsi | 10 +
arch/arm/boot/dts/stih41x-b2000.dtsi | 12 +
arch/arm/configs/multi_v7_defconfig | 1 +
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
10 files changed, 451 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
create mode 100644 drivers/input/keyboard/st-keyscan.c
--
1.9.0
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-05 3:39 ` Gabriel FERNANDEZ
(?)
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Gabriel Fernandez,
Giuseppe Condorelli
This patch adds ST Keyscan driver to use the keypad hw a subset
of ST boards provide. Specific board setup will be put in the
given dt.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
.../devicetree/bindings/input/st-keypad.txt | 50 ++++
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
4 files changed, 386 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
create mode 100644 drivers/input/keyboard/st-keyscan.c
diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
new file mode 100644
index 0000000..63eb07a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/st-keypad.txt
@@ -0,0 +1,50 @@
+* ST Keypad controller device tree bindings
+
+The ST keypad controller device tree binding is based on the
+matrix-keymap.
+
+Required properties:
+
+- compatible: "st,keypad"
+
+- reg: Register base address of st-keypad controler.
+
+- interrupts: Interrupt numberof st-keypad controler.
+
+- clocks: Must contain one entry, for the module clock.
+ See ../clocks/clock-bindings.txt for details.
+
+- pinctrl-0: Should specify pin control groups used for this controller.
+
+- pinctrl-names: Should contain only one value - "default".
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+- keypad,num-rows: Number of row lines connected to the keypad controller.
+
+- keypad,num-columns: Number of column lines connected to the keypad
+ controller.
+
+- st,debounce_us: Debouncing interval time in microseconds
+
+
+Example:
+
+keyscan: keyscan@fe4b0000 {
+ compatible = "st,keypad";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+
+ keypad,num-rows = <4>;
+ keypad,num-columns = <4>;
+ st,debounce_us = <5000>;
+ linux,keymap = < /* in0 in1 in2 in3 */
+ KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
+ KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
+ KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
+ KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
+};
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a673c9f..9e29125 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
To compile this driver as a module, choose M here: the
module will be called stowaway.
+config KEYBOARD_ST_KEYSCAN
+ tristate "STMicroelectronics keyscan support"
+ depends on ARCH_STI
+ select INPUT_EVDEV
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use a keypad attached to the keyscan block
+ on some STMicroelectronics SoC devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stm-keyscan.
+
config KEYBOARD_SUNKBD
tristate "Sun Type 4 and Type 5 keyboard"
select SERIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index a699b61..5fd020a 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
+obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
new file mode 100644
index 0000000..606cc19
--- /dev/null
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -0,0 +1,323 @@
+/*
+ * STMicroelectronics Key Scanning driver
+ *
+ * Copyright (c) 2014 STMicroelectonics Ltd.
+ * Author: Stuart Menefy <stuart.menefy@st.com>
+ *
+ * Based on sh_keysc.c, copyright 2008 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/input/matrix_keypad.h>
+
+#define ST_KEYSCAN_MAXKEYS 16
+
+#define KEYSCAN_CONFIG_OFF 0x000
+#define KEYSCAN_CONFIG_ENABLE 1
+#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
+#define KEYSCAN_MATRIX_STATE_OFF 0x008
+#define KEYSCAN_MATRIX_DIM_OFF 0x00c
+#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
+#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
+
+struct keypad_platform_data {
+ const struct matrix_keymap_data *keymap_data;
+ unsigned int num_out_pads;
+ unsigned int num_in_pads;
+ unsigned int debounce_us;
+};
+
+struct keyscan_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *clk;
+ struct input_dev *input_dev;
+ struct keypad_platform_data *config;
+ unsigned int last_state;
+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
+};
+
+static irqreturn_t keyscan_isr(int irq, void *dev_id)
+{
+ struct platform_device *pdev = dev_id;
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+ int state;
+ int change;
+
+ state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
+ change = priv->last_state ^ state;
+
+ while (change) {
+ int scancode = __ffs(change);
+ int down = state & (1 << scancode);
+
+ input_report_key(priv->input_dev, priv->keycodes[scancode],
+ down);
+ change ^= 1 << scancode;
+ };
+
+ input_sync(priv->input_dev);
+
+ priv->last_state = state;
+
+ return IRQ_HANDLED;
+}
+
+static void keyscan_start(struct keyscan_priv *priv)
+{
+ clk_enable(priv->clk);
+
+ writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
+ priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
+
+ writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
+ ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
+ priv->base + KEYSCAN_MATRIX_DIM_OFF);
+
+ writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
+}
+
+static void keyscan_stop(struct keyscan_priv *priv)
+{
+ writel(0, priv->base + KEYSCAN_CONFIG_OFF);
+
+ clk_disable(priv->clk);
+}
+
+static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
+{
+ struct device *dev = st_kp->input_dev->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct keypad_platform_data *pdata;
+ int error;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(dev, "failed to allocate driver pdata\n");
+ return -ENOMEM;
+ }
+
+ error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
+ &pdata->num_in_pads);
+ if (error) {
+ dev_err(dev, "failed to parse keypad params\n");
+ return error;
+ }
+
+ of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
+
+ st_kp->config = pdata;
+
+ dev_info(dev, "rows=%d col=%d debounce=%d\n",
+ pdata->num_out_pads,
+ pdata->num_in_pads,
+ pdata->debounce_us);
+
+ error = of_property_read_u32_array(np, "linux,keymap",
+ st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
+ if (error) {
+ dev_err(dev, "failed to parse keymap\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int __init keyscan_probe(struct platform_device *pdev)
+{
+ struct keypad_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct keyscan_priv *st_kp;
+ struct input_dev *input_dev;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int len;
+ int error;
+ int i;
+
+ if (!pdata && !pdev->dev.of_node) {
+ dev_err(&pdev->dev, "no keymap defined\n");
+ return -EINVAL;
+ }
+
+ st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
+ if (!st_kp) {
+ dev_err(dev, "failed to allocate driver data\n");
+ return -ENOMEM;
+ }
+ st_kp->config = pdata;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "no I/O memory specified\n");
+ return -ENXIO;
+ }
+
+ len = resource_size(res);
+ if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
+ dev_err(dev, "failed to reserve I/O memory\n");
+ return -EBUSY;
+ }
+
+ st_kp->base = devm_ioremap_nocache(dev, res->start, len);
+ if (st_kp->base == NULL) {
+ dev_err(dev, "failed to remap I/O memory\n");
+ return -ENXIO;
+ }
+
+ st_kp->irq = platform_get_irq(pdev, 0);
+ if (st_kp->irq < 0) {
+ dev_err(dev, "no IRQ specified\n");
+ return -ENXIO;
+ }
+
+ error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
+ pdev->name, pdev);
+ if (error) {
+ dev_err(dev, "failed to request IRQ\n");
+ return error;
+ }
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev) {
+ dev_err(&pdev->dev, "failed to allocate the input device\n");
+ return -ENOMEM;
+ }
+
+ st_kp->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(st_kp->clk)) {
+ dev_err(dev, "cannot get clock");
+ return PTR_ERR(st_kp->clk);
+ }
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ dev_err(dev, "failed to allocate input device\n");
+ return -ENOMEM;
+ }
+ st_kp->input_dev = input_dev;
+
+ input_dev->name = pdev->name;
+ input_dev->phys = "keyscan-keys/input0";
+ input_dev->dev.parent = dev;
+
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->id.vendor = 0x0001;
+ input_dev->id.product = 0x0001;
+ input_dev->id.version = 0x0100;
+
+ if (!pdata) {
+ error = keypad_matrix_key_parse_dt(st_kp);
+ if (error)
+ return error;
+ pdata = st_kp->config;
+ }
+
+ input_dev->keycode = st_kp->keycodes;
+ input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
+ input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
+
+ for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
+ input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
+ __clear_bit(KEY_RESERVED, input_dev->keybit);
+
+ error = input_register_device(input_dev);
+ if (error) {
+ dev_err(dev, "failed to register input device\n");
+ input_free_device(input_dev);
+ platform_set_drvdata(pdev, NULL);
+ return error;
+ }
+
+ platform_set_drvdata(pdev, st_kp);
+
+ keyscan_start(st_kp);
+
+ device_set_wakeup_capable(&pdev->dev, 1);
+
+ return 0;
+}
+
+static int __exit keyscan_remove(struct platform_device *pdev)
+{
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ keyscan_stop(priv);
+
+ input_unregister_device(priv->input_dev);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static int keyscan_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(priv->irq);
+ else
+ keyscan_stop(priv);
+
+ return 0;
+}
+
+static int keyscan_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(priv->irq);
+ else
+ keyscan_start(priv);
+
+ return 0;
+}
+
+static const struct dev_pm_ops keyscan_dev_pm_ops = {
+ .suspend = keyscan_suspend,
+ .resume = keyscan_resume,
+};
+
+static const struct of_device_id keyscan_of_match[] = {
+ { .compatible = "st,keypad" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, keyscan_of_match);
+
+__refdata struct platform_driver keyscan_device_driver = {
+ .probe = keyscan_probe,
+ .remove = __exit_p(keyscan_remove),
+ .driver = {
+ .name = "st-keyscan",
+ .pm = &keyscan_dev_pm_ops,
+ .of_match_table = of_match_ptr(keyscan_of_match),
+ }
+};
+
+static int __init keyscan_init(void)
+{
+ return platform_driver_register(&keyscan_device_driver);
+}
+
+static void __exit keyscan_exit(void)
+{
+ platform_driver_unregister(&keyscan_device_driver);
+}
+
+module_init(keyscan_init);
+module_exit(keyscan_exit);
+
+MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
+MODULE_LICENSE("GPL");
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, kernel, linux-doc, linux-kernel, Giuseppe Condorelli,
linux-input, Lee Jones, Gabriel Fernandez, linux-arm-kernel
This patch adds ST Keyscan driver to use the keypad hw a subset
of ST boards provide. Specific board setup will be put in the
given dt.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
.../devicetree/bindings/input/st-keypad.txt | 50 ++++
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
4 files changed, 386 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
create mode 100644 drivers/input/keyboard/st-keyscan.c
diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
new file mode 100644
index 0000000..63eb07a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/st-keypad.txt
@@ -0,0 +1,50 @@
+* ST Keypad controller device tree bindings
+
+The ST keypad controller device tree binding is based on the
+matrix-keymap.
+
+Required properties:
+
+- compatible: "st,keypad"
+
+- reg: Register base address of st-keypad controler.
+
+- interrupts: Interrupt numberof st-keypad controler.
+
+- clocks: Must contain one entry, for the module clock.
+ See ../clocks/clock-bindings.txt for details.
+
+- pinctrl-0: Should specify pin control groups used for this controller.
+
+- pinctrl-names: Should contain only one value - "default".
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+- keypad,num-rows: Number of row lines connected to the keypad controller.
+
+- keypad,num-columns: Number of column lines connected to the keypad
+ controller.
+
+- st,debounce_us: Debouncing interval time in microseconds
+
+
+Example:
+
+keyscan: keyscan@fe4b0000 {
+ compatible = "st,keypad";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+
+ keypad,num-rows = <4>;
+ keypad,num-columns = <4>;
+ st,debounce_us = <5000>;
+ linux,keymap = < /* in0 in1 in2 in3 */
+ KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
+ KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
+ KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
+ KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
+};
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a673c9f..9e29125 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
To compile this driver as a module, choose M here: the
module will be called stowaway.
+config KEYBOARD_ST_KEYSCAN
+ tristate "STMicroelectronics keyscan support"
+ depends on ARCH_STI
+ select INPUT_EVDEV
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use a keypad attached to the keyscan block
+ on some STMicroelectronics SoC devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stm-keyscan.
+
config KEYBOARD_SUNKBD
tristate "Sun Type 4 and Type 5 keyboard"
select SERIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index a699b61..5fd020a 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
+obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
new file mode 100644
index 0000000..606cc19
--- /dev/null
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -0,0 +1,323 @@
+/*
+ * STMicroelectronics Key Scanning driver
+ *
+ * Copyright (c) 2014 STMicroelectonics Ltd.
+ * Author: Stuart Menefy <stuart.menefy@st.com>
+ *
+ * Based on sh_keysc.c, copyright 2008 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/input/matrix_keypad.h>
+
+#define ST_KEYSCAN_MAXKEYS 16
+
+#define KEYSCAN_CONFIG_OFF 0x000
+#define KEYSCAN_CONFIG_ENABLE 1
+#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
+#define KEYSCAN_MATRIX_STATE_OFF 0x008
+#define KEYSCAN_MATRIX_DIM_OFF 0x00c
+#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
+#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
+
+struct keypad_platform_data {
+ const struct matrix_keymap_data *keymap_data;
+ unsigned int num_out_pads;
+ unsigned int num_in_pads;
+ unsigned int debounce_us;
+};
+
+struct keyscan_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *clk;
+ struct input_dev *input_dev;
+ struct keypad_platform_data *config;
+ unsigned int last_state;
+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
+};
+
+static irqreturn_t keyscan_isr(int irq, void *dev_id)
+{
+ struct platform_device *pdev = dev_id;
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+ int state;
+ int change;
+
+ state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
+ change = priv->last_state ^ state;
+
+ while (change) {
+ int scancode = __ffs(change);
+ int down = state & (1 << scancode);
+
+ input_report_key(priv->input_dev, priv->keycodes[scancode],
+ down);
+ change ^= 1 << scancode;
+ };
+
+ input_sync(priv->input_dev);
+
+ priv->last_state = state;
+
+ return IRQ_HANDLED;
+}
+
+static void keyscan_start(struct keyscan_priv *priv)
+{
+ clk_enable(priv->clk);
+
+ writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
+ priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
+
+ writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
+ ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
+ priv->base + KEYSCAN_MATRIX_DIM_OFF);
+
+ writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
+}
+
+static void keyscan_stop(struct keyscan_priv *priv)
+{
+ writel(0, priv->base + KEYSCAN_CONFIG_OFF);
+
+ clk_disable(priv->clk);
+}
+
+static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
+{
+ struct device *dev = st_kp->input_dev->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct keypad_platform_data *pdata;
+ int error;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(dev, "failed to allocate driver pdata\n");
+ return -ENOMEM;
+ }
+
+ error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
+ &pdata->num_in_pads);
+ if (error) {
+ dev_err(dev, "failed to parse keypad params\n");
+ return error;
+ }
+
+ of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
+
+ st_kp->config = pdata;
+
+ dev_info(dev, "rows=%d col=%d debounce=%d\n",
+ pdata->num_out_pads,
+ pdata->num_in_pads,
+ pdata->debounce_us);
+
+ error = of_property_read_u32_array(np, "linux,keymap",
+ st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
+ if (error) {
+ dev_err(dev, "failed to parse keymap\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int __init keyscan_probe(struct platform_device *pdev)
+{
+ struct keypad_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct keyscan_priv *st_kp;
+ struct input_dev *input_dev;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int len;
+ int error;
+ int i;
+
+ if (!pdata && !pdev->dev.of_node) {
+ dev_err(&pdev->dev, "no keymap defined\n");
+ return -EINVAL;
+ }
+
+ st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
+ if (!st_kp) {
+ dev_err(dev, "failed to allocate driver data\n");
+ return -ENOMEM;
+ }
+ st_kp->config = pdata;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "no I/O memory specified\n");
+ return -ENXIO;
+ }
+
+ len = resource_size(res);
+ if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
+ dev_err(dev, "failed to reserve I/O memory\n");
+ return -EBUSY;
+ }
+
+ st_kp->base = devm_ioremap_nocache(dev, res->start, len);
+ if (st_kp->base == NULL) {
+ dev_err(dev, "failed to remap I/O memory\n");
+ return -ENXIO;
+ }
+
+ st_kp->irq = platform_get_irq(pdev, 0);
+ if (st_kp->irq < 0) {
+ dev_err(dev, "no IRQ specified\n");
+ return -ENXIO;
+ }
+
+ error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
+ pdev->name, pdev);
+ if (error) {
+ dev_err(dev, "failed to request IRQ\n");
+ return error;
+ }
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev) {
+ dev_err(&pdev->dev, "failed to allocate the input device\n");
+ return -ENOMEM;
+ }
+
+ st_kp->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(st_kp->clk)) {
+ dev_err(dev, "cannot get clock");
+ return PTR_ERR(st_kp->clk);
+ }
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ dev_err(dev, "failed to allocate input device\n");
+ return -ENOMEM;
+ }
+ st_kp->input_dev = input_dev;
+
+ input_dev->name = pdev->name;
+ input_dev->phys = "keyscan-keys/input0";
+ input_dev->dev.parent = dev;
+
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->id.vendor = 0x0001;
+ input_dev->id.product = 0x0001;
+ input_dev->id.version = 0x0100;
+
+ if (!pdata) {
+ error = keypad_matrix_key_parse_dt(st_kp);
+ if (error)
+ return error;
+ pdata = st_kp->config;
+ }
+
+ input_dev->keycode = st_kp->keycodes;
+ input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
+ input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
+
+ for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
+ input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
+ __clear_bit(KEY_RESERVED, input_dev->keybit);
+
+ error = input_register_device(input_dev);
+ if (error) {
+ dev_err(dev, "failed to register input device\n");
+ input_free_device(input_dev);
+ platform_set_drvdata(pdev, NULL);
+ return error;
+ }
+
+ platform_set_drvdata(pdev, st_kp);
+
+ keyscan_start(st_kp);
+
+ device_set_wakeup_capable(&pdev->dev, 1);
+
+ return 0;
+}
+
+static int __exit keyscan_remove(struct platform_device *pdev)
+{
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ keyscan_stop(priv);
+
+ input_unregister_device(priv->input_dev);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static int keyscan_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(priv->irq);
+ else
+ keyscan_stop(priv);
+
+ return 0;
+}
+
+static int keyscan_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(priv->irq);
+ else
+ keyscan_start(priv);
+
+ return 0;
+}
+
+static const struct dev_pm_ops keyscan_dev_pm_ops = {
+ .suspend = keyscan_suspend,
+ .resume = keyscan_resume,
+};
+
+static const struct of_device_id keyscan_of_match[] = {
+ { .compatible = "st,keypad" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, keyscan_of_match);
+
+__refdata struct platform_driver keyscan_device_driver = {
+ .probe = keyscan_probe,
+ .remove = __exit_p(keyscan_remove),
+ .driver = {
+ .name = "st-keyscan",
+ .pm = &keyscan_dev_pm_ops,
+ .of_match_table = of_match_ptr(keyscan_of_match),
+ }
+};
+
+static int __init keyscan_init(void)
+{
+ return platform_driver_register(&keyscan_device_driver);
+}
+
+static void __exit keyscan_exit(void)
+{
+ platform_driver_unregister(&keyscan_device_driver);
+}
+
+module_init(keyscan_init);
+module_exit(keyscan_exit);
+
+MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
+MODULE_LICENSE("GPL");
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds ST Keyscan driver to use the keypad hw a subset
of ST boards provide. Specific board setup will be put in the
given dt.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
.../devicetree/bindings/input/st-keypad.txt | 50 ++++
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
4 files changed, 386 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
create mode 100644 drivers/input/keyboard/st-keyscan.c
diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
new file mode 100644
index 0000000..63eb07a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/st-keypad.txt
@@ -0,0 +1,50 @@
+* ST Keypad controller device tree bindings
+
+The ST keypad controller device tree binding is based on the
+matrix-keymap.
+
+Required properties:
+
+- compatible: "st,keypad"
+
+- reg: Register base address of st-keypad controler.
+
+- interrupts: Interrupt numberof st-keypad controler.
+
+- clocks: Must contain one entry, for the module clock.
+ See ../clocks/clock-bindings.txt for details.
+
+- pinctrl-0: Should specify pin control groups used for this controller.
+
+- pinctrl-names: Should contain only one value - "default".
+
+- linux,keymap: The keymap for keys as described in the binding document
+ devicetree/bindings/input/matrix-keymap.txt.
+
+- keypad,num-rows: Number of row lines connected to the keypad controller.
+
+- keypad,num-columns: Number of column lines connected to the keypad
+ controller.
+
+- st,debounce_us: Debouncing interval time in microseconds
+
+
+Example:
+
+keyscan: keyscan at fe4b0000 {
+ compatible = "st,keypad";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+
+ keypad,num-rows = <4>;
+ keypad,num-columns = <4>;
+ st,debounce_us = <5000>;
+ linux,keymap = < /* in0 in1 in2 in3 */
+ KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
+ KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
+ KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
+ KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
+};
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index a673c9f..9e29125 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
To compile this driver as a module, choose M here: the
module will be called stowaway.
+config KEYBOARD_ST_KEYSCAN
+ tristate "STMicroelectronics keyscan support"
+ depends on ARCH_STI
+ select INPUT_EVDEV
+ select INPUT_MATRIXKMAP
+ help
+ Say Y here if you want to use a keypad attached to the keyscan block
+ on some STMicroelectronics SoC devices.
+
+ To compile this driver as a module, choose M here: the
+ module will be called stm-keyscan.
+
config KEYBOARD_SUNKBD
tristate "Sun Type 4 and Type 5 keyboard"
select SERIO
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index a699b61..5fd020a 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
+obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
new file mode 100644
index 0000000..606cc19
--- /dev/null
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -0,0 +1,323 @@
+/*
+ * STMicroelectronics Key Scanning driver
+ *
+ * Copyright (c) 2014 STMicroelectonics Ltd.
+ * Author: Stuart Menefy <stuart.menefy@st.com>
+ *
+ * Based on sh_keysc.c, copyright 2008 Magnus Damm
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/input/matrix_keypad.h>
+
+#define ST_KEYSCAN_MAXKEYS 16
+
+#define KEYSCAN_CONFIG_OFF 0x000
+#define KEYSCAN_CONFIG_ENABLE 1
+#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
+#define KEYSCAN_MATRIX_STATE_OFF 0x008
+#define KEYSCAN_MATRIX_DIM_OFF 0x00c
+#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
+#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
+
+struct keypad_platform_data {
+ const struct matrix_keymap_data *keymap_data;
+ unsigned int num_out_pads;
+ unsigned int num_in_pads;
+ unsigned int debounce_us;
+};
+
+struct keyscan_priv {
+ void __iomem *base;
+ int irq;
+ struct clk *clk;
+ struct input_dev *input_dev;
+ struct keypad_platform_data *config;
+ unsigned int last_state;
+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
+};
+
+static irqreturn_t keyscan_isr(int irq, void *dev_id)
+{
+ struct platform_device *pdev = dev_id;
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+ int state;
+ int change;
+
+ state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
+ change = priv->last_state ^ state;
+
+ while (change) {
+ int scancode = __ffs(change);
+ int down = state & (1 << scancode);
+
+ input_report_key(priv->input_dev, priv->keycodes[scancode],
+ down);
+ change ^= 1 << scancode;
+ };
+
+ input_sync(priv->input_dev);
+
+ priv->last_state = state;
+
+ return IRQ_HANDLED;
+}
+
+static void keyscan_start(struct keyscan_priv *priv)
+{
+ clk_enable(priv->clk);
+
+ writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
+ priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
+
+ writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
+ ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
+ priv->base + KEYSCAN_MATRIX_DIM_OFF);
+
+ writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
+}
+
+static void keyscan_stop(struct keyscan_priv *priv)
+{
+ writel(0, priv->base + KEYSCAN_CONFIG_OFF);
+
+ clk_disable(priv->clk);
+}
+
+static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
+{
+ struct device *dev = st_kp->input_dev->dev.parent;
+ struct device_node *np = dev->of_node;
+ struct keypad_platform_data *pdata;
+ int error;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(dev, "failed to allocate driver pdata\n");
+ return -ENOMEM;
+ }
+
+ error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
+ &pdata->num_in_pads);
+ if (error) {
+ dev_err(dev, "failed to parse keypad params\n");
+ return error;
+ }
+
+ of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
+
+ st_kp->config = pdata;
+
+ dev_info(dev, "rows=%d col=%d debounce=%d\n",
+ pdata->num_out_pads,
+ pdata->num_in_pads,
+ pdata->debounce_us);
+
+ error = of_property_read_u32_array(np, "linux,keymap",
+ st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
+ if (error) {
+ dev_err(dev, "failed to parse keymap\n");
+ return error;
+ }
+
+ return 0;
+}
+
+static int __init keyscan_probe(struct platform_device *pdev)
+{
+ struct keypad_platform_data *pdata =
+ dev_get_platdata(&pdev->dev);
+ struct keyscan_priv *st_kp;
+ struct input_dev *input_dev;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+ int len;
+ int error;
+ int i;
+
+ if (!pdata && !pdev->dev.of_node) {
+ dev_err(&pdev->dev, "no keymap defined\n");
+ return -EINVAL;
+ }
+
+ st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
+ if (!st_kp) {
+ dev_err(dev, "failed to allocate driver data\n");
+ return -ENOMEM;
+ }
+ st_kp->config = pdata;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res) {
+ dev_err(dev, "no I/O memory specified\n");
+ return -ENXIO;
+ }
+
+ len = resource_size(res);
+ if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
+ dev_err(dev, "failed to reserve I/O memory\n");
+ return -EBUSY;
+ }
+
+ st_kp->base = devm_ioremap_nocache(dev, res->start, len);
+ if (st_kp->base == NULL) {
+ dev_err(dev, "failed to remap I/O memory\n");
+ return -ENXIO;
+ }
+
+ st_kp->irq = platform_get_irq(pdev, 0);
+ if (st_kp->irq < 0) {
+ dev_err(dev, "no IRQ specified\n");
+ return -ENXIO;
+ }
+
+ error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
+ pdev->name, pdev);
+ if (error) {
+ dev_err(dev, "failed to request IRQ\n");
+ return error;
+ }
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev) {
+ dev_err(&pdev->dev, "failed to allocate the input device\n");
+ return -ENOMEM;
+ }
+
+ st_kp->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(st_kp->clk)) {
+ dev_err(dev, "cannot get clock");
+ return PTR_ERR(st_kp->clk);
+ }
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ dev_err(dev, "failed to allocate input device\n");
+ return -ENOMEM;
+ }
+ st_kp->input_dev = input_dev;
+
+ input_dev->name = pdev->name;
+ input_dev->phys = "keyscan-keys/input0";
+ input_dev->dev.parent = dev;
+
+ input_dev->id.bustype = BUS_HOST;
+ input_dev->id.vendor = 0x0001;
+ input_dev->id.product = 0x0001;
+ input_dev->id.version = 0x0100;
+
+ if (!pdata) {
+ error = keypad_matrix_key_parse_dt(st_kp);
+ if (error)
+ return error;
+ pdata = st_kp->config;
+ }
+
+ input_dev->keycode = st_kp->keycodes;
+ input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
+ input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
+
+ for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
+ input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
+ __clear_bit(KEY_RESERVED, input_dev->keybit);
+
+ error = input_register_device(input_dev);
+ if (error) {
+ dev_err(dev, "failed to register input device\n");
+ input_free_device(input_dev);
+ platform_set_drvdata(pdev, NULL);
+ return error;
+ }
+
+ platform_set_drvdata(pdev, st_kp);
+
+ keyscan_start(st_kp);
+
+ device_set_wakeup_capable(&pdev->dev, 1);
+
+ return 0;
+}
+
+static int __exit keyscan_remove(struct platform_device *pdev)
+{
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ keyscan_stop(priv);
+
+ input_unregister_device(priv->input_dev);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static int keyscan_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ enable_irq_wake(priv->irq);
+ else
+ keyscan_stop(priv);
+
+ return 0;
+}
+
+static int keyscan_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct keyscan_priv *priv = platform_get_drvdata(pdev);
+
+ if (device_may_wakeup(dev))
+ disable_irq_wake(priv->irq);
+ else
+ keyscan_start(priv);
+
+ return 0;
+}
+
+static const struct dev_pm_ops keyscan_dev_pm_ops = {
+ .suspend = keyscan_suspend,
+ .resume = keyscan_resume,
+};
+
+static const struct of_device_id keyscan_of_match[] = {
+ { .compatible = "st,keypad" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, keyscan_of_match);
+
+__refdata struct platform_driver keyscan_device_driver = {
+ .probe = keyscan_probe,
+ .remove = __exit_p(keyscan_remove),
+ .driver = {
+ .name = "st-keyscan",
+ .pm = &keyscan_dev_pm_ops,
+ .of_match_table = of_match_ptr(keyscan_of_match),
+ }
+};
+
+static int __init keyscan_init(void)
+{
+ return platform_driver_register(&keyscan_device_driver);
+}
+
+static void __exit keyscan_exit(void)
+{
+ platform_driver_unregister(&keyscan_device_driver);
+}
+
+module_init(keyscan_init);
+module_exit(keyscan_exit);
+
+MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
+MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
+MODULE_LICENSE("GPL");
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 2/5] ARM: STi: DT: add keyscan for stih415
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Giuseppe CONDORELLI,
Gabriel Fernandez
From: Giuseppe CONDORELLI <giuseppe.condorelli@st.com>
Add keyscan support for stih415.
It is put disabled by default because it is not enabled on all boards
Also there are PIOs conflict with already claimed lines.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 ++++++++++++++++
arch/arm/boot/dts/stih415.dtsi | 10 ++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/stih415-pinctrl.dtsi b/arch/arm/boot/dts/stih415-pinctrl.dtsi
index e56449d..731e4b1 100644
--- a/arch/arm/boot/dts/stih415-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih415-pinctrl.dtsi
@@ -87,6 +87,22 @@
};
};
+ keyscan {
+ pinctrl_keyscan: keyscan {
+ st,pins {
+ keyin0 = <&PIO0 2 ALT2 IN>;
+ keyin1 = <&PIO0 3 ALT2 IN>;
+ keyin2 = <&PIO0 4 ALT2 IN>;
+ keyin3 = <&PIO2 6 ALT2 IN>;
+
+ keyout0 = <&PIO1 6 ALT2 OUT>;
+ keyout1 = <&PIO1 7 ALT2 OUT>;
+ keyout2 = <&PIO0 6 ALT2 OUT>;
+ keyout3 = <&PIO2 7 ALT2 OUT>;
+ };
+ };
+ };
+
sbc_i2c0 {
pinctrl_sbc_i2c0_default: sbc_i2c0-default {
st,pins {
diff --git a/arch/arm/boot/dts/stih415.dtsi b/arch/arm/boot/dts/stih415.dtsi
index d9c7dd1..5930467 100644
--- a/arch/arm/boot/dts/stih415.dtsi
+++ b/arch/arm/boot/dts/stih415.dtsi
@@ -136,5 +136,15 @@
status = "disabled";
};
+
+ keyscan: keyscan@fe4b0000 {
+ compatible = "st,keypad";
+ status = "disabled";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 2/5] ARM: STi: DT: add keyscan for stih415
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
From: Giuseppe CONDORELLI <giuseppe.condorelli@st.com>
Add keyscan support for stih415.
It is put disabled by default because it is not enabled on all boards
Also there are PIOs conflict with already claimed lines.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 ++++++++++++++++
arch/arm/boot/dts/stih415.dtsi | 10 ++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/stih415-pinctrl.dtsi b/arch/arm/boot/dts/stih415-pinctrl.dtsi
index e56449d..731e4b1 100644
--- a/arch/arm/boot/dts/stih415-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih415-pinctrl.dtsi
@@ -87,6 +87,22 @@
};
};
+ keyscan {
+ pinctrl_keyscan: keyscan {
+ st,pins {
+ keyin0 = <&PIO0 2 ALT2 IN>;
+ keyin1 = <&PIO0 3 ALT2 IN>;
+ keyin2 = <&PIO0 4 ALT2 IN>;
+ keyin3 = <&PIO2 6 ALT2 IN>;
+
+ keyout0 = <&PIO1 6 ALT2 OUT>;
+ keyout1 = <&PIO1 7 ALT2 OUT>;
+ keyout2 = <&PIO0 6 ALT2 OUT>;
+ keyout3 = <&PIO2 7 ALT2 OUT>;
+ };
+ };
+ };
+
sbc_i2c0 {
pinctrl_sbc_i2c0_default: sbc_i2c0-default {
st,pins {
diff --git a/arch/arm/boot/dts/stih415.dtsi b/arch/arm/boot/dts/stih415.dtsi
index d9c7dd1..5930467 100644
--- a/arch/arm/boot/dts/stih415.dtsi
+++ b/arch/arm/boot/dts/stih415.dtsi
@@ -136,5 +136,15 @@
status = "disabled";
};
+
+ keyscan: keyscan at fe4b0000 {
+ compatible = "st,keypad";
+ status = "disabled";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 3/5] ARM: STi: DT: add keyscan for stih416
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Gabriel Fernandez,
Giuseppe Condorelli
Add keyscan support for stih416.
It is disabled by default given that it is not enabled on all boards.
Also there are PIOs conflict with already claimed lines.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 ++++++++++++++++
arch/arm/boot/dts/stih416.dtsi | 10 ++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/stih416-pinctrl.dtsi b/arch/arm/boot/dts/stih416-pinctrl.dtsi
index b29ff4b..4e0ebed 100644
--- a/arch/arm/boot/dts/stih416-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih416-pinctrl.dtsi
@@ -98,6 +98,22 @@
};
};
+ keyscan {
+ pinctrl_keyscan: keyscan {
+ st,pins {
+ keyin0 = <&PIO0 2 ALT2 IN>;
+ keyin1 = <&PIO0 3 ALT2 IN>;
+ keyin2 = <&PIO0 4 ALT2 IN>;
+ keyin3 = <&PIO2 6 ALT2 IN>;
+
+ keyout0 = <&PIO1 6 ALT2 OUT>;
+ keyout1 = <&PIO1 7 ALT2 OUT>;
+ keyout2 = <&PIO0 6 ALT2 OUT>;
+ keyout3 = <&PIO2 7 ALT2 OUT>;
+ };
+ };
+ };
+
sbc_i2c0 {
pinctrl_sbc_i2c0_default: sbc_i2c0-default {
st,pins {
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index b7ab47b..9f20b21 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -145,5 +145,15 @@
status = "disabled";
};
+
+ keyscan: keyscan@fe4b0000 {
+ compatible = "st,keypad";
+ status = "disabled";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 3/5] ARM: STi: DT: add keyscan for stih416
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
Add keyscan support for stih416.
It is disabled by default given that it is not enabled on all boards.
Also there are PIOs conflict with already claimed lines.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 ++++++++++++++++
arch/arm/boot/dts/stih416.dtsi | 10 ++++++++++
2 files changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/stih416-pinctrl.dtsi b/arch/arm/boot/dts/stih416-pinctrl.dtsi
index b29ff4b..4e0ebed 100644
--- a/arch/arm/boot/dts/stih416-pinctrl.dtsi
+++ b/arch/arm/boot/dts/stih416-pinctrl.dtsi
@@ -98,6 +98,22 @@
};
};
+ keyscan {
+ pinctrl_keyscan: keyscan {
+ st,pins {
+ keyin0 = <&PIO0 2 ALT2 IN>;
+ keyin1 = <&PIO0 3 ALT2 IN>;
+ keyin2 = <&PIO0 4 ALT2 IN>;
+ keyin3 = <&PIO2 6 ALT2 IN>;
+
+ keyout0 = <&PIO1 6 ALT2 OUT>;
+ keyout1 = <&PIO1 7 ALT2 OUT>;
+ keyout2 = <&PIO0 6 ALT2 OUT>;
+ keyout3 = <&PIO2 7 ALT2 OUT>;
+ };
+ };
+ };
+
sbc_i2c0 {
pinctrl_sbc_i2c0_default: sbc_i2c0-default {
st,pins {
diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index b7ab47b..9f20b21 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -145,5 +145,15 @@
status = "disabled";
};
+
+ keyscan: keyscan at fe4b0000 {
+ compatible = "st,keypad";
+ status = "disabled";
+ reg = <0xfe4b0000 0x2000>;
+ interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
+ clocks = <&CLK_SYSIN>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_keyscan>;
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Gabriel Fernandez,
Giuseppe Condorelli
Add keyscan setup for stih415/h416 b2000.
Both have same raw/column lines number, debounce time and keymap.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih41x-b2000.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
index 1e6aa92..cf06beb 100644
--- a/arch/arm/boot/dts/stih41x-b2000.dtsi
+++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
@@ -6,6 +6,7 @@
* it under the terms of the GNU General Public License version 2 as
* publishhed by the Free Software Foundation.
*/
+#include <dt-bindings/input/input.h>
/ {
memory{
@@ -46,5 +47,16 @@
status = "okay";
};
+
+ keyscan: keyscan@fe4b0000 {
+ keypad,num-rows = <4>;
+ keypad,num-columns = <4>;
+ st,debounce_us = <5000>;
+ linux,keymap = < /* in0 in1 in2 in3 */
+ KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
+ KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
+ KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
+ KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
Add keyscan setup for stih415/h416 b2000.
Both have same raw/column lines number, debounce time and keymap.
Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/boot/dts/stih41x-b2000.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
index 1e6aa92..cf06beb 100644
--- a/arch/arm/boot/dts/stih41x-b2000.dtsi
+++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
@@ -6,6 +6,7 @@
* it under the terms of the GNU General Public License version 2 as
* publishhed by the Free Software Foundation.
*/
+#include <dt-bindings/input/input.h>
/ {
memory{
@@ -46,5 +47,16 @@
status = "okay";
};
+
+ keyscan: keyscan at fe4b0000 {
+ keypad,num-rows = <4>;
+ keypad,num-columns = <4>;
+ st,debounce_us = <5000>;
+ linux,keymap = < /* in0 in1 in2 in3 */
+ KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
+ KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
+ KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
+ KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
+ };
};
};
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 5/5] ARM: multi_v7_defconfig: add st-keyscan driver
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely
Cc: devicetree, linux-doc, linux-kernel, linux-arm-kernel,
linux-input, kernel, Lee Jones, Gabriel Fernandez
This patch adds KEYBOARD_ST_KEYSCAN config
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/configs/multi_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index ee69829..5e926981 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -131,6 +131,7 @@ CONFIG_INPUT_EVDEV=y
CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_TEGRA=y
CONFIG_KEYBOARD_SPEAR=y
+CONFIG_KEYBOARD_ST_KEYSCAN=y
CONFIG_KEYBOARD_CROS_EC=y
CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_MISC=y
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 5/5] ARM: multi_v7_defconfig: add st-keyscan driver
@ 2014-03-05 3:39 ` Gabriel FERNANDEZ
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel FERNANDEZ @ 2014-03-05 3:39 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds KEYBOARD_ST_KEYSCAN config
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
---
arch/arm/configs/multi_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
index ee69829..5e926981 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -131,6 +131,7 @@ CONFIG_INPUT_EVDEV=y
CONFIG_KEYBOARD_GPIO=y
CONFIG_KEYBOARD_TEGRA=y
CONFIG_KEYBOARD_SPEAR=y
+CONFIG_KEYBOARD_ST_KEYSCAN=y
CONFIG_KEYBOARD_CROS_EC=y
CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_MISC=y
--
1.9.0
^ permalink raw reply related [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-05 6:46 ` Dmitry Torokhov
-1 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-05 6:46 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Russell King, Grant Likely, devicetree, linux-doc,
linux-kernel, linux-arm-kernel, linux-input, kernel, Lee Jones,
Giuseppe Condorelli
Hi Gabriel,
On Wed, Mar 05, 2014 at 04:39:28AM +0100, Gabriel FERNANDEZ wrote:
> This patch adds ST Keyscan driver to use the keypad hw a subset
> of ST boards provide. Specific board setup will be put in the
> given dt.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> drivers/input/keyboard/Kconfig | 12 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
> 4 files changed, 386 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
> create mode 100644 drivers/input/keyboard/st-keyscan.c
>
> diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
> new file mode 100644
> index 0000000..63eb07a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/st-keypad.txt
> @@ -0,0 +1,50 @@
> +* ST Keypad controller device tree bindings
> +
> +The ST keypad controller device tree binding is based on the
> +matrix-keymap.
> +
> +Required properties:
> +
> +- compatible: "st,keypad"
> +
> +- reg: Register base address of st-keypad controler.
> +
> +- interrupts: Interrupt numberof st-keypad controler.
> +
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
> +
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +
> +- pinctrl-names: Should contain only one value - "default".
> +
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad
> + controller.
> +
> +- st,debounce_us: Debouncing interval time in microseconds
> +
> +
> +Example:
> +
> +keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_keyscan>;
> +
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> +};
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a673c9f..9e29125 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
> To compile this driver as a module, choose M here: the
> module will be called stowaway.
>
> +config KEYBOARD_ST_KEYSCAN
> + tristate "STMicroelectronics keyscan support"
> + depends on ARCH_STI
> + select INPUT_EVDEV
> + select INPUT_MATRIXKMAP
> + help
> + Say Y here if you want to use a keypad attached to the keyscan block
> + on some STMicroelectronics SoC devices.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called stm-keyscan.
> +
> config KEYBOARD_SUNKBD
> tristate "Sun Type 4 and Type 5 keyboard"
> select SERIO
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a699b61..5fd020a 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> new file mode 100644
> index 0000000..606cc19
> --- /dev/null
> +++ b/drivers/input/keyboard/st-keyscan.c
> @@ -0,0 +1,323 @@
> +/*
> + * STMicroelectronics Key Scanning driver
> + *
> + * Copyright (c) 2014 STMicroelectonics Ltd.
> + * Author: Stuart Menefy <stuart.menefy@st.com>
> + *
> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +#define ST_KEYSCAN_MAXKEYS 16
> +
> +#define KEYSCAN_CONFIG_OFF 0x000
> +#define KEYSCAN_CONFIG_ENABLE 1
> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
> +#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
> +#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
> +
> +struct keypad_platform_data {
> + const struct matrix_keymap_data *keymap_data;
> + unsigned int num_out_pads;
> + unsigned int num_in_pads;
> + unsigned int debounce_us;
> +};
> +
> +struct keyscan_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct input_dev *input_dev;
> + struct keypad_platform_data *config;
> + unsigned int last_state;
> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
> +};
> +
> +static irqreturn_t keyscan_isr(int irq, void *dev_id)
> +{
> + struct platform_device *pdev = dev_id;
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> + int state;
> + int change;
> +
> + state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
> + change = priv->last_state ^ state;
> +
> + while (change) {
> + int scancode = __ffs(change);
> + int down = state & (1 << scancode);
> +
> + input_report_key(priv->input_dev, priv->keycodes[scancode],
> + down);
> + change ^= 1 << scancode;
> + };
> +
> + input_sync(priv->input_dev);
> +
> + priv->last_state = state;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void keyscan_start(struct keyscan_priv *priv)
> +{
> + clk_enable(priv->clk);
> +
> + writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
> + priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
> +
> + writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
> + ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
> + priv->base + KEYSCAN_MATRIX_DIM_OFF);
> +
> + writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
> +}
> +
> +static void keyscan_stop(struct keyscan_priv *priv)
> +{
> + writel(0, priv->base + KEYSCAN_CONFIG_OFF);
> +
> + clk_disable(priv->clk);
> +}
> +
> +static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
> +{
> + struct device *dev = st_kp->input_dev->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct keypad_platform_data *pdata;
> + int error;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "failed to allocate driver pdata\n");
> + return -ENOMEM;
> + }
> +
> + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> + &pdata->num_in_pads);
> + if (error) {
> + dev_err(dev, "failed to parse keypad params\n");
> + return error;
> + }
> +
> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
> +
> + st_kp->config = pdata;
> +
> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
> + pdata->num_out_pads,
> + pdata->num_in_pads,
> + pdata->debounce_us);
> +
> + error = of_property_read_u32_array(np, "linux,keymap",
> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
> + if (error) {
> + dev_err(dev, "failed to parse keymap\n");
> + return error;
> + }
Please use standard format for matrix keymap so that you can use
matrix_keypad_build_keymap().
> +
> + return 0;
> +}
> +
> +static int __init keyscan_probe(struct platform_device *pdev)
> +{
> + struct keypad_platform_data *pdata =
> + dev_get_platdata(&pdev->dev);
const struct ...
> + struct keyscan_priv *st_kp;
> + struct input_dev *input_dev;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + int len;
> + int error;
> + int i;
> +
> + if (!pdata && !pdev->dev.of_node) {
> + dev_err(&pdev->dev, "no keymap defined\n");
> + return -EINVAL;
> + }
> +
> + st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
> + if (!st_kp) {
> + dev_err(dev, "failed to allocate driver data\n");
> + return -ENOMEM;
> + }
> + st_kp->config = pdata;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
> +
> + st_kp->irq = platform_get_irq(pdev, 0);
> + if (st_kp->irq < 0) {
> + dev_err(dev, "no IRQ specified\n");
> + return -ENXIO;
> + }
> +
> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
> + pdev->name, pdev);
> + if (error) {
> + dev_err(dev, "failed to request IRQ\n");
> + return error;
> + }
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev) {
> + dev_err(&pdev->dev, "failed to allocate the input device\n");
> + return -ENOMEM;
> + }
> +
> + st_kp->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(st_kp->clk)) {
> + dev_err(dev, "cannot get clock");
> + return PTR_ERR(st_kp->clk);
> + }
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + dev_err(dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
You already allocated it once a few lines above.
> + st_kp->input_dev = input_dev;
> +
> + input_dev->name = pdev->name;
> + input_dev->phys = "keyscan-keys/input0";
> + input_dev->dev.parent = dev;
> +
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->id.vendor = 0x0001;
> + input_dev->id.product = 0x0001;
> + input_dev->id.version = 0x0100;
> +
> + if (!pdata) {
> + error = keypad_matrix_key_parse_dt(st_kp);
> + if (error)
> + return error;
> + pdata = st_kp->config;
> + }
> +
> + input_dev->keycode = st_kp->keycodes;
> + input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
> + input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
> +
> + for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
> + input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
> + __clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + error = input_register_device(input_dev);
> + if (error) {
> + dev_err(dev, "failed to register input device\n");
> + input_free_device(input_dev);
> + platform_set_drvdata(pdev, NULL);
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, st_kp);
> +
> + keyscan_start(st_kp);
> +
> + device_set_wakeup_capable(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int __exit keyscan_remove(struct platform_device *pdev)
Why is this marked as __exit? I can unbind device from driver without
unloading module (via sysfs).
> +{
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + keyscan_stop(priv);
Call to keyscan_stop() shoudl go into keyscan_close() implementation.
> +
> + input_unregister_device(priv->input_dev);
Not needed since you are trying to use devres-managed input device.
> + platform_set_drvdata(pdev, NULL);
Not needed. In fact, if you move keyscan_stop() into keyscan_close() you
should be able to get rid of keyscan_remove().
> +
> + return 0;
> +}
> +
> +static int keyscan_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + enable_irq_wake(priv->irq);
> + else
> + keyscan_stop(priv);
> +
> + return 0;
> +}
> +
> +static int keyscan_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + disable_irq_wake(priv->irq);
> + else
> + keyscan_start(priv);
> +
> + return 0;
> +}
Guard suspend/resume with CONFIG_PM_SLEEP?
> +
> +static const struct dev_pm_ops keyscan_dev_pm_ops = {
> + .suspend = keyscan_suspend,
> + .resume = keyscan_resume,
> +};
Make it SIMPLE_DEV_PM_OPS().
> +
> +static const struct of_device_id keyscan_of_match[] = {
> + { .compatible = "st,keypad" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, keyscan_of_match);
> +
> +__refdata struct platform_driver keyscan_device_driver = {
What is this refdata business?
> + .probe = keyscan_probe,
> + .remove = __exit_p(keyscan_remove),
> + .driver = {
> + .name = "st-keyscan",
> + .pm = &keyscan_dev_pm_ops,
> + .of_match_table = of_match_ptr(keyscan_of_match),
> + }
> +};
> +
> +static int __init keyscan_init(void)
> +{
> + return platform_driver_register(&keyscan_device_driver);
> +}
> +
> +static void __exit keyscan_exit(void)
> +{
> + platform_driver_unregister(&keyscan_device_driver);
> +}
> +
> +module_init(keyscan_init);
> +module_exit(keyscan_exit);
I think we have module_platform_drriver() for this.
> +
> +MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
> +MODULE_LICENSE("GPL");
> --
> 1.9.0
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-05 6:46 ` Dmitry Torokhov
0 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-05 6:46 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gabriel,
On Wed, Mar 05, 2014 at 04:39:28AM +0100, Gabriel FERNANDEZ wrote:
> This patch adds ST Keyscan driver to use the keypad hw a subset
> of ST boards provide. Specific board setup will be put in the
> given dt.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> drivers/input/keyboard/Kconfig | 12 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
> 4 files changed, 386 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
> create mode 100644 drivers/input/keyboard/st-keyscan.c
>
> diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
> new file mode 100644
> index 0000000..63eb07a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/st-keypad.txt
> @@ -0,0 +1,50 @@
> +* ST Keypad controller device tree bindings
> +
> +The ST keypad controller device tree binding is based on the
> +matrix-keymap.
> +
> +Required properties:
> +
> +- compatible: "st,keypad"
> +
> +- reg: Register base address of st-keypad controler.
> +
> +- interrupts: Interrupt numberof st-keypad controler.
> +
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
> +
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +
> +- pinctrl-names: Should contain only one value - "default".
> +
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad
> + controller.
> +
> +- st,debounce_us: Debouncing interval time in microseconds
> +
> +
> +Example:
> +
> +keyscan: keyscan at fe4b0000 {
> + compatible = "st,keypad";
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_keyscan>;
> +
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> +};
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a673c9f..9e29125 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
> To compile this driver as a module, choose M here: the
> module will be called stowaway.
>
> +config KEYBOARD_ST_KEYSCAN
> + tristate "STMicroelectronics keyscan support"
> + depends on ARCH_STI
> + select INPUT_EVDEV
> + select INPUT_MATRIXKMAP
> + help
> + Say Y here if you want to use a keypad attached to the keyscan block
> + on some STMicroelectronics SoC devices.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called stm-keyscan.
> +
> config KEYBOARD_SUNKBD
> tristate "Sun Type 4 and Type 5 keyboard"
> select SERIO
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a699b61..5fd020a 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> new file mode 100644
> index 0000000..606cc19
> --- /dev/null
> +++ b/drivers/input/keyboard/st-keyscan.c
> @@ -0,0 +1,323 @@
> +/*
> + * STMicroelectronics Key Scanning driver
> + *
> + * Copyright (c) 2014 STMicroelectonics Ltd.
> + * Author: Stuart Menefy <stuart.menefy@st.com>
> + *
> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +#define ST_KEYSCAN_MAXKEYS 16
> +
> +#define KEYSCAN_CONFIG_OFF 0x000
> +#define KEYSCAN_CONFIG_ENABLE 1
> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
> +#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
> +#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
> +
> +struct keypad_platform_data {
> + const struct matrix_keymap_data *keymap_data;
> + unsigned int num_out_pads;
> + unsigned int num_in_pads;
> + unsigned int debounce_us;
> +};
> +
> +struct keyscan_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct input_dev *input_dev;
> + struct keypad_platform_data *config;
> + unsigned int last_state;
> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
> +};
> +
> +static irqreturn_t keyscan_isr(int irq, void *dev_id)
> +{
> + struct platform_device *pdev = dev_id;
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> + int state;
> + int change;
> +
> + state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
> + change = priv->last_state ^ state;
> +
> + while (change) {
> + int scancode = __ffs(change);
> + int down = state & (1 << scancode);
> +
> + input_report_key(priv->input_dev, priv->keycodes[scancode],
> + down);
> + change ^= 1 << scancode;
> + };
> +
> + input_sync(priv->input_dev);
> +
> + priv->last_state = state;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void keyscan_start(struct keyscan_priv *priv)
> +{
> + clk_enable(priv->clk);
> +
> + writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
> + priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
> +
> + writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
> + ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
> + priv->base + KEYSCAN_MATRIX_DIM_OFF);
> +
> + writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
> +}
> +
> +static void keyscan_stop(struct keyscan_priv *priv)
> +{
> + writel(0, priv->base + KEYSCAN_CONFIG_OFF);
> +
> + clk_disable(priv->clk);
> +}
> +
> +static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
> +{
> + struct device *dev = st_kp->input_dev->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct keypad_platform_data *pdata;
> + int error;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "failed to allocate driver pdata\n");
> + return -ENOMEM;
> + }
> +
> + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> + &pdata->num_in_pads);
> + if (error) {
> + dev_err(dev, "failed to parse keypad params\n");
> + return error;
> + }
> +
> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
> +
> + st_kp->config = pdata;
> +
> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
> + pdata->num_out_pads,
> + pdata->num_in_pads,
> + pdata->debounce_us);
> +
> + error = of_property_read_u32_array(np, "linux,keymap",
> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
> + if (error) {
> + dev_err(dev, "failed to parse keymap\n");
> + return error;
> + }
Please use standard format for matrix keymap so that you can use
matrix_keypad_build_keymap().
> +
> + return 0;
> +}
> +
> +static int __init keyscan_probe(struct platform_device *pdev)
> +{
> + struct keypad_platform_data *pdata =
> + dev_get_platdata(&pdev->dev);
const struct ...
> + struct keyscan_priv *st_kp;
> + struct input_dev *input_dev;
> + struct device *dev = &pdev->dev;
> + struct resource *res;
> + int len;
> + int error;
> + int i;
> +
> + if (!pdata && !pdev->dev.of_node) {
> + dev_err(&pdev->dev, "no keymap defined\n");
> + return -EINVAL;
> + }
> +
> + st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
> + if (!st_kp) {
> + dev_err(dev, "failed to allocate driver data\n");
> + return -ENOMEM;
> + }
> + st_kp->config = pdata;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
> +
> + st_kp->irq = platform_get_irq(pdev, 0);
> + if (st_kp->irq < 0) {
> + dev_err(dev, "no IRQ specified\n");
> + return -ENXIO;
> + }
> +
> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
> + pdev->name, pdev);
> + if (error) {
> + dev_err(dev, "failed to request IRQ\n");
> + return error;
> + }
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev) {
> + dev_err(&pdev->dev, "failed to allocate the input device\n");
> + return -ENOMEM;
> + }
> +
> + st_kp->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(st_kp->clk)) {
> + dev_err(dev, "cannot get clock");
> + return PTR_ERR(st_kp->clk);
> + }
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + dev_err(dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
You already allocated it once a few lines above.
> + st_kp->input_dev = input_dev;
> +
> + input_dev->name = pdev->name;
> + input_dev->phys = "keyscan-keys/input0";
> + input_dev->dev.parent = dev;
> +
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->id.vendor = 0x0001;
> + input_dev->id.product = 0x0001;
> + input_dev->id.version = 0x0100;
> +
> + if (!pdata) {
> + error = keypad_matrix_key_parse_dt(st_kp);
> + if (error)
> + return error;
> + pdata = st_kp->config;
> + }
> +
> + input_dev->keycode = st_kp->keycodes;
> + input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
> + input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
> +
> + for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
> + input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
> + __clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + error = input_register_device(input_dev);
> + if (error) {
> + dev_err(dev, "failed to register input device\n");
> + input_free_device(input_dev);
> + platform_set_drvdata(pdev, NULL);
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, st_kp);
> +
> + keyscan_start(st_kp);
> +
> + device_set_wakeup_capable(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int __exit keyscan_remove(struct platform_device *pdev)
Why is this marked as __exit? I can unbind device from driver without
unloading module (via sysfs).
> +{
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + keyscan_stop(priv);
Call to keyscan_stop() shoudl go into keyscan_close() implementation.
> +
> + input_unregister_device(priv->input_dev);
Not needed since you are trying to use devres-managed input device.
> + platform_set_drvdata(pdev, NULL);
Not needed. In fact, if you move keyscan_stop() into keyscan_close() you
should be able to get rid of keyscan_remove().
> +
> + return 0;
> +}
> +
> +static int keyscan_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + enable_irq_wake(priv->irq);
> + else
> + keyscan_stop(priv);
> +
> + return 0;
> +}
> +
> +static int keyscan_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + if (device_may_wakeup(dev))
> + disable_irq_wake(priv->irq);
> + else
> + keyscan_start(priv);
> +
> + return 0;
> +}
Guard suspend/resume with CONFIG_PM_SLEEP?
> +
> +static const struct dev_pm_ops keyscan_dev_pm_ops = {
> + .suspend = keyscan_suspend,
> + .resume = keyscan_resume,
> +};
Make it SIMPLE_DEV_PM_OPS().
> +
> +static const struct of_device_id keyscan_of_match[] = {
> + { .compatible = "st,keypad" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, keyscan_of_match);
> +
> +__refdata struct platform_driver keyscan_device_driver = {
What is this refdata business?
> + .probe = keyscan_probe,
> + .remove = __exit_p(keyscan_remove),
> + .driver = {
> + .name = "st-keyscan",
> + .pm = &keyscan_dev_pm_ops,
> + .of_match_table = of_match_ptr(keyscan_of_match),
> + }
> +};
> +
> +static int __init keyscan_init(void)
> +{
> + return platform_driver_register(&keyscan_device_driver);
> +}
> +
> +static void __exit keyscan_exit(void)
> +{
> + platform_driver_unregister(&keyscan_device_driver);
> +}
> +
> +module_init(keyscan_init);
> +module_exit(keyscan_exit);
I think we have module_platform_drriver() for this.
> +
> +MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
> +MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
> +MODULE_LICENSE("GPL");
> --
> 1.9.0
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-05 6:46 ` Dmitry Torokhov
(?)
@ 2014-03-07 4:51 ` Gabriel Fernandez
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-07 4:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
Rob Landley, Russell King, Grant Likely, devicetree, linux-doc,
linux-kernel, linux-arm-kernel, linux-input, kernel, Lee Jones,
Giuseppe Condorelli
[-- Attachment #1: Type: text/plain, Size: 14359 bytes --]
Hi Dmitry,
Thanks for yours remarks.
Best regards
Gabriel
On 03/05/2014 07:46 AM, Dmitry Torokhov wrote:
> Hi Gabriel,
>
> On Wed, Mar 05, 2014 at 04:39:28AM +0100, Gabriel FERNANDEZ wrote:
>> This patch adds ST Keyscan driver to use the keypad hw a subset
>> of ST boards provide. Specific board setup will be put in the
>> given dt.
>>
>> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>> ---
>> .../devicetree/bindings/input/st-keypad.txt | 50 ++++
>> drivers/input/keyboard/Kconfig | 12 +
>> drivers/input/keyboard/Makefile | 1 +
>> drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
>> 4 files changed, 386 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
>> create mode 100644 drivers/input/keyboard/st-keyscan.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
>> new file mode 100644
>> index 0000000..63eb07a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/st-keypad.txt
>> @@ -0,0 +1,50 @@
>> +* ST Keypad controller device tree bindings
>> +
>> +The ST keypad controller device tree binding is based on the
>> +matrix-keymap.
>> +
>> +Required properties:
>> +
>> +- compatible: "st,keypad"
>> +
>> +- reg: Register base address of st-keypad controler.
>> +
>> +- interrupts: Interrupt numberof st-keypad controler.
>> +
>> +- clocks: Must contain one entry, for the module clock.
>> + See ../clocks/clock-bindings.txt for details.
>> +
>> +- pinctrl-0: Should specify pin control groups used for this controller.
>> +
>> +- pinctrl-names: Should contain only one value - "default".
>> +
>> +- linux,keymap: The keymap for keys as described in the binding document
>> + devicetree/bindings/input/matrix-keymap.txt.
>> +
>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the keypad
>> + controller.
>> +
>> +- st,debounce_us: Debouncing interval time in microseconds
>> +
>> +
>> +Example:
>> +
>> +keyscan: keyscan@fe4b0000 {
>> + compatible = "st,keypad";
>> + reg = <0xfe4b0000 0x2000>;
>> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
>> + clocks = <&CLK_SYSIN>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_keyscan>;
>> +
>> + keypad,num-rows = <4>;
>> + keypad,num-columns = <4>;
>> + st,debounce_us = <5000>;
>> + linux,keymap = < /* in0 in1 in2 in3 */
>> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
>> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
>> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
>> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
>> +};
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index a673c9f..9e29125 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
>> To compile this driver as a module, choose M here: the
>> module will be called stowaway.
>>
>> +config KEYBOARD_ST_KEYSCAN
>> + tristate "STMicroelectronics keyscan support"
>> + depends on ARCH_STI
>> + select INPUT_EVDEV
>> + select INPUT_MATRIXKMAP
>> + help
>> + Say Y here if you want to use a keypad attached to the keyscan block
>> + on some STMicroelectronics SoC devices.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called stm-keyscan.
>> +
>> config KEYBOARD_SUNKBD
>> tristate "Sun Type 4 and Type 5 keyboard"
>> select SERIO
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index a699b61..5fd020a 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
>> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
>> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
>> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
>> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
>> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
>> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
>> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
>> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
>> new file mode 100644
>> index 0000000..606cc19
>> --- /dev/null
>> +++ b/drivers/input/keyboard/st-keyscan.c
>> @@ -0,0 +1,323 @@
>> +/*
>> + * STMicroelectronics Key Scanning driver
>> + *
>> + * Copyright (c) 2014 STMicroelectonics Ltd.
>> + * Author: Stuart Menefy <stuart.menefy@st.com>
>> + *
>> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/input/matrix_keypad.h>
>> +
>> +#define ST_KEYSCAN_MAXKEYS 16
>> +
>> +#define KEYSCAN_CONFIG_OFF 0x000
>> +#define KEYSCAN_CONFIG_ENABLE 1
>> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
>> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
>> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
>> +#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
>> +#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
>> +
>> +struct keypad_platform_data {
>> + const struct matrix_keymap_data *keymap_data;
>> + unsigned int num_out_pads;
>> + unsigned int num_in_pads;
>> + unsigned int debounce_us;
>> +};
>> +
>> +struct keyscan_priv {
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clk;
>> + struct input_dev *input_dev;
>> + struct keypad_platform_data *config;
>> + unsigned int last_state;
>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
>> +};
>> +
>> +static irqreturn_t keyscan_isr(int irq, void *dev_id)
>> +{
>> + struct platform_device *pdev = dev_id;
>> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
>> + int state;
>> + int change;
>> +
>> + state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
>> + change = priv->last_state ^ state;
>> +
>> + while (change) {
>> + int scancode = __ffs(change);
>> + int down = state & (1 << scancode);
>> +
>> + input_report_key(priv->input_dev, priv->keycodes[scancode],
>> + down);
>> + change ^= 1 << scancode;
>> + };
>> +
>> + input_sync(priv->input_dev);
>> +
>> + priv->last_state = state;
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void keyscan_start(struct keyscan_priv *priv)
>> +{
>> + clk_enable(priv->clk);
>> +
>> + writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
>> + priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
>> +
>> + writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
>> + ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
>> + priv->base + KEYSCAN_MATRIX_DIM_OFF);
>> +
>> + writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
>> +}
>> +
>> +static void keyscan_stop(struct keyscan_priv *priv)
>> +{
>> + writel(0, priv->base + KEYSCAN_CONFIG_OFF);
>> +
>> + clk_disable(priv->clk);
>> +}
>> +
>> +static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
>> +{
>> + struct device *dev = st_kp->input_dev->dev.parent;
>> + struct device_node *np = dev->of_node;
>> + struct keypad_platform_data *pdata;
>> + int error;
>> +
>> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
>> + if (!pdata) {
>> + dev_err(dev, "failed to allocate driver pdata\n");
>> + return -ENOMEM;
>> + }
>> +
>> + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
>> + &pdata->num_in_pads);
>> + if (error) {
>> + dev_err(dev, "failed to parse keypad params\n");
>> + return error;
>> + }
>> +
>> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
>> +
>> + st_kp->config = pdata;
>> +
>> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
>> + pdata->num_out_pads,
>> + pdata->num_in_pads,
>> + pdata->debounce_us);
>> +
>> + error = of_property_read_u32_array(np, "linux,keymap",
>> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
>> + if (error) {
>> + dev_err(dev, "failed to parse keymap\n");
>> + return error;
>> + }
> Please use standard format for matrix keymap so that you can use
> matrix_keypad_build_keymap().
ok
>
>> +
>> + return 0;
>> +}
>> +
>> +static int __init keyscan_probe(struct platform_device *pdev)
>> +{
>> + struct keypad_platform_data *pdata =
>> + dev_get_platdata(&pdev->dev);
> const struct ...
ok
>
>> + struct keyscan_priv *st_kp;
>> + struct input_dev *input_dev;
>> + struct device *dev = &pdev->dev;
>> + struct resource *res;
>> + int len;
>> + int error;
>> + int i;
>> +
>> + if (!pdata && !pdev->dev.of_node) {
>> + dev_err(&pdev->dev, "no keymap defined\n");
>> + return -EINVAL;
>> + }
>> +
>> + st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
>> + if (!st_kp) {
>> + dev_err(dev, "failed to allocate driver data\n");
>> + return -ENOMEM;
>> + }
>> + st_kp->config = pdata;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + dev_err(dev, "no I/O memory specified\n");
>> + return -ENXIO;
>> + }
>> +
>> + len = resource_size(res);
>> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
>> + dev_err(dev, "failed to reserve I/O memory\n");
>> + return -EBUSY;
>> + }
>> +
>> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
>> + if (st_kp->base == NULL) {
>> + dev_err(dev, "failed to remap I/O memory\n");
>> + return -ENXIO;
>> + }
>> +
>> + st_kp->irq = platform_get_irq(pdev, 0);
>> + if (st_kp->irq < 0) {
>> + dev_err(dev, "no IRQ specified\n");
>> + return -ENXIO;
>> + }
>> +
>> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
>> + pdev->name, pdev);
>> + if (error) {
>> + dev_err(dev, "failed to request IRQ\n");
>> + return error;
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate the input device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + st_kp->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(st_kp->clk)) {
>> + dev_err(dev, "cannot get clock");
>> + return PTR_ERR(st_kp->clk);
>> + }
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + dev_err(dev, "failed to allocate input device\n");
>> + return -ENOMEM;
>> + }
> You already allocated it once a few lines above.
right
**
>
>> + st_kp->input_dev = input_dev;
>> +
>> + input_dev->name = pdev->name;
>> + input_dev->phys = "keyscan-keys/input0";
>> + input_dev->dev.parent = dev;
>> +
>> + input_dev->id.bustype = BUS_HOST;
>> + input_dev->id.vendor = 0x0001;
>> + input_dev->id.product = 0x0001;
>> + input_dev->id.version = 0x0100;
>> +
>> + if (!pdata) {
>> + error = keypad_matrix_key_parse_dt(st_kp);
>> + if (error)
>> + return error;
>> + pdata = st_kp->config;
>> + }
>> +
>> + input_dev->keycode = st_kp->keycodes;
>> + input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
>> + input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
>> +
>> + for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
>> + input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
>> + __clear_bit(KEY_RESERVED, input_dev->keybit);
>> +
>> + error = input_register_device(input_dev);
>> + if (error) {
>> + dev_err(dev, "failed to register input device\n");
>> + input_free_device(input_dev);
>> + platform_set_drvdata(pdev, NULL);
>> + return error;
>> + }
>> +
>> + platform_set_drvdata(pdev, st_kp);
>> +
>> + keyscan_start(st_kp);
>> +
>> + device_set_wakeup_capable(&pdev->dev, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int __exit keyscan_remove(struct platform_device *pdev)
> Why is this marked as __exit? I can unbind device from driver without
> unloading module (via sysfs).
yes you are right
>> +{
>> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
>> +
>> + keyscan_stop(priv);
> Call to keyscan_stop() shoudl go into keyscan_close() implementation.
ok
>
>> +
>> + input_unregister_device(priv->input_dev);
> Not needed since you are trying to use devres-managed input device.
ok
>
>> + platform_set_drvdata(pdev, NULL);
> Not needed. In fact, if you move keyscan_stop() into keyscan_close() you
> should be able to get rid of keyscan_remove().
ok
>
>> +
>> + return 0;
>> +}
>> +
>> +static int keyscan_suspend(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
>> +
>> + if (device_may_wakeup(dev))
>> + enable_irq_wake(priv->irq);
>> + else
>> + keyscan_stop(priv);
>> +
>> + return 0;
>> +}
>> +
>> +static int keyscan_resume(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
>> +
>> + if (device_may_wakeup(dev))
>> + disable_irq_wake(priv->irq);
>> + else
>> + keyscan_start(priv);
>> +
>> + return 0;
>> +}
> Guard suspend/resume with CONFIG_PM_SLEEP?
i will check this point.
>
>> +
>> +static const struct dev_pm_ops keyscan_dev_pm_ops = {
>> + .suspend = keyscan_suspend,
>> + .resume = keyscan_resume,
>> +};
> Make it SIMPLE_DEV_PM_OPS().
ok
>
>> +
>> +static const struct of_device_id keyscan_of_match[] = {
>> + { .compatible = "st,keypad" },
>> + { },
>> +};
>> +MODULE_DEVICE_TABLE(of, keyscan_of_match);
>> +
>> +__refdata struct platform_driver keyscan_device_driver = {
> What is this refdata business?
sorry, forgot this line...
>
>> + .probe = keyscan_probe,
>> + .remove = __exit_p(keyscan_remove),
>> + .driver = {
>> + .name = "st-keyscan",
>> + .pm = &keyscan_dev_pm_ops,
>> + .of_match_table = of_match_ptr(keyscan_of_match),
>> + }
>> +};
>> +
>> +static int __init keyscan_init(void)
>> +{
>> + return platform_driver_register(&keyscan_device_driver);
>> +}
>> +
>> +static void __exit keyscan_exit(void)
>> +{
>> + platform_driver_unregister(&keyscan_device_driver);
>> +}
>> +
>> +module_init(keyscan_init);
>> +module_exit(keyscan_exit);
> I think we have module_platform_drriver() for this.
ok
>
>> +
>> +MODULE_AUTHOR("Stuart Menefy <stuart.menefy@st.com>");
>> +MODULE_DESCRIPTION("STMicroelectronics keyscan device driver");
>> +MODULE_LICENSE("GPL");
>> --
>> 1.9.0
>>
> Thanks.
>
[-- Attachment #2: Type: text/html, Size: 17173 bytes --]
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 5/5] ARM: multi_v7_defconfig: add st-keyscan driver
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-10 10:34 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 10:34 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel
> This patch adds KEYBOARD_ST_KEYSCAN config
>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/configs/multi_v7_defconfig | 1 +
> 1 file changed, 1 insertion(+)
You should capitalise ST Keyscan in the $SUBJECT line, but other than that:
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index ee69829..5e926981 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -131,6 +131,7 @@ CONFIG_INPUT_EVDEV=y
> CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_TEGRA=y
> CONFIG_KEYBOARD_SPEAR=y
> +CONFIG_KEYBOARD_ST_KEYSCAN=y
> CONFIG_KEYBOARD_CROS_EC=y
> CONFIG_MOUSE_PS2_ELANTECH=y
> CONFIG_INPUT_MISC=y
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 5/5] ARM: multi_v7_defconfig: add st-keyscan driver
@ 2014-03-10 10:34 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 10:34 UTC (permalink / raw)
To: linux-arm-kernel
> This patch adds KEYBOARD_ST_KEYSCAN config
>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/configs/multi_v7_defconfig | 1 +
> 1 file changed, 1 insertion(+)
You should capitalise ST Keyscan in the $SUBJECT line, but other than that:
Acked-by: Lee Jones <lee.jones@linaro.org>
> diff --git a/arch/arm/configs/multi_v7_defconfig b/arch/arm/configs/multi_v7_defconfig
> index ee69829..5e926981 100644
> --- a/arch/arm/configs/multi_v7_defconfig
> +++ b/arch/arm/configs/multi_v7_defconfig
> @@ -131,6 +131,7 @@ CONFIG_INPUT_EVDEV=y
> CONFIG_KEYBOARD_GPIO=y
> CONFIG_KEYBOARD_TEGRA=y
> CONFIG_KEYBOARD_SPEAR=y
> +CONFIG_KEYBOARD_ST_KEYSCAN=y
> CONFIG_KEYBOARD_CROS_EC=y
> CONFIG_MOUSE_PS2_ELANTECH=y
> CONFIG_INPUT_MISC=y
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-05 3:39 ` Gabriel FERNANDEZ
@ 2014-03-10 11:48 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:48 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
Hi Gabi,
Sorry for the delay. It was a hectic week last week.
As promised:
> This patch adds ST Keyscan driver to use the keypad hw a subset
> of ST boards provide. Specific board setup will be put in the
> given dt.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
Are you sure these are in the correct order?
What is the history of this commit?
> ---
> .../devicetree/bindings/input/st-keypad.txt | 50 ++++
This should be submitted as a seperate patch.
> drivers/input/keyboard/Kconfig | 12 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
> 4 files changed, 386 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
> create mode 100644 drivers/input/keyboard/st-keyscan.c
>
> diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
> new file mode 100644
> index 0000000..63eb07a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/st-keypad.txt
> @@ -0,0 +1,50 @@
> +* ST Keypad controller device tree bindings
s/device tree/Device Tree
> +
> +The ST keypad controller device tree binding is based on the
As above.
> +matrix-keymap.
> +
> +Required properties:
> +
> +- compatible: "st,keypad"
I think there will be subsequent st,keypad drivers? Is there any way
we can make this compatible string more specific to _this_ device?
st,stih4xx-keypad?
> +- reg: Register base address of st-keypad controler.
s/address/address and size AND s/controler/controller
> +- interrupts: Interrupt numberof st-keypad controler.
s/numberof/number for the
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
Great!
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +
> +- pinctrl-names: Should contain only one value - "default".
Like to ../pinctrl/pinctrl-bindings.txt
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad
> + controller.
> +
> +- st,debounce_us: Debouncing interval time in microseconds
I'm sure there will be a shared binding for de-bounce.
If not, there certainly should be.
> +
> +
Superfluous new lines.
> +Example:
> +
> +keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
Is there any way we can make this more specific to _this_ IP?
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_keyscan>;
> +
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
Do these line up in the file? I know Git can be a bit funny about this
sometimes.
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> +};
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a673c9f..9e29125 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
> To compile this driver as a module, choose M here: the
> module will be called stowaway.
>
> +config KEYBOARD_ST_KEYSCAN
> + tristate "STMicroelectronics keyscan support"
> + depends on ARCH_STI
> + select INPUT_EVDEV
> + select INPUT_MATRIXKMAP
> + help
> + Say Y here if you want to use a keypad attached to the keyscan block
> + on some STMicroelectronics SoC devices.
Might be worth mentioning which ones.
> + To compile this driver as a module, choose M here: the
> + module will be called stm-keyscan.
> +
> config KEYBOARD_SUNKBD
> tristate "Sun Type 4 and Type 5 keyboard"
> select SERIO
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a699b61..5fd020a 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> new file mode 100644
> index 0000000..606cc19
> --- /dev/null
> +++ b/drivers/input/keyboard/st-keyscan.c
> @@ -0,0 +1,323 @@
> +/*
> + * STMicroelectronics Key Scanning driver
> + *
> + * Copyright (c) 2014 STMicroelectonics Ltd.
> + * Author: Stuart Menefy <stuart.menefy@st.com>
> + *
> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
Did sh_keysc.c ever exist in Mainline?
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +#define ST_KEYSCAN_MAXKEYS 16
> +
> +#define KEYSCAN_CONFIG_OFF 0x000
> +#define KEYSCAN_CONFIG_ENABLE 1
0x001?
> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
Odd that these are 3 digit padded? Is there a reason for that?
> +#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
> +#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
For all 'ST_KEYSCAN_...'?
> +struct keypad_platform_data {
> + const struct matrix_keymap_data *keymap_data;
> + unsigned int num_out_pads;
> + unsigned int num_in_pads;
> + unsigned int debounce_us;
> +};
> +
> +struct keyscan_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct input_dev *input_dev;
> + struct keypad_platform_data *config;
> + unsigned int last_state;
> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
Seems odd to limit this. Can't the information come from DT
i.e. keypad,num-rows and keypad,num-columns?
> +};
> +
> +static irqreturn_t keyscan_isr(int irq, void *dev_id)
> +{
> + struct platform_device *pdev = dev_id;
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> + int state;
> + int change;
> +
> + state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
> + change = priv->last_state ^ state;
> +
> + while (change) {
> + int scancode = __ffs(change);
> + int down = state & (1 << scancode);
int down = state & BIT(scancode);
> + input_report_key(priv->input_dev, priv->keycodes[scancode],
> + down);
> + change ^= 1 << scancode;
change ^= BIT(scancode);
> + };
> +
> + input_sync(priv->input_dev);
> +
> + priv->last_state = state;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void keyscan_start(struct keyscan_priv *priv)
> +{
> + clk_enable(priv->clk);
> +
> + writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
> + priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
> +
> + writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
> + ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
> + priv->base + KEYSCAN_MATRIX_DIM_OFF);
> +
> + writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
> +}
> +
> +static void keyscan_stop(struct keyscan_priv *priv)
> +{
> + writel(0, priv->base + KEYSCAN_CONFIG_OFF);
> +
> + clk_disable(priv->clk);
> +}
> +
> +static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
> +{
> + struct device *dev = st_kp->input_dev->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct keypad_platform_data *pdata;
> + int error;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "failed to allocate driver pdata\n");
If the system is OOM, this sort of error message will be pretty
redundant. Just return -ENOMEM instead.
> + return -ENOMEM;
> + }
> +
> + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> + &pdata->num_in_pads);
> + if (error) {
> + dev_err(dev, "failed to parse keypad params\n");
> + return error;
Nit: It's pretty unusual to use this for a standard error handling
variable. Consider 'ret' or 'err' as a replacement.
> + }
> +
> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
Isn't this a required property? Might be worth checking the return
value if so.
> + st_kp->config = pdata;
> +
> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
> + pdata->num_out_pads,
> + pdata->num_in_pads,
> + pdata->debounce_us);
Might be worth moving this down passed the final point of failure.
> + error = of_property_read_u32_array(np, "linux,keymap",
> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
> + if (error) {
> + dev_err(dev, "failed to parse keymap\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int __init keyscan_probe(struct platform_device *pdev)
> +{
> + struct keypad_platform_data *pdata =
> + dev_get_platdata(&pdev->dev);
Do we really support platform data still?
> + struct keyscan_priv *st_kp;
> + struct input_dev *input_dev;
> + struct device *dev = &pdev->dev;
dev and pdev are used randomly in probe(). Just remove the dev
de-reference and use &pdev->dev exclusively.
It's pretty common to de-reference 'np = pdev->dev.of_node' though.
> + struct resource *res;
> + int len;
> + int error;
> + int i;
> +
> + if (!pdata && !pdev->dev.of_node) {
> + dev_err(&pdev->dev, "no keymap defined\n");
> + return -EINVAL;
> + }
> +
> + st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
> + if (!st_kp) {
> + dev_err(dev, "failed to allocate driver data\n");
> + return -ENOMEM;
I wouldn't print any messages on -ENOMEM personally. It's not a driver
failure per se, but a system one where the user will soon be notified.
> + }
> + st_kp->config = pdata;
You only want to do this in the !np case.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
Replace the two above with devm_ioremap_resource().
Make sure the IORESOURCE_CACHEABLE is set.
> + st_kp->irq = platform_get_irq(pdev, 0);
> + if (st_kp->irq < 0) {
> + dev_err(dev, "no IRQ specified\n");
> + return -ENXIO;
No such device or address, are you sure?
Perhaps -EINVAL would be better, as one should be specified.
> + }
> +
> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
> + pdev->name, pdev);
> + if (error) {
> + dev_err(dev, "failed to request IRQ\n");
> + return error;
> + }
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev) {
> + dev_err(&pdev->dev, "failed to allocate the input device\n");
> + return -ENOMEM;
> + }
> +
> + st_kp->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(st_kp->clk)) {
> + dev_err(dev, "cannot get clock");
> + return PTR_ERR(st_kp->clk);
> + }
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + dev_err(dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
Wasn't this done already?
> + st_kp->input_dev = input_dev;
> +
> + input_dev->name = pdev->name;
> + input_dev->phys = "keyscan-keys/input0";
> + input_dev->dev.parent = dev;
> +
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->id.vendor = 0x0001;
> + input_dev->id.product = 0x0001;
> + input_dev->id.version = 0x0100;
Any chance we can #define these?
> + if (!pdata) {
> + error = keypad_matrix_key_parse_dt(st_kp);
> + if (error)
> + return error;
> + pdata = st_kp->config;
Is pdata used after this?
> + }
> +
> + input_dev->keycode = st_kp->keycodes;
> + input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
> + input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
> +
> + for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
> + input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
> + __clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + error = input_register_device(input_dev);
> + if (error) {
> + dev_err(dev, "failed to register input device\n");
> + input_free_device(input_dev);
> + platform_set_drvdata(pdev, NULL);
You don't need to do this anymore. It's taken care of elsewhere.
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, st_kp);
> +
> + keyscan_start(st_kp);
> +
> + device_set_wakeup_capable(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int __exit keyscan_remove(struct platform_device *pdev)
> +{
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + keyscan_stop(priv);
> +
> + input_unregister_device(priv->input_dev);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
I already saw that Dmitry commented on the rest of the file.
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 11:48 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:48 UTC (permalink / raw)
To: linux-arm-kernel
Hi Gabi,
Sorry for the delay. It was a hectic week last week.
As promised:
> This patch adds ST Keyscan driver to use the keypad hw a subset
> of ST boards provide. Specific board setup will be put in the
> given dt.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
Are you sure these are in the correct order?
What is the history of this commit?
> ---
> .../devicetree/bindings/input/st-keypad.txt | 50 ++++
This should be submitted as a seperate patch.
> drivers/input/keyboard/Kconfig | 12 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/st-keyscan.c | 323 +++++++++++++++++++++
> 4 files changed, 386 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/st-keypad.txt
> create mode 100644 drivers/input/keyboard/st-keyscan.c
>
> diff --git a/Documentation/devicetree/bindings/input/st-keypad.txt b/Documentation/devicetree/bindings/input/st-keypad.txt
> new file mode 100644
> index 0000000..63eb07a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/st-keypad.txt
> @@ -0,0 +1,50 @@
> +* ST Keypad controller device tree bindings
s/device tree/Device Tree
> +
> +The ST keypad controller device tree binding is based on the
As above.
> +matrix-keymap.
> +
> +Required properties:
> +
> +- compatible: "st,keypad"
I think there will be subsequent st,keypad drivers? Is there any way
we can make this compatible string more specific to _this_ device?
st,stih4xx-keypad?
> +- reg: Register base address of st-keypad controler.
s/address/address and size AND s/controler/controller
> +- interrupts: Interrupt numberof st-keypad controler.
s/numberof/number for the
> +- clocks: Must contain one entry, for the module clock.
> + See ../clocks/clock-bindings.txt for details.
Great!
> +- pinctrl-0: Should specify pin control groups used for this controller.
> +
> +- pinctrl-names: Should contain only one value - "default".
Like to ../pinctrl/pinctrl-bindings.txt
> +- linux,keymap: The keymap for keys as described in the binding document
> + devicetree/bindings/input/matrix-keymap.txt.
> +
> +- keypad,num-rows: Number of row lines connected to the keypad controller.
> +
> +- keypad,num-columns: Number of column lines connected to the keypad
> + controller.
> +
> +- st,debounce_us: Debouncing interval time in microseconds
I'm sure there will be a shared binding for de-bounce.
If not, there certainly should be.
> +
> +
Superfluous new lines.
> +Example:
> +
> +keyscan: keyscan at fe4b0000 {
> + compatible = "st,keypad";
Is there any way we can make this more specific to _this_ IP?
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_keyscan>;
> +
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
Do these line up in the file? I know Git can be a bit funny about this
sometimes.
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> +};
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a673c9f..9e29125 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -512,6 +512,18 @@ config KEYBOARD_STOWAWAY
> To compile this driver as a module, choose M here: the
> module will be called stowaway.
>
> +config KEYBOARD_ST_KEYSCAN
> + tristate "STMicroelectronics keyscan support"
> + depends on ARCH_STI
> + select INPUT_EVDEV
> + select INPUT_MATRIXKMAP
> + help
> + Say Y here if you want to use a keypad attached to the keyscan block
> + on some STMicroelectronics SoC devices.
Might be worth mentioning which ones.
> + To compile this driver as a module, choose M here: the
> + module will be called stm-keyscan.
> +
> config KEYBOARD_SUNKBD
> tristate "Sun Type 4 and Type 5 keyboard"
> select SERIO
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a699b61..5fd020a 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> new file mode 100644
> index 0000000..606cc19
> --- /dev/null
> +++ b/drivers/input/keyboard/st-keyscan.c
> @@ -0,0 +1,323 @@
> +/*
> + * STMicroelectronics Key Scanning driver
> + *
> + * Copyright (c) 2014 STMicroelectonics Ltd.
> + * Author: Stuart Menefy <stuart.menefy@st.com>
> + *
> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
Did sh_keysc.c ever exist in Mainline?
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/input/matrix_keypad.h>
> +
> +#define ST_KEYSCAN_MAXKEYS 16
> +
> +#define KEYSCAN_CONFIG_OFF 0x000
> +#define KEYSCAN_CONFIG_ENABLE 1
0x001?
> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
Odd that these are 3 digit padded? Is there a reason for that?
> +#define KEYSCAN_MATRIX_DIM_X_SHIFT 0
> +#define KEYSCAN_MATRIX_DIM_Y_SHIFT 2
For all 'ST_KEYSCAN_...'?
> +struct keypad_platform_data {
> + const struct matrix_keymap_data *keymap_data;
> + unsigned int num_out_pads;
> + unsigned int num_in_pads;
> + unsigned int debounce_us;
> +};
> +
> +struct keyscan_priv {
> + void __iomem *base;
> + int irq;
> + struct clk *clk;
> + struct input_dev *input_dev;
> + struct keypad_platform_data *config;
> + unsigned int last_state;
> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
Seems odd to limit this. Can't the information come from DT
i.e. keypad,num-rows and keypad,num-columns?
> +};
> +
> +static irqreturn_t keyscan_isr(int irq, void *dev_id)
> +{
> + struct platform_device *pdev = dev_id;
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> + int state;
> + int change;
> +
> + state = readl(priv->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
> + change = priv->last_state ^ state;
> +
> + while (change) {
> + int scancode = __ffs(change);
> + int down = state & (1 << scancode);
int down = state & BIT(scancode);
> + input_report_key(priv->input_dev, priv->keycodes[scancode],
> + down);
> + change ^= 1 << scancode;
change ^= BIT(scancode);
> + };
> +
> + input_sync(priv->input_dev);
> +
> + priv->last_state = state;
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void keyscan_start(struct keyscan_priv *priv)
> +{
> + clk_enable(priv->clk);
> +
> + writel(priv->config->debounce_us * (clk_get_rate(priv->clk) / 1000000),
> + priv->base + KEYSCAN_DEBOUNCE_TIME_OFF);
> +
> + writel(((priv->config->num_in_pads - 1) << KEYSCAN_MATRIX_DIM_X_SHIFT) |
> + ((priv->config->num_out_pads - 1) << KEYSCAN_MATRIX_DIM_Y_SHIFT),
> + priv->base + KEYSCAN_MATRIX_DIM_OFF);
> +
> + writel(KEYSCAN_CONFIG_ENABLE, priv->base + KEYSCAN_CONFIG_OFF);
> +}
> +
> +static void keyscan_stop(struct keyscan_priv *priv)
> +{
> + writel(0, priv->base + KEYSCAN_CONFIG_OFF);
> +
> + clk_disable(priv->clk);
> +}
> +
> +static int keypad_matrix_key_parse_dt(struct keyscan_priv *st_kp)
> +{
> + struct device *dev = st_kp->input_dev->dev.parent;
> + struct device_node *np = dev->of_node;
> + struct keypad_platform_data *pdata;
> + int error;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> + if (!pdata) {
> + dev_err(dev, "failed to allocate driver pdata\n");
If the system is OOM, this sort of error message will be pretty
redundant. Just return -ENOMEM instead.
> + return -ENOMEM;
> + }
> +
> + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> + &pdata->num_in_pads);
> + if (error) {
> + dev_err(dev, "failed to parse keypad params\n");
> + return error;
Nit: It's pretty unusual to use this for a standard error handling
variable. Consider 'ret' or 'err' as a replacement.
> + }
> +
> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
Isn't this a required property? Might be worth checking the return
value if so.
> + st_kp->config = pdata;
> +
> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
> + pdata->num_out_pads,
> + pdata->num_in_pads,
> + pdata->debounce_us);
Might be worth moving this down passed the final point of failure.
> + error = of_property_read_u32_array(np, "linux,keymap",
> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
> + if (error) {
> + dev_err(dev, "failed to parse keymap\n");
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int __init keyscan_probe(struct platform_device *pdev)
> +{
> + struct keypad_platform_data *pdata =
> + dev_get_platdata(&pdev->dev);
Do we really support platform data still?
> + struct keyscan_priv *st_kp;
> + struct input_dev *input_dev;
> + struct device *dev = &pdev->dev;
dev and pdev are used randomly in probe(). Just remove the dev
de-reference and use &pdev->dev exclusively.
It's pretty common to de-reference 'np = pdev->dev.of_node' though.
> + struct resource *res;
> + int len;
> + int error;
> + int i;
> +
> + if (!pdata && !pdev->dev.of_node) {
> + dev_err(&pdev->dev, "no keymap defined\n");
> + return -EINVAL;
> + }
> +
> + st_kp = devm_kzalloc(dev, sizeof(*st_kp), GFP_KERNEL);
> + if (!st_kp) {
> + dev_err(dev, "failed to allocate driver data\n");
> + return -ENOMEM;
I wouldn't print any messages on -ENOMEM personally. It's not a driver
failure per se, but a system one where the user will soon be notified.
> + }
> + st_kp->config = pdata;
You only want to do this in the !np case.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
Replace the two above with devm_ioremap_resource().
Make sure the IORESOURCE_CACHEABLE is set.
> + st_kp->irq = platform_get_irq(pdev, 0);
> + if (st_kp->irq < 0) {
> + dev_err(dev, "no IRQ specified\n");
> + return -ENXIO;
No such device or address, are you sure?
Perhaps -EINVAL would be better, as one should be specified.
> + }
> +
> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
> + pdev->name, pdev);
> + if (error) {
> + dev_err(dev, "failed to request IRQ\n");
> + return error;
> + }
> +
> + input_dev = devm_input_allocate_device(&pdev->dev);
> + if (!input_dev) {
> + dev_err(&pdev->dev, "failed to allocate the input device\n");
> + return -ENOMEM;
> + }
> +
> + st_kp->clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(st_kp->clk)) {
> + dev_err(dev, "cannot get clock");
> + return PTR_ERR(st_kp->clk);
> + }
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + dev_err(dev, "failed to allocate input device\n");
> + return -ENOMEM;
> + }
Wasn't this done already?
> + st_kp->input_dev = input_dev;
> +
> + input_dev->name = pdev->name;
> + input_dev->phys = "keyscan-keys/input0";
> + input_dev->dev.parent = dev;
> +
> + input_dev->id.bustype = BUS_HOST;
> + input_dev->id.vendor = 0x0001;
> + input_dev->id.product = 0x0001;
> + input_dev->id.version = 0x0100;
Any chance we can #define these?
> + if (!pdata) {
> + error = keypad_matrix_key_parse_dt(st_kp);
> + if (error)
> + return error;
> + pdata = st_kp->config;
Is pdata used after this?
> + }
> +
> + input_dev->keycode = st_kp->keycodes;
> + input_dev->keycodesize = sizeof(st_kp->keycodes[0]);
> + input_dev->keycodemax = ARRAY_SIZE(st_kp->keycodes);
> +
> + for (i = 0; i < ST_KEYSCAN_MAXKEYS; i++)
> + input_set_capability(input_dev, EV_KEY, st_kp->keycodes[i]);
> + __clear_bit(KEY_RESERVED, input_dev->keybit);
> +
> + error = input_register_device(input_dev);
> + if (error) {
> + dev_err(dev, "failed to register input device\n");
> + input_free_device(input_dev);
> + platform_set_drvdata(pdev, NULL);
You don't need to do this anymore. It's taken care of elsewhere.
> + return error;
> + }
> +
> + platform_set_drvdata(pdev, st_kp);
> +
> + keyscan_start(st_kp);
> +
> + device_set_wakeup_capable(&pdev->dev, 1);
> +
> + return 0;
> +}
> +
> +static int __exit keyscan_remove(struct platform_device *pdev)
> +{
> + struct keyscan_priv *priv = platform_get_drvdata(pdev);
> +
> + keyscan_stop(priv);
> +
> + input_unregister_device(priv->input_dev);
> + platform_set_drvdata(pdev, NULL);
> +
> + return 0;
I already saw that Dmitry commented on the rest of the file.
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 2/5] ARM: STi: DT: add keyscan for stih415
2014-03-05 3:39 ` Gabriel FERNANDEZ
(?)
@ 2014-03-10 11:50 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:50 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe CONDORELLI
> From: Giuseppe CONDORELLI <giuseppe.condorelli@st.com>
>
> Add keyscan support for stih415.
> It is put disabled by default because it is not enabled on all boards
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih415.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
<snip>
> + keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
> + status = "disabled";
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
Small nit here, but otherwise:
Acked-by: Lee Jones <lee.jones@linaro.org>
> + pinctrl-0 = <&pinctrl_keyscan>;
> + };
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 2/5] ARM: STi: DT: add keyscan for stih415
@ 2014-03-10 11:50 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:50 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe CONDORELLI
> From: Giuseppe CONDORELLI <giuseppe.condorelli@st.com>
>
> Add keyscan support for stih415.
> It is put disabled by default because it is not enabled on all boards
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih415.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
<snip>
> + keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
> + status = "disabled";
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
Small nit here, but otherwise:
Acked-by: Lee Jones <lee.jones@linaro.org>
> + pinctrl-0 = <&pinctrl_keyscan>;
> + };
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 2/5] ARM: STi: DT: add keyscan for stih415
@ 2014-03-10 11:50 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:50 UTC (permalink / raw)
To: linux-arm-kernel
> From: Giuseppe CONDORELLI <giuseppe.condorelli@st.com>
>
> Add keyscan support for stih415.
> It is put disabled by default because it is not enabled on all boards
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih415-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih415.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
<snip>
> + keyscan: keyscan at fe4b0000 {
> + compatible = "st,keypad";
> + status = "disabled";
> + reg = <0xfe4b0000 0x2000>;
> + interrupts = <GIC_SPI 212 IRQ_TYPE_NONE>;
> + clocks = <&CLK_SYSIN>;
> + pinctrl-names = "default";
Small nit here, but otherwise:
Acked-by: Lee Jones <lee.jones@linaro.org>
> + pinctrl-0 = <&pinctrl_keyscan>;
> + };
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 3/5] ARM: STi: DT: add keyscan for stih416
2014-03-05 3:39 ` Gabriel FERNANDEZ
(?)
@ 2014-03-10 11:52 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:52 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> Add keyscan support for stih416.
> It is disabled by default given that it is not enabled on all boards.
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih416.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
Looks good:
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 3/5] ARM: STi: DT: add keyscan for stih416
@ 2014-03-10 11:52 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:52 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> Add keyscan support for stih416.
> It is disabled by default given that it is not enabled on all boards.
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih416.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
Looks good:
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 3/5] ARM: STi: DT: add keyscan for stih416
@ 2014-03-10 11:52 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:52 UTC (permalink / raw)
To: linux-arm-kernel
> Add keyscan support for stih416.
> It is disabled by default given that it is not enabled on all boards.
> Also there are PIOs conflict with already claimed lines.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih416-pinctrl.dtsi | 16 ++++++++++++++++
> arch/arm/boot/dts/stih416.dtsi | 10 ++++++++++
> 2 files changed, 26 insertions(+)
Looks good:
Acked-by: Lee Jones <lee.jones@linaro.org>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000
2014-03-05 3:39 ` Gabriel FERNANDEZ
(?)
@ 2014-03-10 11:54 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:54 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
On Wed, 05 Mar 2014, Gabriel FERNANDEZ wrote:
> Add keyscan setup for stih415/h416 b2000.
> Both have same raw/column lines number, debounce time and keymap.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih41x-b2000.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
> index 1e6aa92..cf06beb 100644
> --- a/arch/arm/boot/dts/stih41x-b2000.dtsi
> +++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
> @@ -6,6 +6,7 @@
> * it under the terms of the GNU General Public License version 2 as
> * publishhed by the Free Software Foundation.
> */
> +#include <dt-bindings/input/input.h>
> / {
>
> memory{
> @@ -46,5 +47,16 @@
>
> status = "okay";
> };
> +
> + keyscan: keyscan@fe4b0000 {
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
Nit: We should try to line the headers up with their associated values
(perhaps they just look misalined in the email and are actually fine
in the text file?)
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> + };
Patch looks good though:
Acked-by: Lee Jones <lee.jones@linaro.org>
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000
@ 2014-03-10 11:54 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:54 UTC (permalink / raw)
To: Gabriel FERNANDEZ
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
On Wed, 05 Mar 2014, Gabriel FERNANDEZ wrote:
> Add keyscan setup for stih415/h416 b2000.
> Both have same raw/column lines number, debounce time and keymap.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih41x-b2000.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
> index 1e6aa92..cf06beb 100644
> --- a/arch/arm/boot/dts/stih41x-b2000.dtsi
> +++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
> @@ -6,6 +6,7 @@
> * it under the terms of the GNU General Public License version 2 as
> * publishhed by the Free Software Foundation.
> */
> +#include <dt-bindings/input/input.h>
> / {
>
> memory{
> @@ -46,5 +47,16 @@
>
> status = "okay";
> };
> +
> + keyscan: keyscan@fe4b0000 {
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
Nit: We should try to line the headers up with their associated values
(perhaps they just look misalined in the email and are actually fine
in the text file?)
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> + };
Patch looks good though:
Acked-by: Lee Jones <lee.jones@linaro.org>
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000
@ 2014-03-10 11:54 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 11:54 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, 05 Mar 2014, Gabriel FERNANDEZ wrote:
> Add keyscan setup for stih415/h416 b2000.
> Both have same raw/column lines number, debounce time and keymap.
>
> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> ---
> arch/arm/boot/dts/stih41x-b2000.dtsi | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/arch/arm/boot/dts/stih41x-b2000.dtsi b/arch/arm/boot/dts/stih41x-b2000.dtsi
> index 1e6aa92..cf06beb 100644
> --- a/arch/arm/boot/dts/stih41x-b2000.dtsi
> +++ b/arch/arm/boot/dts/stih41x-b2000.dtsi
> @@ -6,6 +6,7 @@
> * it under the terms of the GNU General Public License version 2 as
> * publishhed by the Free Software Foundation.
> */
> +#include <dt-bindings/input/input.h>
> / {
>
> memory{
> @@ -46,5 +47,16 @@
>
> status = "okay";
> };
> +
> + keyscan: keyscan at fe4b0000 {
> + keypad,num-rows = <4>;
> + keypad,num-columns = <4>;
> + st,debounce_us = <5000>;
> + linux,keymap = < /* in0 in1 in2 in3 */
Nit: We should try to line the headers up with their associated values
(perhaps they just look misalined in the email and are actually fine
in the text file?)
> + KEY_F13 KEY_F9 KEY_F5 KEY_F1 /* out0 */
> + KEY_F14 KEY_F10 KEY_F6 KEY_F2 /* out1 */
> + KEY_F15 KEY_F11 KEY_F7 KEY_F3 /* out2 */
> + KEY_F16 KEY_F12 KEY_F8 KEY_F4 >; /* out3 */
> + };
Patch looks good though:
Acked-by: Lee Jones <lee.jones@linaro.org>
> };
> };
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-10 11:48 ` Lee Jones
@ 2014-03-10 15:28 ` Dmitry Torokhov
-1 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-10 15:28 UTC (permalink / raw)
To: Lee Jones
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
Hi Lee,
On Mon, Mar 10, 2014 at 11:48:19AM +0000, Lee Jones wrote:
> Hi Gabi,
>
> Sorry for the delay. It was a hectic week last week.
>
> As promised:
>
> > This patch adds ST Keyscan driver to use the keypad hw a subset
> > of ST boards provide. Specific board setup will be put in the
> > given dt.
> >
> > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> Are you sure these are in the correct order?
>
> What is the history of this commit?
>
> > ---
> > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
>
> This should be submitted as a seperate patch.
Why do we have such requirement? To me it would make more sense to add
binding documentation in the same commit as the code that uses these
bindings.
[...]
> > +
> > + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> > + &pdata->num_in_pads);
> > + if (error) {
> > + dev_err(dev, "failed to parse keypad params\n");
> > + return error;
>
> Nit: It's pretty unusual to use this for a standard error handling
> variable. Consider 'ret' or 'err' as a replacement.
I like "error", in fact there are a lot of these in input. I use "error" for
data that is only returned from error path and "retval" when the same
variable is returned in both success and error paths.
> > +
> > + input_dev->id.bustype = BUS_HOST;
> > + input_dev->id.vendor = 0x0001;
> > + input_dev->id.product = 0x0001;
> > + input_dev->id.version = 0x0100;
>
> Any chance we can #define these?
Even better would be not use 0x0001 as vendor as there (unfortunately)
quite a few other drivers use it already. Either omit or chose something
else. Does ST have PCI or USB VID assigned?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 15:28 ` Dmitry Torokhov
0 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-10 15:28 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lee,
On Mon, Mar 10, 2014 at 11:48:19AM +0000, Lee Jones wrote:
> Hi Gabi,
>
> Sorry for the delay. It was a hectic week last week.
>
> As promised:
>
> > This patch adds ST Keyscan driver to use the keypad hw a subset
> > of ST boards provide. Specific board setup will be put in the
> > given dt.
> >
> > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
>
> Are you sure these are in the correct order?
>
> What is the history of this commit?
>
> > ---
> > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
>
> This should be submitted as a seperate patch.
Why do we have such requirement? To me it would make more sense to add
binding documentation in the same commit as the code that uses these
bindings.
[...]
> > +
> > + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> > + &pdata->num_in_pads);
> > + if (error) {
> > + dev_err(dev, "failed to parse keypad params\n");
> > + return error;
>
> Nit: It's pretty unusual to use this for a standard error handling
> variable. Consider 'ret' or 'err' as a replacement.
I like "error", in fact there are a lot of these in input. I use "error" for
data that is only returned from error path and "retval" when the same
variable is returned in both success and error paths.
> > +
> > + input_dev->id.bustype = BUS_HOST;
> > + input_dev->id.vendor = 0x0001;
> > + input_dev->id.product = 0x0001;
> > + input_dev->id.version = 0x0100;
>
> Any chance we can #define these?
Even better would be not use 0x0001 as vendor as there (unfortunately)
quite a few other drivers use it already. Either omit or chose something
else. Does ST have PCI or USB VID assigned?
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-10 15:28 ` Dmitry Torokhov
@ 2014-03-10 15:38 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 15:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > of ST boards provide. Specific board setup will be put in the
> > > given dt.
> > >
> > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >
> > Are you sure these are in the correct order?
> >
> > What is the history of this commit?
> >
> > > ---
> > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> >
> > This should be submitted as a seperate patch.
>
> Why do we have such requirement? To me it would make more sense to add
> binding documentation in the same commit as the code that uses these
> bindings.
I'm inclined to agree with you and that's actually how we used to do
it, but a decision was made by the DT guys at one of the Kernel
Summits to submit Documentation as a separate patch.
> [...]
>
> > > +
> > > + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> > > + &pdata->num_in_pads);
> > > + if (error) {
> > > + dev_err(dev, "failed to parse keypad params\n");
> > > + return error;
> >
> > Nit: It's pretty unusual to use this for a standard error handling
> > variable. Consider 'ret' or 'err' as a replacement.
>
> I like "error", in fact there are a lot of these in input. I use "error" for
> data that is only returned from error path and "retval" when the same
> variable is returned in both success and error paths.
If that's your preference then I'm cool with it too. Scrap my comment.
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 15:38 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 15:38 UTC (permalink / raw)
To: linux-arm-kernel
> > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > of ST boards provide. Specific board setup will be put in the
> > > given dt.
> > >
> > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >
> > Are you sure these are in the correct order?
> >
> > What is the history of this commit?
> >
> > > ---
> > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> >
> > This should be submitted as a seperate patch.
>
> Why do we have such requirement? To me it would make more sense to add
> binding documentation in the same commit as the code that uses these
> bindings.
I'm inclined to agree with you and that's actually how we used to do
it, but a decision was made by the DT guys at one of the Kernel
Summits to submit Documentation as a separate patch.
> [...]
>
> > > +
> > > + error = matrix_keypad_parse_of_params(dev, &pdata->num_out_pads,
> > > + &pdata->num_in_pads);
> > > + if (error) {
> > > + dev_err(dev, "failed to parse keypad params\n");
> > > + return error;
> >
> > Nit: It's pretty unusual to use this for a standard error handling
> > variable. Consider 'ret' or 'err' as a replacement.
>
> I like "error", in fact there are a lot of these in input. I use "error" for
> data that is only returned from error path and "retval" when the same
> variable is returned in both success and error paths.
If that's your preference then I'm cool with it too. Scrap my comment.
[...]
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-10 15:38 ` Lee Jones
@ 2014-03-10 15:58 ` Dmitry Torokhov
-1 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-10 15:58 UTC (permalink / raw)
To: Lee Jones
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
On Mon, Mar 10, 2014 at 03:38:15PM +0000, Lee Jones wrote:
> > > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > > of ST boards provide. Specific board setup will be put in the
> > > > given dt.
> > > >
> > > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > >
> > > Are you sure these are in the correct order?
> > >
> > > What is the history of this commit?
> > >
> > > > ---
> > > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> > >
> > > This should be submitted as a seperate patch.
> >
> > Why do we have such requirement? To me it would make more sense to add
> > binding documentation in the same commit as the code that uses these
> > bindings.
>
> I'm inclined to agree with you and that's actually how we used to do
> it, but a decision was made by the DT guys at one of the Kernel
> Summits to submit Documentation as a separate patch.
Do you have background for this decision? To me it is akin splitting
header file into a separate patch.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 15:58 ` Dmitry Torokhov
0 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-10 15:58 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Mar 10, 2014 at 03:38:15PM +0000, Lee Jones wrote:
> > > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > > of ST boards provide. Specific board setup will be put in the
> > > > given dt.
> > > >
> > > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > >
> > > Are you sure these are in the correct order?
> > >
> > > What is the history of this commit?
> > >
> > > > ---
> > > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> > >
> > > This should be submitted as a seperate patch.
> >
> > Why do we have such requirement? To me it would make more sense to add
> > binding documentation in the same commit as the code that uses these
> > bindings.
>
> I'm inclined to agree with you and that's actually how we used to do
> it, but a decision was made by the DT guys at one of the Kernel
> Summits to submit Documentation as a separate patch.
Do you have background for this decision? To me it is akin splitting
header file into a separate patch.
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-10 15:58 ` Dmitry Torokhov
(?)
@ 2014-03-10 16:35 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 16:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> > > > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > > > of ST boards provide. Specific board setup will be put in the
> > > > > given dt.
> > > > >
> > > > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > > >
> > > > Are you sure these are in the correct order?
> > > >
> > > > What is the history of this commit?
> > > >
> > > > > ---
> > > > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> > > >
> > > > This should be submitted as a seperate patch.
> > >
> > > Why do we have such requirement? To me it would make more sense to add
> > > binding documentation in the same commit as the code that uses these
> > > bindings.
> >
> > I'm inclined to agree with you and that's actually how we used to do
> > it, but a decision was made by the DT guys at one of the Kernel
> > Summits to submit Documentation as a separate patch.
>
> Do you have background for this decision? To me it is akin splitting
> header file into a separate patch.
The discussion/decision was verbal unfortunately.
I don't really mind either way. I'm just attempting to enforce the
decisions that were made by the forces-that-be. Perhaps ping
devicetree@vger.kernel.org with me in CC for more clarification if
required.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 16:35 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 16:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Gabriel FERNANDEZ, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> > > > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > > > of ST boards provide. Specific board setup will be put in the
> > > > > given dt.
> > > > >
> > > > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > > >
> > > > Are you sure these are in the correct order?
> > > >
> > > > What is the history of this commit?
> > > >
> > > > > ---
> > > > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> > > >
> > > > This should be submitted as a seperate patch.
> > >
> > > Why do we have such requirement? To me it would make more sense to add
> > > binding documentation in the same commit as the code that uses these
> > > bindings.
> >
> > I'm inclined to agree with you and that's actually how we used to do
> > it, but a decision was made by the DT guys at one of the Kernel
> > Summits to submit Documentation as a separate patch.
>
> Do you have background for this decision? To me it is akin splitting
> header file into a separate patch.
The discussion/decision was verbal unfortunately.
I don't really mind either way. I'm just attempting to enforce the
decisions that were made by the forces-that-be. Perhaps ping
devicetree@vger.kernel.org with me in CC for more clarification if
required.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-10 16:35 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-10 16:35 UTC (permalink / raw)
To: linux-arm-kernel
> > > > > This patch adds ST Keyscan driver to use the keypad hw a subset
> > > > > of ST boards provide. Specific board setup will be put in the
> > > > > given dt.
> > > > >
> > > > > Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> > > > > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> > > >
> > > > Are you sure these are in the correct order?
> > > >
> > > > What is the history of this commit?
> > > >
> > > > > ---
> > > > > .../devicetree/bindings/input/st-keypad.txt | 50 ++++
> > > >
> > > > This should be submitted as a seperate patch.
> > >
> > > Why do we have such requirement? To me it would make more sense to add
> > > binding documentation in the same commit as the code that uses these
> > > bindings.
> >
> > I'm inclined to agree with you and that's actually how we used to do
> > it, but a decision was made by the DT guys at one of the Kernel
> > Summits to submit Documentation as a separate patch.
>
> Do you have background for this decision? To me it is akin splitting
> header file into a separate patch.
The discussion/decision was verbal unfortunately.
I don't really mind either way. I'm just attempting to enforce the
decisions that were made by the forces-that-be. Perhaps ping
devicetree at vger.kernel.org with me in CC for more clarification if
required.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-10 11:48 ` Lee Jones
(?)
@ 2014-03-14 10:13 ` Gabriel Fernandez
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-14 10:13 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
Hi Lee,
On 03/10/2014 12:48 PM, Lee Jones wrote:
> Hi Gabi,
>
> Sorry for the delay. It was a hectic week last week.
>
> As promised:
>
>> This patch adds ST Keyscan driver to use the keypad hw a subset
>> of ST boards provide. Specific board setup will be put in the
>> given dt.
>>
>> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Are you sure these are in the correct order?
ok i change the order
>> +- linux,keymap: The keymap for keys as described in the binding document
>> + devicetree/bindings/input/matrix-keymap.txt.
>> +
>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the keypad
>> + controller.
>> +
>> +- st,debounce_us: Debouncing interval time in microseconds
> I'm sure there will be a shared binding for de-bounce.
>
> If not, there certainly should be.
you want to refer to "debounce-interval" ?
>
> +Example:
> +
> +keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
> Is there any way we can make this more specific to _this_ IP?
for my knowledge this IP is the same for stixxxx platform.
>
>> + To compile this driver as a module, choose M here: the
>> + module will be called stm-keyscan.
>> +
>> config KEYBOARD_SUNKBD
>> tristate "Sun Type 4 and Type 5 keyboard"
>> select SERIO
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index a699b61..5fd020a 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
>> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
>> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
>> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
>> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
>> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
>> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
>> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
>> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
>> new file mode 100644
>> index 0000000..606cc19
>> --- /dev/null
>> +++ b/drivers/input/keyboard/st-keyscan.c
>> @@ -0,0 +1,323 @@
>> +/*
>> + * STMicroelectronics Key Scanning driver
>> + *
>> + * Copyright (c) 2014 STMicroelectonics Ltd.
>> + * Author: Stuart Menefy <stuart.menefy@st.com>
>> + *
>> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
> Did sh_keysc.c ever exist in Mainline?
i think no, i 'll suppress "Based on sh_keysc.c, copyright 2008 Magnus Damm"
>
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/input/matrix_keypad.h>
>> +
>> +#define ST_KEYSCAN_MAXKEYS 16
>> +
>> +#define KEYSCAN_CONFIG_OFF 0x000
>> +#define KEYSCAN_CONFIG_ENABLE 1
> 0x001?
>
>> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
>> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
>> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
> Odd that these are 3 digit padded? Is there a reason for that?
no reason for the padded, i will change that.
>> +struct keypad_platform_data {
>> + const struct matrix_keymap_data *keymap_data;
>> + unsigned int num_out_pads;
>> + unsigned int num_in_pads;
>> + unsigned int debounce_us;
>> +};
>> +
>> +struct keyscan_priv {
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clk;
>> + struct input_dev *input_dev;
>> + struct keypad_platform_data *config;
>> + unsigned int last_state;
>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
> Seems odd to limit this. Can't the information come from DT
> i.e. keypad,num-rows and keypad,num-columns?
>
i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into 'n_cols'
> Nit: It's pretty unusual to use this for a standard error handling
> variable. Consider 'ret' or 'err' as a replacement.
>
>> + }
>> +
>> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
> Isn't this a required property? Might be worth checking the return
> value if so.
no mandatory property, i will update the dt binding.
>
>> + st_kp->config = pdata;
>> +
>> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
>> + pdata->num_out_pads,
>> + pdata->num_in_pads,
>> + pdata->debounce_us);
> Might be worth moving this down passed the final point of failure.
>
>> + error = of_property_read_u32_array(np, "linux,keymap",
>> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
>> + if (error) {
>> + dev_err(dev, "failed to parse keymap\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init keyscan_probe(struct platform_device *pdev)
>> +{
>> + struct keypad_platform_data *pdata =
>> + dev_get_platdata(&pdev->dev);
> Do we really support platform data still?
no, i will remove that.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
> Replace the two above with devm_ioremap_resource().
>
> Make sure the IORESOURCE_CACHEABLE is set.
ok, we are in no cacheable use case.
>
>> + }
>> +
>> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
>> + pdev->name, pdev);
>> + if (error) {
>> + dev_err(dev, "failed to request IRQ\n");
>> + return error;
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate the input device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + st_kp->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(st_kp->clk)) {
>> + dev_err(dev, "cannot get clock");
>> + return PTR_ERR(st_kp->clk);
>> + }
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + dev_err(dev, "failed to allocate input device\n");
>> + return -ENOMEM;
>> + }
> Wasn't this done already?
yes, crap these lines.
>> + st_kp->input_dev = input_dev;
>> +
>> + input_dev->name = pdev->name;
>> + input_dev->phys = "keyscan-keys/input0";
>> + input_dev->dev.parent = dev;
>> +
>> + input_dev->id.bustype = BUS_HOST;
>> + input_dev->id.vendor = 0x0001;
>> + input_dev->id.product = 0x0001;
>> + input_dev->id.version = 0x0100;
> Any chance we can #define these?
i will follow Dmitry remark (omit these information)
>
>> + if (!pdata) {
>> + error = keypad_matrix_key_parse_dt(st_kp);
>> + if (error)
>> + return error;
>> + pdata = st_kp->config;
> Is pdata used after this?
no, i will remove platform data support
Thanks
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-14 10:13 ` Gabriel Fernandez
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-14 10:13 UTC (permalink / raw)
To: Lee Jones
Cc: Mark Rutland, devicetree, Russell King, kernel, Pawel Moll,
Ian Campbell, Dmitry Torokhov, linux-doc, linux-kernel,
Rob Herring, Giuseppe Condorelli, Rob Landley, Kumar Gala,
Grant Likely, linux-input, linux-arm-kernel
Hi Lee,
On 03/10/2014 12:48 PM, Lee Jones wrote:
> Hi Gabi,
>
> Sorry for the delay. It was a hectic week last week.
>
> As promised:
>
>> This patch adds ST Keyscan driver to use the keypad hw a subset
>> of ST boards provide. Specific board setup will be put in the
>> given dt.
>>
>> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Are you sure these are in the correct order?
ok i change the order
>> +- linux,keymap: The keymap for keys as described in the binding document
>> + devicetree/bindings/input/matrix-keymap.txt.
>> +
>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the keypad
>> + controller.
>> +
>> +- st,debounce_us: Debouncing interval time in microseconds
> I'm sure there will be a shared binding for de-bounce.
>
> If not, there certainly should be.
you want to refer to "debounce-interval" ?
>
> +Example:
> +
> +keyscan: keyscan@fe4b0000 {
> + compatible = "st,keypad";
> Is there any way we can make this more specific to _this_ IP?
for my knowledge this IP is the same for stixxxx platform.
>
>> + To compile this driver as a module, choose M here: the
>> + module will be called stm-keyscan.
>> +
>> config KEYBOARD_SUNKBD
>> tristate "Sun Type 4 and Type 5 keyboard"
>> select SERIO
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index a699b61..5fd020a 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
>> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
>> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
>> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
>> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
>> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
>> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
>> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
>> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
>> new file mode 100644
>> index 0000000..606cc19
>> --- /dev/null
>> +++ b/drivers/input/keyboard/st-keyscan.c
>> @@ -0,0 +1,323 @@
>> +/*
>> + * STMicroelectronics Key Scanning driver
>> + *
>> + * Copyright (c) 2014 STMicroelectonics Ltd.
>> + * Author: Stuart Menefy <stuart.menefy@st.com>
>> + *
>> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
> Did sh_keysc.c ever exist in Mainline?
i think no, i 'll suppress "Based on sh_keysc.c, copyright 2008 Magnus Damm"
>
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/input/matrix_keypad.h>
>> +
>> +#define ST_KEYSCAN_MAXKEYS 16
>> +
>> +#define KEYSCAN_CONFIG_OFF 0x000
>> +#define KEYSCAN_CONFIG_ENABLE 1
> 0x001?
>
>> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
>> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
>> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
> Odd that these are 3 digit padded? Is there a reason for that?
no reason for the padded, i will change that.
>> +struct keypad_platform_data {
>> + const struct matrix_keymap_data *keymap_data;
>> + unsigned int num_out_pads;
>> + unsigned int num_in_pads;
>> + unsigned int debounce_us;
>> +};
>> +
>> +struct keyscan_priv {
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clk;
>> + struct input_dev *input_dev;
>> + struct keypad_platform_data *config;
>> + unsigned int last_state;
>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
> Seems odd to limit this. Can't the information come from DT
> i.e. keypad,num-rows and keypad,num-columns?
>
i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into 'n_cols'
> Nit: It's pretty unusual to use this for a standard error handling
> variable. Consider 'ret' or 'err' as a replacement.
>
>> + }
>> +
>> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
> Isn't this a required property? Might be worth checking the return
> value if so.
no mandatory property, i will update the dt binding.
>
>> + st_kp->config = pdata;
>> +
>> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
>> + pdata->num_out_pads,
>> + pdata->num_in_pads,
>> + pdata->debounce_us);
> Might be worth moving this down passed the final point of failure.
>
>> + error = of_property_read_u32_array(np, "linux,keymap",
>> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
>> + if (error) {
>> + dev_err(dev, "failed to parse keymap\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init keyscan_probe(struct platform_device *pdev)
>> +{
>> + struct keypad_platform_data *pdata =
>> + dev_get_platdata(&pdev->dev);
> Do we really support platform data still?
no, i will remove that.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
> Replace the two above with devm_ioremap_resource().
>
> Make sure the IORESOURCE_CACHEABLE is set.
ok, we are in no cacheable use case.
>
>> + }
>> +
>> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
>> + pdev->name, pdev);
>> + if (error) {
>> + dev_err(dev, "failed to request IRQ\n");
>> + return error;
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate the input device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + st_kp->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(st_kp->clk)) {
>> + dev_err(dev, "cannot get clock");
>> + return PTR_ERR(st_kp->clk);
>> + }
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + dev_err(dev, "failed to allocate input device\n");
>> + return -ENOMEM;
>> + }
> Wasn't this done already?
yes, crap these lines.
>> + st_kp->input_dev = input_dev;
>> +
>> + input_dev->name = pdev->name;
>> + input_dev->phys = "keyscan-keys/input0";
>> + input_dev->dev.parent = dev;
>> +
>> + input_dev->id.bustype = BUS_HOST;
>> + input_dev->id.vendor = 0x0001;
>> + input_dev->id.product = 0x0001;
>> + input_dev->id.version = 0x0100;
> Any chance we can #define these?
i will follow Dmitry remark (omit these information)
>
>> + if (!pdata) {
>> + error = keypad_matrix_key_parse_dt(st_kp);
>> + if (error)
>> + return error;
>> + pdata = st_kp->config;
> Is pdata used after this?
no, i will remove platform data support
Thanks
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-14 10:13 ` Gabriel Fernandez
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-14 10:13 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lee,
On 03/10/2014 12:48 PM, Lee Jones wrote:
> Hi Gabi,
>
> Sorry for the delay. It was a hectic week last week.
>
> As promised:
>
>> This patch adds ST Keyscan driver to use the keypad hw a subset
>> of ST boards provide. Specific board setup will be put in the
>> given dt.
>>
>> Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> Are you sure these are in the correct order?
ok i change the order
>> +- linux,keymap: The keymap for keys as described in the binding document
>> + devicetree/bindings/input/matrix-keymap.txt.
>> +
>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the keypad
>> + controller.
>> +
>> +- st,debounce_us: Debouncing interval time in microseconds
> I'm sure there will be a shared binding for de-bounce.
>
> If not, there certainly should be.
you want to refer to "debounce-interval" ?
>
> +Example:
> +
> +keyscan: keyscan at fe4b0000 {
> + compatible = "st,keypad";
> Is there any way we can make this more specific to _this_ IP?
for my knowledge this IP is the same for stixxxx platform.
>
>> + To compile this driver as a module, choose M here: the
>> + module will be called stm-keyscan.
>> +
>> config KEYBOARD_SUNKBD
>> tristate "Sun Type 4 and Type 5 keyboard"
>> select SERIO
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index a699b61..5fd020a 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
>> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
>> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
>> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
>> +obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
>> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
>> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
>> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
>> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
>> new file mode 100644
>> index 0000000..606cc19
>> --- /dev/null
>> +++ b/drivers/input/keyboard/st-keyscan.c
>> @@ -0,0 +1,323 @@
>> +/*
>> + * STMicroelectronics Key Scanning driver
>> + *
>> + * Copyright (c) 2014 STMicroelectonics Ltd.
>> + * Author: Stuart Menefy <stuart.menefy@st.com>
>> + *
>> + * Based on sh_keysc.c, copyright 2008 Magnus Damm
> Did sh_keysc.c ever exist in Mainline?
i think no, i 'll suppress "Based on sh_keysc.c, copyright 2008 Magnus Damm"
>
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/clk.h>
>> +#include <linux/io.h>
>> +#include <linux/input/matrix_keypad.h>
>> +
>> +#define ST_KEYSCAN_MAXKEYS 16
>> +
>> +#define KEYSCAN_CONFIG_OFF 0x000
>> +#define KEYSCAN_CONFIG_ENABLE 1
> 0x001?
>
>> +#define KEYSCAN_DEBOUNCE_TIME_OFF 0x004
>> +#define KEYSCAN_MATRIX_STATE_OFF 0x008
>> +#define KEYSCAN_MATRIX_DIM_OFF 0x00c
> Odd that these are 3 digit padded? Is there a reason for that?
no reason for the padded, i will change that.
>> +struct keypad_platform_data {
>> + const struct matrix_keymap_data *keymap_data;
>> + unsigned int num_out_pads;
>> + unsigned int num_in_pads;
>> + unsigned int debounce_us;
>> +};
>> +
>> +struct keyscan_priv {
>> + void __iomem *base;
>> + int irq;
>> + struct clk *clk;
>> + struct input_dev *input_dev;
>> + struct keypad_platform_data *config;
>> + unsigned int last_state;
>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
> Seems odd to limit this. Can't the information come from DT
> i.e. keypad,num-rows and keypad,num-columns?
>
i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into 'n_cols'
> Nit: It's pretty unusual to use this for a standard error handling
> variable. Consider 'ret' or 'err' as a replacement.
>
>> + }
>> +
>> + of_property_read_u32(np, "st,debounce_us", &pdata->debounce_us);
> Isn't this a required property? Might be worth checking the return
> value if so.
no mandatory property, i will update the dt binding.
>
>> + st_kp->config = pdata;
>> +
>> + dev_info(dev, "rows=%d col=%d debounce=%d\n",
>> + pdata->num_out_pads,
>> + pdata->num_in_pads,
>> + pdata->debounce_us);
> Might be worth moving this down passed the final point of failure.
>
>> + error = of_property_read_u32_array(np, "linux,keymap",
>> + st_kp->keycodes, ST_KEYSCAN_MAXKEYS);
>> + if (error) {
>> + dev_err(dev, "failed to parse keymap\n");
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int __init keyscan_probe(struct platform_device *pdev)
>> +{
>> + struct keypad_platform_data *pdata =
>> + dev_get_platdata(&pdev->dev);
> Do we really support platform data still?
no, i will remove that.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res) {
> + dev_err(dev, "no I/O memory specified\n");
> + return -ENXIO;
> + }
> +
> + len = resource_size(res);
> + if (!devm_request_mem_region(dev, res->start, len, pdev->name)) {
> + dev_err(dev, "failed to reserve I/O memory\n");
> + return -EBUSY;
> + }
> +
> + st_kp->base = devm_ioremap_nocache(dev, res->start, len);
> + if (st_kp->base == NULL) {
> + dev_err(dev, "failed to remap I/O memory\n");
> + return -ENXIO;
> + }
> Replace the two above with devm_ioremap_resource().
>
> Make sure the IORESOURCE_CACHEABLE is set.
ok, we are in no cacheable use case.
>
>> + }
>> +
>> + error = devm_request_irq(dev, st_kp->irq, keyscan_isr, 0,
>> + pdev->name, pdev);
>> + if (error) {
>> + dev_err(dev, "failed to request IRQ\n");
>> + return error;
>> + }
>> +
>> + input_dev = devm_input_allocate_device(&pdev->dev);
>> + if (!input_dev) {
>> + dev_err(&pdev->dev, "failed to allocate the input device\n");
>> + return -ENOMEM;
>> + }
>> +
>> + st_kp->clk = devm_clk_get(dev, NULL);
>> + if (IS_ERR(st_kp->clk)) {
>> + dev_err(dev, "cannot get clock");
>> + return PTR_ERR(st_kp->clk);
>> + }
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + dev_err(dev, "failed to allocate input device\n");
>> + return -ENOMEM;
>> + }
> Wasn't this done already?
yes, crap these lines.
>> + st_kp->input_dev = input_dev;
>> +
>> + input_dev->name = pdev->name;
>> + input_dev->phys = "keyscan-keys/input0";
>> + input_dev->dev.parent = dev;
>> +
>> + input_dev->id.bustype = BUS_HOST;
>> + input_dev->id.vendor = 0x0001;
>> + input_dev->id.product = 0x0001;
>> + input_dev->id.version = 0x0100;
> Any chance we can #define these?
i will follow Dmitry remark (omit these information)
>
>> + if (!pdata) {
>> + error = keypad_matrix_key_parse_dt(st_kp);
>> + if (error)
>> + return error;
>> + pdata = st_kp->config;
> Is pdata used after this?
no, i will remove platform data support
Thanks
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-14 10:13 ` Gabriel Fernandez
(?)
@ 2014-03-14 10:42 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-14 10:42 UTC (permalink / raw)
To: Gabriel Fernandez
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> >Sorry for the delay. It was a hectic week last week.
> >
> >As promised:
> >
> >>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>of ST boards provide. Specific board setup will be put in the
> >>given dt.
> >>
> >>Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> >>Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >Are you sure these are in the correct order?
> ok i change the order
I'm not saying they are in the wrong order, I'm just asking. Who wrote
the patch? Has it changed since?
> >>+- linux,keymap: The keymap for keys as described in the binding document
> >>+ devicetree/bindings/input/matrix-keymap.txt.
> >>+
> >>+- keypad,num-rows: Number of row lines connected to the keypad controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the keypad
> >>+ controller.
> >>+
> >>+- st,debounce_us: Debouncing interval time in microseconds
> >I'm sure there will be a shared binding for de-bounce.
> >
> >If not, there certainly should be.
>
> you want to refer to "debounce-interval" ?
That sounds more generic, but if it's not documented as such, then
please consider doing so.
> >+Example:
> >+
> >+keyscan: keyscan@fe4b0000 {
> >+ compatible = "st,keypad";
> >Is there any way we can make this more specific to _this_ IP?
> for my knowledge this IP is the same for stixxxx platform.
So st,stix-keypad, or st,sti4x-keypad?
I'm just thinking about future proofing the architecture. What if ST
released stj which has a different keypad IP?
> >>+struct keyscan_priv {
> >>+ void __iomem *base;
> >>+ int irq;
> >>+ struct clk *clk;
> >>+ struct input_dev *input_dev;
> >>+ struct keypad_platform_data *config;
> >>+ unsigned int last_state;
> >>+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
> >Seems odd to limit this. Can't the information come from DT
> >i.e. keypad,num-rows and keypad,num-columns?
> >
> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
> 'n_cols'
That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-14 10:42 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-14 10:42 UTC (permalink / raw)
To: Gabriel Fernandez
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> >Sorry for the delay. It was a hectic week last week.
> >
> >As promised:
> >
> >>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>of ST boards provide. Specific board setup will be put in the
> >>given dt.
> >>
> >>Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> >>Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >Are you sure these are in the correct order?
> ok i change the order
I'm not saying they are in the wrong order, I'm just asking. Who wrote
the patch? Has it changed since?
> >>+- linux,keymap: The keymap for keys as described in the binding document
> >>+ devicetree/bindings/input/matrix-keymap.txt.
> >>+
> >>+- keypad,num-rows: Number of row lines connected to the keypad controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the keypad
> >>+ controller.
> >>+
> >>+- st,debounce_us: Debouncing interval time in microseconds
> >I'm sure there will be a shared binding for de-bounce.
> >
> >If not, there certainly should be.
>
> you want to refer to "debounce-interval" ?
That sounds more generic, but if it's not documented as such, then
please consider doing so.
> >+Example:
> >+
> >+keyscan: keyscan@fe4b0000 {
> >+ compatible = "st,keypad";
> >Is there any way we can make this more specific to _this_ IP?
> for my knowledge this IP is the same for stixxxx platform.
So st,stix-keypad, or st,sti4x-keypad?
I'm just thinking about future proofing the architecture. What if ST
released stj which has a different keypad IP?
> >>+struct keyscan_priv {
> >>+ void __iomem *base;
> >>+ int irq;
> >>+ struct clk *clk;
> >>+ struct input_dev *input_dev;
> >>+ struct keypad_platform_data *config;
> >>+ unsigned int last_state;
> >>+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
> >Seems odd to limit this. Can't the information come from DT
> >i.e. keypad,num-rows and keypad,num-columns?
> >
> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
> 'n_cols'
That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-14 10:42 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-14 10:42 UTC (permalink / raw)
To: linux-arm-kernel
> >Sorry for the delay. It was a hectic week last week.
> >
> >As promised:
> >
> >>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>of ST boards provide. Specific board setup will be put in the
> >>given dt.
> >>
> >>Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> >>Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >Are you sure these are in the correct order?
> ok i change the order
I'm not saying they are in the wrong order, I'm just asking. Who wrote
the patch? Has it changed since?
> >>+- linux,keymap: The keymap for keys as described in the binding document
> >>+ devicetree/bindings/input/matrix-keymap.txt.
> >>+
> >>+- keypad,num-rows: Number of row lines connected to the keypad controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the keypad
> >>+ controller.
> >>+
> >>+- st,debounce_us: Debouncing interval time in microseconds
> >I'm sure there will be a shared binding for de-bounce.
> >
> >If not, there certainly should be.
>
> you want to refer to "debounce-interval" ?
That sounds more generic, but if it's not documented as such, then
please consider doing so.
> >+Example:
> >+
> >+keyscan: keyscan at fe4b0000 {
> >+ compatible = "st,keypad";
> >Is there any way we can make this more specific to _this_ IP?
> for my knowledge this IP is the same for stixxxx platform.
So st,stix-keypad, or st,sti4x-keypad?
I'm just thinking about future proofing the architecture. What if ST
released stj which has a different keypad IP?
> >>+struct keyscan_priv {
> >>+ void __iomem *base;
> >>+ int irq;
> >>+ struct clk *clk;
> >>+ struct input_dev *input_dev;
> >>+ struct keypad_platform_data *config;
> >>+ unsigned int last_state;
> >>+ u32 keycodes[ST_KEYSCAN_MAXKEYS];
> >Seems odd to limit this. Can't the information come from DT
> >i.e. keypad,num-rows and keypad,num-columns?
> >
> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
> 'n_cols'
That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-14 10:13 ` Gabriel Fernandez
@ 2014-03-14 15:26 ` Dmitry Torokhov
-1 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-14 15:26 UTC (permalink / raw)
To: Gabriel Fernandez
Cc: Lee Jones, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Rob Landley, Russell King, Grant Likely, devicetree,
linux-doc, linux-kernel, linux-arm-kernel, linux-input, kernel,
Giuseppe Condorelli
On Fri, Mar 14, 2014 at 11:13:17AM +0100, Gabriel Fernandez wrote:
> Hi Lee,
>
> On 03/10/2014 12:48 PM, Lee Jones wrote:
> >Hi Gabi,
> >
> >Sorry for the delay. It was a hectic week last week.
> >
> >As promised:
> >
> >>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>of ST boards provide. Specific board setup will be put in the
> >>given dt.
> >>
> >>Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> >>Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >Are you sure these are in the correct order?
> ok i change the order
>
> >>+- linux,keymap: The keymap for keys as described in the binding document
> >>+ devicetree/bindings/input/matrix-keymap.txt.
> >>+
> >>+- keypad,num-rows: Number of row lines connected to the keypad controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the keypad
> >>+ controller.
> >>+
> >>+- st,debounce_us: Debouncing interval time in microseconds
> >I'm sure there will be a shared binding for de-bounce.
> >
> >If not, there certainly should be.
>
> you want to refer to "debounce-interval" ?
>
> >
> >+Example:
> >+
> >+keyscan: keyscan@fe4b0000 {
> >+ compatible = "st,keypad";
> >Is there any way we can make this more specific to _this_ IP?
> for my knowledge this IP is the same for stixxxx platform.
>
>
> >
> >>+ To compile this driver as a module, choose M here: the
> >>+ module will be called stm-keyscan.
> >>+
> >> config KEYBOARD_SUNKBD
> >> tristate "Sun Type 4 and Type 5 keyboard"
> >> select SERIO
> >>diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> >>index a699b61..5fd020a 100644
> >>--- a/drivers/input/keyboard/Makefile
> >>+++ b/drivers/input/keyboard/Makefile
> >>@@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> >> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> >> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> >> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> >>+obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> >> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> >> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> >> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> >>diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> >>new file mode 100644
> >>index 0000000..606cc19
> >>--- /dev/null
> >>+++ b/drivers/input/keyboard/st-keyscan.c
> >>@@ -0,0 +1,323 @@
> >>+/*
> >>+ * STMicroelectronics Key Scanning driver
> >>+ *
> >>+ * Copyright (c) 2014 STMicroelectonics Ltd.
> >>+ * Author: Stuart Menefy <stuart.menefy@st.com>
> >>+ *
> >>+ * Based on sh_keysc.c, copyright 2008 Magnus Damm
> >Did sh_keysc.c ever exist in Mainline?
> i think no, i 'll suppress "Based on sh_keysc.c, copyright 2008 Magnus Damm"
It is still in here:
[dtor@dtor-d630 work]$ git log --oneline --
drivers/input/keyboard/sh_keysc.c | wc -l
31
[dtor@dtor-d630 work]$
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-14 15:26 ` Dmitry Torokhov
0 siblings, 0 replies; 51+ messages in thread
From: Dmitry Torokhov @ 2014-03-14 15:26 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 14, 2014 at 11:13:17AM +0100, Gabriel Fernandez wrote:
> Hi Lee,
>
> On 03/10/2014 12:48 PM, Lee Jones wrote:
> >Hi Gabi,
> >
> >Sorry for the delay. It was a hectic week last week.
> >
> >As promised:
> >
> >>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>of ST boards provide. Specific board setup will be put in the
> >>given dt.
> >>
> >>Signed-off-by: Giuseppe Condorelli <giuseppe.condorelli@st.com>
> >>Signed-off-by: Gabriel Fernandez <gabriel.fernandez@st.com>
> >Are you sure these are in the correct order?
> ok i change the order
>
> >>+- linux,keymap: The keymap for keys as described in the binding document
> >>+ devicetree/bindings/input/matrix-keymap.txt.
> >>+
> >>+- keypad,num-rows: Number of row lines connected to the keypad controller.
> >>+
> >>+- keypad,num-columns: Number of column lines connected to the keypad
> >>+ controller.
> >>+
> >>+- st,debounce_us: Debouncing interval time in microseconds
> >I'm sure there will be a shared binding for de-bounce.
> >
> >If not, there certainly should be.
>
> you want to refer to "debounce-interval" ?
>
> >
> >+Example:
> >+
> >+keyscan: keyscan at fe4b0000 {
> >+ compatible = "st,keypad";
> >Is there any way we can make this more specific to _this_ IP?
> for my knowledge this IP is the same for stixxxx platform.
>
>
> >
> >>+ To compile this driver as a module, choose M here: the
> >>+ module will be called stm-keyscan.
> >>+
> >> config KEYBOARD_SUNKBD
> >> tristate "Sun Type 4 and Type 5 keyboard"
> >> select SERIO
> >>diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> >>index a699b61..5fd020a 100644
> >>--- a/drivers/input/keyboard/Makefile
> >>+++ b/drivers/input/keyboard/Makefile
> >>@@ -50,6 +50,7 @@ obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
> >> obj-$(CONFIG_KEYBOARD_SPEAR) += spear-keyboard.o
> >> obj-$(CONFIG_KEYBOARD_STMPE) += stmpe-keypad.o
> >> obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> >>+obj-$(CONFIG_KEYBOARD_ST_KEYSCAN) += st-keyscan.o
> >> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> >> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> >> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> >>diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> >>new file mode 100644
> >>index 0000000..606cc19
> >>--- /dev/null
> >>+++ b/drivers/input/keyboard/st-keyscan.c
> >>@@ -0,0 +1,323 @@
> >>+/*
> >>+ * STMicroelectronics Key Scanning driver
> >>+ *
> >>+ * Copyright (c) 2014 STMicroelectonics Ltd.
> >>+ * Author: Stuart Menefy <stuart.menefy@st.com>
> >>+ *
> >>+ * Based on sh_keysc.c, copyright 2008 Magnus Damm
> >Did sh_keysc.c ever exist in Mainline?
> i think no, i 'll suppress "Based on sh_keysc.c, copyright 2008 Magnus Damm"
It is still in here:
[dtor at dtor-d630 work]$ git log --oneline --
drivers/input/keyboard/sh_keysc.c | wc -l
31
[dtor at dtor-d630 work]$
--
Dmitry
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-14 10:42 ` Lee Jones
(?)
@ 2014-03-18 10:25 ` Gabriel Fernandez
-1 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-18 10:25 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
Hi Lee,
On 03/14/2014 11:42 AM, Lee Jones wrote:
>>> Sorry for the delay. It was a hectic week last week.
>>>
>>> As promised:
>>>
>>>> This patch adds ST Keyscan driver to use the keypad hw a subset
>>>> of ST boards provide. Specific board setup will be put in the
>>>> given dt.
>>>>
>>>> Signed-off-by: Giuseppe Condorelli<giuseppe.condorelli@st.com>
>>>> Signed-off-by: Gabriel Fernandez<gabriel.fernandez@st.com>
>>> Are you sure these are in the correct order?
>> ok i change the order
> I'm not saying they are in the wrong order, I'm just asking. Who wrote
> the patch? Has it changed since?
Sorry...
I wrote the patch, then Guiseppe has changed and tested, and I re-modifiedit
>>>> +- linux,keymap: The keymap for keys as described in the binding document
>>>> + devicetree/bindings/input/matrix-keymap.txt.
>>>> +
>>>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>>>> +
>>>> +- keypad,num-columns: Number of column lines connected to the keypad
>>>> + controller.
>>>> +
>>>> +- st,debounce_us: Debouncing interval time in microseconds
>>> I'm sure there will be a shared binding for de-bounce.
>>>
>>> If not, there certainly should be.
>> you want to refer to "debounce-interval" ?
> That sounds more generic, but if it's not documented as such, then
> please consider doing so.
>
>>> +Example:
>>> +
>>> +keyscan: keyscan@fe4b0000 {
>>> + compatible = "st,keypad";
>>> Is there any way we can make this more specific to _this_ IP?
>> for my knowledge this IP is the same for stixxxx platform.
> So st,stix-keypad, or st,sti4x-keypad?
>
> I'm just thinking about future proofing the architecture. What if ST
> released stj which has a different keypad IP?
After discussing internally with st "st,sti-keyscan" is better
>>>> +struct keyscan_priv {
>>>> + void __iomem *base;
>>>> + int irq;
>>>> + struct clk *clk;
>>>> + struct input_dev *input_dev;
>>>> + struct keypad_platform_data *config;
>>>> + unsigned int last_state;
>>>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
>>> Seems odd to limit this. Can't the information come from DT
>>> i.e. keypad,num-rows and keypad,num-columns?
>>>
>> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
>> 'n_cols'
> That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
> dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
ok
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-18 10:25 ` Gabriel Fernandez
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-18 10:25 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
Hi Lee,
On 03/14/2014 11:42 AM, Lee Jones wrote:
>>> Sorry for the delay. It was a hectic week last week.
>>>
>>> As promised:
>>>
>>>> This patch adds ST Keyscan driver to use the keypad hw a subset
>>>> of ST boards provide. Specific board setup will be put in the
>>>> given dt.
>>>>
>>>> Signed-off-by: Giuseppe Condorelli<giuseppe.condorelli@st.com>
>>>> Signed-off-by: Gabriel Fernandez<gabriel.fernandez@st.com>
>>> Are you sure these are in the correct order?
>> ok i change the order
> I'm not saying they are in the wrong order, I'm just asking. Who wrote
> the patch? Has it changed since?
Sorry...
I wrote the patch, then Guiseppe has changed and tested, and I re-modifiedit
>>>> +- linux,keymap: The keymap for keys as described in the binding document
>>>> + devicetree/bindings/input/matrix-keymap.txt.
>>>> +
>>>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>>>> +
>>>> +- keypad,num-columns: Number of column lines connected to the keypad
>>>> + controller.
>>>> +
>>>> +- st,debounce_us: Debouncing interval time in microseconds
>>> I'm sure there will be a shared binding for de-bounce.
>>>
>>> If not, there certainly should be.
>> you want to refer to "debounce-interval" ?
> That sounds more generic, but if it's not documented as such, then
> please consider doing so.
>
>>> +Example:
>>> +
>>> +keyscan: keyscan@fe4b0000 {
>>> + compatible = "st,keypad";
>>> Is there any way we can make this more specific to _this_ IP?
>> for my knowledge this IP is the same for stixxxx platform.
> So st,stix-keypad, or st,sti4x-keypad?
>
> I'm just thinking about future proofing the architecture. What if ST
> released stj which has a different keypad IP?
After discussing internally with st "st,sti-keyscan" is better
>>>> +struct keyscan_priv {
>>>> + void __iomem *base;
>>>> + int irq;
>>>> + struct clk *clk;
>>>> + struct input_dev *input_dev;
>>>> + struct keypad_platform_data *config;
>>>> + unsigned int last_state;
>>>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
>>> Seems odd to limit this. Can't the information come from DT
>>> i.e. keypad,num-rows and keypad,num-columns?
>>>
>> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
>> 'n_cols'
> That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
> dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
ok
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-18 10:25 ` Gabriel Fernandez
0 siblings, 0 replies; 51+ messages in thread
From: Gabriel Fernandez @ 2014-03-18 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Hi Lee,
On 03/14/2014 11:42 AM, Lee Jones wrote:
>>> Sorry for the delay. It was a hectic week last week.
>>>
>>> As promised:
>>>
>>>> This patch adds ST Keyscan driver to use the keypad hw a subset
>>>> of ST boards provide. Specific board setup will be put in the
>>>> given dt.
>>>>
>>>> Signed-off-by: Giuseppe Condorelli<giuseppe.condorelli@st.com>
>>>> Signed-off-by: Gabriel Fernandez<gabriel.fernandez@st.com>
>>> Are you sure these are in the correct order?
>> ok i change the order
> I'm not saying they are in the wrong order, I'm just asking. Who wrote
> the patch? Has it changed since?
Sorry...
I wrote the patch, then Guiseppe has changed and tested, and I re-modifiedit
>>>> +- linux,keymap: The keymap for keys as described in the binding document
>>>> + devicetree/bindings/input/matrix-keymap.txt.
>>>> +
>>>> +- keypad,num-rows: Number of row lines connected to the keypad controller.
>>>> +
>>>> +- keypad,num-columns: Number of column lines connected to the keypad
>>>> + controller.
>>>> +
>>>> +- st,debounce_us: Debouncing interval time in microseconds
>>> I'm sure there will be a shared binding for de-bounce.
>>>
>>> If not, there certainly should be.
>> you want to refer to "debounce-interval" ?
> That sounds more generic, but if it's not documented as such, then
> please consider doing so.
>
>>> +Example:
>>> +
>>> +keyscan: keyscan at fe4b0000 {
>>> + compatible = "st,keypad";
>>> Is there any way we can make this more specific to _this_ IP?
>> for my knowledge this IP is the same for stixxxx platform.
> So st,stix-keypad, or st,sti4x-keypad?
>
> I'm just thinking about future proofing the architecture. What if ST
> released stj which has a different keypad IP?
After discussing internally with st "st,sti-keyscan" is better
>>>> +struct keyscan_priv {
>>>> + void __iomem *base;
>>>> + int irq;
>>>> + struct clk *clk;
>>>> + struct input_dev *input_dev;
>>>> + struct keypad_platform_data *config;
>>>> + unsigned int last_state;
>>>> + u32 keycodes[ST_KEYSCAN_MAXKEYS];
>>> Seems odd to limit this. Can't the information come from DT
>>> i.e. keypad,num-rows and keypad,num-columns?
>>>
>> i 'll rename 'num_out_pads' into 'n_rows' and 'num_in_pads' into
>> 'n_cols'
> That's not quite what I meant, I mean can't ST_KEY_MAXKEYS be more
> dynamic and be obtained from (keypad,num-rows * keypad,num-columns)?
ok
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
2014-03-18 10:25 ` Gabriel Fernandez
@ 2014-03-18 11:01 ` Lee Jones
-1 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-18 11:01 UTC (permalink / raw)
To: Gabriel Fernandez
Cc: Dmitry Torokhov, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, Rob Landley, Russell King,
Grant Likely, devicetree, linux-doc, linux-kernel,
linux-arm-kernel, linux-input, kernel, Giuseppe Condorelli
> >>>Sorry for the delay. It was a hectic week last week.
> >>>
> >>>As promised:
> >>>
> >>>>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>>>of ST boards provide. Specific board setup will be put in the
> >>>>given dt.
> >>>>
> >>>>Signed-off-by: Giuseppe Condorelli<giuseppe.condorelli@st.com>
> >>>>Signed-off-by: Gabriel Fernandez<gabriel.fernandez@st.com>
> >>>Are you sure these are in the correct order?
> >>ok i change the order
> >I'm not saying they are in the wrong order, I'm just asking. Who wrote
> >the patch? Has it changed since?
> Sorry...
> I wrote the patch, then Guiseppe has changed and tested, and I re-modifiedit
Ah, then it starts to get very complicated. :)
If you wrote the patch, you need to be at the top of the list.
<snip>
> >>>+keyscan: keyscan@fe4b0000 {
> >>>+ compatible = "st,keypad";
> >>>Is there any way we can make this more specific to _this_ IP?
> >>for my knowledge this IP is the same for stixxxx platform.
> >So st,stix-keypad, or st,sti4x-keypad?
> >
> >I'm just thinking about future proofing the architecture. What if ST
> >released stj which has a different keypad IP?
> After discussing internally with st "st,sti-keyscan" is better
Perfect.
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver
@ 2014-03-18 11:01 ` Lee Jones
0 siblings, 0 replies; 51+ messages in thread
From: Lee Jones @ 2014-03-18 11:01 UTC (permalink / raw)
To: linux-arm-kernel
> >>>Sorry for the delay. It was a hectic week last week.
> >>>
> >>>As promised:
> >>>
> >>>>This patch adds ST Keyscan driver to use the keypad hw a subset
> >>>>of ST boards provide. Specific board setup will be put in the
> >>>>given dt.
> >>>>
> >>>>Signed-off-by: Giuseppe Condorelli<giuseppe.condorelli@st.com>
> >>>>Signed-off-by: Gabriel Fernandez<gabriel.fernandez@st.com>
> >>>Are you sure these are in the correct order?
> >>ok i change the order
> >I'm not saying they are in the wrong order, I'm just asking. Who wrote
> >the patch? Has it changed since?
> Sorry...
> I wrote the patch, then Guiseppe has changed and tested, and I re-modifiedit
Ah, then it starts to get very complicated. :)
If you wrote the patch, you need to be at the top of the list.
<snip>
> >>>+keyscan: keyscan at fe4b0000 {
> >>>+ compatible = "st,keypad";
> >>>Is there any way we can make this more specific to _this_ IP?
> >>for my knowledge this IP is the same for stixxxx platform.
> >So st,stix-keypad, or st,sti4x-keypad?
> >
> >I'm just thinking about future proofing the architecture. What if ST
> >released stj which has a different keypad IP?
> After discussing internally with st "st,sti-keyscan" is better
Perfect.
<snip>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 51+ messages in thread
end of thread, other threads:[~2014-03-18 11:01 UTC | newest]
Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-05 3:39 [PATCH 0/5] Add ST Keyscan driver Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-05 3:39 ` [PATCH 1/5] drivers: input: keyboard: st-keyscan: add keyscan driver Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-05 6:46 ` Dmitry Torokhov
2014-03-05 6:46 ` Dmitry Torokhov
2014-03-07 4:51 ` Gabriel Fernandez
2014-03-10 11:48 ` Lee Jones
2014-03-10 11:48 ` Lee Jones
2014-03-10 15:28 ` Dmitry Torokhov
2014-03-10 15:28 ` Dmitry Torokhov
2014-03-10 15:38 ` Lee Jones
2014-03-10 15:38 ` Lee Jones
2014-03-10 15:58 ` Dmitry Torokhov
2014-03-10 15:58 ` Dmitry Torokhov
2014-03-10 16:35 ` Lee Jones
2014-03-10 16:35 ` Lee Jones
2014-03-10 16:35 ` Lee Jones
2014-03-14 10:13 ` Gabriel Fernandez
2014-03-14 10:13 ` Gabriel Fernandez
2014-03-14 10:13 ` Gabriel Fernandez
2014-03-14 10:42 ` Lee Jones
2014-03-14 10:42 ` Lee Jones
2014-03-14 10:42 ` Lee Jones
2014-03-18 10:25 ` Gabriel Fernandez
2014-03-18 10:25 ` Gabriel Fernandez
2014-03-18 10:25 ` Gabriel Fernandez
2014-03-18 11:01 ` Lee Jones
2014-03-18 11:01 ` Lee Jones
2014-03-14 15:26 ` Dmitry Torokhov
2014-03-14 15:26 ` Dmitry Torokhov
2014-03-05 3:39 ` [PATCH 2/5] ARM: STi: DT: add keyscan for stih415 Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-10 11:50 ` Lee Jones
2014-03-10 11:50 ` Lee Jones
2014-03-10 11:50 ` Lee Jones
2014-03-05 3:39 ` [PATCH 3/5] ARM: STi: DT: add keyscan for stih416 Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-10 11:52 ` Lee Jones
2014-03-10 11:52 ` Lee Jones
2014-03-10 11:52 ` Lee Jones
2014-03-05 3:39 ` [PATCH 4/5] ARM: STi: DT: add keyscan for stih41x-b2000 Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-10 11:54 ` Lee Jones
2014-03-10 11:54 ` Lee Jones
2014-03-10 11:54 ` Lee Jones
2014-03-05 3:39 ` [PATCH 5/5] ARM: multi_v7_defconfig: add st-keyscan driver Gabriel FERNANDEZ
2014-03-05 3:39 ` Gabriel FERNANDEZ
2014-03-10 10:34 ` Lee Jones
2014-03-10 10:34 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.