public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
@ 2012-04-25  9:26 Sourav Poddar
  2012-04-25  9:26 ` [PATCHv4 2/2] arm/dts: omap4-sdp: Add keypad data Sourav Poddar
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Sourav Poddar @ 2012-04-25  9:26 UTC (permalink / raw)
  To: linux-arm-kernel

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: Stephen Warren <swarren@wwwdotorg.org>
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>
---
v3->v4: Fix stephen warren's comment
 - Made use of the (linux, keymap) bindings as 
  described in matrix-keyboard binding documentation.
 
 .../devicetree/bindings/input/omap-keypad.txt      |   31 +++++
 drivers/input/keyboard/omap4-keypad.c              |  118 ++++++++++++++++----
 2 files changed, 126 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..722425b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/omap-keypad.txt
@@ -0,0 +1,31 @@
+* 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, in addition to those specified by
+the shared matrix-keyboard bindings:
+- keypad,num-rows: Number of row lines connected to the keypad
+  controller.
+
+- keypad,num-columns: Number of column lines connected to the
+  keypad controller.
+
+Optional Properties specific to linux:
+- linux,keypad-no-autorepeat: do no enable autorepeat feature.
+
+Example:
+        keypad at 4ae1c000{
+                compatible = "ti,omap4-keypad";
+                keypad,num-rows = <2>;
+                keypad,num-columns = <8>;
+		linux,keypad-no-autorepeat;
+	};
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index e809ac0..bce300e 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,101 @@ 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)
+{
+	struct matrix_keymap_data *keymap_data;
+	struct device_node *np = dev->of_node;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
+	uint32_t *keymap;
+	int proplen = 0, i;
+	const __be32 *prop = of_get_property(np, "linux,keymap", &proplen);
+
+	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;
+
+	keymap_data->keymap_size = proplen / sizeof(u32);
+
+	keymap = devm_kzalloc(dev,
+		sizeof(uint32_t) * keymap_data->keymap_size, GFP_KERNEL);
+	if (!keymap) {
+		dev_err(dev, "could not allocate memory for keymap\n");
+		return NULL;
+	}
+	keypad_data->keymap_data->keymap = keymap;
+
+	for (i = 0; i < keymap_data->keymap_size; i++) {
+		u32 tmp = be32_to_cpup(prop+i);
+		int key_code, row, col;
+
+		row = (tmp >> 24) & 0xff;
+		col = (tmp >> 16) & 0xff;
+		key_code = tmp & 0xffff;
+		*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);
+	} else {
+		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 +284,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);
@@ -236,10 +300,10 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
 		goto err_release_mem;
 	}
 
+	keypad_data->rows = num_rows;
+	keypad_data->cols = num_cols;
 	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 +327,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 +354,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 +392,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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv4 2/2] arm/dts: omap4-sdp: Add keypad data
  2012-04-25  9:26 [PATCHv4 1/2] drivers: input: keypad: Add device tree support Sourav Poddar
@ 2012-04-25  9:26 ` Sourav Poddar
  2012-04-25 10:52 ` [PATCHv4 1/2] drivers: input: keypad: Add device tree support Russell King - ARM Linux
  2012-04-25 15:48 ` Stephen Warren
  2 siblings, 0 replies; 10+ messages in thread
From: Sourav Poddar @ 2012-04-25  9:26 UTC (permalink / raw)
  To: linux-arm-kernel

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: Stephen Warren <swarren@wwwdotorg.org>
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>
---
v3->v4:
Express keymap binding as (linux,keymap) in the format described in 
Documentation/devicetree/bindings/input/matrix-keymap.txt
 arch/arm/boot/dts/omap4-sdp.dts |   63 +++++++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap4.dtsi    |    5 +++
 2 files changed, 68 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 63c6b2b..0c9b3bf 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -17,4 +17,67 @@
 		device_type = "memory";
 		reg = <0x80000000 0x40000000>; /* 1 GB */
 	};
+
+	keypad at 4ae1c000 {
+		keypad,num-rows = <8>;
+		keypad,num-columns = <8>;
+		linux,keymap = < 0x00000012
+			0x00010013
+			0x00020014
+			0x00030066
+			0x0004003f
+			0x00060017
+			0x0007002a
+			0x01000020
+			0x01010021
+			0x01020022
+			0x01030054
+			0x01040040
+			0x01060025
+			0x0107001c
+			0x0200002d
+			0x0201002e
+			0x0202002f
+			0x0203006b
+			0x02040041
+			0x02060034
+			0x0207003a
+			0x0300002c
+			0x0301004e
+			0x03020030
+			0x0303003b
+			0x03040042
+			0x03060018
+			0x03070039
+			0x04000011
+			0x04010015
+			0x04020016
+			0x0403003c
+			0x04040073
+			0x04060026
+			0x04070069
+			0x0500001f
+			0x05010023
+			0x05020024
+			0x0503003d
+			0x05040043
+			0x05050072
+			0x05060032
+			0x0507006a
+			0x06000010
+			0x0601001e
+			0x06020031
+			0x0603005c
+			0x0604000e
+			0x06060019
+			0x06070067
+			0x07000094
+			0x07010095
+			0x070200ca
+			0x070300cb
+			0x0704003e
+			0x07060160
+			0x0707006c >;
+		linux,input-no-autorepeat;
+	};
 };
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 at 4ae1c000 {
+		compatible = "ti,omap4-keypad";
+		ti,hwmods = "kbd";
+	};
 };
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25  9:26 [PATCHv4 1/2] drivers: input: keypad: Add device tree support Sourav Poddar
  2012-04-25  9:26 ` [PATCHv4 2/2] arm/dts: omap4-sdp: Add keypad data Sourav Poddar
@ 2012-04-25 10:52 ` Russell King - ARM Linux
  2012-04-25 11:38   ` Roland Stigge
  2012-04-25 15:48 ` Stephen Warren
  2 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux @ 2012-04-25 10:52 UTC (permalink / raw)
  To: linux-arm-kernel

I think you and Roland Stigge need to put your heads together and come
up with a common DT binding for matrix keypad devices, rather than
inventing N different platform specific bindings.

On Wed, Apr 25, 2012 at 02:56:43PM +0530, Sourav Poddar wrote:
> 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: Stephen Warren <swarren@wwwdotorg.org>
> 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>
> ---
> v3->v4: Fix stephen warren's comment
>  - Made use of the (linux, keymap) bindings as 
>   described in matrix-keyboard binding documentation.
>  
>  .../devicetree/bindings/input/omap-keypad.txt      |   31 +++++
>  drivers/input/keyboard/omap4-keypad.c              |  118 ++++++++++++++++----
>  2 files changed, 126 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..722425b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/omap-keypad.txt
> @@ -0,0 +1,31 @@
> +* 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, in addition to those specified by
> +the shared matrix-keyboard bindings:
> +- keypad,num-rows: Number of row lines connected to the keypad
> +  controller.
> +
> +- keypad,num-columns: Number of column lines connected to the
> +  keypad controller.
> +
> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> +
> +Example:
> +        keypad at 4ae1c000{
> +                compatible = "ti,omap4-keypad";
> +                keypad,num-rows = <2>;
> +                keypad,num-columns = <8>;
> +		linux,keypad-no-autorepeat;
> +	};
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index e809ac0..bce300e 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,101 @@ 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)
> +{
> +	struct matrix_keymap_data *keymap_data;
> +	struct device_node *np = dev->of_node;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct omap4_keypad *keypad_data = platform_get_drvdata(pdev);
> +	uint32_t *keymap;
> +	int proplen = 0, i;
> +	const __be32 *prop = of_get_property(np, "linux,keymap", &proplen);
> +
> +	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;
> +
> +	keymap_data->keymap_size = proplen / sizeof(u32);
> +
> +	keymap = devm_kzalloc(dev,
> +		sizeof(uint32_t) * keymap_data->keymap_size, GFP_KERNEL);
> +	if (!keymap) {
> +		dev_err(dev, "could not allocate memory for keymap\n");
> +		return NULL;
> +	}
> +	keypad_data->keymap_data->keymap = keymap;
> +
> +	for (i = 0; i < keymap_data->keymap_size; i++) {
> +		u32 tmp = be32_to_cpup(prop+i);
> +		int key_code, row, col;
> +
> +		row = (tmp >> 24) & 0xff;
> +		col = (tmp >> 16) & 0xff;
> +		key_code = tmp & 0xffff;
> +		*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);
> +	} else {
> +		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 +284,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);
> @@ -236,10 +300,10 @@ static int __devinit omap4_keypad_probe(struct platform_device *pdev)
>  		goto err_release_mem;
>  	}
>  
> +	keypad_data->rows = num_rows;
> +	keypad_data->cols = num_cols;
>  	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 +327,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 +354,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 +392,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
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25 10:52 ` [PATCHv4 1/2] drivers: input: keypad: Add device tree support Russell King - ARM Linux
@ 2012-04-25 11:38   ` Roland Stigge
  2012-04-25 11:56     ` Poddar, Sourav
  0 siblings, 1 reply; 10+ messages in thread
From: Roland Stigge @ 2012-04-25 11:38 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell and Sourav,

On 04/25/2012 12:52 PM, Russell King - ARM Linux wrote:
> I think you and Roland Stigge need to put your heads together and come
> up with a common DT binding for matrix keypad devices, rather than
> inventing N different platform specific bindings.
> 
>> +Required Board Specific Properties, in addition to those specified by
>> +the shared matrix-keyboard bindings:
>> +- keypad,num-rows: Number of row lines connected to the keypad
>> +  controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the
>> +  keypad controller.

Right - the LPC32xx key scanner always has a square matrix design (#rows
== #cols), but I'm fine with re-using properties "keypad,num-rows" and
"keypad,num-columns" instead of "nxp,matrix-size".

Besides, I still have "nxp,debounce-delay-ms" and "nxp,scan-delay-ms" -
do I need to change this to some default properties I don't know of (yet)?

And: I re-used the DT subnodes strategy for the keycodes ("linux,code")
of the samsung-keypad driver, see
Documentation/devicetree/bindings/input/samsung-keypad.txt - can I
continue using it?

Thanks in advance,

Roland

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25 11:38   ` Roland Stigge
@ 2012-04-25 11:56     ` Poddar, Sourav
  2012-04-25 12:15       ` Roland Stigge
  0 siblings, 1 reply; 10+ messages in thread
From: Poddar, Sourav @ 2012-04-25 11:56 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roland,

On Wed, Apr 25, 2012 at 5:08 PM, Roland Stigge <stigge@antcom.de> wrote:
> Hi Russell and Sourav,
>
> On 04/25/2012 12:52 PM, Russell King - ARM Linux wrote:
>> I think you and Roland Stigge need to put your heads together and come
>> up with a common DT binding for matrix keypad devices, rather than
>> inventing N different platform specific bindings.
>>
>>> +Required Board Specific Properties, in addition to those specified by
>>> +the shared matrix-keyboard bindings:
>>> +- keypad,num-rows: Number of row lines connected to the keypad
>>> + ?controller.
>>> +
>>> +- keypad,num-columns: Number of column lines connected to the
>>> + ?keypad controller.
>
> Right - the LPC32xx key scanner always has a square matrix design (#rows
> == #cols), but I'm fine with re-using properties "keypad,num-rows" and
> "keypad,num-columns" instead of "nxp,matrix-size".
>
> Besides, I still have "nxp,debounce-delay-ms" and "nxp,scan-delay-ms" -
> do I need to change this to some default properties I don't know of (yet)?
>
I think these are the custom properties which should be kept in your bindings.
I dont see anything of these sort in
Documentation/devicetree/bindings/input/matrix-keymap.txt .
> And: I re-used the DT subnodes strategy for the keycodes ("linux,code")
> of the samsung-keypad driver, see
> Documentation/devicetree/bindings/input/samsung-keypad.txt - can I
> continue using it?

Even I was using the bindings as used by samsung-keypad driver. But according
to the comments and logically too, we should use the common matrix keyboard
bindings already descibed in
Documentation/devicetree/bindings/input/matrix-keymap.txt.
So that we can get rid of multiple bindings for different platform.

~Sourav
> Thanks in advance,
>
> Roland

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25 11:56     ` Poddar, Sourav
@ 2012-04-25 12:15       ` Roland Stigge
  2012-04-25 13:00         ` Poddar, Sourav
  0 siblings, 1 reply; 10+ messages in thread
From: Roland Stigge @ 2012-04-25 12:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hi!

On 04/25/2012 01:56 PM, Poddar, Sourav wrote:
> Even I was using the bindings as used by samsung-keypad driver. But according
> to the comments and logically too, we should use the common matrix keyboard
> bindings already descibed in
> Documentation/devicetree/bindings/input/matrix-keymap.txt.

Fine with me. Can switch over to
Documentation/devicetree/bindings/input/matrix-keymap.txt

Maybe samsung-keypad should also be converted because it's subnodes
structure duplicates exactly the same thing (row,col,code) in a
different way, and we both were distracted by it, ignoring
matrix-keymap.txt? Or is it already too late (both samsung-keypad and
matrix-keypad both being standards in use) and we should put a
recommendation into one of them to use the other one for new designs?

Thanks,

Roland

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25 12:15       ` Roland Stigge
@ 2012-04-25 13:00         ` Poddar, Sourav
  0 siblings, 0 replies; 10+ messages in thread
From: Poddar, Sourav @ 2012-04-25 13:00 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Roland,

On Wed, Apr 25, 2012 at 5:45 PM, Roland Stigge <stigge@antcom.de> wrote:
> Hi!
>
> On 04/25/2012 01:56 PM, Poddar, Sourav wrote:
>> Even I was using the bindings as used by samsung-keypad driver. But according
>> to the comments and logically too, we should use the common matrix keyboard
>> bindings already descibed in
>> Documentation/devicetree/bindings/input/matrix-keymap.txt.
>
> Fine with me. Can switch over to
> Documentation/devicetree/bindings/input/matrix-keymap.txt
>
> Maybe samsung-keypad should also be converted because it's subnodes
> structure duplicates exactly the same thing (row,col,code) in a
> different way, and we both were distracted by it, ignoring
> matrix-keymap.txt? Or is it already too late (both samsung-keypad and

Not sure about the standards, but surely have seen few others like tegra
keyboards using common matrix-keyboard bindings.

One more comment which I saw in some other mail thread about
using the following
              key_1 {
                        keypad,row = <0>;
                        keypad,column = <3>;
                        linux,code = <2>;
                };
is that when there are too many keys, the whole data become quilte huge.
These problem get sorted out on using matrix-keymap bindings, since there
is a single value computed in order to describe a particular key.

~Sourav

> matrix-keypad both being standards in use) and we should put a
> recommendation into one of them to use the other one for new designs?
>

> Thanks,
>
> Roland
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25  9:26 [PATCHv4 1/2] drivers: input: keypad: Add device tree support Sourav Poddar
  2012-04-25  9:26 ` [PATCHv4 2/2] arm/dts: omap4-sdp: Add keypad data Sourav Poddar
  2012-04-25 10:52 ` [PATCHv4 1/2] drivers: input: keypad: Add device tree support Russell King - ARM Linux
@ 2012-04-25 15:48 ` Stephen Warren
  2012-04-26  6:33   ` Poddar, Sourav
  2 siblings, 1 reply; 10+ messages in thread
From: Stephen Warren @ 2012-04-25 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

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

> diff --git a/Documentation/devicetree/bindings/input/omap-keypad.txt b/Documentation/devicetree/bindings/input/omap-keypad.txt

> +Required Board Specific Properties, in addition to those specified by
> +the shared matrix-keyboard bindings:
> +- keypad,num-rows: Number of row lines connected to the keypad
> +  controller.
> +
> +- keypad,num-columns: Number of column lines connected to the
> +  keypad controller.

I'm not sure if "keypad," is the appropriate prefix here. If the
properties are specific to this one TI/OMAP binding, "ti," might be a
better prefix. However, if they're more generally applicable, maybe
"linux," to match "linux,keymap"?

> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.

It's probably worth considering whether num-rows/columns should be part
of matrix-keyboard.txt, and whether linux,keypad-no-autorepeat should be
part of some core keyboard binding. Some of the properties in
tegra-kbc.txt might be worth generalizing too (debounce/repeat delay,
ghost filter)?

> +Example:
> +        keypad at 4ae1c000{
> +                compatible = "ti,omap4-keypad";
> +                keypad,num-rows = <2>;
> +                keypad,num-columns = <8>;
> +		linux,keypad-no-autorepeat;
> +	};

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-25 15:48 ` Stephen Warren
@ 2012-04-26  6:33   ` Poddar, Sourav
  2012-04-26 14:39     ` Stephen Warren
  0 siblings, 1 reply; 10+ messages in thread
From: Poddar, Sourav @ 2012-04-26  6:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On Wed, Apr 25, 2012 at 9:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/25/2012 03:26 AM, Sourav Poddar wrote:
>> Update the Documentation with omap4 keypad device tree
>> binding information.
>> Add device tree support for omap4 keypad driver.
>
>> diff --git a/Documentation/devicetree/bindings/input/omap-keypad.txt b/Documentation/devicetree/bindings/input/omap-keypad.txt
>
>> +Required Board Specific Properties, in addition to those specified by
>> +the shared matrix-keyboard bindings:
>> +- keypad,num-rows: Number of row lines connected to the keypad
>> + ?controller.
>> +
>> +- keypad,num-columns: Number of column lines connected to the
>> + ?keypad controller.
>
> I'm not sure if "keypad," is the appropriate prefix here. If the
> properties are specific to this one TI/OMAP binding, "ti," might be a
> better prefix. However, if they're more generally applicable, maybe
> "linux," to match "linux,keymap"?
>
Yes, the idea was to make this binding more generic. Hence no "ti/omap"
prefix were used.
'linux' prefix denotes bindings specific to linux, keymap has
keycodes which is linux specific. Hence, linux,keymap looks
appropriate. But num rows/ cols are keyboard specific, so I am not
sure if linux prefix goes well here ??
>> +Optional Properties specific to linux:
>> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
>
> It's probably worth considering whether num-rows/columns should be part
> of matrix-keyboard.txt, and whether linux,keypad-no-autorepeat should be
> part of some core keyboard binding. Some of the properties in
> tegra-kbc.txt might be worth generalizing too (debounce/repeat delay,
> ghost filter)?
>
Yes, I also think that its a good idea to make some of the above
binding more generic. Though, I think "linux,keypad-no-autorepeat"
will still be a custom binding, since its totally on the platform
whether to use
autorepeat feature or not?
>> +Example:
>> + ? ? ? ?keypad at 4ae1c000{

>> + ? ? ? ? ? ? ? ?compatible = "ti,omap4-keypad";
>> + ? ? ? ? ? ? ? ?keypad,num-rows = <2>;
>> + ? ? ? ? ? ? ? ?keypad,num-columns = <8>;
>> + ? ? ? ? ? ? linux,keypad-no-autorepeat;
>> + ? ? };

~Sourav

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCHv4 1/2] drivers: input: keypad: Add device tree support
  2012-04-26  6:33   ` Poddar, Sourav
@ 2012-04-26 14:39     ` Stephen Warren
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Warren @ 2012-04-26 14:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/26/2012 12:33 AM, Poddar, Sourav wrote:
> On Wed, Apr 25, 2012 at 9:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
...
>> It's probably worth considering whether num-rows/columns should be part
>> of matrix-keyboard.txt, and whether linux,keypad-no-autorepeat should be
>> part of some core keyboard binding. Some of the properties in
>> tegra-kbc.txt might be worth generalizing too (debounce/repeat delay,
>> ghost filter)?
>
> Yes, I also think that its a good idea to make some of the above
> binding more generic. Though, I think "linux,keypad-no-autorepeat"
> will still be a custom binding, since its totally on the platform
> whether to use
> autorepeat feature or not?

I think the concept of whether autorepeat should be enabled is pretty
generic. The actual value of the property might well be
board/platform-specific, so may differ in individual board .dts files.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-04-26 14:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-25  9:26 [PATCHv4 1/2] drivers: input: keypad: Add device tree support Sourav Poddar
2012-04-25  9:26 ` [PATCHv4 2/2] arm/dts: omap4-sdp: Add keypad data Sourav Poddar
2012-04-25 10:52 ` [PATCHv4 1/2] drivers: input: keypad: Add device tree support Russell King - ARM Linux
2012-04-25 11:38   ` Roland Stigge
2012-04-25 11:56     ` Poddar, Sourav
2012-04-25 12:15       ` Roland Stigge
2012-04-25 13:00         ` Poddar, Sourav
2012-04-25 15:48 ` Stephen Warren
2012-04-26  6:33   ` Poddar, Sourav
2012-04-26 14:39     ` Stephen Warren

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox