[PATCH 0/5] Input: mpr121 - add device tree support

[PATCH 0/5] Input: mpr121 - add device tree support

[Akinobu Mita]

This series attempts to add device tree support to mpr121 driver which
currently only supports legacy platform data probe.

The first three patches in this series are cleanup and bug fixes that
I found while playing with this driver.  The latter two patches are the
main part of this series.  We'll make use of matrix keypad helper API in
order to reduce code duplication between legacy platform data probe and
device tree probe.

Akinobu Mita (5):
  Input: mpr121 - remove unused field in struct mpr121_touchkey
  Input: mpr121 - set missing event capability
  Input: mpr121 - handle multiple bits change of status register
  Input: mpr121 - use matrix keypad helper API
  Input: mpr121 - add device tree support

 .../devicetree/bindings/input/mpr121-touchkey.txt  |  45 ++++++
 drivers/input/keyboard/Kconfig                     |   1 +
 drivers/input/keyboard/mpr121_touchkey.c           | 177 ++++++++++++++++-----
 3 files changed, 182 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/mpr121-touchkey.txt

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Rob Herring <robh@kernel.org>
-- 
2.7.4

[PATCH 1/5] Input: mpr121 - remove unused field in struct mpr121_touchkey

[Akinobu Mita]

Remove unused key_val field in struct mpr121_touchkey.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/input/keyboard/mpr121_touchkey.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index 0fd612d..d40e0d1 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -59,7 +59,6 @@
 struct mpr121_touchkey {
 	struct i2c_client	*client;
 	struct input_dev	*input_dev;
-	unsigned int		key_val;
 	unsigned int		statusbits;
 	unsigned int		keycount;
 	u16			keycodes[MPR121_MAX_KEY_COUNT];
-- 
2.7.4

[PATCH 2/5] Input: mpr121 - set missing event capability

[Akinobu Mita]

This driver reports misc scan input events on the sensor's status
register changes.  But the event capability for them was not set in the
device initialization, so these events were ignored.

This change adds the missing event capability.

Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/keyboard/mpr121_touchkey.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index d40e0d1..a2c5305 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -230,6 +230,7 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 	input_dev->id.bustype = BUS_I2C;
 	input_dev->dev.parent = &client->dev;
 	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
 
 	input_dev->keycode = mpr121->keycodes;
 	input_dev->keycodesize = sizeof(mpr121->keycodes[0]);
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] Input: mpr121 - handle multiple bits change of status register

[Akinobu Mita]

This driver reports input events on their interrupts which are triggered
by the sensor's status register changes.  But only single bit change is
reported in the interrupt handler.  So if there are multiple bits are
changed at almost the same time, other press or release events are ignored.

This fixes it by detecting all changed bits in the status register.

Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/keyboard/mpr121_touchkey.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index a2c5305..c809f70 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -86,7 +86,8 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 	struct mpr121_touchkey *mpr121 = dev_id;
 	struct i2c_client *client = mpr121->client;
 	struct input_dev *input = mpr121->input_dev;
-	unsigned int key_num, key_val, pressed;
+	unsigned int bit_changed;
+	unsigned int key_num;
 	int reg;
 
 	reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
@@ -104,18 +105,25 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 
 	reg &= TOUCH_STATUS_MASK;
 	/* use old press bit to figure out which bit changed */
-	key_num = ffs(reg ^ mpr121->statusbits) - 1;
-	pressed = reg & (1 << key_num);
+	bit_changed = reg ^ mpr121->statusbits;
 	mpr121->statusbits = reg;
+	for (key_num = 0; key_num < mpr121->keycount; key_num++) {
+		unsigned int key_val, pressed;
 
-	key_val = mpr121->keycodes[key_num];
+		if (!(bit_changed & (1 << key_num)))
+			continue;
 
-	input_event(input, EV_MSC, MSC_SCAN, key_num);
-	input_report_key(input, key_val, pressed);
-	input_sync(input);
+		pressed = reg & (1 << key_num);
+		key_val = mpr121->keycodes[key_num];
+
+		input_event(input, EV_MSC, MSC_SCAN, key_num);
+		input_report_key(input, key_val, pressed);
+
+		dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
+			pressed ? "pressed" : "released");
 
-	dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
-		pressed ? "pressed" : "released");
+	}
+	input_sync(input);
 
 out:
 	return IRQ_HANDLED;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] Input: mpr121 - handle multiple bits change of status register

[Dmitry Torokhov]

On Thu, Jan 12, 2017 at 02:42:27AM +0900, Akinobu Mita wrote:
> This driver reports input events on their interrupts which are triggered
> by the sensor's status register changes.  But only single bit change is
> reported in the interrupt handler.  So if there are multiple bits are
> changed at almost the same time, other press or release events are ignored.
> 
> This fixes it by detecting all changed bits in the status register.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  drivers/input/keyboard/mpr121_touchkey.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
> index a2c5305..c809f70 100644
> --- a/drivers/input/keyboard/mpr121_touchkey.c
> +++ b/drivers/input/keyboard/mpr121_touchkey.c
> @@ -86,7 +86,8 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
>  	struct mpr121_touchkey *mpr121 = dev_id;
>  	struct i2c_client *client = mpr121->client;
>  	struct input_dev *input = mpr121->input_dev;
> -	unsigned int key_num, key_val, pressed;
> +	unsigned int bit_changed;
> +	unsigned int key_num;
>  	int reg;
>  
>  	reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
> @@ -104,18 +105,25 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
>  
>  	reg &= TOUCH_STATUS_MASK;
>  	/* use old press bit to figure out which bit changed */
> -	key_num = ffs(reg ^ mpr121->statusbits) - 1;
> -	pressed = reg & (1 << key_num);
> +	bit_changed = reg ^ mpr121->statusbits;
>  	mpr121->statusbits = reg;
> +	for (key_num = 0; key_num < mpr121->keycount; key_num++) {
> +		unsigned int key_val, pressed;

maybe change bit_changed to long and use for_each_set_bit()?

>  
> -	key_val = mpr121->keycodes[key_num];
> +		if (!(bit_changed & (1 << key_num)))
> +			continue;
>  
> -	input_event(input, EV_MSC, MSC_SCAN, key_num);
> -	input_report_key(input, key_val, pressed);
> -	input_sync(input);
> +		pressed = reg & (1 << key_num);
> +		key_val = mpr121->keycodes[key_num];
> +
> +		input_event(input, EV_MSC, MSC_SCAN, key_num);
> +		input_report_key(input, key_val, pressed);
> +
> +		dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
> +			pressed ? "pressed" : "released");
>  
> -	dev_dbg(&client->dev, "key %d %d %s\n", key_num, key_val,
> -		pressed ? "pressed" : "released");
> +	}
> +	input_sync(input);
>  
>  out:
>  	return IRQ_HANDLED;
> -- 
> 2.7.4
> 

Thanks.

-- 
Dmitry

[PATCH 5/5] Input: mpr121 - add device tree support

[Akinobu Mita]

This adds device tree support to mpr121 driver which currently only
supports legacy platform data probe.

Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/input/mpr121-touchkey.txt  | 44 +++++++++++
 drivers/input/keyboard/mpr121_touchkey.c           | 87 +++++++++++++++++++---
 2 files changed, 121 insertions(+), 10 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/mpr121-touchkey.txt

diff --git a/Documentation/devicetree/bindings/input/mpr121-touchkey.txt b/Documentation/devicetree/bindings/input/mpr121-touchkey.txt
new file mode 100644
index 0000000..2c4a53c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/mpr121-touchkey.txt
@@ -0,0 +1,44 @@
+* Freescale MPR121 Controllor
+
+Required Properties:
+- compatible:		Should be "fsl,mpr121-touchkey"
+- reg:			The I2C slave address of the device.
+- interrupts:		The interrupt number to the cpu.
+- vdd-supply:		Phandle to the Vdd power supply.
+- linux,keymap:		The keymap for keys as described in the binding document
+			devicetree/bindings/input/matrix-keymap.txt.
+- keypad,num-rows:	Must be 1
+- keypad,num-columns:	Number of lines connected to the keypad controller.
+
+Optional Properties:
+- wakeup-source:	Use any event on keypad as wakeup event.
+- autorepeat:		Enable autorepeat feature.
+
+Example:
+
+#include "dt-bindings/input/input.h"
+
+	touchkey: mpr121@5a {
+		compatible = "fsl,mpr121-touchkey";
+		reg = <0x5a>;
+		interrupt-parent = <&gpio1>;
+		interrupts = <28 2>;
+		autorepeat;
+		vdd-supply = <&ldo4_reg>;
+		keypad,num-rows = <1>;
+		keypad,num-columns = <12>;
+		linux,keymap = <
+			MATRIX_KEY(0x00, 0x00, KEY_0)
+			MATRIX_KEY(0x00, 0x01, KEY_1)
+			MATRIX_KEY(0x00, 0x02, KEY_2)
+			MATRIX_KEY(0x00, 0x03, KEY_3)
+			MATRIX_KEY(0x00, 0x04, KEY_4)
+			MATRIX_KEY(0x00, 0x05, KEY_5)
+			MATRIX_KEY(0x00, 0x06, KEY_6)
+			MATRIX_KEY(0x00, 0x07, KEY_7)
+			MATRIX_KEY(0x00, 0x08, KEY_8)
+			MATRIX_KEY(0x00, 0x09, KEY_9)
+			MATRIX_KEY(0x00, 0x0a, KEY_A)
+			MATRIX_KEY(0x00, 0x0b, KEY_B)
+		>;
+	};
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index 7e85512..2eb0900 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -20,6 +20,7 @@
 #include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/i2c/mpr121_touchkey.h>
+#include <linux/regulator/consumer.h>
 #include <linux/input/matrix_keypad.h>
 
 /* Register definitions */
@@ -81,6 +82,42 @@ static const struct mpr121_init_register init_reg_table[] = {
 	{ AUTO_CONFIG_CTRL_ADDR, 0x0b },
 };
 
+static void mpr121_vdd_supply_disable(void *data)
+{
+	struct regulator *vdd_supply = data;
+
+	regulator_disable(vdd_supply);
+}
+
+static struct regulator *mpr121_vdd_supply_init(struct device *dev)
+{
+	struct regulator *vdd_supply;
+	int err;
+
+	vdd_supply = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(vdd_supply)) {
+		dev_err(dev, "failed to get vdd regulator: %ld\n",
+			PTR_ERR(vdd_supply));
+		return vdd_supply;
+	}
+
+	err = regulator_enable(vdd_supply);
+	if (err) {
+		dev_err(dev, "failed to enable vdd regulator: %d\n", err);
+		return ERR_PTR(err);
+	}
+
+	err = devm_add_action(dev, mpr121_vdd_supply_disable, vdd_supply);
+	if (err) {
+		regulator_disable(vdd_supply);
+		dev_err(dev, "failed to add disable regulator action: %d\n",
+			err);
+		return ERR_PTR(err);
+	}
+
+	return vdd_supply;
+}
+
 static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 {
 	struct mpr121_touchkey *mpr121 = dev_id;
@@ -130,9 +167,8 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int mpr121_phys_init(const struct mpr121_platform_data *pdata,
-				      struct mpr121_touchkey *mpr121,
-				      struct i2c_client *client)
+static int mpr121_phys_init(int vdd_uv, struct mpr121_touchkey *mpr121,
+				struct i2c_client *client)
 {
 	const struct mpr121_init_register *reg;
 	unsigned char usl, lsl, tl, eleconf;
@@ -162,9 +198,9 @@ static int mpr121_phys_init(const struct mpr121_platform_data *pdata,
 	/*
 	 * Capacitance on sensing input varies and needs to be compensated.
 	 * The internal MPR121-auto-configuration can do this if it's
-	 * registers are set properly (based on pdata->vdd_uv).
+	 * registers are set properly (based on vdd_uv).
 	 */
-	vdd = pdata->vdd_uv / 1000;
+	vdd = vdd_uv / 1000;
 	usl = ((vdd - 700) * 256) / vdd;
 	lsl = (usl * 65) / 100;
 	tl = (usl * 90) / 100;
@@ -198,6 +234,9 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 	const struct mpr121_platform_data *pdata =
 			dev_get_platdata(&client->dev);
 	struct device *dev = &client->dev;
+	bool autorepeat;
+	bool wakeup;
+	int vdd_uv;
 	unsigned int keymap_size;
 	struct matrix_keymap_data *keymap_data;
 	struct mpr121_touchkey *mpr121;
@@ -218,6 +257,9 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 			return -EINVAL;
 		}
 
+		autorepeat = true;
+		wakeup = pdata->wakeup;
+		vdd_uv = pdata->vdd_uv;
 		keymap_size = pdata->keymap_size;
 
 		keymap = devm_kcalloc(dev, keymap_size,
@@ -236,8 +278,23 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 		keymap_data->keymap_size = keymap_size;
 		keymap_data->keymap = keymap;
 	} else {
-		dev_err(&client->dev, "no platform data defined\n");
-		return -EINVAL;
+		unsigned int rows;
+		struct regulator *vdd_supply;
+
+		error = matrix_keypad_parse_of_params(dev, &rows, &keymap_size);
+		if (rows != 1) {
+			dev_err(dev, "number of keypad rows must be 1\n");
+			return -EINVAL;
+		}
+
+		vdd_supply = mpr121_vdd_supply_init(dev);
+		if (IS_ERR(vdd_supply))
+			return PTR_ERR(vdd_supply);
+
+		vdd_uv = regulator_get_voltage(vdd_supply);
+		autorepeat = device_property_present(dev, "autorepeat");
+		wakeup = device_property_read_bool(dev, "wakeup-source");
+		keymap_data = NULL;
 	}
 
 	if (!client->irq) {
@@ -261,15 +318,16 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 	input_dev->name = "Freescale MPR121 Touchkey";
 	input_dev->id.bustype = BUS_I2C;
 	input_dev->dev.parent = &client->dev;
-	input_dev->evbit[0] = BIT_MASK(EV_REP);
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
+	if (autorepeat)
+		__set_bit(EV_REP, input_dev->evbit);
 
 	error = matrix_keypad_build_keymap(keymap_data, NULL, 1, keymap_size,
 						NULL, input_dev);
 	if (error)
 		return error;
 
-	error = mpr121_phys_init(pdata, mpr121, client);
+	error = mpr121_phys_init(vdd_uv, mpr121, client);
 	if (error) {
 		dev_err(&client->dev, "Failed to init register\n");
 		return error;
@@ -289,7 +347,7 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 		return error;
 
 	i2c_set_clientdata(client, mpr121);
-	device_init_wakeup(&client->dev, pdata->wakeup);
+	device_init_wakeup(dev, wakeup);
 
 	return 0;
 }
@@ -330,10 +388,19 @@ static const struct i2c_device_id mpr121_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mpr121_id);
 
+#ifdef CONFIG_OF
+static const struct of_device_id mpr121_touchkey_dt_match_table[] = {
+	{ .compatible = "fsl,mpr121-touchkey" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, mpr121_touchkey_dt_match_table);
+#endif
+
 static struct i2c_driver mpr_touchkey_driver = {
 	.driver = {
 		.name	= "mpr121",
 		.pm	= &mpr121_touchkey_pm_ops,
+		.of_match_table = of_match_ptr(mpr121_touchkey_dt_match_table),
 	},
 	.id_table	= mpr121_id,
 	.probe		= mpr_touchkey_probe,
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/5] Input: mpr121 - use matrix keypad helper API

[Akinobu Mita]

Convert to use matrix_kaypad_build_keymap() helper.

This is preparation for adding device tree support.  This change enables
to share code between legacy platform data probe and device tree probe.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
---
 drivers/input/keyboard/Kconfig           |  1 +
 drivers/input/keyboard/mpr121_touchkey.c | 64 +++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 22 deletions(-)

diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index cbd75cf..b436e71 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -407,6 +407,7 @@ config KEYBOARD_MCS
 config KEYBOARD_MPR121
 	tristate "Freescale MPR121 Touchkey"
 	depends on I2C
+	select INPUT_MATRIXKMAP
 	help
 	  Say Y here if you have Freescale MPR121 touchkey controller
 	  chip in your system.
diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
index c809f70..7e85512 100644
--- a/drivers/input/keyboard/mpr121_touchkey.c
+++ b/drivers/input/keyboard/mpr121_touchkey.c
@@ -20,6 +20,7 @@
 #include <linux/bitops.h>
 #include <linux/interrupt.h>
 #include <linux/i2c/mpr121_touchkey.h>
+#include <linux/input/matrix_keypad.h>
 
 /* Register definitions */
 #define ELE_TOUCH_STATUS_0_ADDR	0x0
@@ -61,7 +62,6 @@ struct mpr121_touchkey {
 	struct input_dev	*input_dev;
 	unsigned int		statusbits;
 	unsigned int		keycount;
-	u16			keycodes[MPR121_MAX_KEY_COUNT];
 };
 
 struct mpr121_init_register {
@@ -88,6 +88,7 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 	struct input_dev *input = mpr121->input_dev;
 	unsigned int bit_changed;
 	unsigned int key_num;
+	const unsigned short *keycode = input->keycode;
 	int reg;
 
 	reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
@@ -114,7 +115,7 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
 			continue;
 
 		pressed = reg & (1 << key_num);
-		key_val = mpr121->keycodes[key_num];
+		key_val = keycode[key_num];
 
 		input_event(input, EV_MSC, MSC_SCAN, key_num);
 		input_report_key(input, key_val, pressed);
@@ -196,23 +197,46 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 {
 	const struct mpr121_platform_data *pdata =
 			dev_get_platdata(&client->dev);
+	struct device *dev = &client->dev;
+	unsigned int keymap_size;
+	struct matrix_keymap_data *keymap_data;
 	struct mpr121_touchkey *mpr121;
 	struct input_dev *input_dev;
 	int error;
 	int i;
 
-	if (!pdata) {
-		dev_err(&client->dev, "no platform data defined\n");
-		return -EINVAL;
-	}
+	if (pdata) {
+		uint32_t *keymap;
 
-	if (!pdata->keymap || !pdata->keymap_size) {
-		dev_err(&client->dev, "missing keymap data\n");
-		return -EINVAL;
-	}
+		if (!pdata->keymap || !pdata->keymap_size) {
+			dev_err(dev, "missing keymap data\n");
+			return -EINVAL;
+		}
+
+		if (pdata->keymap_size > MPR121_MAX_KEY_COUNT) {
+			dev_err(dev, "too many keys defined\n");
+			return -EINVAL;
+		}
+
+		keymap_size = pdata->keymap_size;
+
+		keymap = devm_kcalloc(dev, keymap_size,
+				sizeof(keymap_data->keymap[0]), GFP_KERNEL);
+		if (!keymap)
+			return -ENOMEM;
 
-	if (pdata->keymap_size > MPR121_MAX_KEY_COUNT) {
-		dev_err(&client->dev, "too many keys defined\n");
+		for (i = 0; i < keymap_size; i++)
+			keymap[i] = KEY(0, i, pdata->keymap[i]);
+
+		keymap_data = devm_kzalloc(dev, sizeof(*keymap_data),
+					GFP_KERNEL);
+		if (!keymap_data)
+			return -ENOMEM;
+
+		keymap_data->keymap_size = keymap_size;
+		keymap_data->keymap = keymap;
+	} else {
+		dev_err(&client->dev, "no platform data defined\n");
 		return -EINVAL;
 	}
 
@@ -232,22 +256,18 @@ static int mpr_touchkey_probe(struct i2c_client *client,
 
 	mpr121->client = client;
 	mpr121->input_dev = input_dev;
-	mpr121->keycount = pdata->keymap_size;
+	mpr121->keycount = keymap_size;
 
 	input_dev->name = "Freescale MPR121 Touchkey";
 	input_dev->id.bustype = BUS_I2C;
 	input_dev->dev.parent = &client->dev;
-	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
+	input_dev->evbit[0] = BIT_MASK(EV_REP);
 	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
 
-	input_dev->keycode = mpr121->keycodes;
-	input_dev->keycodesize = sizeof(mpr121->keycodes[0]);
-	input_dev->keycodemax = mpr121->keycount;
-
-	for (i = 0; i < pdata->keymap_size; i++) {
-		input_set_capability(input_dev, EV_KEY, pdata->keymap[i]);
-		mpr121->keycodes[i] = pdata->keymap[i];
-	}
+	error = matrix_keypad_build_keymap(keymap_data, NULL, 1, keymap_size,
+						NULL, input_dev);
+	if (error)
+		return error;
 
 	error = mpr121_phys_init(pdata, mpr121, client);
 	if (error) {
-- 
2.7.4

Re: [PATCH 4/5] Input: mpr121 - use matrix keypad helper API

[Dmitry Torokhov]

Hi Akinobu,

On Thu, Jan 12, 2017 at 02:42:28AM +0900, Akinobu Mita wrote:
> Convert to use matrix_kaypad_build_keymap() helper.
> 
> This is preparation for adding device tree support.  This change enables
> to share code between legacy platform data probe and device tree probe.

I feel that matrix keymap is overkill here. We have standardish
linus,keycodes property for non-matrix keys/buttons; I think it should
be used here as well. For example of parsing see atmel_captouch_probe().

Also, I do not see anyone in mainline actually using mpr121 platform
data and the driver was merged in 2011. Therefore let's use generic
device properties to fetch keymap and other parameters and get rid of
platform data. If there are legacy (non-DT) boards they can define
property sets in their board-specific code.

Also, could you make a patch removing #ifdef CONFIG_PM_SLEEP and mark
suspend/resume handlers as __maybe_unused instead?

Thanks!

> 
> Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Akinobu Mita <akinobu.mita-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/input/keyboard/Kconfig           |  1 +
>  drivers/input/keyboard/mpr121_touchkey.c | 64 +++++++++++++++++++++-----------
>  2 files changed, 43 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index cbd75cf..b436e71 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -407,6 +407,7 @@ config KEYBOARD_MCS
>  config KEYBOARD_MPR121
>  	tristate "Freescale MPR121 Touchkey"
>  	depends on I2C
> +	select INPUT_MATRIXKMAP
>  	help
>  	  Say Y here if you have Freescale MPR121 touchkey controller
>  	  chip in your system.
> diff --git a/drivers/input/keyboard/mpr121_touchkey.c b/drivers/input/keyboard/mpr121_touchkey.c
> index c809f70..7e85512 100644
> --- a/drivers/input/keyboard/mpr121_touchkey.c
> +++ b/drivers/input/keyboard/mpr121_touchkey.c
> @@ -20,6 +20,7 @@
>  #include <linux/bitops.h>
>  #include <linux/interrupt.h>
>  #include <linux/i2c/mpr121_touchkey.h>
> +#include <linux/input/matrix_keypad.h>
>  
>  /* Register definitions */
>  #define ELE_TOUCH_STATUS_0_ADDR	0x0
> @@ -61,7 +62,6 @@ struct mpr121_touchkey {
>  	struct input_dev	*input_dev;
>  	unsigned int		statusbits;
>  	unsigned int		keycount;
> -	u16			keycodes[MPR121_MAX_KEY_COUNT];
>  };
>  
>  struct mpr121_init_register {
> @@ -88,6 +88,7 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
>  	struct input_dev *input = mpr121->input_dev;
>  	unsigned int bit_changed;
>  	unsigned int key_num;
> +	const unsigned short *keycode = input->keycode;
>  	int reg;
>  
>  	reg = i2c_smbus_read_byte_data(client, ELE_TOUCH_STATUS_1_ADDR);
> @@ -114,7 +115,7 @@ static irqreturn_t mpr_touchkey_interrupt(int irq, void *dev_id)
>  			continue;
>  
>  		pressed = reg & (1 << key_num);
> -		key_val = mpr121->keycodes[key_num];
> +		key_val = keycode[key_num];
>  
>  		input_event(input, EV_MSC, MSC_SCAN, key_num);
>  		input_report_key(input, key_val, pressed);
> @@ -196,23 +197,46 @@ static int mpr_touchkey_probe(struct i2c_client *client,
>  {
>  	const struct mpr121_platform_data *pdata =
>  			dev_get_platdata(&client->dev);
> +	struct device *dev = &client->dev;
> +	unsigned int keymap_size;
> +	struct matrix_keymap_data *keymap_data;
>  	struct mpr121_touchkey *mpr121;
>  	struct input_dev *input_dev;
>  	int error;
>  	int i;
>  
> -	if (!pdata) {
> -		dev_err(&client->dev, "no platform data defined\n");
> -		return -EINVAL;
> -	}
> +	if (pdata) {
> +		uint32_t *keymap;
>  
> -	if (!pdata->keymap || !pdata->keymap_size) {
> -		dev_err(&client->dev, "missing keymap data\n");
> -		return -EINVAL;
> -	}
> +		if (!pdata->keymap || !pdata->keymap_size) {
> +			dev_err(dev, "missing keymap data\n");
> +			return -EINVAL;
> +		}
> +
> +		if (pdata->keymap_size > MPR121_MAX_KEY_COUNT) {
> +			dev_err(dev, "too many keys defined\n");
> +			return -EINVAL;
> +		}
> +
> +		keymap_size = pdata->keymap_size;
> +
> +		keymap = devm_kcalloc(dev, keymap_size,
> +				sizeof(keymap_data->keymap[0]), GFP_KERNEL);
> +		if (!keymap)
> +			return -ENOMEM;
>  
> -	if (pdata->keymap_size > MPR121_MAX_KEY_COUNT) {
> -		dev_err(&client->dev, "too many keys defined\n");
> +		for (i = 0; i < keymap_size; i++)
> +			keymap[i] = KEY(0, i, pdata->keymap[i]);
> +
> +		keymap_data = devm_kzalloc(dev, sizeof(*keymap_data),
> +					GFP_KERNEL);
> +		if (!keymap_data)
> +			return -ENOMEM;
> +
> +		keymap_data->keymap_size = keymap_size;
> +		keymap_data->keymap = keymap;
> +	} else {
> +		dev_err(&client->dev, "no platform data defined\n");
>  		return -EINVAL;
>  	}
>  
> @@ -232,22 +256,18 @@ static int mpr_touchkey_probe(struct i2c_client *client,
>  
>  	mpr121->client = client;
>  	mpr121->input_dev = input_dev;
> -	mpr121->keycount = pdata->keymap_size;
> +	mpr121->keycount = keymap_size;
>  
>  	input_dev->name = "Freescale MPR121 Touchkey";
>  	input_dev->id.bustype = BUS_I2C;
>  	input_dev->dev.parent = &client->dev;
> -	input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_REP);
> +	input_dev->evbit[0] = BIT_MASK(EV_REP);
>  	input_set_capability(input_dev, EV_MSC, MSC_SCAN);
>  
> -	input_dev->keycode = mpr121->keycodes;
> -	input_dev->keycodesize = sizeof(mpr121->keycodes[0]);
> -	input_dev->keycodemax = mpr121->keycount;
> -
> -	for (i = 0; i < pdata->keymap_size; i++) {
> -		input_set_capability(input_dev, EV_KEY, pdata->keymap[i]);
> -		mpr121->keycodes[i] = pdata->keymap[i];
> -	}
> +	error = matrix_keypad_build_keymap(keymap_data, NULL, 1, keymap_size,
> +						NULL, input_dev);
> +	if (error)
> +		return error;
>  
>  	error = mpr121_phys_init(pdata, mpr121, client);
>  	if (error) {
> -- 
> 2.7.4
> 

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html