All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] Input: gpio_keys_polled - keep button data constant
@ 2016-10-28 23:14 Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 2/6] Input: gpio_keys_polled - always use gpiod_get_value_cansleep Dmitry Torokhov
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Linus Walleij, Hans de Goede, Mika Westerberg,
	Geert Uytterhoeven, linux-kernel

Commit 633a21d80b4a ("input: gpio_keys_polled: Add support for GPIO
descriptors") placed gpio descriptor into gpio_keys_button structure, which
is supposed to be part of platform data and not modifiable by the driver.
To keep the data constant, let's move the descriptor to
gpio_keys_button_data structure instead.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c        |  10 +--
 drivers/input/keyboard/gpio_keys_polled.c | 105 +++++++++++++++++-------------
 include/linux/gpio_keys.h                 |   4 +-
 3 files changed, 64 insertions(+), 55 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 2909365..890eb39 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -624,7 +624,6 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 	struct gpio_keys_button *button;
 	int error;
 	int nbuttons;
-	int i;
 
 	node = dev->of_node;
 	if (!node)
@@ -640,19 +639,18 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
-	pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
+	button = (struct gpio_keys_button *)(pdata + 1);
+
+	pdata->buttons = button;
 	pdata->nbuttons = nbuttons;
 
 	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
 
 	of_property_read_string(node, "label", &pdata->name);
 
-	i = 0;
 	for_each_available_child_of_node(node, pp) {
 		enum of_gpio_flags flags;
 
-		button = &pdata->buttons[i++];
-
 		button->gpio = of_get_gpio_flags(pp, 0, &flags);
 		if (button->gpio < 0) {
 			error = button->gpio;
@@ -694,6 +692,8 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 		if (of_property_read_u32(pp, "debounce-interval",
 					 &button->debounce_interval))
 			button->debounce_interval = 5;
+
+		button++;
 	}
 
 	if (pdata->nbuttons == 0)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 62bdb1d..2cf4078 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -30,6 +30,7 @@
 #define DRV_NAME	"gpio-keys-polled"
 
 struct gpio_keys_button_data {
+	struct gpio_desc *gpiod;
 	int last_state;
 	int count;
 	int threshold;
@@ -46,7 +47,7 @@ struct gpio_keys_polled_dev {
 };
 
 static void gpio_keys_button_event(struct input_polled_dev *dev,
-				   struct gpio_keys_button *button,
+				   const struct gpio_keys_button *button,
 				   int state)
 {
 	struct gpio_keys_polled_dev *bdev = dev->private;
@@ -70,15 +71,15 @@ static void gpio_keys_button_event(struct input_polled_dev *dev,
 }
 
 static void gpio_keys_polled_check_state(struct input_polled_dev *dev,
-					 struct gpio_keys_button *button,
+					 const struct gpio_keys_button *button,
 					 struct gpio_keys_button_data *bdata)
 {
 	int state;
 
 	if (bdata->can_sleep)
-		state = !!gpiod_get_value_cansleep(button->gpiod);
+		state = !!gpiod_get_value_cansleep(bdata->gpiod);
 	else
-		state = !!gpiod_get_value(button->gpiod);
+		state = !!gpiod_get_value(bdata->gpiod);
 
 	gpio_keys_button_event(dev, button, state);
 
@@ -142,48 +143,35 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
 		pdata->disable(bdev->dev);
 }
 
-static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct device *dev)
+static struct gpio_keys_platform_data *
+gpio_keys_polled_get_devtree_pdata(struct device *dev)
 {
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
 	struct fwnode_handle *child;
-	int error;
 	int nbuttons;
 
 	nbuttons = device_get_child_node_count(dev);
 	if (nbuttons == 0)
-		return NULL;
+		return ERR_PTR(-EINVAL);
 
 	pdata = devm_kzalloc(dev, sizeof(*pdata) + nbuttons * sizeof(*button),
 			     GFP_KERNEL);
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
-	pdata->buttons = (struct gpio_keys_button *)(pdata + 1);
+	button = (struct gpio_keys_button *)(pdata + 1);
+
+	pdata->buttons = button;
+	pdata->nbuttons = nbuttons;
 
 	pdata->rep = device_property_present(dev, "autorepeat");
 	device_property_read_u32(dev, "poll-interval", &pdata->poll_interval);
 
 	device_for_each_child_node(dev, child) {
-		struct gpio_desc *desc;
-
-		desc = devm_get_gpiod_from_child(dev, NULL, child);
-		if (IS_ERR(desc)) {
-			error = PTR_ERR(desc);
-			if (error != -EPROBE_DEFER)
-				dev_err(dev,
-					"Failed to get gpio flags, error: %d\n",
-					error);
-			fwnode_handle_put(child);
-			return ERR_PTR(error);
-		}
-
-		button = &pdata->buttons[pdata->nbuttons++];
-		button->gpiod = desc;
-
-		if (fwnode_property_read_u32(child, "linux,code", &button->code)) {
-			dev_err(dev, "Button without keycode: %d\n",
-				pdata->nbuttons - 1);
+		if (fwnode_property_read_u32(child, "linux,code",
+					     &button->code)) {
+			dev_err(dev, "button without keycode\n");
 			fwnode_handle_put(child);
 			return ERR_PTR(-EINVAL);
 		}
@@ -206,10 +194,9 @@ static struct gpio_keys_platform_data *gpio_keys_polled_get_devtree_pdata(struct
 		if (fwnode_property_read_u32(child, "debounce-interval",
 					     &button->debounce_interval))
 			button->debounce_interval = 5;
-	}
 
-	if (pdata->nbuttons == 0)
-		return ERR_PTR(-EINVAL);
+		button++;
+	}
 
 	return pdata;
 }
@@ -220,7 +207,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
 	int i, min = 0, max = 0;
 
 	for (i = 0; i < pdata->nbuttons; i++) {
-		struct gpio_keys_button *button = &pdata->buttons[i];
+		const struct gpio_keys_button *button = &pdata->buttons[i];
 
 		if (button->type != EV_ABS || button->code != code)
 			continue;
@@ -230,6 +217,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
 		if (button->value > max)
 			max = button->value;
 	}
+
 	input_set_abs_params(input, code, min, max, 0, 0);
 }
 
@@ -242,6 +230,7 @@ MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
 static int gpio_keys_polled_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct fwnode_handle *child = NULL;
 	const struct gpio_keys_platform_data *pdata = dev_get_platdata(dev);
 	struct gpio_keys_polled_dev *bdev;
 	struct input_polled_dev *poll_dev;
@@ -254,10 +243,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 		pdata = gpio_keys_polled_get_devtree_pdata(dev);
 		if (IS_ERR(pdata))
 			return PTR_ERR(pdata);
-		if (!pdata) {
-			dev_err(dev, "missing platform data\n");
-			return -EINVAL;
-		}
 	}
 
 	if (!pdata->poll_interval) {
@@ -300,20 +285,40 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 		__set_bit(EV_REP, input->evbit);
 
 	for (i = 0; i < pdata->nbuttons; i++) {
-		struct gpio_keys_button *button = &pdata->buttons[i];
+		const struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_keys_button_data *bdata = &bdev->data[i];
 		unsigned int type = button->type ?: EV_KEY;
 
 		if (button->wakeup) {
 			dev_err(dev, DRV_NAME " does not support wakeup\n");
+			fwnode_handle_put(child);
 			return -EINVAL;
 		}
 
-		/*
-		 * Legacy GPIO number so request the GPIO here and
-		 * convert it to descriptor.
-		 */
-		if (!button->gpiod && gpio_is_valid(button->gpio)) {
+		if (!dev_get_platdata(dev)) {
+			/* No legacy static platform data */
+			child = device_get_next_child_node(dev, child);
+			if (!child) {
+				dev_err(dev, "missing child device node\n");
+				return -EINVAL;
+			}
+
+			bdata->gpiod = devm_get_gpiod_from_child(dev, NULL,
+								 child);
+			if (IS_ERR(bdata->gpiod)) {
+				error = PTR_ERR(bdata->gpiod);
+				if (error != -EPROBE_DEFER)
+					dev_err(dev,
+						"failed to get gpio: %d\n",
+						error);
+				fwnode_handle_put(child);
+				return error;
+			}
+		} else if (gpio_is_valid(button->gpio)) {
+			/*
+			 * Legacy GPIO number so request the GPIO here and
+			 * convert it to descriptor.
+			 */
 			unsigned flags = GPIOF_IN;
 
 			if (button->active_low)
@@ -322,18 +327,22 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 			error = devm_gpio_request_one(&pdev->dev, button->gpio,
 					flags, button->desc ? : DRV_NAME);
 			if (error) {
-				dev_err(dev, "unable to claim gpio %u, err=%d\n",
+				dev_err(dev,
+					"unable to claim gpio %u, err=%d\n",
 					button->gpio, error);
 				return error;
 			}
 
-			button->gpiod = gpio_to_desc(button->gpio);
+			bdata->gpiod = gpio_to_desc(button->gpio);
+			if (!bdata->gpiod) {
+				dev_err(dev,
+					"unable to convert gpio %u to descriptor\n",
+					button->gpio);
+				return -EINVAL;
+			}
 		}
 
-		if (IS_ERR(button->gpiod))
-			return PTR_ERR(button->gpiod);
-
-		bdata->can_sleep = gpiod_cansleep(button->gpiod);
+		bdata->can_sleep = gpiod_cansleep(bdata->gpiod);
 		bdata->last_state = -1;
 		bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
 						pdata->poll_interval);
@@ -344,6 +353,8 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 							button->code);
 	}
 
+	fwnode_handle_put(child);
+
 	bdev->poll_dev = poll_dev;
 	bdev->dev = dev;
 	bdev->pdata = pdata;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index ee2d8c6..d1250ad 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -2,7 +2,6 @@
 #define _GPIO_KEYS_H
 
 struct device;
-struct gpio_desc;
 
 /**
  * struct gpio_keys_button - configuration parameters
@@ -31,7 +30,6 @@ struct gpio_keys_button {
 	bool can_disable;
 	int value;
 	unsigned int irq;
-	struct gpio_desc *gpiod;
 };
 
 /**
@@ -46,7 +44,7 @@ struct gpio_keys_button {
  * @name:		input device name
  */
 struct gpio_keys_platform_data {
-	struct gpio_keys_button *buttons;
+	const struct gpio_keys_button *buttons;
 	int nbuttons;
 	unsigned int poll_interval;
 	unsigned int rep:1;
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 2/6] Input: gpio_keys_polled - always use gpiod_get_value_cansleep
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
@ 2016-10-28 23:14 ` Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 3/6] Input: gpio_keys - annotate PM methods as __maybe_unused Dmitry Torokhov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Linus Walleij, Hans de Goede, Mika Westerberg,
	Geert Uytterhoeven, linux-kernel

It does not matter if given GPIO may sleep or not when reading state,
polling is always done in a non-atomic context, so we should always
be able to simply use gpiod_get_value_cansleep().

Also let's note in the logs when we fail to read gpio state.

Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys_polled.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 2cf4078..72b3503 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -34,7 +34,6 @@ struct gpio_keys_button_data {
 	int last_state;
 	int count;
 	int threshold;
-	int can_sleep;
 };
 
 struct gpio_keys_polled_dev {
@@ -76,16 +75,17 @@ static void gpio_keys_polled_check_state(struct input_polled_dev *dev,
 {
 	int state;
 
-	if (bdata->can_sleep)
-		state = !!gpiod_get_value_cansleep(bdata->gpiod);
-	else
-		state = !!gpiod_get_value(bdata->gpiod);
-
-	gpio_keys_button_event(dev, button, state);
+	state = gpiod_get_value_cansleep(bdata->gpiod);
+	if (state < 0) {
+		dev_err(dev->input->dev.parent,
+			"failed to get gpio state: %d\n", state);
+	} else {
+		gpio_keys_button_event(dev, button, state);
 
-	if (state != bdata->last_state) {
-		bdata->count = 0;
-		bdata->last_state = state;
+		if (state != bdata->last_state) {
+			bdata->count = 0;
+			bdata->last_state = state;
+		}
 	}
 }
 
@@ -342,7 +342,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 			}
 		}
 
-		bdata->can_sleep = gpiod_cansleep(bdata->gpiod);
 		bdata->last_state = -1;
 		bdata->threshold = DIV_ROUND_UP(button->debounce_interval,
 						pdata->poll_interval);
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 3/6] Input: gpio_keys - annotate PM methods as __maybe_unused
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 2/6] Input: gpio_keys_polled - always use gpiod_get_value_cansleep Dmitry Torokhov
@ 2016-10-28 23:14 ` Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 4/6] Input: gpio_keys - fix leaking DT node references Dmitry Torokhov
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Linus Walleij, Hans de Goede, Mika Westerberg,
	Geert Uytterhoeven, linux-kernel

Instead of using #ifdef, let's mark suspend and resume methods as
__maybe_unused to provide better compile coverage.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 890eb39..8f7c20b 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -814,8 +814,7 @@ static int gpio_keys_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int gpio_keys_suspend(struct device *dev)
+static int __maybe_unused gpio_keys_suspend(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
 	struct input_dev *input = ddata->input;
@@ -837,7 +836,7 @@ static int gpio_keys_suspend(struct device *dev)
 	return 0;
 }
 
-static int gpio_keys_resume(struct device *dev)
+static int __maybe_unused gpio_keys_resume(struct device *dev)
 {
 	struct gpio_keys_drvdata *ddata = dev_get_drvdata(dev);
 	struct input_dev *input = ddata->input;
@@ -863,7 +862,6 @@ static int gpio_keys_resume(struct device *dev)
 	gpio_keys_report_state(ddata);
 	return 0;
 }
-#endif
 
 static SIMPLE_DEV_PM_OPS(gpio_keys_pm_ops, gpio_keys_suspend, gpio_keys_resume);
 
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 4/6] Input: gpio_keys - fix leaking DT node references
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 2/6] Input: gpio_keys_polled - always use gpiod_get_value_cansleep Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 3/6] Input: gpio_keys - annotate PM methods as __maybe_unused Dmitry Torokhov
@ 2016-10-28 23:14 ` Dmitry Torokhov
  2016-10-28 23:14 ` [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors Dmitry Torokhov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Linus Walleij, Hans de Goede, Mika Westerberg,
	Geert Uytterhoeven, linux-kernel

for_each_available_child_of_node(node, pp) takes reference to 'pp' and
drops it when attempting next iteration. However if we exit the loop early
we need to drop the reference ourselves.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 8f7c20b..d75a25c 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -659,6 +659,7 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 					dev_err(dev,
 						"Failed to get gpio flags, error: %d\n",
 						error);
+				of_node_put(pp);
 				return ERR_PTR(error);
 			}
 		} else {
@@ -669,12 +670,14 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 
 		if (!gpio_is_valid(button->gpio) && !button->irq) {
 			dev_err(dev, "Found button without gpios or irqs\n");
+			of_node_put(pp);
 			return ERR_PTR(-EINVAL);
 		}
 
 		if (of_property_read_u32(pp, "linux,code", &button->code)) {
 			dev_err(dev, "Button without keycode: 0x%x\n",
 				button->gpio);
+			of_node_put(pp);
 			return ERR_PTR(-EINVAL);
 		}
 
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
                   ` (2 preceding siblings ...)
  2016-10-28 23:14 ` [PATCH 4/6] Input: gpio_keys - fix leaking DT node references Dmitry Torokhov
@ 2016-10-28 23:14 ` Dmitry Torokhov
  2016-10-29  8:38   ` Linus Walleij
  2016-10-28 23:14 ` [PATCH 6/6] Input: gpio_keys - switch to using generic device properties Dmitry Torokhov
  2016-10-31 10:41 ` [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Mika Westerberg
  5 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Geert Uytterhoeven, Linus Walleij, Hans de Goede,
	Mika Westerberg, linux-kernel

From: Geert Uytterhoeven <geert+renesas@glider.be>

GPIO descriptors are the preferred way over legacy GPIO numbers
nowadays. Convert the driver to use GPIO descriptors internally but
still allow passing legacy GPIO numbers from platform data to support
existing platforms.

Based on commits 633a21d80b4a2cd6 ("input: gpio_keys_polled: Add support
for GPIO descriptors") and 1ae5ddb6f8837558 ("Input: gpio_keys_polled -
request GPIO pin as input.").

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index d75a25c..0f04cb1 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -26,6 +26,7 @@
 #include <linux/gpio_keys.h>
 #include <linux/workqueue.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <linux/of_gpio.h>
@@ -35,6 +36,7 @@
 struct gpio_button_data {
 	const struct gpio_keys_button *button;
 	struct input_dev *input;
+	struct gpio_desc *gpiod;
 
 	struct timer_list release_timer;
 	unsigned int release_delay;	/* in msecs, for IRQ-only buttons */
@@ -140,7 +142,7 @@ static void gpio_keys_disable_button(struct gpio_button_data *bdata)
 		 */
 		disable_irq(bdata->irq);
 
-		if (gpio_is_valid(bdata->button->gpio))
+		if (bdata->gpiod)
 			cancel_delayed_work_sync(&bdata->work);
 		else
 			del_timer_sync(&bdata->release_timer);
@@ -358,19 +360,20 @@ static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
 	const struct gpio_keys_button *button = bdata->button;
 	struct input_dev *input = bdata->input;
 	unsigned int type = button->type ?: EV_KEY;
-	int state = gpio_get_value_cansleep(button->gpio);
+	int state;
 
+	state = gpiod_get_value_cansleep(bdata->gpiod);
 	if (state < 0) {
-		dev_err(input->dev.parent, "failed to get gpio state\n");
+		dev_err(input->dev.parent,
+			"failed to get gpio state: %d\n", state);
 		return;
 	}
 
-	state = (state ? 1 : 0) ^ button->active_low;
 	if (type == EV_ABS) {
 		if (state)
 			input_event(input, type, button->code, button->value);
 	} else {
-		input_event(input, type, button->code, !!state);
+		input_event(input, type, button->code, state);
 	}
 	input_sync(input);
 }
@@ -456,7 +459,7 @@ static void gpio_keys_quiesce_key(void *data)
 {
 	struct gpio_button_data *bdata = data;
 
-	if (gpio_is_valid(bdata->button->gpio))
+	if (bdata->gpiod)
 		cancel_delayed_work_sync(&bdata->work);
 	else
 		del_timer_sync(&bdata->release_timer);
@@ -478,18 +481,30 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	bdata->button = button;
 	spin_lock_init(&bdata->lock);
 
+	/*
+	 * Legacy GPIO number, so request the GPIO here and
+	 * convert it to descriptor.
+	 */
 	if (gpio_is_valid(button->gpio)) {
+		unsigned flags = GPIOF_IN;
+
+		if (button->active_low)
+			flags |= GPIOF_ACTIVE_LOW;
 
-		error = devm_gpio_request_one(&pdev->dev, button->gpio,
-					      GPIOF_IN, desc);
+		error = devm_gpio_request_one(&pdev->dev, button->gpio, flags,
+					      desc);
 		if (error < 0) {
 			dev_err(dev, "Failed to request GPIO %d, error %d\n",
 				button->gpio, error);
 			return error;
 		}
 
+		bdata->gpiod = gpio_to_desc(button->gpio);
+		if (!bdata->gpiod)
+			return -EINVAL;
+
 		if (button->debounce_interval) {
-			error = gpio_set_debounce(button->gpio,
+			error = gpiod_set_debounce(bdata->gpiod,
 					button->debounce_interval * 1000);
 			/* use timer if gpiolib doesn't provide debounce */
 			if (error < 0)
@@ -500,7 +515,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		if (button->irq) {
 			bdata->irq = button->irq;
 		} else {
-			irq = gpio_to_irq(button->gpio);
+			irq = gpiod_to_irq(bdata->gpiod);
 			if (irq < 0) {
 				error = irq;
 				dev_err(dev,
@@ -575,7 +590,7 @@ static void gpio_keys_report_state(struct gpio_keys_drvdata *ddata)
 
 	for (i = 0; i < ddata->pdata->nbuttons; i++) {
 		struct gpio_button_data *bdata = &ddata->data[i];
-		if (gpio_is_valid(bdata->button->gpio))
+		if (bdata->gpiod)
 			gpio_keys_gpio_report_event(bdata);
 	}
 	input_sync(input);
-- 
2.8.0.rc3.226.g39d4020

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

* [PATCH 6/6] Input: gpio_keys - switch to using generic device properties
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
                   ` (3 preceding siblings ...)
  2016-10-28 23:14 ` [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors Dmitry Torokhov
@ 2016-10-28 23:14 ` Dmitry Torokhov
  2016-10-31 10:44   ` Mika Westerberg
  2016-10-31 10:41 ` [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Mika Westerberg
  5 siblings, 1 reply; 9+ messages in thread
From: Dmitry Torokhov @ 2016-10-28 23:14 UTC (permalink / raw)
  To: linux-input
  Cc: Linus Walleij, Hans de Goede, Mika Westerberg,
	Geert Uytterhoeven, linux-kernel

Make use of the device property API in this driver so that both OF based
systems and ACPI based systems can use this driver.

Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/gpio_keys.c | 141 ++++++++++++++++++-------------------
 1 file changed, 69 insertions(+), 72 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 0f04cb1..5576f2a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -28,8 +28,6 @@
 #include <linux/gpio.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
-#include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 #include <linux/spinlock.h>
 
@@ -468,7 +466,8 @@ static void gpio_keys_quiesce_key(void *data)
 static int gpio_keys_setup_key(struct platform_device *pdev,
 				struct input_dev *input,
 				struct gpio_button_data *bdata,
-				const struct gpio_keys_button *button)
+				const struct gpio_keys_button *button,
+				struct fwnode_handle *child)
 {
 	const char *desc = button->desc ? button->desc : "gpio_keys";
 	struct device *dev = &pdev->dev;
@@ -481,11 +480,28 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 	bdata->button = button;
 	spin_lock_init(&bdata->lock);
 
-	/*
-	 * Legacy GPIO number, so request the GPIO here and
-	 * convert it to descriptor.
-	 */
-	if (gpio_is_valid(button->gpio)) {
+	if (child) {
+		bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child);
+		if (IS_ERR(bdata->gpiod)) {
+			error = PTR_ERR(bdata->gpiod);
+			if (error == -ENOENT) {
+				/*
+				 * GPIO is optional, we may be dealing with
+				 * purely interrupt-driven setup.
+				 */
+				bdata->gpiod = NULL;
+			} else {
+				if (error != -EPROBE_DEFER)
+					dev_err(dev, "failed to get gpio: %d\n",
+						error);
+				return error;
+			}
+		}
+	} else if (gpio_is_valid(button->gpio)) {
+		/*
+		 * Legacy GPIO number, so request the GPIO here and
+		 * convert it to descriptor.
+		 */
 		unsigned flags = GPIOF_IN;
 
 		if (button->active_low)
@@ -502,7 +518,9 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 		bdata->gpiod = gpio_to_desc(button->gpio);
 		if (!bdata->gpiod)
 			return -EINVAL;
+	}
 
+	if (bdata->gpiod) {
 		if (button->debounce_interval) {
 			error = gpiod_set_debounce(bdata->gpiod,
 					button->debounce_interval * 1000);
@@ -533,9 +551,10 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
 
 	} else {
 		if (!button->irq) {
-			dev_err(dev, "No IRQ specified\n");
+			dev_err(dev, "Found button without gpio or irq\n");
 			return -EINVAL;
 		}
+
 		bdata->irq = button->irq;
 
 		if (button->type && button->type != EV_KEY) {
@@ -627,24 +646,18 @@ static void gpio_keys_close(struct input_dev *input)
  * Handlers for alternative sources of platform_data
  */
 
-#ifdef CONFIG_OF
 /*
- * Translate OpenFirmware node properties into platform_data
+ * Translate properties into platform_data
  */
 static struct gpio_keys_platform_data *
 gpio_keys_get_devtree_pdata(struct device *dev)
 {
-	struct device_node *node, *pp;
 	struct gpio_keys_platform_data *pdata;
 	struct gpio_keys_button *button;
-	int error;
+	struct fwnode_handle *child;
 	int nbuttons;
 
-	node = dev->of_node;
-	if (!node)
-		return ERR_PTR(-ENODEV);
-
-	nbuttons = of_get_available_child_count(node);
+	nbuttons = device_get_child_node_count(dev);
 	if (nbuttons == 0)
 		return ERR_PTR(-ENODEV);
 
@@ -659,64 +672,43 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 	pdata->buttons = button;
 	pdata->nbuttons = nbuttons;
 
-	pdata->rep = !!of_get_property(node, "autorepeat", NULL);
+	pdata->rep = device_property_read_bool(dev, "autorepeat");
 
-	of_property_read_string(node, "label", &pdata->name);
+	device_property_read_string(dev, "label", &pdata->name);
 
-	for_each_available_child_of_node(node, pp) {
-		enum of_gpio_flags flags;
+	device_for_each_child_node(dev, child) {
+		if (is_of_node(child))
+			button->irq =
+				irq_of_parse_and_map(to_of_node(child), 0);
 
-		button->gpio = of_get_gpio_flags(pp, 0, &flags);
-		if (button->gpio < 0) {
-			error = button->gpio;
-			if (error != -ENOENT) {
-				if (error != -EPROBE_DEFER)
-					dev_err(dev,
-						"Failed to get gpio flags, error: %d\n",
-						error);
-				of_node_put(pp);
-				return ERR_PTR(error);
-			}
-		} else {
-			button->active_low = flags & OF_GPIO_ACTIVE_LOW;
-		}
-
-		button->irq = irq_of_parse_and_map(pp, 0);
-
-		if (!gpio_is_valid(button->gpio) && !button->irq) {
-			dev_err(dev, "Found button without gpios or irqs\n");
-			of_node_put(pp);
-			return ERR_PTR(-EINVAL);
-		}
-
-		if (of_property_read_u32(pp, "linux,code", &button->code)) {
-			dev_err(dev, "Button without keycode: 0x%x\n",
-				button->gpio);
-			of_node_put(pp);
+		if (fwnode_property_read_u32(child, "linux,code",
+					     &button->code)) {
+			dev_err(dev, "Button without keycode\n");
+			fwnode_handle_put(child);
 			return ERR_PTR(-EINVAL);
 		}
 
-		button->desc = of_get_property(pp, "label", NULL);
+		fwnode_property_read_string(child, "label", &button->desc);
 
-		if (of_property_read_u32(pp, "linux,input-type", &button->type))
+		if (fwnode_property_read_u32(child, "linux,input-type",
+					     &button->type))
 			button->type = EV_KEY;
 
-		button->wakeup = of_property_read_bool(pp, "wakeup-source") ||
-				 /* legacy name */
-				 of_property_read_bool(pp, "gpio-key,wakeup");
+		button->wakeup =
+			fwnode_property_read_bool(child, "wakeup-source") ||
+			/* legacy name */
+			fwnode_property_read_bool(child, "gpio-key,wakeup");
 
-		button->can_disable = !!of_get_property(pp, "linux,can-disable", NULL);
+		button->can_disable =
+			fwnode_property_read_bool(child, "linux,can-disable");
 
-		if (of_property_read_u32(pp, "debounce-interval",
+		if (fwnode_property_read_u32(child, "debounce-interval",
 					 &button->debounce_interval))
 			button->debounce_interval = 5;
 
 		button++;
 	}
 
-	if (pdata->nbuttons == 0)
-		return ERR_PTR(-EINVAL);
-
 	return pdata;
 }
 
@@ -726,20 +718,11 @@ static const struct of_device_id gpio_keys_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, gpio_keys_of_match);
 
-#else
-
-static inline struct gpio_keys_platform_data *
-gpio_keys_get_devtree_pdata(struct device *dev)
-{
-	return ERR_PTR(-ENODEV);
-}
-
-#endif
-
 static int gpio_keys_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	const struct gpio_keys_platform_data *pdata = dev_get_platdata(dev);
+	struct fwnode_handle *child = NULL;
 	struct gpio_keys_drvdata *ddata;
 	struct input_dev *input;
 	size_t size;
@@ -792,14 +775,28 @@ static int gpio_keys_probe(struct platform_device *pdev)
 		const struct gpio_keys_button *button = &pdata->buttons[i];
 		struct gpio_button_data *bdata = &ddata->data[i];
 
-		error = gpio_keys_setup_key(pdev, input, bdata, button);
-		if (error)
+		if (!dev_get_platdata(dev)) {
+			child = device_get_next_child_node(&pdev->dev, child);
+			if (!child) {
+				dev_err(&pdev->dev,
+					"missing child device node for entry %d\n",
+					i);
+				return -EINVAL;
+			}
+		}
+
+		error = gpio_keys_setup_key(pdev, input, bdata, button, child);
+		if (error) {
+			fwnode_handle_put(child);
 			return error;
+		}
 
 		if (button->wakeup)
 			wakeup = 1;
 	}
 
+	fwnode_handle_put(child);
+
 	error = sysfs_create_group(&pdev->dev.kobj, &gpio_keys_attr_group);
 	if (error) {
 		dev_err(dev, "Unable to export keys/switches, error: %d\n",
@@ -889,7 +886,7 @@ static struct platform_driver gpio_keys_device_driver = {
 	.driver		= {
 		.name	= "gpio-keys",
 		.pm	= &gpio_keys_pm_ops,
-		.of_match_table = of_match_ptr(gpio_keys_of_match),
+		.of_match_table = gpio_keys_of_match,
 	}
 };
 
-- 
2.8.0.rc3.226.g39d4020

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

* Re: [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors
  2016-10-28 23:14 ` [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors Dmitry Torokhov
@ 2016-10-29  8:38   ` Linus Walleij
  0 siblings, 0 replies; 9+ messages in thread
From: Linus Walleij @ 2016-10-29  8:38 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Linux Input, Geert Uytterhoeven, Hans de Goede, Mika Westerberg,
	linux-kernel

On Sat, Oct 29, 2016 at 1:14 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> From: Geert Uytterhoeven <geert+renesas@glider.be>
>
> GPIO descriptors are the preferred way over legacy GPIO numbers
> nowadays. Convert the driver to use GPIO descriptors internally but
> still allow passing legacy GPIO numbers from platform data to support
> existing platforms.
>
> Based on commits 633a21d80b4a2cd6 ("input: gpio_keys_polled: Add support
> for GPIO descriptors") and 1ae5ddb6f8837558 ("Input: gpio_keys_polled -
> request GPIO pin as input.").
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

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

* Re: [PATCH 1/6] Input: gpio_keys_polled - keep button data constant
  2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
                   ` (4 preceding siblings ...)
  2016-10-28 23:14 ` [PATCH 6/6] Input: gpio_keys - switch to using generic device properties Dmitry Torokhov
@ 2016-10-31 10:41 ` Mika Westerberg
  5 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2016-10-31 10:41 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Linus Walleij, Hans de Goede, Geert Uytterhoeven,
	linux-kernel

On Fri, Oct 28, 2016 at 04:14:39PM -0700, Dmitry Torokhov wrote:
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index ee2d8c6..d1250ad 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -2,7 +2,6 @@
>  #define _GPIO_KEYS_H
>  
>  struct device;
> -struct gpio_desc;
>  
>  /**
>   * struct gpio_keys_button - configuration parameters
> @@ -31,7 +30,6 @@ struct gpio_keys_button {
>  	bool can_disable;
>  	int value;
>  	unsigned int irq;
> -	struct gpio_desc *gpiod;

Please remove the kernel-doc comment of this member as well.

>  };

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

* Re: [PATCH 6/6] Input: gpio_keys - switch to using generic device properties
  2016-10-28 23:14 ` [PATCH 6/6] Input: gpio_keys - switch to using generic device properties Dmitry Torokhov
@ 2016-10-31 10:44   ` Mika Westerberg
  0 siblings, 0 replies; 9+ messages in thread
From: Mika Westerberg @ 2016-10-31 10:44 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Linus Walleij, Hans de Goede, Geert Uytterhoeven,
	linux-kernel

On Fri, Oct 28, 2016 at 04:14:44PM -0700, Dmitry Torokhov wrote:
> Make use of the device property API in this driver so that both OF based
> systems and ACPI based systems can use this driver.
> 
> Suggested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

I tested 1-6 on Intel Joule with both gpio_keys and gpio_keys_polled and
it works fine. Thanks for doing this!

For the whole series,

Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>

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

end of thread, other threads:[~2016-10-31 10:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28 23:14 [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Dmitry Torokhov
2016-10-28 23:14 ` [PATCH 2/6] Input: gpio_keys_polled - always use gpiod_get_value_cansleep Dmitry Torokhov
2016-10-28 23:14 ` [PATCH 3/6] Input: gpio_keys - annotate PM methods as __maybe_unused Dmitry Torokhov
2016-10-28 23:14 ` [PATCH 4/6] Input: gpio_keys - fix leaking DT node references Dmitry Torokhov
2016-10-28 23:14 ` [PATCH 5/6] Input: gpio_keys - add support for GPIO descriptors Dmitry Torokhov
2016-10-29  8:38   ` Linus Walleij
2016-10-28 23:14 ` [PATCH 6/6] Input: gpio_keys - switch to using generic device properties Dmitry Torokhov
2016-10-31 10:44   ` Mika Westerberg
2016-10-31 10:41 ` [PATCH 1/6] Input: gpio_keys_polled - keep button data constant Mika Westerberg

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.