linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH 3/3] max7359_keypad: implement DT bindings
       [not found] ` <pquS6-2DV-9@gated-at.bofh.it>
@ 2015-05-17 16:55   ` Evgeniy Dushistov
  2015-05-18 16:53     ` Dmitry Torokhov
  2015-05-18 17:02     ` Dmitry Torokhov
  0 siblings, 2 replies; 5+ messages in thread
From: Evgeniy Dushistov @ 2015-05-17 16:55 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: devicetree, linux-kernel, linux-input

On Fri, May 15, 2015 at 11:00:02PM +0200, Dmitry Torokhov wrote:
> On Thu, May 14, 2015 at 05:38:03AM +0300, Evgeniy Dushistov wrote:
> > +		maxim,debounce_reg = /bits/ 8 <0x5F>;
> > +		maxim,ports_reg = /bits/ 8 <0xFF>;
> 
> Specifying exact size for properties is quite uncommon; I think the
> usual recommendation is is to use the "standard" u32 and validate the
> range in parser function.
> 

Using of u8 has advantages, it is possible on compilation stage
(dts->dtb) check that you enter right value, but
because of DT validation tools are not part of mainline,
I replace u8 with u32 as you suggest, see patch bellow.

> > +		MATRIX_KEY(0, 7, KEY_RESERVED)
> > +		...
> 
> 
> Indent one more level? Also, maybe fill with something other than
> KEY_RESERVED?
> 

Fixed, see patch bellow.

> 
> 
 
> >  	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
> > +	if (!keymap_data) {
> > +		error = max7359_parse_dt(&client->dev, &init_state);
> > +		if (error) {
> > +			dev_err(&client->dev, "platform data null, and no DT data\n");
> > +			return error;
> 
> Both debounce and ports values are optional and we'll fail building
> keymap if there are no platform data nor device tree data, so I would
> drop this check and the stub for max7359_parse_dt() as well - we have
> stubs for property parsing anyway.
> 

Because of u32/u8 check at now max7359_parse_dt can return not 0.

> -- 
> Dmitry
> 

Thanks, for review. 

Please add me to CC, because of I'm not the part
of any mailing list (too huge traffic for me).
Here patch with fixes mentioned above:


max7359_keypad: implement DT bindings

Signed-off-by: Evgeniy A. Dushistov <dushistov@mail.ru>
---
 .../devicetree/bindings/input/max7359-keypad.txt   | 33 ++++++++++
 drivers/input/keyboard/max7359_keypad.c            | 70 +++++++++++++++++++++-
 2 files changed, 100 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/max7359-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/max7359-keypad.txt b/Documentation/devicetree/bindings/input/max7359-keypad.txt
new file mode 100644
index 0000000..5bd4a4f
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/max7359-keypad.txt
@@ -0,0 +1,33 @@
+* MAX7359 keypda device tree bindings
+
+Required Properties:
+- compatible:		Should be "maxim,max7359"
+- linux,keymap:		The definition can be found at
+			bindings/input/matrix-keymap.txt
+
+Optional Properties:
+- maxim,debounce_reg: u32, in short allow control debounce,
+see max7359 datasheet for details
+- maxim,ports_reg: u32, in short allow control what pin used,
+and what pins should be configured as GPIO
+
+Example:
+max7359_keypad@38 {
+	compatible = "maxim,max7359";
+	reg = <0x38>;
+	interrupts = <28 IRQ_TYPE_LEVEL_LOW>;/*gpio_156*/
+	interrupt-parent = <&gpio5>;
+	maxim,debounce_reg = <0x5F>;
+	maxim,ports_reg = <0xFF>;
+	linux,keymap = <
+	MATRIX_KEY(0, 0, KEY_F1)
+	MATRIX_KEY(0, 1, KEY_RESERVED)
+	MATRIX_KEY(0, 2, KEY_F2)
+	MATRIX_KEY(0, 3, KEY_RESERVED)
+	MATRIX_KEY(0, 4, KEY_M)
+	MATRIX_KEY(0, 5, KEY_L)
+	MATRIX_KEY(0, 6, KEY_P)
+	MATRIX_KEY(0, 7, KEY_ESC)
+	...
+	>;
+};
diff --git a/drivers/input/keyboard/max7359_keypad.c b/drivers/input/keyboard/max7359_keypad.c
index 5091133..60b4d29 100644
--- a/drivers/input/keyboard/max7359_keypad.c
+++ b/drivers/input/keyboard/max7359_keypad.c
@@ -20,6 +20,7 @@
 #include <linux/pm.h>
 #include <linux/input.h>
 #include <linux/input/matrix_keypad.h>
+#include <linux/of.h>
 
 #define MAX7359_MAX_KEY_ROWS	8
 #define MAX7359_MAX_KEY_COLS	8
@@ -64,6 +65,11 @@ struct max7359_keypad {
 	struct i2c_client *client;
 };
 
+struct max7359_initial_state {
+	u8 debounce_val;
+	u8 ports_val;
+};
+
 static int max7359_write_reg(struct i2c_client *client, u8 reg, u8 val)
 {
 	int ret = i2c_smbus_write_byte_data(client, reg, val);
@@ -143,24 +149,64 @@ static void max7359_close(struct input_dev *dev)
 	max7359_fall_deepsleep(keypad->client);
 }
 
-static void max7359_initialize(struct i2c_client *client)
+static void max7359_initialize(struct i2c_client *client,
+			       const struct max7359_initial_state *init_state)
 {
 	max7359_write_reg(client, MAX7359_REG_CONFIG,
 		MAX7359_CFG_KEY_RELEASE | /* Key release enable */
 		MAX7359_CFG_WAKEUP); /* Key press wakeup enable */
 
 	/* Full key-scan functionality */
-	max7359_write_reg(client, MAX7359_REG_DEBOUNCE, 0x1F);
+	max7359_write_reg(client, MAX7359_REG_DEBOUNCE,
+			  init_state->debounce_val);
 
 	/* nINT asserts every debounce cycles */
 	max7359_write_reg(client, MAX7359_REG_INTERRUPT, 0x01);
+	max7359_write_reg(client, MAX7359_REG_PORTS, init_state->ports_val);
 
 	max7359_fall_deepsleep(client);
 }
 
+#ifdef CONFIG_OF
+static int max7359_parse_dt(struct device *dev,
+			    struct max7359_initial_state *init_state)
+{
+	struct device_node *np = dev->of_node;
+	u32 prop;
+
+	if (!of_property_read_u32(np, "maxim,debounce_reg", &prop)) {
+		if (prop > 0xFF) {
+			dev_err(dev, "expect 8bit value for maxim,debounce_reg\n");
+			return -EINVAL;
+		}
+		init_state->debounce_val = prop;
+	}
+
+	if (!of_property_read_u32(np, "maxim,ports_reg", &prop)) {
+		if (prop > 0xFF) {
+			dev_err(dev, "expect 8bit value for maxim,ports_reg\n");
+			return -EINVAL;
+		}
+		init_state->ports_val = prop;
+	}
+
+	return 0;
+}
+#else
+static inline int max7359_parse_dt(struct device *dev,
+				   struct max7359_initial_state *init_state)
+{
+	return -EINVAL;
+}
+#endif
+
 static int max7359_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
+	struct max7359_initial_state init_state = {
+		.debounce_val = 0x1F,
+		.ports_val = 0xFE,
+	};
 	const struct matrix_keymap_data *keymap_data =
 			dev_get_platdata(&client->dev);
 	struct max7359_keypad *keypad;
@@ -181,6 +227,15 @@ static int max7359_probe(struct i2c_client *client,
 	}
 
 	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
+	if (!keymap_data) {
+		error = max7359_parse_dt(&client->dev, &init_state);
+		if (error) {
+			dev_err(&client->dev, "Invalid DT data\n");
+			return error;
+		}
+		dev_dbg(&client->dev, "Init vals: debounce %X, ports %X\n",
+			init_state.debounce_val, init_state.ports_val);
+	}
 
 	keypad = devm_kzalloc(&client->dev, sizeof(struct max7359_keypad),
 			      GFP_KERNEL);
@@ -239,7 +294,7 @@ static int max7359_probe(struct i2c_client *client,
 	}
 
 	/* Initialize MAX7359 */
-	max7359_initialize(client);
+	max7359_initialize(client, &init_state);
 
 	i2c_set_clientdata(client, keypad);
 	device_init_wakeup(&client->dev, 1);
@@ -282,10 +337,19 @@ static const struct i2c_device_id max7359_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, max7359_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id max7359_dt_match[] = {
+	{ .compatible = "maxim,max7359" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max7359_dt_match);
+#endif
+
 static struct i2c_driver max7359_i2c_driver = {
 	.driver = {
 		.name = "max7359",
 		.pm   = &max7359_pm,
+		.of_match_table = of_match_ptr(max7359_dt_match),
 	},
 	.probe		= max7359_probe,
 	.id_table	= max7359_ids,
-- 
2.3.6


-- 
/Evgeniy

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

* Re: [PATCH 3/3] max7359_keypad: implement DT bindings
  2015-05-17 16:55   ` [PATCH 3/3] max7359_keypad: implement DT bindings Evgeniy Dushistov
@ 2015-05-18 16:53     ` Dmitry Torokhov
  2015-05-18 17:02     ` Dmitry Torokhov
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-05-18 16:53 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-input

On Sun, May 17, 2015 at 07:55:02PM +0300, Evgeniy Dushistov wrote:
> On Fri, May 15, 2015 at 11:00:02PM +0200, Dmitry Torokhov wrote:
> > On Thu, May 14, 2015 at 05:38:03AM +0300, Evgeniy Dushistov wrote:
> > > +		maxim,debounce_reg = /bits/ 8 <0x5F>;
> > > +		maxim,ports_reg = /bits/ 8 <0xFF>;
> > 
> > Specifying exact size for properties is quite uncommon; I think the
> > usual recommendation is is to use the "standard" u32 and validate the
> > range in parser function.
> > 
> 
> Using of u8 has advantages, it is possible on compilation stage
> (dts->dtb) check that you enter right value, but
> because of DT validation tools are not part of mainline,
> I replace u8 with u32 as you suggest, see patch bellow.

If you get DT folks OK on using /bits/ I'm fine with keeping it.

-- 
Dmitry

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

* Re: [PATCH 3/3] max7359_keypad: implement DT bindings
  2015-05-17 16:55   ` [PATCH 3/3] max7359_keypad: implement DT bindings Evgeniy Dushistov
  2015-05-18 16:53     ` Dmitry Torokhov
@ 2015-05-18 17:02     ` Dmitry Torokhov
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-05-18 17:02 UTC (permalink / raw)
  To: Grant Likely, robh+dt, Evgeniy Dushistov
  Cc: devicetree, linux-kernel, linux-input

On Sun, May 17, 2015 at 07:55:02PM +0300, Evgeniy Dushistov wrote:
> On Fri, May 15, 2015 at 11:00:02PM +0200, Dmitry Torokhov wrote:
> > On Thu, May 14, 2015 at 05:38:03AM +0300, Evgeniy Dushistov wrote:
> > > +		maxim,debounce_reg = /bits/ 8 <0x5F>;
> > > +		maxim,ports_reg = /bits/ 8 <0xFF>;
> > 
> > Specifying exact size for properties is quite uncommon; I think the
> > usual recommendation is is to use the "standard" u32 and validate the
> > range in parser function.
> > 
> 
> Using of u8 has advantages, it is possible on compilation stage
> (dts->dtb) check that you enter right value, but
> because of DT validation tools are not part of mainline,
> I replace u8 with u32 as you suggest, see patch bellow.

If you get DT folks to OK using /bits/ - that's fine. Grant, Rob, any
guidance here?

> 
> > > +		MATRIX_KEY(0, 7, KEY_RESERVED)
> > > +		...
> > 
> > 
> > Indent one more level? Also, maybe fill with something other than
> > KEY_RESERVED?
> > 
> 
> Fixed, see patch bellow.
> 
> > 
> > 
>  
> > >  	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
> > > +	if (!keymap_data) {
> > > +		error = max7359_parse_dt(&client->dev, &init_state);
> > > +		if (error) {
> > > +			dev_err(&client->dev, "platform data null, and no DT data\n");
> > > +			return error;
> > 
> > Both debounce and ports values are optional and we'll fail building
> > keymap if there are no platform data nor device tree data, so I would
> > drop this check and the stub for max7359_parse_dt() as well - we have
> > stubs for property parsing anyway.
> > 
> 
> Because of u32/u8 check at now max7359_parse_dt can return not 0.
> 
> > -- 
> > Dmitry
> > 
> 
> Thanks, for review. 
> 
> Please add me to CC, because of I'm not the part
> of any mailing list (too huge traffic for me).

Umm, if you want to be replied (or CCed) to, why do you make sure that
replies do not go to you by default??? Why do you set "Mail-Followup-To"
to exclude yourself?

> Mail-Followup-To: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
> 	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
> 	linux-input@vger.kernel.org

Thanks.

-- 
Dmitry

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

* Re: [PATCH 3/3] max7359_keypad: implement DT bindings
  2015-05-14  2:38 Evgeniy Dushistov
@ 2015-05-15 20:57 ` Dmitry Torokhov
  0 siblings, 0 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2015-05-15 20:57 UTC (permalink / raw)
  To: devicetree, linux-kernel, linux-input

On Thu, May 14, 2015 at 05:38:03AM +0300, Evgeniy Dushistov wrote:
> max7359_keypad: implement DT bindings
> 
> Signed-off-by: Evgeniy A. Dushistov <dushistov@mail.ru>
> ---
>  .../devicetree/bindings/input/max7359-keypad.txt   | 33 ++++++++++++
>  drivers/input/keyboard/max7359_keypad.c            | 60 ++++++++++++++++++++--
>  2 files changed, 90 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/max7359-keypad.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/max7359-keypad.txt b/Documentation/devicetree/bindings/input/max7359-keypad.txt
> new file mode 100644
> index 0000000..0a3c381
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/max7359-keypad.txt
> @@ -0,0 +1,33 @@
> +* MAX7359 keypda device tree bindings
> +
> +Required Properties:
> +- compatible:		Should be "maxim,max7359"
> +- linux,keymap:		The definition can be found at
> +			bindings/input/matrix-keymap.txt
> +
> +Optional Properties:
> +- maxim,debounce_reg: u8, in short allow control debounce,
> +see max7359 datasheet for details
> +- maxim,ports_reg: u8, in short allow control what pin used,
> +and what pins should be configured as GPIO
> +
> +Example:
> +	max7359_keypad@38 {
> +		compatible = "maxim,max7359";
> +		reg = <0x38>;
> +		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;/*gpio_156*/
> +		interrupt-parent = <&gpio5>;
> +		maxim,debounce_reg = /bits/ 8 <0x5F>;
> +		maxim,ports_reg = /bits/ 8 <0xFF>;

Specifying exact size for properties is quite uncommon; I think the
usual recommendation is is to use the "standard" u32 and validate the
range in parser function.

> +		linux,keymap = <
> +		MATRIX_KEY(0, 0, KEY_RESERVED) /*row 0 not used*/
> +		MATRIX_KEY(0, 1, KEY_RESERVED)
> +		MATRIX_KEY(0, 2, KEY_RESERVED)
> +		MATRIX_KEY(0, 3, KEY_RESERVED)
> +		MATRIX_KEY(0, 4, KEY_RESERVED)
> +		MATRIX_KEY(0, 5, KEY_RESERVED)
> +		MATRIX_KEY(0, 6, KEY_RESERVED)
> +		MATRIX_KEY(0, 7, KEY_RESERVED)
> +		...

Indent one more level? Also, maybe fill with something other than
KEY_RESERVED?

> +		>;
> +	};
> diff --git a/drivers/input/keyboard/max7359_keypad.c b/drivers/input/keyboard/max7359_keypad.c
> index 5091133..0164f2e 100644
> --- a/drivers/input/keyboard/max7359_keypad.c
> +++ b/drivers/input/keyboard/max7359_keypad.c
> @@ -20,6 +20,7 @@
>  #include <linux/pm.h>
>  #include <linux/input.h>
>  #include <linux/input/matrix_keypad.h>
> +#include <linux/of.h>
>  
>  #define MAX7359_MAX_KEY_ROWS	8
>  #define MAX7359_MAX_KEY_COLS	8
> @@ -64,6 +65,11 @@ struct max7359_keypad {
>  	struct i2c_client *client;
>  };
>  
> +struct max7359_initial_state {
> +	u8 debounce_val;
> +	u8 ports_val;
> +};
> +
>  static int max7359_write_reg(struct i2c_client *client, u8 reg, u8 val)
>  {
>  	int ret = i2c_smbus_write_byte_data(client, reg, val);
> @@ -143,24 +149,54 @@ static void max7359_close(struct input_dev *dev)
>  	max7359_fall_deepsleep(keypad->client);
>  }
>  
> -static void max7359_initialize(struct i2c_client *client)
> +static void max7359_initialize(struct i2c_client *client,
> +			       const struct max7359_initial_state *init_state)
>  {
>  	max7359_write_reg(client, MAX7359_REG_CONFIG,
>  		MAX7359_CFG_KEY_RELEASE | /* Key release enable */
>  		MAX7359_CFG_WAKEUP); /* Key press wakeup enable */
>  
>  	/* Full key-scan functionality */
> -	max7359_write_reg(client, MAX7359_REG_DEBOUNCE, 0x1F);
> +	max7359_write_reg(client, MAX7359_REG_DEBOUNCE,
> +			  init_state->debounce_val);
>  
>  	/* nINT asserts every debounce cycles */
>  	max7359_write_reg(client, MAX7359_REG_INTERRUPT, 0x01);
> +	max7359_write_reg(client, MAX7359_REG_PORTS, init_state->ports_val);
>  
>  	max7359_fall_deepsleep(client);
>  }
>  
> +#ifdef CONFIG_OF
> +static int max7359_parse_dt(struct device *dev,
> +			    struct max7359_initial_state *init_state)
> +{
> +	struct device_node *np = dev->of_node;
> +	u8 prop;
> +
> +	if (!of_property_read_u8(np, "maxim,debounce_reg", &prop))
> +		init_state->debounce_val = prop;
> +
> +	if (!of_property_read_u8(np, "maxim,ports_reg", &prop))
> +		init_state->ports_val = prop;
> +
> +	return 0;
> +}
> +#else
> +static inline int max7359_parse_dt(struct device *dev,
> +				   struct max7359_initial_state *init_state)
> +{
> +	return -EINVAL;
> +}
> +#endif
> +
>  static int max7359_probe(struct i2c_client *client,
>  					const struct i2c_device_id *id)
>  {
> +	struct max7359_initial_state init_state = {
> +		.debounce_val = 0x1F,
> +		.ports_val = 0xFE,
> +	};
>  	const struct matrix_keymap_data *keymap_data =
>  			dev_get_platdata(&client->dev);
>  	struct max7359_keypad *keypad;
> @@ -181,6 +217,15 @@ static int max7359_probe(struct i2c_client *client,
>  	}
>  
>  	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
> +	if (!keymap_data) {
> +		error = max7359_parse_dt(&client->dev, &init_state);
> +		if (error) {
> +			dev_err(&client->dev, "platform data null, and no DT data\n");
> +			return error;

Both debounce and ports values are optional and we'll fail building
keymap if there are no platform data nor device tree data, so I would
drop this check and the stub for max7359_parse_dt() as well - we have
stubs for property parsing anyway.

> +		}
> +		dev_dbg(&client->dev, "Init vals: debounce %X, ports %X\n",
> +			init_state.debounce_val, init_state.ports_val);
> +	}
>  
>  	keypad = devm_kzalloc(&client->dev, sizeof(struct max7359_keypad),
>  			      GFP_KERNEL);
> @@ -239,7 +284,7 @@ static int max7359_probe(struct i2c_client *client,
>  	}
>  
>  	/* Initialize MAX7359 */
> -	max7359_initialize(client);
> +	max7359_initialize(client, &init_state);
>  
>  	i2c_set_clientdata(client, keypad);
>  	device_init_wakeup(&client->dev, 1);
> @@ -282,10 +327,19 @@ static const struct i2c_device_id max7359_ids[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, max7359_ids);
>  
> +#ifdef CONFIG_OF
> +static const struct of_device_id max7359_dt_match[] = {
> +	{ .compatible = "maxim,max7359" },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, max7359_dt_match);
> +#endif
> +
>  static struct i2c_driver max7359_i2c_driver = {
>  	.driver = {
>  		.name = "max7359",
>  		.pm   = &max7359_pm,
> +		.of_match_table = of_match_ptr(max7359_dt_match),
>  	},
>  	.probe		= max7359_probe,
>  	.id_table	= max7359_ids,
> -- 
> 2.3.6
> 
> -- 
> /Evgeniy

Thanks.

-- 
Dmitry

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

* [PATCH 3/3] max7359_keypad: implement DT bindings
@ 2015-05-14  2:38 Evgeniy Dushistov
  2015-05-15 20:57 ` Dmitry Torokhov
  0 siblings, 1 reply; 5+ messages in thread
From: Evgeniy Dushistov @ 2015-05-14  2:38 UTC (permalink / raw)
  To: Dmitry Torokhov, devicetree; +Cc: linux-kernel, linux-input

max7359_keypad: implement DT bindings

Signed-off-by: Evgeniy A. Dushistov <dushistov@mail.ru>
---
 .../devicetree/bindings/input/max7359-keypad.txt   | 33 ++++++++++++
 drivers/input/keyboard/max7359_keypad.c            | 60 ++++++++++++++++++++--
 2 files changed, 90 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/max7359-keypad.txt

diff --git a/Documentation/devicetree/bindings/input/max7359-keypad.txt b/Documentation/devicetree/bindings/input/max7359-keypad.txt
new file mode 100644
index 0000000..0a3c381
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/max7359-keypad.txt
@@ -0,0 +1,33 @@
+* MAX7359 keypda device tree bindings
+
+Required Properties:
+- compatible:		Should be "maxim,max7359"
+- linux,keymap:		The definition can be found at
+			bindings/input/matrix-keymap.txt
+
+Optional Properties:
+- maxim,debounce_reg: u8, in short allow control debounce,
+see max7359 datasheet for details
+- maxim,ports_reg: u8, in short allow control what pin used,
+and what pins should be configured as GPIO
+
+Example:
+	max7359_keypad@38 {
+		compatible = "maxim,max7359";
+		reg = <0x38>;
+		interrupts = <28 IRQ_TYPE_LEVEL_LOW>;/*gpio_156*/
+		interrupt-parent = <&gpio5>;
+		maxim,debounce_reg = /bits/ 8 <0x5F>;
+		maxim,ports_reg = /bits/ 8 <0xFF>;
+		linux,keymap = <
+		MATRIX_KEY(0, 0, KEY_RESERVED) /*row 0 not used*/
+		MATRIX_KEY(0, 1, KEY_RESERVED)
+		MATRIX_KEY(0, 2, KEY_RESERVED)
+		MATRIX_KEY(0, 3, KEY_RESERVED)
+		MATRIX_KEY(0, 4, KEY_RESERVED)
+		MATRIX_KEY(0, 5, KEY_RESERVED)
+		MATRIX_KEY(0, 6, KEY_RESERVED)
+		MATRIX_KEY(0, 7, KEY_RESERVED)
+		...
+		>;
+	};
diff --git a/drivers/input/keyboard/max7359_keypad.c b/drivers/input/keyboard/max7359_keypad.c
index 5091133..0164f2e 100644
--- a/drivers/input/keyboard/max7359_keypad.c
+++ b/drivers/input/keyboard/max7359_keypad.c
@@ -20,6 +20,7 @@
 #include <linux/pm.h>
 #include <linux/input.h>
 #include <linux/input/matrix_keypad.h>
+#include <linux/of.h>
 
 #define MAX7359_MAX_KEY_ROWS	8
 #define MAX7359_MAX_KEY_COLS	8
@@ -64,6 +65,11 @@ struct max7359_keypad {
 	struct i2c_client *client;
 };
 
+struct max7359_initial_state {
+	u8 debounce_val;
+	u8 ports_val;
+};
+
 static int max7359_write_reg(struct i2c_client *client, u8 reg, u8 val)
 {
 	int ret = i2c_smbus_write_byte_data(client, reg, val);
@@ -143,24 +149,54 @@ static void max7359_close(struct input_dev *dev)
 	max7359_fall_deepsleep(keypad->client);
 }
 
-static void max7359_initialize(struct i2c_client *client)
+static void max7359_initialize(struct i2c_client *client,
+			       const struct max7359_initial_state *init_state)
 {
 	max7359_write_reg(client, MAX7359_REG_CONFIG,
 		MAX7359_CFG_KEY_RELEASE | /* Key release enable */
 		MAX7359_CFG_WAKEUP); /* Key press wakeup enable */
 
 	/* Full key-scan functionality */
-	max7359_write_reg(client, MAX7359_REG_DEBOUNCE, 0x1F);
+	max7359_write_reg(client, MAX7359_REG_DEBOUNCE,
+			  init_state->debounce_val);
 
 	/* nINT asserts every debounce cycles */
 	max7359_write_reg(client, MAX7359_REG_INTERRUPT, 0x01);
+	max7359_write_reg(client, MAX7359_REG_PORTS, init_state->ports_val);
 
 	max7359_fall_deepsleep(client);
 }
 
+#ifdef CONFIG_OF
+static int max7359_parse_dt(struct device *dev,
+			    struct max7359_initial_state *init_state)
+{
+	struct device_node *np = dev->of_node;
+	u8 prop;
+
+	if (!of_property_read_u8(np, "maxim,debounce_reg", &prop))
+		init_state->debounce_val = prop;
+
+	if (!of_property_read_u8(np, "maxim,ports_reg", &prop))
+		init_state->ports_val = prop;
+
+	return 0;
+}
+#else
+static inline int max7359_parse_dt(struct device *dev,
+				   struct max7359_initial_state *init_state)
+{
+	return -EINVAL;
+}
+#endif
+
 static int max7359_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
+	struct max7359_initial_state init_state = {
+		.debounce_val = 0x1F,
+		.ports_val = 0xFE,
+	};
 	const struct matrix_keymap_data *keymap_data =
 			dev_get_platdata(&client->dev);
 	struct max7359_keypad *keypad;
@@ -181,6 +217,15 @@ static int max7359_probe(struct i2c_client *client,
 	}
 
 	dev_dbg(&client->dev, "keys FIFO is 0x%02x\n", ret);
+	if (!keymap_data) {
+		error = max7359_parse_dt(&client->dev, &init_state);
+		if (error) {
+			dev_err(&client->dev, "platform data null, and no DT data\n");
+			return error;
+		}
+		dev_dbg(&client->dev, "Init vals: debounce %X, ports %X\n",
+			init_state.debounce_val, init_state.ports_val);
+	}
 
 	keypad = devm_kzalloc(&client->dev, sizeof(struct max7359_keypad),
 			      GFP_KERNEL);
@@ -239,7 +284,7 @@ static int max7359_probe(struct i2c_client *client,
 	}
 
 	/* Initialize MAX7359 */
-	max7359_initialize(client);
+	max7359_initialize(client, &init_state);
 
 	i2c_set_clientdata(client, keypad);
 	device_init_wakeup(&client->dev, 1);
@@ -282,10 +327,19 @@ static const struct i2c_device_id max7359_ids[] = {
 };
 MODULE_DEVICE_TABLE(i2c, max7359_ids);
 
+#ifdef CONFIG_OF
+static const struct of_device_id max7359_dt_match[] = {
+	{ .compatible = "maxim,max7359" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, max7359_dt_match);
+#endif
+
 static struct i2c_driver max7359_i2c_driver = {
 	.driver = {
 		.name = "max7359",
 		.pm   = &max7359_pm,
+		.of_match_table = of_match_ptr(max7359_dt_match),
 	},
 	.probe		= max7359_probe,
 	.id_table	= max7359_ids,
-- 
2.3.6

-- 
/Evgeniy

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

end of thread, other threads:[~2015-05-18 17:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <ppRxn-3aD-5@gated-at.bofh.it>
     [not found] ` <pquS6-2DV-9@gated-at.bofh.it>
2015-05-17 16:55   ` [PATCH 3/3] max7359_keypad: implement DT bindings Evgeniy Dushistov
2015-05-18 16:53     ` Dmitry Torokhov
2015-05-18 17:02     ` Dmitry Torokhov
2015-05-14  2:38 Evgeniy Dushistov
2015-05-15 20:57 ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).