All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RESEND] input: gpio_keys_polled: convert to dt
@ 2012-06-20 11:34 ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-20 11:34 UTC (permalink / raw)
  Cc: Alexandre Pereira da Silva, Dmitry Torokhov, Grant Likely,
	Rob Herring, JJ Ding, Roland Stigge, linux-input, linux-kernel,
	devicetree-discuss

Add device tree support to gpio_keys_polled.c

Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
Tested-by: Roland Stigge <stigge@antcom.de>
---
 drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 20c8ab1..a64b361 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,6 +25,8 @@
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 #include <linux/gpio_keys.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
 
 #define DRV_NAME	"gpio-keys-polled"
 
@@ -38,7 +40,7 @@ struct gpio_keys_button_data {
 struct gpio_keys_polled_dev {
 	struct input_polled_dev *poll_dev;
 	struct device *dev;
-	struct gpio_keys_platform_data *pdata;
+	struct gpio_keys_platform_data pdata;
 	struct gpio_keys_button_data data[0];
 };
 
@@ -67,11 +69,11 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
 static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 	struct input_dev *input = dev->input;
 	int i;
 
-	for (i = 0; i < bdev->pdata->nbuttons; i++) {
+	for (i = 0; i < pdata->nbuttons; i++) {
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
 		if (bdata->count < bdata->threshold)
@@ -85,7 +87,7 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 static void gpio_keys_polled_open(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 
 	if (pdata->enable)
 		pdata->enable(bdev->dev);
@@ -94,23 +96,125 @@ static void gpio_keys_polled_open(struct input_polled_dev *dev)
 static void gpio_keys_polled_close(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 
 	if (pdata->disable)
 		pdata->disable(bdev->dev);
 }
+#ifdef CONFIG_OF
+static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *pdata)
+{
+	struct device_node *node, *pp;
+	int i;
+	struct gpio_keys_button *buttons;
+	u32 reg;
+
+	node = dev->of_node;
+	if (node == NULL)
+		return -ENODEV;
+
+	memset(pdata, 0, sizeof *pdata);
+
+	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
+
+	if (of_property_read_u32(node, "poll-interval", &reg) == 0)
+		pdata->poll_interval = reg;
+
+	/* First count the subnodes */
+	pdata->nbuttons = 0;
+	pp = NULL;
+	while ((pp = of_get_next_child(node, pp)))
+		pdata->nbuttons++;
+
+	if (pdata->nbuttons == 0)
+		return -ENODEV;
+
+	buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
+	if (!buttons)
+		return -ENOMEM;
+
+	pp = NULL;
+	i = 0;
+	while ((pp = of_get_next_child(node, pp))) {
+		enum of_gpio_flags flags;
+
+		if (!of_find_property(pp, "gpios", NULL)) {
+			pdata->nbuttons--;
+			dev_warn(dev, "Found button without gpios\n");
+			continue;
+		}
+		buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
+		buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+		if (of_property_read_u32(pp, "linux,code", &reg)) {
+			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
+			goto out_fail;
+		}
+		buttons[i].code = reg;
+
+		buttons[i].desc = of_get_property(pp, "label", NULL);
+
+		if (of_property_read_u32(pp, "linux,input-type", &reg) == 0)
+			buttons[i].type = reg;
+		else
+			buttons[i].type = EV_KEY;
+
+		buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
+
+		if (of_property_read_u32(pp, "debounce-interval", &reg) == 0)
+			buttons[i].debounce_interval = reg;
+		else
+			buttons[i].debounce_interval = 5;
+
+		i++;
+	}
+
+	pdata->buttons = buttons;
+
+	return 0;
+
+out_fail:
+	kfree(buttons);
+	return -ENODEV;
+}
+
+static struct of_device_id gpio_keys_polled_of_match[] = {
+	{ .compatible = "gpio-keys-polled", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+#else
+
+static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *altp)
+{
+	return -ENODEV;
+}
+
+#define gpio_keys_polled_of_match NULL
+
+#endif
 
 static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 {
 	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
+	struct gpio_keys_platform_data alt_pdata;
 	struct gpio_keys_polled_dev *bdev;
 	struct input_polled_dev *poll_dev;
 	struct input_dev *input;
 	int error;
 	int i;
 
-	if (!pdata || !pdata->poll_interval)
+	if (!pdata) {
+	    error = gpio_keys_polled_get_devtree_pdata(dev, &alt_pdata);
+		if (error)
+			return error;
+		pdata = &alt_pdata;
+	}
+
+	if (!pdata->poll_interval)
 		return -EINVAL;
 
 	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
@@ -184,7 +288,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 
 	bdev->poll_dev = poll_dev;
 	bdev->dev = dev;
-	bdev->pdata = pdata;
+	bdev->pdata = *pdata;
 	platform_set_drvdata(pdev, bdev);
 
 	error = input_register_polled_device(poll_dev);
@@ -217,7 +321,7 @@ err_free_bdev:
 static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
 {
 	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 	int i;
 
 	input_unregister_polled_device(bdev->poll_dev);
@@ -239,6 +343,7 @@ static struct platform_driver gpio_keys_polled_driver = {
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = gpio_keys_polled_of_match,
 	},
 };
 module_platform_driver(gpio_keys_polled_driver);
-- 
1.7.10


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

* [PATCH RESEND] input: gpio_keys_polled: convert to dt
@ 2012-06-20 11:34 ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-20 11:34 UTC (permalink / raw)
  Cc: Alexandre Pereira da Silva, Dmitry Torokhov, Grant Likely,
	Rob Herring, JJ Ding, Roland Stigge, linux-input, linux-kernel,
	devicetree-discuss

Add device tree support to gpio_keys_polled.c

Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
Tested-by: Roland Stigge <stigge@antcom.de>
---
 drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
 1 file changed, 113 insertions(+), 8 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 20c8ab1..a64b361 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -25,6 +25,8 @@
 #include <linux/platform_device.h>
 #include <linux/gpio.h>
 #include <linux/gpio_keys.h>
+#include <linux/of_platform.h>
+#include <linux/of_gpio.h>
 
 #define DRV_NAME	"gpio-keys-polled"
 
@@ -38,7 +40,7 @@ struct gpio_keys_button_data {
 struct gpio_keys_polled_dev {
 	struct input_polled_dev *poll_dev;
 	struct device *dev;
-	struct gpio_keys_platform_data *pdata;
+	struct gpio_keys_platform_data pdata;
 	struct gpio_keys_button_data data[0];
 };
 
@@ -67,11 +69,11 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
 static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 	struct input_dev *input = dev->input;
 	int i;
 
-	for (i = 0; i < bdev->pdata->nbuttons; i++) {
+	for (i = 0; i < pdata->nbuttons; i++) {
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 
 		if (bdata->count < bdata->threshold)
@@ -85,7 +87,7 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
 static void gpio_keys_polled_open(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 
 	if (pdata->enable)
 		pdata->enable(bdev->dev);
@@ -94,23 +96,125 @@ static void gpio_keys_polled_open(struct input_polled_dev *dev)
 static void gpio_keys_polled_close(struct input_polled_dev *dev)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 
 	if (pdata->disable)
 		pdata->disable(bdev->dev);
 }
+#ifdef CONFIG_OF
+static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *pdata)
+{
+	struct device_node *node, *pp;
+	int i;
+	struct gpio_keys_button *buttons;
+	u32 reg;
+
+	node = dev->of_node;
+	if (node == NULL)
+		return -ENODEV;
+
+	memset(pdata, 0, sizeof *pdata);
+
+	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
+
+	if (of_property_read_u32(node, "poll-interval", &reg) == 0)
+		pdata->poll_interval = reg;
+
+	/* First count the subnodes */
+	pdata->nbuttons = 0;
+	pp = NULL;
+	while ((pp = of_get_next_child(node, pp)))
+		pdata->nbuttons++;
+
+	if (pdata->nbuttons == 0)
+		return -ENODEV;
+
+	buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
+	if (!buttons)
+		return -ENOMEM;
+
+	pp = NULL;
+	i = 0;
+	while ((pp = of_get_next_child(node, pp))) {
+		enum of_gpio_flags flags;
+
+		if (!of_find_property(pp, "gpios", NULL)) {
+			pdata->nbuttons--;
+			dev_warn(dev, "Found button without gpios\n");
+			continue;
+		}
+		buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
+		buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
+
+		if (of_property_read_u32(pp, "linux,code", &reg)) {
+			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
+			goto out_fail;
+		}
+		buttons[i].code = reg;
+
+		buttons[i].desc = of_get_property(pp, "label", NULL);
+
+		if (of_property_read_u32(pp, "linux,input-type", &reg) == 0)
+			buttons[i].type = reg;
+		else
+			buttons[i].type = EV_KEY;
+
+		buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
+
+		if (of_property_read_u32(pp, "debounce-interval", &reg) == 0)
+			buttons[i].debounce_interval = reg;
+		else
+			buttons[i].debounce_interval = 5;
+
+		i++;
+	}
+
+	pdata->buttons = buttons;
+
+	return 0;
+
+out_fail:
+	kfree(buttons);
+	return -ENODEV;
+}
+
+static struct of_device_id gpio_keys_polled_of_match[] = {
+	{ .compatible = "gpio-keys-polled", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+#else
+
+static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
+			    struct gpio_keys_platform_data *altp)
+{
+	return -ENODEV;
+}
+
+#define gpio_keys_polled_of_match NULL
+
+#endif
 
 static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 {
 	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
 	struct device *dev = &pdev->dev;
+	struct gpio_keys_platform_data alt_pdata;
 	struct gpio_keys_polled_dev *bdev;
 	struct input_polled_dev *poll_dev;
 	struct input_dev *input;
 	int error;
 	int i;
 
-	if (!pdata || !pdata->poll_interval)
+	if (!pdata) {
+	    error = gpio_keys_polled_get_devtree_pdata(dev, &alt_pdata);
+		if (error)
+			return error;
+		pdata = &alt_pdata;
+	}
+
+	if (!pdata->poll_interval)
 		return -EINVAL;
 
 	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
@@ -184,7 +288,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
 
 	bdev->poll_dev = poll_dev;
 	bdev->dev = dev;
-	bdev->pdata = pdata;
+	bdev->pdata = *pdata;
 	platform_set_drvdata(pdev, bdev);
 
 	error = input_register_polled_device(poll_dev);
@@ -217,7 +321,7 @@ err_free_bdev:
 static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
 {
 	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
-	struct gpio_keys_platform_data *pdata = bdev->pdata;
+	struct gpio_keys_platform_data *pdata = &bdev->pdata;
 	int i;
 
 	input_unregister_polled_device(bdev->poll_dev);
@@ -239,6 +343,7 @@ static struct platform_driver gpio_keys_polled_driver = {
 	.driver	= {
 		.name	= DRV_NAME,
 		.owner	= THIS_MODULE,
+		.of_match_table = gpio_keys_polled_of_match,
 	},
 };
 module_platform_driver(gpio_keys_polled_driver);
-- 
1.7.10


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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
  2012-06-20 11:34 ` Alexandre Pereira da Silva
  (?)
@ 2012-06-21  9:18 ` Dmitry Torokhov
  2012-06-21 11:37     ` Alexandre Pereira da Silva
  -1 siblings, 1 reply; 10+ messages in thread
From: Dmitry Torokhov @ 2012-06-21  9:18 UTC (permalink / raw)
  To: Alexandre Pereira da Silva
  Cc: Grant Likely, Rob Herring, JJ Ding, Roland Stigge, linux-input,
	linux-kernel, devicetree-discuss

Hi Alexandre,

On Wed, Jun 20, 2012 at 08:34:21AM -0300, Alexandre Pereira da Silva wrote:
> Add device tree support to gpio_keys_polled.c
> 
> Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
> Tested-by: Roland Stigge <stigge@antcom.de>
> ---
>  drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index 20c8ab1..a64b361 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -25,6 +25,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/gpio.h>
>  #include <linux/gpio_keys.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
>  
>  #define DRV_NAME	"gpio-keys-polled"
>  
> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>  struct gpio_keys_polled_dev {
>  	struct input_polled_dev *poll_dev;
>  	struct device *dev;
> -	struct gpio_keys_platform_data *pdata;
> +	struct gpio_keys_platform_data pdata;

I am not sure why this change is needed. Also it seems that the driver
leaks DT-created pdata on unload.

> +
> +#define gpio_keys_polled_of_match NULL

Please use of_match_ptr() instead.

Thanks.

-- 
Dmitry

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
  2012-06-21  9:18 ` Dmitry Torokhov
@ 2012-06-21 11:37     ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-21 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, JJ Ding, Roland Stigge, linux-input,
	linux-kernel, devicetree-discuss

On Thu, Jun 21, 2012 at 6:18 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>>  struct gpio_keys_polled_dev {
>>       struct input_polled_dev *poll_dev;
>>       struct device *dev;
>> -     struct gpio_keys_platform_data *pdata;
>> +     struct gpio_keys_platform_data pdata;
>
> I am not sure why this change is needed. Also it seems that the driver
> leaks DT-created pdata on unload.

Pdata is now a member of gpio_keys_polled_dev now and that one is
kzalloc'ed at probe and kfree'd at the removal and errors. I see no
leaks there.

I could kzalloc and kfree gpio_keys_polled_dev independently instead,
to avoid all those pointer changes if you think it's better.

This is mostly a clone from the dt handling in gpio_keys, and
comparing both now,
I see that it will leak on pdata->buttons, so I will fix that.

>> +
>> +#define gpio_keys_polled_of_match NULL
>
> Please use of_match_ptr() instead.

Thanks for the suggestion, I will implement that.

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
@ 2012-06-21 11:37     ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-21 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, JJ Ding, Roland Stigge, linux-input,
	linux-kernel, devicetree-discuss

On Thu, Jun 21, 2012 at 6:18 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>>  struct gpio_keys_polled_dev {
>>       struct input_polled_dev *poll_dev;
>>       struct device *dev;
>> -     struct gpio_keys_platform_data *pdata;
>> +     struct gpio_keys_platform_data pdata;
>
> I am not sure why this change is needed. Also it seems that the driver
> leaks DT-created pdata on unload.

Pdata is now a member of gpio_keys_polled_dev now and that one is
kzalloc'ed at probe and kfree'd at the removal and errors. I see no
leaks there.

I could kzalloc and kfree gpio_keys_polled_dev independently instead,
to avoid all those pointer changes if you think it's better.

This is mostly a clone from the dt handling in gpio_keys, and
comparing both now,
I see that it will leak on pdata->buttons, so I will fix that.

>> +
>> +#define gpio_keys_polled_of_match NULL
>
> Please use of_match_ptr() instead.

Thanks for the suggestion, I will implement that.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
  2012-06-21 11:37     ` Alexandre Pereira da Silva
@ 2012-06-25  8:03       ` Dmitry Torokhov
  -1 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2012-06-25  8:03 UTC (permalink / raw)
  To: Alexandre Pereira da Silva
  Cc: Grant Likely, Rob Herring, JJ Ding, Roland Stigge, linux-input,
	linux-kernel, devicetree-discuss

On Thu, Jun 21, 2012 at 08:37:18AM -0300, Alexandre Pereira da Silva wrote:
> On Thu, Jun 21, 2012 at 6:18 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
> >>  struct gpio_keys_polled_dev {
> >>       struct input_polled_dev *poll_dev;
> >>       struct device *dev;
> >> -     struct gpio_keys_platform_data *pdata;
> >> +     struct gpio_keys_platform_data pdata;
> >
> > I am not sure why this change is needed. Also it seems that the driver
> > leaks DT-created pdata on unload.
> 
> Pdata is now a member of gpio_keys_polled_dev now and that one is
> kzalloc'ed at probe and kfree'd at the removal and errors. I see no
> leaks there.

Ah, I missed that you used on-stack instance to hold parsed DT data...

> 
> I could kzalloc and kfree gpio_keys_polled_dev independently instead,
> to avoid all those pointer changes if you think it's better.
> 
> This is mostly a clone from the dt handling in gpio_keys, and
> comparing both now,
> I see that it will leak on pdata->buttons, so I will fix that.
> 
> >> +
> >> +#define gpio_keys_polled_of_match NULL
> >
> > Please use of_match_ptr() instead.
> 
> Thanks for the suggestion, I will implement that.

-- 
Dmitry

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
@ 2012-06-25  8:03       ` Dmitry Torokhov
  0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Torokhov @ 2012-06-25  8:03 UTC (permalink / raw)
  To: Alexandre Pereira da Silva
  Cc: Grant Likely, Rob Herring, JJ Ding, Roland Stigge, linux-input,
	linux-kernel, devicetree-discuss

On Thu, Jun 21, 2012 at 08:37:18AM -0300, Alexandre Pereira da Silva wrote:
> On Thu, Jun 21, 2012 at 6:18 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
> >>  struct gpio_keys_polled_dev {
> >>       struct input_polled_dev *poll_dev;
> >>       struct device *dev;
> >> -     struct gpio_keys_platform_data *pdata;
> >> +     struct gpio_keys_platform_data pdata;
> >
> > I am not sure why this change is needed. Also it seems that the driver
> > leaks DT-created pdata on unload.
> 
> Pdata is now a member of gpio_keys_polled_dev now and that one is
> kzalloc'ed at probe and kfree'd at the removal and errors. I see no
> leaks there.

Ah, I missed that you used on-stack instance to hold parsed DT data...

> 
> I could kzalloc and kfree gpio_keys_polled_dev independently instead,
> to avoid all those pointer changes if you think it's better.
> 
> This is mostly a clone from the dt handling in gpio_keys, and
> comparing both now,
> I see that it will leak on pdata->buttons, so I will fix that.
> 
> >> +
> >> +#define gpio_keys_polled_of_match NULL
> >
> > Please use of_match_ptr() instead.
> 
> Thanks for the suggestion, I will implement that.

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

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
  2012-06-20 11:34 ` Alexandre Pereira da Silva
  (?)
  (?)
@ 2012-06-25 12:43 ` Rob Herring
  2012-06-25 15:04     ` Alexandre Pereira da Silva
  -1 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2012-06-25 12:43 UTC (permalink / raw)
  To: Alexandre Pereira da Silva
  Cc: Dmitry Torokhov, Grant Likely, JJ Ding, Roland Stigge,
	linux-input, linux-kernel, devicetree-discuss

On 06/20/2012 06:34 AM, Alexandre Pereira da Silva wrote:
> Add device tree support to gpio_keys_polled.c
> 
> Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
> Tested-by: Roland Stigge <stigge@antcom.de>
> ---
>  drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
>  1 file changed, 113 insertions(+), 8 deletions(-)

Needs binding documentation.

> 
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index 20c8ab1..a64b361 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -25,6 +25,8 @@
>  #include <linux/platform_device.h>
>  #include <linux/gpio.h>
>  #include <linux/gpio_keys.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_gpio.h>
>  
>  #define DRV_NAME	"gpio-keys-polled"
>  
> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>  struct gpio_keys_polled_dev {
>  	struct input_polled_dev *poll_dev;
>  	struct device *dev;
> -	struct gpio_keys_platform_data *pdata;
> +	struct gpio_keys_platform_data pdata;
>  	struct gpio_keys_button_data data[0];
>  };
>  
> @@ -67,11 +69,11 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
>  static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>  {
>  	struct gpio_keys_polled_dev *bdev = dev->private;
> -	struct gpio_keys_platform_data *pdata = bdev->pdata;
> +	struct gpio_keys_platform_data *pdata = &bdev->pdata;
>  	struct input_dev *input = dev->input;
>  	int i;
>  
> -	for (i = 0; i < bdev->pdata->nbuttons; i++) {
> +	for (i = 0; i < pdata->nbuttons; i++) {
>  		struct gpio_keys_button_data *bdata = &bdev->data[i];
>  
>  		if (bdata->count < bdata->threshold)
> @@ -85,7 +87,7 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>  static void gpio_keys_polled_open(struct input_polled_dev *dev)
>  {
>  	struct gpio_keys_polled_dev *bdev = dev->private;
> -	struct gpio_keys_platform_data *pdata = bdev->pdata;
> +	struct gpio_keys_platform_data *pdata = &bdev->pdata;
>  
>  	if (pdata->enable)
>  		pdata->enable(bdev->dev);
> @@ -94,23 +96,125 @@ static void gpio_keys_polled_open(struct input_polled_dev *dev)
>  static void gpio_keys_polled_close(struct input_polled_dev *dev)
>  {
>  	struct gpio_keys_polled_dev *bdev = dev->private;
> -	struct gpio_keys_platform_data *pdata = bdev->pdata;
> +	struct gpio_keys_platform_data *pdata = &bdev->pdata;
>  
>  	if (pdata->disable)
>  		pdata->disable(bdev->dev);
>  }
> +#ifdef CONFIG_OF
> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
> +			    struct gpio_keys_platform_data *pdata)
> +{
> +	struct device_node *node, *pp;
> +	int i;
> +	struct gpio_keys_button *buttons;
> +	u32 reg;
> +
> +	node = dev->of_node;
> +	if (node == NULL)
> +		return -ENODEV;
> +
> +	memset(pdata, 0, sizeof *pdata);

parentheses around *pdata.

> +
> +	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
> +
> +	if (of_property_read_u32(node, "poll-interval", &reg) == 0)
> +		pdata->poll_interval = reg;

You can just do this instead of the if:

of_property_read_u32(node, "poll-interval", &pdata->poll_interval);


> +
> +	/* First count the subnodes */
> +	pdata->nbuttons = 0;

You already to this to 0.

> +	pp = NULL;
> +	while ((pp = of_get_next_child(node, pp)))
> +		pdata->nbuttons++;

of_get_child_count()

> +
> +	if (pdata->nbuttons == 0)
> +		return -ENODEV;
> +
> +	buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
> +	if (!buttons)
> +		return -ENOMEM;
> +
> +	pp = NULL;
> +	i = 0;
> +	while ((pp = of_get_next_child(node, pp))) {

for_each_child_of_node

> +		enum of_gpio_flags flags;
> +
> +		if (!of_find_property(pp, "gpios", NULL)) {
> +			pdata->nbuttons--;
> +			dev_warn(dev, "Found button without gpios\n");
> +			continue;
> +		}
> +		buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
> +		buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
> +
> +		if (of_property_read_u32(pp, "linux,code", &reg)) {
> +			dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
> +			goto out_fail;
> +		}
> +		buttons[i].code = reg;
> +
> +		buttons[i].desc = of_get_property(pp, "label", NULL);
> +
> +		if (of_property_read_u32(pp, "linux,input-type", &reg) == 0)
> +			buttons[i].type = reg;
> +		else
> +			buttons[i].type = EV_KEY;

if (of_property_read_u32(pp, "linux,input-type", &buttons[i].type))
	buttons[i].type = EV_KEY;

> +
> +		buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
> +
> +		if (of_property_read_u32(pp, "debounce-interval", &reg) == 0)
> +			buttons[i].debounce_interval = reg;
> +		else
> +			buttons[i].debounce_interval = 5;

ditto

> +
> +		i++;
> +	}
> +
> +	pdata->buttons = buttons;
> +
> +	return 0;
> +
> +out_fail:
> +	kfree(buttons);
> +	return -ENODEV;
> +}
> +
> +static struct of_device_id gpio_keys_polled_of_match[] = {
> +	{ .compatible = "gpio-keys-polled", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
> +#else
> +
> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
> +			    struct gpio_keys_platform_data *altp)
> +{
> +	return -ENODEV;
> +}
> +
> +#define gpio_keys_polled_of_match NULL
> +
> +#endif
>  
>  static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>  {
>  	struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>  	struct device *dev = &pdev->dev;
> +	struct gpio_keys_platform_data alt_pdata;
>  	struct gpio_keys_polled_dev *bdev;
>  	struct input_polled_dev *poll_dev;
>  	struct input_dev *input;
>  	int error;
>  	int i;
>  
> -	if (!pdata || !pdata->poll_interval)
> +	if (!pdata) {
> +	    error = gpio_keys_polled_get_devtree_pdata(dev, &alt_pdata);
> +		if (error)
> +			return error;
> +		pdata = &alt_pdata;
> +	}
> +
> +	if (!pdata->poll_interval)
>  		return -EINVAL;
>  
>  	bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
> @@ -184,7 +288,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>  
>  	bdev->poll_dev = poll_dev;
>  	bdev->dev = dev;
> -	bdev->pdata = pdata;
> +	bdev->pdata = *pdata;
>  	platform_set_drvdata(pdev, bdev);
>  
>  	error = input_register_polled_device(poll_dev);
> @@ -217,7 +321,7 @@ err_free_bdev:
>  static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
>  {
>  	struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
> -	struct gpio_keys_platform_data *pdata = bdev->pdata;
> +	struct gpio_keys_platform_data *pdata = &bdev->pdata;
>  	int i;
>  
>  	input_unregister_polled_device(bdev->poll_dev);
> @@ -239,6 +343,7 @@ static struct platform_driver gpio_keys_polled_driver = {
>  	.driver	= {
>  		.name	= DRV_NAME,
>  		.owner	= THIS_MODULE,
> +		.of_match_table = gpio_keys_polled_of_match,
>  	},
>  };
>  module_platform_driver(gpio_keys_polled_driver);

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
  2012-06-25 12:43 ` Rob Herring
@ 2012-06-25 15:04     ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-25 15:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: Dmitry Torokhov, Grant Likely, JJ Ding, Roland Stigge,
	linux-input, linux-kernel, devicetree-discuss

On Mon, Jun 25, 2012 at 9:43 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 06/20/2012 06:34 AM, Alexandre Pereira da Silva wrote:
>> Add device tree support to gpio_keys_polled.c
>>
>> Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
>> Tested-by: Roland Stigge <stigge@antcom.de>
>> ---
>>  drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
>>  1 file changed, 113 insertions(+), 8 deletions(-)
>
> Needs binding documentation.
>
>>
>> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
>> index 20c8ab1..a64b361 100644
>> --- a/drivers/input/keyboard/gpio_keys_polled.c
>> +++ b/drivers/input/keyboard/gpio_keys_polled.c
>> @@ -25,6 +25,8 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/gpio.h>
>>  #include <linux/gpio_keys.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_gpio.h>
>>
>>  #define DRV_NAME     "gpio-keys-polled"
>>
>> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>>  struct gpio_keys_polled_dev {
>>       struct input_polled_dev *poll_dev;
>>       struct device *dev;
>> -     struct gpio_keys_platform_data *pdata;
>> +     struct gpio_keys_platform_data pdata;
>>       struct gpio_keys_button_data data[0];
>>  };
>>
>> @@ -67,11 +69,11 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
>>  static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>       struct input_dev *input = dev->input;
>>       int i;
>>
>> -     for (i = 0; i < bdev->pdata->nbuttons; i++) {
>> +     for (i = 0; i < pdata->nbuttons; i++) {
>>               struct gpio_keys_button_data *bdata = &bdev->data[i];
>>
>>               if (bdata->count < bdata->threshold)
>> @@ -85,7 +87,7 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>>  static void gpio_keys_polled_open(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>
>>       if (pdata->enable)
>>               pdata->enable(bdev->dev);
>> @@ -94,23 +96,125 @@ static void gpio_keys_polled_open(struct input_polled_dev *dev)
>>  static void gpio_keys_polled_close(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>
>>       if (pdata->disable)
>>               pdata->disable(bdev->dev);
>>  }
>> +#ifdef CONFIG_OF
>> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
>> +                         struct gpio_keys_platform_data *pdata)
>> +{
>> +     struct device_node *node, *pp;
>> +     int i;
>> +     struct gpio_keys_button *buttons;
>> +     u32 reg;
>> +
>> +     node = dev->of_node;
>> +     if (node == NULL)
>> +             return -ENODEV;
>> +
>> +     memset(pdata, 0, sizeof *pdata);
>
> parentheses around *pdata.
>
>> +
>> +     pdata->rep = !!of_get_property(node, "autorepeat", NULL);
>> +
>> +     if (of_property_read_u32(node, "poll-interval", &reg) == 0)
>> +             pdata->poll_interval = reg;
>
> You can just do this instead of the if:
>
> of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
>
>
>> +
>> +     /* First count the subnodes */
>> +     pdata->nbuttons = 0;
>
> You already to this to 0.
>
>> +     pp = NULL;
>> +     while ((pp = of_get_next_child(node, pp)))
>> +             pdata->nbuttons++;
>
> of_get_child_count()
>
>> +
>> +     if (pdata->nbuttons == 0)
>> +             return -ENODEV;
>> +
>> +     buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
>> +     if (!buttons)
>> +             return -ENOMEM;
>> +
>> +     pp = NULL;
>> +     i = 0;
>> +     while ((pp = of_get_next_child(node, pp))) {
>
> for_each_child_of_node
>
>> +             enum of_gpio_flags flags;
>> +
>> +             if (!of_find_property(pp, "gpios", NULL)) {
>> +                     pdata->nbuttons--;
>> +                     dev_warn(dev, "Found button without gpios\n");
>> +                     continue;
>> +             }
>> +             buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
>> +             buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
>> +
>> +             if (of_property_read_u32(pp, "linux,code", &reg)) {
>> +                     dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
>> +                     goto out_fail;
>> +             }
>> +             buttons[i].code = reg;
>> +
>> +             buttons[i].desc = of_get_property(pp, "label", NULL);
>> +
>> +             if (of_property_read_u32(pp, "linux,input-type", &reg) == 0)
>> +                     buttons[i].type = reg;
>> +             else
>> +                     buttons[i].type = EV_KEY;
>
> if (of_property_read_u32(pp, "linux,input-type", &buttons[i].type))
>        buttons[i].type = EV_KEY;
>
>> +
>> +             buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
>> +
>> +             if (of_property_read_u32(pp, "debounce-interval", &reg) == 0)
>> +                     buttons[i].debounce_interval = reg;
>> +             else
>> +                     buttons[i].debounce_interval = 5;
>
> ditto
>
>> +
>> +             i++;
>> +     }
>> +
>> +     pdata->buttons = buttons;
>> +
>> +     return 0;
>> +
>> +out_fail:
>> +     kfree(buttons);
>> +     return -ENODEV;
>> +}
>> +
>> +static struct of_device_id gpio_keys_polled_of_match[] = {
>> +     { .compatible = "gpio-keys-polled", },
>> +     { },
>> +};
>> +MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
>> +#else
>> +
>> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
>> +                         struct gpio_keys_platform_data *altp)
>> +{
>> +     return -ENODEV;
>> +}
>> +
>> +#define gpio_keys_polled_of_match NULL
>> +
>> +#endif
>>
>>  static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>>  {
>>       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>>       struct device *dev = &pdev->dev;
>> +     struct gpio_keys_platform_data alt_pdata;
>>       struct gpio_keys_polled_dev *bdev;
>>       struct input_polled_dev *poll_dev;
>>       struct input_dev *input;
>>       int error;
>>       int i;
>>
>> -     if (!pdata || !pdata->poll_interval)
>> +     if (!pdata) {
>> +         error = gpio_keys_polled_get_devtree_pdata(dev, &alt_pdata);
>> +             if (error)
>> +                     return error;
>> +             pdata = &alt_pdata;
>> +     }
>> +
>> +     if (!pdata->poll_interval)
>>               return -EINVAL;
>>
>>       bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
>> @@ -184,7 +288,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>>
>>       bdev->poll_dev = poll_dev;
>>       bdev->dev = dev;
>> -     bdev->pdata = pdata;
>> +     bdev->pdata = *pdata;
>>       platform_set_drvdata(pdev, bdev);
>>
>>       error = input_register_polled_device(poll_dev);
>> @@ -217,7 +321,7 @@ err_free_bdev:
>>  static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>       int i;
>>
>>       input_unregister_polled_device(bdev->poll_dev);
>> @@ -239,6 +343,7 @@ static struct platform_driver gpio_keys_polled_driver = {
>>       .driver = {
>>               .name   = DRV_NAME,
>>               .owner  = THIS_MODULE,
>> +             .of_match_table = gpio_keys_polled_of_match,
>>       },
>>  };
>>  module_platform_driver(gpio_keys_polled_driver);

Thanks for reviewing. I will submit patches for both gpio-keys and
gpio-keys-polled to address those issues.

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

* Re: [PATCH RESEND] input: gpio_keys_polled: convert to dt
@ 2012-06-25 15:04     ` Alexandre Pereira da Silva
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Pereira da Silva @ 2012-06-25 15:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: Dmitry Torokhov, Grant Likely, JJ Ding, Roland Stigge,
	linux-input, linux-kernel, devicetree-discuss

On Mon, Jun 25, 2012 at 9:43 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 06/20/2012 06:34 AM, Alexandre Pereira da Silva wrote:
>> Add device tree support to gpio_keys_polled.c
>>
>> Signed-off-by: Alexandre Pereira da Silva <aletes.xgr@gmail.com>
>> Tested-by: Roland Stigge <stigge@antcom.de>
>> ---
>>  drivers/input/keyboard/gpio_keys_polled.c |  121 +++++++++++++++++++++++++++--
>>  1 file changed, 113 insertions(+), 8 deletions(-)
>
> Needs binding documentation.
>
>>
>> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
>> index 20c8ab1..a64b361 100644
>> --- a/drivers/input/keyboard/gpio_keys_polled.c
>> +++ b/drivers/input/keyboard/gpio_keys_polled.c
>> @@ -25,6 +25,8 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/gpio.h>
>>  #include <linux/gpio_keys.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/of_gpio.h>
>>
>>  #define DRV_NAME     "gpio-keys-polled"
>>
>> @@ -38,7 +40,7 @@ struct gpio_keys_button_data {
>>  struct gpio_keys_polled_dev {
>>       struct input_polled_dev *poll_dev;
>>       struct device *dev;
>> -     struct gpio_keys_platform_data *pdata;
>> +     struct gpio_keys_platform_data pdata;
>>       struct gpio_keys_button_data data[0];
>>  };
>>
>> @@ -67,11 +69,11 @@ static void gpio_keys_polled_check_state(struct input_dev *input,
>>  static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>       struct input_dev *input = dev->input;
>>       int i;
>>
>> -     for (i = 0; i < bdev->pdata->nbuttons; i++) {
>> +     for (i = 0; i < pdata->nbuttons; i++) {
>>               struct gpio_keys_button_data *bdata = &bdev->data[i];
>>
>>               if (bdata->count < bdata->threshold)
>> @@ -85,7 +87,7 @@ static void gpio_keys_polled_poll(struct input_polled_dev *dev)
>>  static void gpio_keys_polled_open(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>
>>       if (pdata->enable)
>>               pdata->enable(bdev->dev);
>> @@ -94,23 +96,125 @@ static void gpio_keys_polled_open(struct input_polled_dev *dev)
>>  static void gpio_keys_polled_close(struct input_polled_dev *dev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = dev->private;
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>
>>       if (pdata->disable)
>>               pdata->disable(bdev->dev);
>>  }
>> +#ifdef CONFIG_OF
>> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
>> +                         struct gpio_keys_platform_data *pdata)
>> +{
>> +     struct device_node *node, *pp;
>> +     int i;
>> +     struct gpio_keys_button *buttons;
>> +     u32 reg;
>> +
>> +     node = dev->of_node;
>> +     if (node == NULL)
>> +             return -ENODEV;
>> +
>> +     memset(pdata, 0, sizeof *pdata);
>
> parentheses around *pdata.
>
>> +
>> +     pdata->rep = !!of_get_property(node, "autorepeat", NULL);
>> +
>> +     if (of_property_read_u32(node, "poll-interval", &reg) == 0)
>> +             pdata->poll_interval = reg;
>
> You can just do this instead of the if:
>
> of_property_read_u32(node, "poll-interval", &pdata->poll_interval);
>
>
>> +
>> +     /* First count the subnodes */
>> +     pdata->nbuttons = 0;
>
> You already to this to 0.
>
>> +     pp = NULL;
>> +     while ((pp = of_get_next_child(node, pp)))
>> +             pdata->nbuttons++;
>
> of_get_child_count()
>
>> +
>> +     if (pdata->nbuttons == 0)
>> +             return -ENODEV;
>> +
>> +     buttons = kzalloc(pdata->nbuttons * (sizeof *buttons), GFP_KERNEL);
>> +     if (!buttons)
>> +             return -ENOMEM;
>> +
>> +     pp = NULL;
>> +     i = 0;
>> +     while ((pp = of_get_next_child(node, pp))) {
>
> for_each_child_of_node
>
>> +             enum of_gpio_flags flags;
>> +
>> +             if (!of_find_property(pp, "gpios", NULL)) {
>> +                     pdata->nbuttons--;
>> +                     dev_warn(dev, "Found button without gpios\n");
>> +                     continue;
>> +             }
>> +             buttons[i].gpio = of_get_gpio_flags(pp, 0, &flags);
>> +             buttons[i].active_low = flags & OF_GPIO_ACTIVE_LOW;
>> +
>> +             if (of_property_read_u32(pp, "linux,code", &reg)) {
>> +                     dev_err(dev, "Button without keycode: 0x%x\n", buttons[i].gpio);
>> +                     goto out_fail;
>> +             }
>> +             buttons[i].code = reg;
>> +
>> +             buttons[i].desc = of_get_property(pp, "label", NULL);
>> +
>> +             if (of_property_read_u32(pp, "linux,input-type", &reg) == 0)
>> +                     buttons[i].type = reg;
>> +             else
>> +                     buttons[i].type = EV_KEY;
>
> if (of_property_read_u32(pp, "linux,input-type", &buttons[i].type))
>        buttons[i].type = EV_KEY;
>
>> +
>> +             buttons[i].wakeup = !!of_get_property(pp, "gpio-key,wakeup", NULL);
>> +
>> +             if (of_property_read_u32(pp, "debounce-interval", &reg) == 0)
>> +                     buttons[i].debounce_interval = reg;
>> +             else
>> +                     buttons[i].debounce_interval = 5;
>
> ditto
>
>> +
>> +             i++;
>> +     }
>> +
>> +     pdata->buttons = buttons;
>> +
>> +     return 0;
>> +
>> +out_fail:
>> +     kfree(buttons);
>> +     return -ENODEV;
>> +}
>> +
>> +static struct of_device_id gpio_keys_polled_of_match[] = {
>> +     { .compatible = "gpio-keys-polled", },
>> +     { },
>> +};
>> +MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
>> +#else
>> +
>> +static int gpio_keys_polled_get_devtree_pdata(struct device *dev,
>> +                         struct gpio_keys_platform_data *altp)
>> +{
>> +     return -ENODEV;
>> +}
>> +
>> +#define gpio_keys_polled_of_match NULL
>> +
>> +#endif
>>
>>  static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>>  {
>>       struct gpio_keys_platform_data *pdata = pdev->dev.platform_data;
>>       struct device *dev = &pdev->dev;
>> +     struct gpio_keys_platform_data alt_pdata;
>>       struct gpio_keys_polled_dev *bdev;
>>       struct input_polled_dev *poll_dev;
>>       struct input_dev *input;
>>       int error;
>>       int i;
>>
>> -     if (!pdata || !pdata->poll_interval)
>> +     if (!pdata) {
>> +         error = gpio_keys_polled_get_devtree_pdata(dev, &alt_pdata);
>> +             if (error)
>> +                     return error;
>> +             pdata = &alt_pdata;
>> +     }
>> +
>> +     if (!pdata->poll_interval)
>>               return -EINVAL;
>>
>>       bdev = kzalloc(sizeof(struct gpio_keys_polled_dev) +
>> @@ -184,7 +288,7 @@ static int __devinit gpio_keys_polled_probe(struct platform_device *pdev)
>>
>>       bdev->poll_dev = poll_dev;
>>       bdev->dev = dev;
>> -     bdev->pdata = pdata;
>> +     bdev->pdata = *pdata;
>>       platform_set_drvdata(pdev, bdev);
>>
>>       error = input_register_polled_device(poll_dev);
>> @@ -217,7 +321,7 @@ err_free_bdev:
>>  static int __devexit gpio_keys_polled_remove(struct platform_device *pdev)
>>  {
>>       struct gpio_keys_polled_dev *bdev = platform_get_drvdata(pdev);
>> -     struct gpio_keys_platform_data *pdata = bdev->pdata;
>> +     struct gpio_keys_platform_data *pdata = &bdev->pdata;
>>       int i;
>>
>>       input_unregister_polled_device(bdev->poll_dev);
>> @@ -239,6 +343,7 @@ static struct platform_driver gpio_keys_polled_driver = {
>>       .driver = {
>>               .name   = DRV_NAME,
>>               .owner  = THIS_MODULE,
>> +             .of_match_table = gpio_keys_polled_of_match,
>>       },
>>  };
>>  module_platform_driver(gpio_keys_polled_driver);

Thanks for reviewing. I will submit patches for both gpio-keys and
gpio-keys-polled to address those issues.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2012-06-25 15:04 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-20 11:34 [PATCH RESEND] input: gpio_keys_polled: convert to dt Alexandre Pereira da Silva
2012-06-20 11:34 ` Alexandre Pereira da Silva
2012-06-21  9:18 ` Dmitry Torokhov
2012-06-21 11:37   ` Alexandre Pereira da Silva
2012-06-21 11:37     ` Alexandre Pereira da Silva
2012-06-25  8:03     ` Dmitry Torokhov
2012-06-25  8:03       ` Dmitry Torokhov
2012-06-25 12:43 ` Rob Herring
2012-06-25 15:04   ` Alexandre Pereira da Silva
2012-06-25 15:04     ` Alexandre Pereira da Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.