Re: [PATCHv3 1/2] drivers: input: keypad: Add device tree support

Re: [PATCHv3 1/2] drivers: input: keypad: Add device tree support

[Stephen Warren]

On 04/23/2012 09:27 AM, Sourav Poddar wrote:
> Update the Documentation with omap4 keypad device tree
> binding information.
> Add device tree support for omap4 keypad driver.

Shouldn't this use the already-defined matrix keyboard bindings at
Documentation/devicetree/bindings/input/matrix-keymap.txt?

(I asked this before, but don't recall seeing any response explaining
why a new binding was needed. Sorry if I missed it.)

[PATCHv3 1/2] drivers: input: keypad: Add device tree support

[Sourav Poddar]

Update the Documentation with omap4 keypad device tree
binding information.
Add device tree support for omap4 keypad driver.

Tested on omap4430 sdp with 3.4-rc3.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Felipe Balbi <balbi@ti.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Randy Dunlap <rdunlap@xenotime.net>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
Changes since v2:
- Include conditional setting of autorepeat feature based
  on device tree binding.
 .../devicetree/bindings/input/omap-keypad.txt      |   55 +++++++++
 drivers/input/keyboard/omap4-keypad.c              |  121 ++++++++++++++++----
 2 files changed, 153 insertions(+), 23 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/omap-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/omap-keypad.txt b/Documentation/devicetree/bindings/input/omap-keypad.txt
new file mode 100644
index 0000000..fda1e26
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/omap-keypad.txt
@@ -0,0 +1,55 @@
+* TI's Keypad Controller device tree bindings
+
+TI's Keypad controller is used to interface a SoC with a matrix-type
+keypad device. The keypad controller supports multiple row and column lines.
+A key can be placed at each intersection of a unique row and a unique column.
+The keypad controller can sense a key-press and key-release and report the
+event using a interrupt to the cpu.
+
+Required SoC Specific Properties:
+- compatible: should be one of the following
+   - "ti,omap4-keypad": For controllers compatible with omap4 keypad
+      controller.
+
+Required Board Specific Properties:
+- keypad,num-rows: Number of row lines connected to the keypad
+  controller.
+
+- keypad,num-columns: Number of column lines connected to the
+  keypad controller.
+
+- Keys represented as child nodes: Each key connected to the keypad
+  controller is represented as a child node to the keypad controller
+  device node and should include the following properties.
+  - keypad,row: the row number to which the key is connected.
+  - keypad,column: the column number to which the key is connected.
+  - linux,code: the key-code to be reported when the key is pressed
+    and released.
+
+Optional Properties specific to linux:
+- linux,keypad-no-autorepeat: do no enable autorepeat feature.
+
+Example:
+        keypad@4ae1c000{
+                compatible = "ti,omap4-keypad";
+                keypad,num-rows = <2>;
+                keypad,num-columns = <8>;
+
+		key_1 {
+                        keypad,row = <0>;
+                        keypad,column = <3>;
+                        linux,code = <2>;
+                };
+
+                key_2 {
+                        keypad,row = <0>;
+                        keypad,column = <4>;
+                        linux,code = <3>;
+                };
+
+                key_3 {
+                        keypad,row = <0>;
+                        keypad,column = <5>;
+                        linux,code = <4>;
+                };
+        };
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index e809ac0..5dde7b2 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -27,6 +27,7 @@
 #include <linux/platform_device.h>
 #include <linux/errno.h>
 #include <linux/io.h>
+#include <linux/of.h>
 #include <linux/input.h>
 #include <linux/slab.h>
 #include <linux/pm_runtime.h>
@@ -70,6 +71,7 @@
 
 struct omap4_keypad {
 	struct input_dev *input;
+	struct matrix_keymap_data *keymap_data;
 
 	void __iomem *base;
 	int irq;
@@ -77,6 +79,7 @@ struct omap4_keypad {
 	unsigned int rows;
 	unsigned int cols;
 	unsigned int row_shift;
+	bool no_autorepeat;
 	unsigned char key_state[8];
 	unsigned short keymap[];
 };
@@ -174,24 +177,106 @@ static void omap4_keypad_close(struct input_dev *input)
 	pm_runtime_put_sync(input->dev.parent);
 }
 
+static struct omap4_keypad *omap_keypad_parse_dt(struct device *dev,
+					uint32_t num_rows, uint32_t num_cols)
+{
+	struct matrix_keymap_data *keymap_data;
+	struct device_node *np = dev->of_node, *key_np;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
+	uint32_t *keymap;
+	unsigned int key_count = 0;
+
+	keypad_data->rows = num_rows;
+	keypad_data->cols = num_cols;
+
+	keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
+	if (!keymap_data) {
+		dev_err(dev, "could not allocate memory for keymap data\n");
+		return NULL;
+	}
+	keypad_data->keymap_data = keymap_data;
+
+	for_each_child_of_node(np, key_np)
+		key_count++;
+
+	keymap_data->keymap_size = key_count;
+	keymap = devm_kzalloc(dev, sizeof(uint32_t) * key_count, GFP_KERNEL);
+	if (!keymap) {
+		dev_err(dev, "could not allocate memory for keymap\n");
+		return NULL;
+	}
+	keypad_data->keymap_data->keymap = keymap;
+
+	for_each_child_of_node(np, key_np) {
+		u32 row, col, key_code;
+		of_property_read_u32(key_np, "keypad,row", &row);
+		of_property_read_u32(key_np, "keypad,column", &col);
+		of_property_read_u32(key_np, "linux,code", &key_code);
+		*keymap++ = KEY(row, col, key_code);
+	}
+
+	if (of_get_property(np, "linux,input-no-autorepeat", NULL))
+		keypad_data->no_autorepeat = true;
+
+	return keypad_data;
+}
+
 static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
 	const struct omap4_keypad_platform_data *pdata;
 	struct omap4_keypad *keypad_data;
 	struct input_dev *input_dev;
 	struct resource *res;
 	resource_size_t size;
-	unsigned int row_shift, max_keys;
+	unsigned int row_shift = 0, max_keys = 0;
+	uint32_t num_rows = 0, num_cols = 0;
 	int irq;
 	int error;
 
 	/* platform data */
 	pdata = pdev->dev.platform_data;
-	if (!pdata) {
+	if (np) {
+		of_property_read_u32(np, "keypad,num-rows", &num_rows);
+		of_property_read_u32(np, "keypad,num-columns", &num_cols);
+		if (!num_rows || !num_cols) {
+			dev_err(&pdev->dev, "number of keypad rows/columns not specified\n");
+			return -EINVAL;
+		}
+	} else if (pdata) {
+		num_rows = pdata->rows;
+		num_cols = pdata->cols;
+	} else {
 		dev_err(&pdev->dev, "no platform data defined\n");
 		return -EINVAL;
 	}
 
+	row_shift = get_count_order(num_cols);
+	max_keys = num_rows << row_shift;
+
+	keypad_data = devm_kzalloc(dev, sizeof(struct omap4_keypad) +
+			max_keys * sizeof(keypad_data->keymap[0]),
+				GFP_KERNEL);
+
+	if (!keypad_data) {
+		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(pdev, keypad_data);
+
+	if (np) {
+		keypad_data = omap_keypad_parse_dt(&pdev->dev,
+						num_rows, num_cols);
+	} else {
+		keypad_data->rows = num_rows;
+		keypad_data->cols = num_cols;
+		keypad_data->keymap_data =
+				(struct matrix_keymap_data *)pdata->keymap_data;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
 		dev_err(&pdev->dev, "no base address specified\n");
@@ -204,22 +289,6 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	if (!pdata->keymap_data) {
-		dev_err(&pdev->dev, "no keymap data defined\n");
-		return -EINVAL;
-	}
-
-	row_shift = get_count_order(pdata->cols);
-	max_keys = pdata->rows << row_shift;
-
-	keypad_data = kzalloc(sizeof(struct omap4_keypad) +
-				max_keys * sizeof(keypad_data->keymap[0]),
-			      GFP_KERNEL);
-	if (!keypad_data) {
-		dev_err(&pdev->dev, "keypad_data memory allocation failed\n");
-		return -ENOMEM;
-	}
-
 	size = resource_size(res);
 
 	res = request_mem_region(res->start, size, pdev->name);
@@ -238,8 +307,6 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 
 	keypad_data->irq = irq;
 	keypad_data->row_shift = row_shift;
-	keypad_data->rows = pdata->rows;
-	keypad_data->cols = pdata->cols;
 
 	/* input device allocation */
 	keypad_data->input = input_dev = input_allocate_device();
@@ -263,13 +330,15 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 	input_dev->keycodemax	= max_keys;
 
 	__set_bit(EV_KEY, input_dev->evbit);
-	__set_bit(EV_REP, input_dev->evbit);
+
+	if (!keypad_data->no_autorepeat)
+		__set_bit(EV_REP, input_dev->evbit);
 
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
 
 	input_set_drvdata(input_dev, keypad_data);
 
-	matrix_keypad_build_keymap(pdata->keymap_data, row_shift,
+	matrix_keypad_build_keymap(keypad_data->keymap_data, row_shift,
 			input_dev->keycode, input_dev->keybit);
 
 	error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
@@ -288,7 +357,6 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 		goto err_pm_disable;
 	}
 
-	platform_set_drvdata(pdev, keypad_data);
 	return 0;
 
 err_pm_disable:
@@ -327,12 +395,19 @@ static int __devexit omap4_keypad_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id omap_keypad_dt_match[] = {
+	{ .compatible = "ti,omap4-keypad" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, omap_keypad_dt_match);
+
 static struct platform_driver omap4_keypad_driver = {
 	.probe		= omap4_keypad_probe,
 	.remove		= __devexit_p(omap4_keypad_remove),
 	.driver		= {
 		.name	= "omap4-keypad",
 		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(omap_keypad_dt_match),
 	},
 };
 module_platform_driver(omap4_keypad_driver);
-- 
1.7.1

[PATCHv3 2/2] arm/dts: omap4-sdp: Add keypad data

[Sourav Poddar]

Add keypad data node in omap4 device tree file.
Also fill the device tree binding parameters
with the required value in "omap4-sdp" dts file.

Tested on omap44330 sdp with 3.4-rc3 kernel.

Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Sourav Poddar <sourav.poddar@ti.com>
---
Changes since v2:
 -Includes no-autorepeat device tree binding.
 arch/arm/boot/dts/omap4-sdp.dts |  292 +++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap4.dtsi    |    5 +
 2 files changed, 297 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 63c6b2b..1d2d6cd 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -17,4 +17,296 @@
 		device_type = "memory";
 		reg = <0x80000000 0x40000000>; /* 1 GB */
 	};
+
+	keypad@4ae1c000 {
+		keypad,num-rows = <8>;
+		keypad,num-columns = <8>;
+		linux,input-no-autorepeat;
+
+		key_E {
+			keypad,row = <0>;
+			keypad,column = <0>;
+			linux,code = <18>;
+		};
+		key_R {
+			keypad,row = <0>;
+			keypad,column = <1>;
+			linux,code = <19>;
+		};
+		key_T {
+			keypad,row = <0>;
+			keypad,column = <2>;
+			linux,code = <20>;
+		};
+		key_HOME {
+			keypad,row = <0>;
+			keypad,column = <3>;
+			linux,code = <102>;
+		};
+		key_F5 {
+			keypad,row = <0>;
+			keypad,column = <4>;
+			linux,code = <63>;
+		};
+		key_I {
+			keypad,row = <0>;
+			keypad,column = <6>;
+			linux,code = <23>;
+		};
+		key_LEFTSHIFT {
+			keypad,row = <0>;
+			keypad,column = <7>;
+			linux,code = <42>;
+		};
+		key_D {
+			keypad,row = <1>;
+			keypad,column = <0>;
+			linux,code = <32>;
+		};
+		key_F {
+			keypad,row = <1>;
+			keypad,column = <1>;
+			linux,code = <33>;
+		};
+		key_G {
+			keypad,row = <1>;
+			keypad,column = <2>;
+			linux,code = <34>;
+		};
+		key_SEND {
+			keypad,row = <1>;
+			keypad,column = <3>;
+			linux,code = <84>;
+		};
+		key_F6 {
+			keypad,row = <1>;
+			keypad,column = <4>;
+			linux,code = <64>;
+		};
+		key_K {
+			keypad,row = <1>;
+			keypad,column = <6>;
+			linux,code = <37>;
+		};
+		key_ENTER {
+			keypad,row = <1>;
+			keypad,column = <7>;
+			linux,code = <28>;
+		};
+		key_X {
+			keypad,row = <2>;
+			keypad,column = <0>;
+			linux,code = <45>;
+		};
+		key_C {
+			keypad,row = <2>;
+			keypad,column = <1>;
+			linux,code = <46>;
+		};
+		key_V {
+			keypad,row = <2>;
+			keypad,column = <2>;
+			linux,code = <47>;
+		};
+		key_END {
+			keypad,row = <2>;
+			keypad,column = <3>;
+			linux,code = <107>;
+		};
+		key_F7 {
+			keypad,row = <2>;
+			keypad,column = <4>;
+			linux,code = <65>;
+		};
+		key_DOT {
+			keypad,row = <2>;
+			keypad,column = <6>;
+			linux,code = <52>;
+		};
+		key_CAPSLOCK {
+                        keypad,row = <2>;
+                        keypad,column = <7>;
+                        linux,code = <58>;
+                };
+		key_Z {
+			keypad,row = <3>;
+			keypad,column = <0>;
+			linux,code = <44>;
+		};
+		key_KPLUS {
+			keypad,row = <3>;
+			keypad,column = <1>;
+			linux,code = <78>;
+		};
+		key_B {
+			keypad,row = <3>;
+			keypad,column = <2>;
+			linux,code = <48>;
+		};
+		key_F1 {
+			keypad,row = <3>;
+			keypad,column = <3>;
+			linux,code = <59>;
+		};
+		key_F8 {
+			keypad,row = <3>;
+			keypad,column = <4>;
+			linux,code = <66>;
+		};
+		key_O {
+			keypad,row = <3>;
+			keypad,column = <6>;
+			linux,code = <24>;
+                };
+		key_SPACE {
+			keypad,row = <3>;
+			keypad,column = <7>;
+			linux,code = <57>;
+                };
+		key_W {
+			keypad,row = <4>;
+			keypad,column = <0>;
+			linux,code = <17>;
+                };
+		key_Y {
+			keypad,row = <4>;
+			keypad,column = <1>;
+			linux,code = <21>;
+		};
+		key_U {
+			keypad,row = <4>;
+			keypad,column = <2>;
+			linux,code = <22>;
+		};
+		key_F2 {
+			keypad,row = <4>;
+			keypad,column = <3>;
+			linux,code = <60>;
+		};
+		key_VOLUMEUP {
+			keypad,row = <4>;
+			keypad,column = <4>;
+			linux,code = <115>;
+		};
+		key_L {
+			keypad,row = <4>;
+			keypad,column = <6>;
+			linux,code = <38>;
+		};
+		key_LEFT {
+			keypad,row = <4>;
+			keypad,column = <7>;
+			linux,code = <105>;
+		};
+		key_S {
+			keypad,row = <5>;
+			keypad,column = <0>;
+			linux,code = <31>;
+		};
+		key_H {
+			keypad,row = <5>;
+			keypad,column = <1>;
+			linux,code = <35>;
+		};
+		key_J {
+			keypad,row = <5>;
+			keypad,column = <2>;
+			linux,code = <36>;
+		};
+		key_F3 {
+			keypad,row = <5>;
+			keypad,column = <3>;
+			linux,code = <61>;
+		};
+		key_F9 {
+			keypad,row = <5>;
+			keypad,column = <4>;
+			linux,code = <67>;
+		};
+		key_VOLUMEDOWN {
+			keypad,row = <5>;
+			keypad,column = <5>;
+			linux,code = <114>;
+		};
+		key_M {
+			keypad,row = <5>;
+			keypad,column = <6>;
+			linux,code = <50>;
+		};
+		key_RIGHT {
+			keypad,row = <5>;
+			keypad,column = <7>;
+			linux,code = <106>;
+		};
+		key_Q {
+			keypad,row = <6>;
+			keypad,column = <0>;
+			linux,code = <16>;
+		};
+		key_A {
+			keypad,row = <6>;
+			keypad,column = <1>;
+			linux,code = <30>;
+                };
+		key_N {
+			keypad,row = <6>;
+			keypad,column = <2>;
+			linux,code = <49>;
+                };
+		key_BACK {
+			keypad,row = <6>;
+			keypad,column = <3>;
+			linux,code = <92>;
+                };
+		key_BACKSPACE {
+			keypad,row = <6>;
+			keypad,column = <4>;
+			linux,code = <14>;
+		};
+		key_P {
+			keypad,row = <6>;
+			keypad,column = <6>;
+			linux,code = <25>;
+		};
+		key_UP {
+			keypad,row = <6>;
+			keypad,column = <7>;
+			linux,code = <103>;
+		};
+		key_PROG1 {
+			keypad,row = <7>;
+			keypad,column = <0>;
+			linux,code = <148>;
+		};
+		key_PROG2 {
+			keypad,row = <7>;
+			keypad,column = <1>;
+			linux,code = <149>;
+		};
+		key_PROG3 {
+			keypad,row = <7>;
+			keypad,column = <2>;
+			linux,code = <202>;
+                };
+		key_PROG4 {
+			keypad,row = <7>;
+			keypad,column = <3>;
+			linux,code = <203>;
+		};
+		key_F4 {
+			keypad,row = <7>;
+			keypad,column = <4>;
+			linux,code = <62>;
+		};
+		key_Ok {
+			keypad,row = <7>;
+			keypad,column = <6>;
+			linux,code = <352>;
+		};
+		key_DOWN {
+			keypad,row = <7>;
+			keypad,column = <7>;
+			linux,code = <108>;
+		};
+	};
 };
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 3d35559..e0f678a 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -156,4 +156,9 @@
 			ti,hwmods = "i2c4";
 		};
 	};
+
+	keypad@4ae1c000 {
+		compatible = "ti,omap4-keypad";
+		ti,hwmods = "kbd";
+	};
 };
-- 
1.7.1

Re: [PATCHv3 1/2] drivers: input: keypad: Add device tree support

[Poddar, Sourav]

Hi Stephen,

On Mon, Apr 23, 2012 at 8:56 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/23/2012 09:27 AM, Sourav Poddar wrote:
>> Update the Documentation with omap4 keypad device tree
>> binding information.
>> Add device tree support for omap4 keypad driver.
>
> Shouldn't this use the already-defined matrix keyboard bindings at
> Documentation/devicetree/bindings/input/matrix-keymap.txt?
>
True, that is one way of describing the linux-keymap, but i think it can be
done  either ways.

Are you hinting at getting rid of Documentation for each seperate modules and
just use a single  Documentation/devicetree/bindings/input/matrix-keymap.txt?


> (I asked this before, but don't recall seeing any response explaining
> why a new binding was needed. Sorry if I missed it.)
Sorry, It was  I  who missed it.

~Sourav

Re: [PATCHv3 1/2] drivers: input: keypad: Add device tree support

[Stephen Warren]

On 04/23/2012 09:40 AM, Poddar, Sourav wrote:
> Hi Stephen,
> 
> On Mon, Apr 23, 2012 at 8:56 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/23/2012 09:27 AM, Sourav Poddar wrote:
>>> Update the Documentation with omap4 keypad device tree
>>> binding information.
>>> Add device tree support for omap4 keypad driver.
>>
>> Shouldn't this use the already-defined matrix keyboard bindings at
>> Documentation/devicetree/bindings/input/matrix-keymap.txt?
>>
> True, that is one way of describing the linux-keymap, but i think it can be
> done  either ways.

Well, there certainly is more than one way to represent the data.
However, it'd be best to avoid creating new bindings for every piece of
hardware when there's already an existing binding that can be used. By
reusing the binding, different HW can be configured in the same way thus
re-using any knowledge someone has of the binding.

> Are you hinting at getting rid of Documentation for each seperate modules and
> just use a single  Documentation/devicetree/bindings/input/matrix-keymap.txt?

You'd probably still need a device-specific binding in order to define
which compatible values were necessary, which reg values were necessary,
to reference the common binding documentation, to document any required
additional custom properties, and document any limitations. For example,
see how Olof wrote Documentation/devicetree/bindings/input/tegra-kbc.txt
based on the common matrix keyboard binding.