Skip to contentSkip to conversation

Input: mcs_touchkey: add device tree support

PATCH In review Beomho Seo 2 participants 3 messages May 21, 2014 – May 22, 2014

Subsystems (2)
HID Input drivers
lore
Share & tools
lore

Beomho Seo

author
#0 lore raw

PATCH 1/2 Input: mcs_touchkey: add device tree support

Add device tree support for mcs touchkey driver.
Tested on exynos 4412 board.

Signed-off-by Beomho Seo <[email protected]>
Cc Joonyoung Shim <[email protected]>
---
 drivers/input/keyboard/mcs_touchkey.c |   73 +++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 4 deletions(-)
drivers/input/keyboard/mcs_touchkey.c +69 -4
index 1da8e0b..9bff47b 100644
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/slab.h>
 #include <linux/pm.h>
+#include <linux/of_gpio.h>
 
 /* MCS5000 Touchkey */
 #define MCS5000_TOUCHKEY_STATUS		0x04

@@ -96,6 +97,60 @@ static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_OF
+static struct mcs_platform_data *mcs_touchkey_parse_dt(struct device *dev)
+{
+	struct mcs_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+	unsigned int keymap[2];
+	unsigned int len;
+	int i = 0;
+	const __be32 *prop;
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Failed to allocate platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	prop = of_get_property(np, "linux,code", &len);
+	if (!prop) {
+		dev_err(dev, "Failed to get code\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (len % sizeof(u32)) {
+		dev_err(dev, "Malformed keycode property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata->keymap_size = len / sizeof(u32);
+
+	if (of_property_read_u32(np, "key_maxval", &pdata->key_maxval)) {
+		dev_err(dev, "Failed to get key max value data\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (pdata->keymap_size > pdata->key_maxval) {
+		dev_err(dev, "Key map size overflow\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (i = 0; i < pdata->keymap_size; i++) {
+		u32 code = be32_to_cpup(prop + i);
+		keymap[i] = MCS_KEY_MAP(i, code);
+	}
+	pdata->keymap = keymap;
+	return pdata;
+}
+#else
+static inline struct mcs_platform_data *mcs_touchkey_parse_dt
+						(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int mcs_touchkey_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {

@@ -107,10 +162,14 @@ static int mcs_touchkey_probe(struct i2c_client *client,
 	int error;
 	int i;
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "no platform data defined\n");
-		return -EINVAL;
+	if (&client->dev.of_node)
+		pdata = mcs_touchkey_parse_dt(&client->dev);
+	else
+		pdata = dev_get_platdata(&client->dev);
+
+	if (IS_ERR(pdata)) {
+		dev_err(&client->dev, "Failed to get platform data\n");
+		return PTR_ERR(pdata);
 	}
 
 	data = kzalloc(sizeof(struct mcs_touchkey_data) +

@@ -262,11 +321,17 @@ static const struct i2c_device_id mcs_touchkey_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mcs_touchkey_id);
 
+static struct of_device_id mcs_touchkey_dt_match[] = {
+	{ .compatible = "mcs5000_touchkey", },
+	{ .compatible = "mcs5080_touchkey", },
+};
+
 static struct i2c_driver mcs_touchkey_driver = {
 	.driver = {
 		.name	= "mcs_touchkey",
 		.owner	= THIS_MODULE,
 		.pm	= &mcs_touchkey_pm_ops,
+		.of_match_table = of_match_ptr(mcs_touchkey_dt_match),
 	},
 	.probe		= mcs_touchkey_probe,
 	.remove		= mcs_touchkey_remove,
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 24 lines · Beomho Seo

Add device tree support for mcs touchkey driver.
Tested on exynos 4412 board.

Signed-off-by Beomho Seo <[email protected]>
Cc Joonyoung Shim <[email protected]>
---
 drivers/input/keyboard/mcs_touchkey.c |   73 +++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 4 deletions(-)
drivers/input/keyboard/mcs_touchkey.c +1 -0
index 1da8e0b..9bff47b 100644
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/slab.h>
 #include <linux/pm.h>
+#include <linux/of_gpio.h>
 
 /* MCS5000 Touchkey */
 #define MCS5000_TOUCHKEY_STATUS		0x04

@@ -96,6 +97,60 @@ static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 108

Quoted 10 lines
drivers/input/keyboard/mcs_touchkey.c +9 -0
 
+#ifdef CONFIG_OF
+static struct mcs_platform_data *mcs_touchkey_parse_dt(struct device *dev)
+{
+	struct mcs_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+	unsigned int keymap[2];
+	unsigned int len;
+	int i = 0;
+	const __be32 *prop;

Hmm. Almost every use of __be32 *prop values is indicative of something that can be done with existing accessors. I suspect that may be true here...

Quoted 9 lines
drivers/input/keyboard/mcs_touchkey.c +9 -0
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Failed to allocate platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	prop = of_get_property(np, "linux,code", &len);
+	if (!prop) {

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 127

Quoted 10 lines
drivers/input/keyboard/mcs_touchkey.c +10 -0
+		dev_err(dev, "Failed to get code\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (len % sizeof(u32)) {
+		dev_err(dev, "Malformed keycode property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata->keymap_size = len / sizeof(u32);

Use of_property_count_u32_elems. It does all of this and returns a negative error code if anything is wrong.

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 132

Quoted 5 lines
drivers/input/keyboard/mcs_touchkey.c +5 -0
+
+	if (of_property_read_u32(np, "key_maxval", &pdata->key_maxval)) {
+		dev_err(dev, "Failed to get key max value data\n");
+		return ERR_PTR(-EINVAL);
+	}

What is this? This sounds like an implementation detail. Why is it in the DT?

Why doesn't it follow dt conventions ('-' rather than '_')?

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 140

Quoted 8 lines
drivers/input/keyboard/mcs_touchkey.c +8 -0
+
+	if (pdata->keymap_size > pdata->key_maxval) {
+		dev_err(dev, "Key map size overflow\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (i = 0; i < pdata->keymap_size; i++) {
+		u32 code = be32_to_cpup(prop + i);

Use the DT accessors (e.g. of_property_read_u32_index). There is absolutely no reason to touch the raw DTB data here.

Quoted 36 lines
drivers/input/keyboard/mcs_touchkey.c +21 -4
+		keymap[i] = MCS_KEY_MAP(i, code);
+	}
+	pdata->keymap = keymap;
+	return pdata;
+}
+#else
+static inline struct mcs_platform_data *mcs_touchkey_parse_dt
+						(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int mcs_touchkey_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {

@@ -107,10 +162,14 @@ static int mcs_touchkey_probe(struct i2c_client *client,
 	int error;
 	int i;
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "no platform data defined\n");
-		return -EINVAL;
+	if (&client->dev.of_node)
+		pdata = mcs_touchkey_parse_dt(&client->dev);
+	else
+		pdata = dev_get_platdata(&client->dev);
+
+	if (IS_ERR(pdata)) {
+		dev_err(&client->dev, "Failed to get platform data\n");
+		return PTR_ERR(pdata);
 	}
 
 	data = kzalloc(sizeof(struct mcs_touchkey_data) +

drivers/input/keyboard/mcs_touchkey.c static const struct i2c_device_id mcs_touchkey_id[] = { · line 326

Quoted 6 lines
drivers/input/keyboard/mcs_touchkey.c +3 -0
@@ -262,11 +321,17 @@ static const struct i2c_device_id mcs_touchkey_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mcs_touchkey_id);
 
+static struct of_device_id mcs_touchkey_dt_match[] = {
+	{ .compatible = "mcs5000_touchkey", },
+	{ .compatible = "mcs5080_touchkey", },

NAK. These do not follow the "$VENDOR,$DEVICE" pattern.

Mark.

Quoted 14 lines
drivers/input/keyboard/mcs_touchkey.c +3 -0
+};
+
 static struct i2c_driver mcs_touchkey_driver = {
 	.driver = {
 		.name	= "mcs_touchkey",
 		.owner	= THIS_MODULE,
 		.pm	= &mcs_touchkey_pm_ops,
+		.of_match_table = of_match_ptr(mcs_touchkey_dt_match),
 	},
 	.probe		= mcs_touchkey_probe,
 	.remove		= mcs_touchkey_remove,
Signature · 1 line
1.7.9.5
author
#2 lore raw

Hello,

Thank you for your advice.

Quoted 72 lines · Mark Rutland
Quoted 24 lines · Beomho Seo

Add device tree support for mcs touchkey driver.
Tested on exynos 4412 board.

Signed-off-by Beomho Seo <[email protected]>
Cc Joonyoung Shim <[email protected]>
---
 drivers/input/keyboard/mcs_touchkey.c |   73 +++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 4 deletions(-)
drivers/input/keyboard/mcs_touchkey.c +1 -0
index 1da8e0b..9bff47b 100644
@@ -19,6 +19,7 @@
 #include <linux/irq.h>
 #include <linux/slab.h>
 #include <linux/pm.h>
+#include <linux/of_gpio.h>
 
 /* MCS5000 Touchkey */
 #define MCS5000_TOUCHKEY_STATUS		0x04

@@ -96,6 +97,60 @@ static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 108

Quoted 10 lines
drivers/input/keyboard/mcs_touchkey.c +9 -0
 
+#ifdef CONFIG_OF
+static struct mcs_platform_data *mcs_touchkey_parse_dt(struct device *dev)
+{
+	struct mcs_platform_data *pdata;
+	struct device_node *np = dev->of_node;
+	unsigned int keymap[2];
+	unsigned int len;
+	int i = 0;
+	const __be32 *prop;

Hmm. Almost every use of __be32 *prop values is indicative of something that can be done with existing accessors. I suspect that may be true here...

Quoted 9 lines
drivers/input/keyboard/mcs_touchkey.c +9 -0
+
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata) {
+		dev_err(dev, "Failed to allocate platform data\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	prop = of_get_property(np, "linux,code", &len);
+	if (!prop) {

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 127

Quoted 10 lines
drivers/input/keyboard/mcs_touchkey.c +10 -0
+		dev_err(dev, "Failed to get code\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (len % sizeof(u32)) {
+		dev_err(dev, "Malformed keycode property\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	pdata->keymap_size = len / sizeof(u32);

Use of_property_count_u32_elems. It does all of this and returns a negative error code if anything is wrong.

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 132

Quoted 5 lines
drivers/input/keyboard/mcs_touchkey.c +5 -0
+
+	if (of_property_read_u32(np, "key_maxval", &pdata->key_maxval)) {
+		dev_err(dev, "Failed to get key max value data\n");
+		return ERR_PTR(-EINVAL);
+	}

What is this? This sounds like an implementation detail. Why is it in the DT?

Quoted 2 lines

Why doesn't it follow dt conventions ('-' rather than '_')?

OK. I will revise *_parse_dt function on your advice.

Quoted 51 lines

drivers/input/keyboard/mcs_touchkey.c static irqreturn_t mcs_touchkey_interrupt(int irq, void *dev_id) · line 140

Quoted 8 lines
drivers/input/keyboard/mcs_touchkey.c +8 -0
+
+	if (pdata->keymap_size > pdata->key_maxval) {
+		dev_err(dev, "Key map size overflow\n");
+		return ERR_PTR(-EINVAL);
+	}
+
+	for (i = 0; i < pdata->keymap_size; i++) {
+		u32 code = be32_to_cpup(prop + i);

Use the DT accessors (e.g. of_property_read_u32_index). There is absolutely no reason to touch the raw DTB data here.

Quoted 39 lines
drivers/input/keyboard/mcs_touchkey.c +21 -4
+		keymap[i] = MCS_KEY_MAP(i, code);
+	}
+	pdata->keymap = keymap;
+	return pdata;
+}
+#else
+static inline struct mcs_platform_data *mcs_touchkey_parse_dt
+						(struct device *dev)
+{
+	return NULL;
+}
+#endif
+
 static int mcs_touchkey_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {

@@ -107,10 +162,14 @@ static int mcs_touchkey_probe(struct i2c_client *client,
 	int error;
 	int i;
 
-	pdata = dev_get_platdata(&client->dev);
-	if (!pdata) {
-		dev_err(&client->dev, "no platform data defined\n");
-		return -EINVAL;
+	if (&client->dev.of_node)
+		pdata = mcs_touchkey_parse_dt(&client->dev);
+	else
+		pdata = dev_get_platdata(&client->dev);
+
+	if (IS_ERR(pdata)) {
+		dev_err(&client->dev, "Failed to get platform data\n");
+		return PTR_ERR(pdata);
 	}
 
 	data = kzalloc(sizeof(struct mcs_touchkey_data) +

@@ -262,11 +321,17 @@ static const struct i2c_device_id mcs_touchkey_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, mcs_touchkey_id);
Quoted 8 lines

drivers/input/keyboard/mcs_touchkey.c static const struct i2c_device_id mcs_touchkey_id[] = { · line 326

Quoted 3 lines
drivers/input/keyboard/mcs_touchkey.c +3 -0
+static struct of_device_id mcs_touchkey_dt_match[] = {
+	{ .compatible = "mcs5000_touchkey", },
+	{ .compatible = "mcs5080_touchkey", },

NAK. These do not follow the "$VENDOR,$DEVICE" pattern.

Mark.

OK, I will changes to above pattern.
Additionally, Fix bindings documentation and vendor-prefixes.txt.

Quoted 6 lines
Quoted 6 lines
drivers/input/keyboard/mcs_touchkey.c +2 -0
+};
+
 static struct i2c_driver mcs_touchkey_driver = {
 	.driver = {
 		.name	= "mcs_touchkey",
 		.owner	= THIS_MODULE,
Quoted 9 lines
Quoted 8 lines
drivers/input/keyboard/mcs_touchkey.c +1 -0
 		.pm	= &mcs_touchkey_pm_ops,
+		.of_match_table = of_match_ptr(mcs_touchkey_dt_match),
 	},
 	.probe		= mcs_touchkey_probe,
 	.remove		= mcs_touchkey_remove,
Signature · 1 line
1.7.9.5

Again, thank you for your advice.
I'll fix them and send v2 patch soon.

Best regards,
Beomho Seo