[RFC/PATCH 0/3] add adp5589 dt support

[RFC/PATCH 0/3] add adp5589 dt support

[Guido Martínez]

This patchset enables adp5589 keypad configuration via dt. Until now,
this was only done via platform data.

The adp5589 has many functions besides matrix keypad decoding. For now,
we only support keypad related functionality: including scan time and
pull resistors configuration.

  Patch 1 adds the functionality to the driver. There is a minor change
in old code to properly detect the device type if we're using DT.

  Patch 2 adds a DT include file to specify the chip lines for the pull
configuration with symbolic names.

  Patch 3 adds the binding documentation

Guido Martínez (3):
  drivers: input: keyboard: adp5589: add DT support
  DT: input: adp5589: add adp5589 include file
  DT: input: adp5589: add binding documentation

 .../devicetree/bindings/input/adp5589.txt          |  63 +++++++
 drivers/input/keyboard/adp5589-keys.c              | 207 ++++++++++++++++++++-
 include/dt-bindings/input/adp5589.h                |   7 +
 3 files changed, 276 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/input/adp5589.txt
 create mode 100644 include/dt-bindings/input/adp5589.h

-- 
2.0.0.rc0

[RFC/PATCH 1/3] drivers: input: keyboard: adp5589: add DT support

[Guido Martínez]

Add DT support for the Analog ADP5589 matrix keypad decoding functions.

Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
---
 drivers/input/keyboard/adp5589-keys.c | 207 +++++++++++++++++++++++++++++++++-
 1 file changed, 206 insertions(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
index 6329549..2b232c0 100644
--- a/drivers/input/keyboard/adp5589-keys.c
+++ b/drivers/input/keyboard/adp5589-keys.c
@@ -18,7 +18,10 @@
 #include <linux/i2c.h>
 #include <linux/gpio.h>
 #include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/err.h>
 
+#include <linux/input/matrix_keypad.h>
 #include <linux/input/adp5589.h>
 
 /* ADP5589/ADP5585 Common Registers */
@@ -246,6 +249,14 @@ struct adp5589_kpad {
 #endif
 };
 
+static struct of_device_id adp5589_of_match[] = {
+	{
+		.compatible = "adi,adp5589",
+		.data = (void *)ADP5589
+	},
+	{ },
+};
+
 /*
  *  ADP5589 / ADP5585 derivative / variant handling
  */
@@ -858,6 +869,188 @@ static void adp5589_report_switch_state(struct adp5589_kpad *kpad)
 	input_sync(kpad->input);
 }
 
+#ifdef CONFIG_OF
+static int adp5589_key(int row, int col)
+{
+	return col + row * 11;
+}
+
+static int adp5589_dt_read_keymap(struct device *dev,
+				  struct adp5589_kpad_platform_data *pdata,
+				  const struct device_node *node)
+{
+	int i;
+	const u32 *dt_keymap;
+	unsigned short *keymap;
+	int keymap_len;
+
+	dt_keymap = of_get_property(node, "linux,keymap", &keymap_len);
+	if (!dt_keymap) {
+		dev_err(dev, "missing dt keymap\n");
+		return -ENODEV;
+	}
+
+	if (keymap_len % sizeof(u32)) {
+		dev_err(dev, "malformed keymap (len=%i)\n", keymap_len);
+		return -EINVAL;
+	}
+
+	keymap_len /= sizeof(u32);
+
+	keymap = devm_kzalloc(dev, ADP5589_KEYMAPSIZE * sizeof(u32),
+			      GFP_KERNEL);
+	if (!keymap)
+		return -ENOMEM;
+
+	for (i = 0; i < keymap_len; i++) {
+		u32 val;
+		u16 key;
+		u8 row, col;
+
+		val = be32_to_cpup(&dt_keymap[i]);
+
+		row = KEY_ROW(val);
+		col = KEY_COL(val);
+		key = KEY_VAL(val);
+
+		if (row > ADP5589_MAX_ROW_NUM) {
+			dev_err(dev, "invalid row number (%i)\n", row);
+			return -EINVAL;
+		}
+
+		if (col > ADP5589_MAX_COL_NUM) {
+			dev_err(dev, "invalid column number (%i)\n", col);
+			return -EINVAL;
+		}
+
+		pdata->keypad_en_mask |= ADP_ROW(row);
+		pdata->keypad_en_mask |= ADP_COL(col);
+
+		keymap[adp5589_key(row, col)] = key;
+	}
+
+	pdata->keymap = keymap;
+	pdata->keymapsize = ADP5589_KEYMAPSIZE;
+
+	return 0;
+}
+
+static int adp5589_dt_read_pulls(struct device *dev,
+				 struct adp5589_kpad_platform_data *pdata,
+				 const struct device_node *node)
+{
+	unsigned i;
+
+	pdata->pull_dis_mask = 0;
+	pdata->pullup_en_300k = 0;
+	pdata->pullup_en_100k = 0;
+	pdata->pulldown_en_300k = 0;
+
+	of_property_read_u32(node, "adp5589,pulldown-300k",
+			&pdata->pulldown_en_300k);
+
+	of_property_read_u32(node, "adp5589,pullup-300k",
+			&pdata->pullup_en_300k);
+
+	of_property_read_u32(node, "adp5589,pullup-100k",
+			&pdata->pullup_en_100k);
+
+	of_property_read_u32(node, "adp5589,pull-disable",
+			&pdata->pull_dis_mask);
+
+	/* Check for misconfiguration */
+	for (i = 1; i != 0; i <<= 1) {
+		int s = 0;
+
+		if (pdata->pulldown_en_300k & i)
+			s++;
+		if (pdata->pullup_en_300k & i)
+			s++;
+		if (pdata->pullup_en_100k & i)
+			s++;
+		if (pdata->pull_dis_mask & i)
+			s++;
+
+		if (s > 1) {
+			dev_err(dev, "misconfigured pull resistors\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int adp5589_ms_to_cycle_time(unsigned t)
+{
+	if (t >= 40)
+		return ADP5589_SCAN_CYCLE_40ms;
+	else if (t >= 30)
+		return ADP5589_SCAN_CYCLE_30ms;
+	else if (t >= 20)
+		return ADP5589_SCAN_CYCLE_20ms;
+	else
+		return ADP5589_SCAN_CYCLE_10ms;
+}
+
+static int adp5589_dt_fill(struct device *dev,
+			   struct adp5589_kpad_platform_data *pdata,
+			   const struct device_node *node)
+{
+	int error;
+	u32 t;
+
+	error = adp5589_dt_read_keymap(dev, pdata, node);
+	if (error)
+		return error;
+
+	error = adp5589_dt_read_pulls(dev, pdata, node);
+	if (error)
+		return error;
+
+	if (!of_property_read_u32(node, "adp5589,scan-cycle-time-ms", &t))
+		pdata->scan_cycle_time = adp5589_ms_to_cycle_time(t);
+
+	pdata->repeat = !of_property_read_bool(node, "linux,no-autorepeat");
+
+	return 0;
+}
+
+static struct adp5589_kpad_platform_data *
+adp5589_get_dt_data(struct device *dev, int *dev_type)
+{
+	struct device_node *node;
+	const struct of_device_id *match;
+	struct adp5589_kpad_platform_data *pdata;
+	int error;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+
+	if (!pdata)
+		return ERR_PTR(-ENOMEM);
+
+	node = dev->of_node;
+	if (!node) {
+		dev_err(dev, "dt node does not exist\n");
+		return ERR_PTR(-ENODEV);
+	}
+
+	error = adp5589_dt_fill(dev, pdata, node);
+	if (error)
+		return ERR_PTR(error);
+
+	*dev_type = (uintptr_t)match->data;
+	dev->platform_data = pdata;
+
+	return pdata;
+}
+#else
+static struct adp5589_kpad_platform_data *
+adp5589_get_dt_data(struct device *dev, int *dev_type)
+{
+	return ERR_PTR(-ENODEV);
+}
+#endif
+
 static int adp5589_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
@@ -868,6 +1061,7 @@ static int adp5589_probe(struct i2c_client *client,
 	unsigned int revid;
 	int ret, i;
 	int error;
+	int dev_type = 0;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_BYTE_DATA)) {
@@ -876,6 +1070,16 @@ static int adp5589_probe(struct i2c_client *client,
 	}
 
 	if (!pdata) {
+		/* Try with device tree */
+		pdata = adp5589_get_dt_data(&client->dev, &dev_type);
+
+		if (IS_ERR(pdata))
+			pdata = NULL;
+	} else {
+		dev_type = id->driver_data;
+	}
+
+	if (!pdata) {
 		dev_err(&client->dev, "no platform data?\n");
 		return -EINVAL;
 	}
@@ -884,7 +1088,7 @@ static int adp5589_probe(struct i2c_client *client,
 	if (!kpad)
 		return -ENOMEM;
 
-	switch (id->driver_data) {
+	switch (dev_type) {
 	case ADP5585_02:
 		kpad->adp5585_support_row5 = true;
 	case ADP5585_01:
@@ -1101,6 +1305,7 @@ static struct i2c_driver adp5589_driver = {
 		.name = KBUILD_MODNAME,
 		.owner = THIS_MODULE,
 		.pm = &adp5589_dev_pm_ops,
+		.of_match_table = adp5589_of_match,
 	},
 	.probe = adp5589_probe,
 	.remove = adp5589_remove,
-- 
2.0.0.rc0

[RFC/PATCH 2/3] DT: input: adp5589: add adp5589 include file

[Guido Martínez]

Add include file with macros ADP_COL and ADP_ROW, used to specify pull
configuration for the adp5589.

Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
---
 include/dt-bindings/input/adp5589.h | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 include/dt-bindings/input/adp5589.h

diff --git a/include/dt-bindings/input/adp5589.h b/include/dt-bindings/input/adp5589.h
new file mode 100644
index 0000000..5285003
--- /dev/null
+++ b/include/dt-bindings/input/adp5589.h
@@ -0,0 +1,7 @@
+#ifndef _DT_BINDINGS_INPUT_ADP5589_H
+#define _DT_BINDINGS_INPUT_ADP5589_H
+
+#define ADP_ROW(x)	(1 << (x))
+#define ADP_COL(x)	(1 << (8+(x)))
+
+#endif
-- 
2.0.0.rc0

[RFC/PATCH 3/3] DT: input: adp5589: add binding documentation

[Guido Martínez]

Add documentation for the adp5589 DT binding.

Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
---
 .../devicetree/bindings/input/adp5589.txt          | 63 ++++++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/adp5589.txt

diff --git a/Documentation/devicetree/bindings/input/adp5589.txt b/Documentation/devicetree/bindings/input/adp5589.txt
new file mode 100644
index 0000000..d4c295d
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adp5589.txt
@@ -0,0 +1,63 @@
+Binding for the Analog Devices ADP5589 matrix decoder. The ADP5589 is a
+complex chip with many functions including matrix keypad decoding, reset
+event generator and gpio expansion. DT support is limited to keypad
+decoding.
+
+Required properties:
+
+- compatible: Should be "adi,adp5589"
+
+- linux,keymap: An array of packed cells each describing a mapping from
+a key to a linux keycode, as described in [1]. The macro MATRIX_KEY can
+be used to construct each mapping.
+
+Optional properties:
+
+- adp5589,scan-cycle-time-ms: The time between keymap polls in ms.
+Supported valued are 10, 20, 30 and 40. Other values are silently
+rounded down to the nearest supported value. Default: 10ms.
+
+- linux,no-autorepeat: Disable key auto repeat.
+
+- adp5589,pulldown-300k
+- adp5589,pullup-300k
+- adp5589,pullup-100k
+- adp5589,pull-disable: Set pull up/down/disable for the chip lines.
+Each is an OR'd value constructed with the macros ADP_COL and ADP_ROW.
+You can also use ~0 to specify all lines. It is an error for any two of
+these values have bits in common, and the driver will not load if so.
+
+Example node:
+
+&i2c0 {
+	keyboard {
+		compatible = "adi,adp5589";
+		reg = <0x34>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&kb_pins>;
+
+		interrupt-parent = <&gpio1>;
+		interrupts = <19 GPIO_ACTIVE_HIGH>;
+
+		adp5589,scan-cycle-time-ms = <40>;
+
+		/* pull up all lines with a 300k resistor */
+		adp5589,pullup-300k = < (~0) >;
+
+		linux,no-autorepeat;
+		linux,keymap = <
+			MATRIX_KEY(0, 0, KEY_0)
+			MATRIX_KEY(0, 1, KEY_1)
+			MATRIX_KEY(0, 2, KEY_2)
+
+			MATRIX_KEY(1, 0, KEY_F1)
+			MATRIX_KEY(1, 1, KEY_F2)
+			MATRIX_KEY(1, 2, KEY_F3)
+		>;
+	}
+}
+
+-------
+
+[1] Documentation/devicetree/bindings/input/matrix-keymap.txt
-- 
2.0.0.rc0

Re: [RFC/PATCH 1/3] drivers: input: keyboard: adp5589: add DT support

[Dmitry Torokhov]

Hi Guido,

On Wed, May 07, 2014 at 10:00:42AM -0300, Guido Martínez wrote:
> Add DT support for the Analog ADP5589 matrix keypad decoding functions.
> 
> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> ---
>  drivers/input/keyboard/adp5589-keys.c | 207 +++++++++++++++++++++++++++++++++-
>  1 file changed, 206 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> index 6329549..2b232c0 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -18,7 +18,10 @@
>  #include <linux/i2c.h>
>  #include <linux/gpio.h>
>  #include <linux/slab.h>
> +#include <linux/of.h>
> +#include <linux/err.h>
>  
> +#include <linux/input/matrix_keypad.h>
>  #include <linux/input/adp5589.h>
>  
>  /* ADP5589/ADP5585 Common Registers */
> @@ -246,6 +249,14 @@ struct adp5589_kpad {
>  #endif
>  };
>  
> +static struct of_device_id adp5589_of_match[] = {
> +	{
> +		.compatible = "adi,adp5589",
> +		.data = (void *)ADP5589
> +	},
> +	{ },
> +};
> +
>  /*
>   *  ADP5589 / ADP5585 derivative / variant handling
>   */
> @@ -858,6 +869,188 @@ static void adp5589_report_switch_state(struct adp5589_kpad *kpad)
>  	input_sync(kpad->input);
>  }
>  
> +#ifdef CONFIG_OF
> +static int adp5589_key(int row, int col)
> +{
> +	return col + row * 11;
> +}
> +
> +static int adp5589_dt_read_keymap(struct device *dev,
> +				  struct adp5589_kpad_platform_data *pdata,
> +				  const struct device_node *node)
> +{
> +	int i;
> +	const u32 *dt_keymap;
> +	unsigned short *keymap;
> +	int keymap_len;
> +
> +	dt_keymap = of_get_property(node, "linux,keymap", &keymap_len);
> +	if (!dt_keymap) {
> +		dev_err(dev, "missing dt keymap\n");
> +		return -ENODEV;
> +	}
> +
> +	if (keymap_len % sizeof(u32)) {
> +		dev_err(dev, "malformed keymap (len=%i)\n", keymap_len);
> +		return -EINVAL;
> +	}
> +
> +	keymap_len /= sizeof(u32);
> +
> +	keymap = devm_kzalloc(dev, ADP5589_KEYMAPSIZE * sizeof(u32),
> +			      GFP_KERNEL);
> +	if (!keymap)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < keymap_len; i++) {
> +		u32 val;
> +		u16 key;
> +		u8 row, col;
> +
> +		val = be32_to_cpup(&dt_keymap[i]);
> +
> +		row = KEY_ROW(val);
> +		col = KEY_COL(val);
> +		key = KEY_VAL(val);
> +
> +		if (row > ADP5589_MAX_ROW_NUM) {
> +			dev_err(dev, "invalid row number (%i)\n", row);
> +			return -EINVAL;
> +		}
> +
> +		if (col > ADP5589_MAX_COL_NUM) {
> +			dev_err(dev, "invalid column number (%i)\n", col);
> +			return -EINVAL;
> +		}
> +
> +		pdata->keypad_en_mask |= ADP_ROW(row);
> +		pdata->keypad_en_mask |= ADP_COL(col);
> +
> +		keymap[adp5589_key(row, col)] = key;
> +	}
> +
> +	pdata->keymap = keymap;
> +	pdata->keymapsize = ADP5589_KEYMAPSIZE;

I was wondering if we could also move non-DT variant to matrix-keypad
infrastructure and use matrix_keypad_build_keymap and friends to handle
this uniformly.

Thanks.

-- 
Dmitry

Re: [RFC/PATCH 1/3] drivers: input: keyboard: adp5589: add DT support

[Guido Martínez]

Hi Dmitry, sorry for the terribly late reply.

On Sun, May 18, 2014 at 01:40:09PM -0700, Dmitry Torokhov wrote:
> Hi Guido,
> 
> On Wed, May 07, 2014 at 10:00:42AM -0300, Guido Martínez wrote:
> > Add DT support for the Analog ADP5589 matrix keypad decoding functions.
> > 
> > Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> > ---
> >  drivers/input/keyboard/adp5589-keys.c | 207 +++++++++++++++++++++++++++++++++-
> >  1 file changed, 206 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> > index 6329549..2b232c0 100644
> > --- a/drivers/input/keyboard/adp5589-keys.c
> > +++ b/drivers/input/keyboard/adp5589-keys.c
> > @@ -18,7 +18,10 @@
> >  #include <linux/i2c.h>
> >  #include <linux/gpio.h>
> >  #include <linux/slab.h>
> > +#include <linux/of.h>
> > +#include <linux/err.h>
> >  
> > +#include <linux/input/matrix_keypad.h>
> >  #include <linux/input/adp5589.h>
> >  
> >  /* ADP5589/ADP5585 Common Registers */
> > @@ -246,6 +249,14 @@ struct adp5589_kpad {
> >  #endif
> >  };
> >  
> > +static struct of_device_id adp5589_of_match[] = {
> > +	{
> > +		.compatible = "adi,adp5589",
> > +		.data = (void *)ADP5589
> > +	},
> > +	{ },
> > +};
> > +
> >  /*
> >   *  ADP5589 / ADP5585 derivative / variant handling
> >   */
> > @@ -858,6 +869,188 @@ static void adp5589_report_switch_state(struct adp5589_kpad *kpad)
> >  	input_sync(kpad->input);
> >  }
> >  
> > +#ifdef CONFIG_OF
> > +static int adp5589_key(int row, int col)
> > +{
> > +	return col + row * 11;
> > +}
> > +
> > +static int adp5589_dt_read_keymap(struct device *dev,
> > +				  struct adp5589_kpad_platform_data *pdata,
> > +				  const struct device_node *node)
> > +{
> > +	int i;
> > +	const u32 *dt_keymap;
> > +	unsigned short *keymap;
> > +	int keymap_len;
> > +
> > +	dt_keymap = of_get_property(node, "linux,keymap", &keymap_len);
> > +	if (!dt_keymap) {
> > +		dev_err(dev, "missing dt keymap\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (keymap_len % sizeof(u32)) {
> > +		dev_err(dev, "malformed keymap (len=%i)\n", keymap_len);
> > +		return -EINVAL;
> > +	}
> > +
> > +	keymap_len /= sizeof(u32);
> > +
> > +	keymap = devm_kzalloc(dev, ADP5589_KEYMAPSIZE * sizeof(u32),
> > +			      GFP_KERNEL);
> > +	if (!keymap)
> > +		return -ENOMEM;
> > +
> > +	for (i = 0; i < keymap_len; i++) {
> > +		u32 val;
> > +		u16 key;
> > +		u8 row, col;
> > +
> > +		val = be32_to_cpup(&dt_keymap[i]);
> > +
> > +		row = KEY_ROW(val);
> > +		col = KEY_COL(val);
> > +		key = KEY_VAL(val);
> > +
> > +		if (row > ADP5589_MAX_ROW_NUM) {
> > +			dev_err(dev, "invalid row number (%i)\n", row);
> > +			return -EINVAL;
> > +		}
> > +
> > +		if (col > ADP5589_MAX_COL_NUM) {
> > +			dev_err(dev, "invalid column number (%i)\n", col);
> > +			return -EINVAL;
> > +		}
> > +
> > +		pdata->keypad_en_mask |= ADP_ROW(row);
> > +		pdata->keypad_en_mask |= ADP_COL(col);
> > +
> > +		keymap[adp5589_key(row, col)] = key;
> > +	}
> > +
> > +	pdata->keymap = keymap;
> > +	pdata->keymapsize = ADP5589_KEYMAPSIZE;
> 
> I was wondering if we could also move non-DT variant to matrix-keypad
> infrastructure and use matrix_keypad_build_keymap and friends to handle
> this uniformly.

Seems like a good idea, I'll look into it.

Thanks for reviewing this!

-- 
Guido Martínez, VanguardiaSur
www.vanguardiasur.com.ar