Subject: [PATCH 1/2] input: keyboard: gpio_keys_polled: use gpio lookup table

From: Enrico Weigelt <[email protected]>

Support the recently introduced gpio lookup tables for
attaching to gpio lines. So, harcoded gpio numbers aren't
needed anymore.

changes v3:
* fix printf string in gpio_keys_polled_get_gpiod()
* fix unused variable 'error' in gpio_keys_polled_get_gpiod()
* fix uninitialized variable in gpio_keys_polled_get_gpiod_fwnode()

Cc: Dmitry Torokhov <[email protected]>
Cc: [email protected]
Signed-off-by: Enrico Weigelt <[email protected]>
---
drivers/input/keyboard/gpio_keys_polled.c | 166 +++++++++++++++++++++---------
1 file changed, 118 insertions(+), 48 deletions(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 465eecf..91754de 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -21,6 +21,7 @@
#include <linux/platform_device.h>
#include <linux/gpio.h>
#include <linux/gpio/consumer.h>
+#include <linux/gpio/machine.h>
#include <linux/gpio_keys.h>
#include <linux/property.h>

@@ -226,6 +227,118 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
};
MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);

+static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
+ struct device *dev,
+ int idx,
+ const char *desc)
+{
+ struct gpio_desc *gpiod;
+ struct fwnode_handle *child;
+ int x = idx;
+
+ /* get the idx'th child node */
+ child = device_get_next_child_node(dev, NULL);
+ while (child && x) {
+ child = device_get_next_child_node(dev, child);
+ x--;
+ }
+
+ if (!child) {
+ dev_err(dev, "missing oftree child node #%d\n", idx);
+ return ERR_PTR(-EINVAL);
+ }
+
+ gpiod = devm_fwnode_get_gpiod_from_child(dev,
+ NULL,
+ child,
+ GPIOD_IN,
+ desc);
+ if (IS_ERR(gpiod)) {
+ if (PTR_ERR(gpiod) != -EPROBE_DEFER)
+ dev_err(dev,
+ "failed to get gpio: %ld\n",
+ PTR_ERR(gpiod));
+ fwnode_handle_put(child);
+ return gpiod;
+ }
+
+ return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod_legacy(
+ struct device *dev,
+ int idx,
+ const struct gpio_keys_button *button)
+{
+ /*
+ * Legacy GPIO number so request the GPIO here and
+ * convert it to descriptor.
+ */
+ unsigned int flags = GPIOF_IN;
+ struct gpio_desc *gpiod;
+ int error;
+
+ dev_info(dev, "hardcoded gpio IDs are deprecated.\n");
+
+ if (button->active_low)
+ flags |= GPIOF_ACTIVE_LOW;
+
+ error = devm_gpio_request_one(dev, button->gpio,
+ flags, button->desc ? : DRV_NAME);
+ if (error) {
+ dev_err(dev,
+ "unable to claim gpio %u, err=%d\n",
+ button->gpio, error);
+ return ERR_PTR(error);
+ }
+
+ gpiod = gpio_to_desc(button->gpio);
+ if (!gpiod) {
+ dev_err(dev,
+ "unable to convert gpio %u to descriptor\n",
+ button->gpio);
+ return ERR_PTR(-EINVAL);
+ }
+
+ return gpiod;
+}
+
+static struct gpio_desc *gpio_keys_polled_get_gpiod(
+ struct device *dev,
+ int idx,
+ const struct gpio_keys_button *button)
+{
+ struct gpio_desc *gpiod = NULL;
+
+ /* No legacy static platform data - use oftree */
+ if (!dev_get_platdata(dev)) {
+ return gpio_keys_polled_get_gpiod_fwnode(
+ dev, idx, button->desc);
+ }
+
+ gpiod = devm_gpiod_get_index(dev, NULL, idx, GPIOF_IN);
+
+ if (!IS_ERR(gpiod)) {
+ dev_info(dev, "picked gpiod idx %d from gpio table\n", idx);
+ gpiod_set_consumer_name(gpiod, button->desc ? : DRV_NAME);
+ return gpiod;
+ }
+
+ if (PTR_ERR(gpiod) != -ENOENT) {
+ dev_err(dev, "failed fetching gpiod #%d: %ld\n",
+ idx, PTR_ERR(gpiod));
+ return gpiod;
+ }
+
+ /* Use legacy gpio id, if defined */
+ if (gpio_is_valid(button->gpio)) {
+ return gpio_keys_polled_get_gpiod_legacy(
+ dev, idx, button);
+ }
+
+ return ERR_PTR(-ENOENT);
+}
+
static int gpio_keys_polled_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -288,57 +401,14 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)

if (button->wakeup) {
dev_err(dev, DRV_NAME " does not support wakeup\n");
- fwnode_handle_put(child);
return -EINVAL;
}

- 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_fwnode_get_gpiod_from_child(dev,
- NULL, child,
- GPIOD_IN,
- button->desc);
- 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)
- flags |= GPIOF_ACTIVE_LOW;
-
- error = devm_gpio_request_one(dev, button->gpio,
- flags, button->desc ? : DRV_NAME);
- if (error) {
- dev_err(dev,
- "unable to claim gpio %u, err=%d\n",
- button->gpio, error);
- return error;
- }
-
- 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;
- }
+ bdata->gpiod = gpio_keys_polled_get_gpiod(dev, i, button);
+
+ if (IS_ERR(bdata->gpiod)) {
+ dev_err(dev, "failed to fetch gpiod #%d\n", i);
+ return PTR_ERR(bdata->gpiod);
}

bdata->last_state = -1;
--
1.9.1


2019-07-29 19:27:52

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/2] input: keyboard: gpio_keys_polled: use gpio lookup table

Hi Enrico,

On Mon, Jul 29, 2019 at 05:59:53PM +0200, Enrico Weigelt, metux IT consult wrote:
> From: Enrico Weigelt <[email protected]>
>
> Support the recently introduced gpio lookup tables for
> attaching to gpio lines. So, harcoded gpio numbers aren't
> needed anymore.
>
> changes v3:
> * fix printf string in gpio_keys_polled_get_gpiod()
> * fix unused variable 'error' in gpio_keys_polled_get_gpiod()
> * fix uninitialized variable in gpio_keys_polled_get_gpiod_fwnode()

As I think I mentioned a while back I would prefer to get gpiolob
support swnode-backed properties so that the driver would not need to
know about differences between ACPI, DT and static board files.

I just recently re-posted patches for this, let's see if we can get them
landed in the kernel.

Thanks.

--
Dmitry

Subject: Re: [PATCH 1/2] input: keyboard: gpio_keys_polled: use gpio lookup table

On 29.07.19 19:26, Dmitry Torokhov wrote:

Hi,

> As I think I mentioned a while back I would prefer to get gpiolob > support swnode-backed properties so that the driver would not need
to> know about differences between ACPI, DT and static board files.
Indeed would be nice. But I think we should get rid of raw gpio IDs in
favour of gpiod lookup tables first.

> I just recently re-posted patches for this, let's see if we can get them > landed in the kernel.
Can you give me a pointer ?


--mtx


--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2019-07-29 22:01:44

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH 1/2] input: keyboard: gpio_keys_polled: use gpio lookup table

On Mon, Jul 29, 2019 at 08:14:52PM +0200, Enrico Weigelt, metux IT consult wrote:
> On 29.07.19 19:26, Dmitry Torokhov wrote:
>
> Hi,
>
> > As I think I mentioned a while back I would prefer to get gpiolob >
> > support swnode-backed properties so that the driver would not need
> to> know about differences between ACPI, DT and static board files.
> Indeed would be nice. But I think we should get rid of raw gpio IDs in
> favour of gpiod lookup tables first.
>
> > I just recently re-posted patches for this, let's see if we can get them > landed in the kernel.
> Can you give me a pointer ?

https://patchwork.kernel.org/cover/11042915/

I tried putting you on CC list there, did you not get them?

Thanks.

--
Dmitry

Subject: Re: [PATCH 1/2] input: keyboard: gpio_keys_polled: use gpio lookup table

On 29.07.19 20:43, Dmitry Torokhov wrote:

Hi,

> https://patchwork.kernel.org/cover/11042915/

Thanks.

> I tried putting you on CC list there, did you not get them?

hmm, maybe it went to one of the dozens of mailboxes where I didn't
look at careful enough ... I'm currently just sorting by mailing list,
but don't separate out stuff that's directly addressed to me - guess
I'll have to fix up my mail filter rules :o

Regarding your patch:

How should I now setup a proper swnode object and pass it to the
driver ?

--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287