Hello folks,
here're some improvements for the gpio-keys-polled driver.
The first patch isn't in the driver itself, but adds a little module helper
for conditionally declaring oftree module table if oftree is enabled
(only needed for the last patch)
have fun.
--mtx
we don't need to build in oftree probing stuff when oftree isn't
enabled at all.
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
---
drivers/input/keyboard/gpio_keys_polled.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3f773b2..fbccb89 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -147,6 +147,7 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
static struct gpio_keys_platform_data *
gpio_keys_polled_get_devtree_pdata(struct device *dev)
{
+#ifdef CONFIG_OF
struct gpio_keys_platform_data *pdata;
struct gpio_keys_button *button;
struct fwnode_handle *child;
@@ -200,6 +201,9 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
}
return pdata;
+#else /* CONFIG_OF */
+ return ERR_PTR(-ENOENT);
+#endif /* CONFIG_OF */
}
static void gpio_keys_polled_set_abs_params(struct input_dev *input,
@@ -226,7 +230,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
{ .compatible = "gpio-keys-polled", },
{ },
};
-MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
+MODULE_DEVICE_TABLE_OF(gpio_keys_polled_of_match);
static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
struct device *dev,
@@ -452,7 +456,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
.probe = gpio_keys_polled_probe,
.driver = {
.name = DRV_NAME,
+#ifdef CONFIG_OF
.of_match_table = gpio_keys_polled_of_match,
+#endif /* CONFIG_OF */
},
};
module_platform_driver(gpio_keys_polled_driver);
--
1.9.1
Support the recently introduced gpio lookup tables for
attaching to gpio lines. So, harcoded gpio numbers aren't
needed anymore.
Cc: Dmitry Torokhov <[email protected]>
Cc: [email protected]
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
---
drivers/input/keyboard/gpio_keys_polled.c | 167 +++++++++++++++++++++---------
1 file changed, 119 insertions(+), 48 deletions(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index 3312186..3f773b2 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -24,6 +24,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>
@@ -227,6 +228,119 @@ 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;
+
+ /* 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;
+ int error;
+
+ /* 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: %d\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;
@@ -291,57 +405,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
Instead of hardcoding the input name to the driver name
('gpio-keys-polled'), allow the passing a name via platform data
('name' field was already present), but default to old behaviour
in case of NULL.
Cc: Dmitry Torokhov <[email protected]>
Cc: [email protected]
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
---
drivers/input/keyboard/gpio_keys_polled.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index edc7262..3312186 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -272,7 +272,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
input = poll_dev->input;
- input->name = pdev->name;
+ input->name = (pdata->name ? pdata->name : pdev->name);
input->phys = DRV_NAME"/input0";
input->id.bustype = BUS_HOST;
--
1.9.1
Little helper macro that declares an oftree module device table,
if CONFIG_OF is enabled. Otherwise it's just noop.
This is also helpful if some drivers can be built w/ or w/o
oftree support.
Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
---
include/linux/mod_devicetable.h | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 448621c..853e301 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -245,6 +245,15 @@ struct of_device_id {
const void *data;
};
+/*
+ * macro for adding the of module device table only if CONFIG_OF enabled
+ */
+#ifdef CONFIG_OF
+#define MODULE_DEVICE_TABLE_OF(name) MODULE_DEVICE_TABLE(of,name)
+#else
+#define MODULE_DEVICE_TABLE_OF(name)
+#endif /* CONFIG_OF */
+
/* VIO */
struct vio_device_id {
char type[32];
--
1.9.1
Hi Enrico,
On Tue, Apr 16, 2019 at 09:57:22PM +0200, Enrico Weigelt, metux IT consult wrote:
> Little helper macro that declares an oftree module device table,
> if CONFIG_OF is enabled. Otherwise it's just noop.
>
> This is also helpful if some drivers can be built w/ or w/o
> oftree support.
This should go to OF folks, please.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
> ---
> include/linux/mod_devicetable.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 448621c..853e301 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -245,6 +245,15 @@ struct of_device_id {
> const void *data;
> };
>
> +/*
> + * macro for adding the of module device table only if CONFIG_OF enabled
> + */
> +#ifdef CONFIG_OF
> +#define MODULE_DEVICE_TABLE_OF(name) MODULE_DEVICE_TABLE(of,name)
> +#else
> +#define MODULE_DEVICE_TABLE_OF(name)
> +#endif /* CONFIG_OF */
> +
> /* VIO */
> struct vio_device_id {
> char type[32];
> --
> 1.9.1
>
--
Dmitry
Hi Enrico,
On Tue, Apr 16, 2019 at 09:57:24PM +0200, Enrico Weigelt, metux IT consult wrote:
> Support the recently introduced gpio lookup tables for
> attaching to gpio lines. So, harcoded gpio numbers aren't
> needed anymore.
I would prefer if gpiod API could parse static board data/gpio lookup
tables for child nodes, instead of adding this to gpio-keys. Now that
Heikki Krogerus work on software nodes has landed I need to resurrect
my patch to gpiolib.
Thanks.
--
Dmitry
On 19.04.19 09:40, Dmitry Torokhov wrote:
> Hi Enrico,
>
> On Tue, Apr 16, 2019 at 09:57:22PM +0200, Enrico Weigelt, metux IT consult wrote:
>> Little helper macro that declares an oftree module device table,
>> if CONFIG_OF is enabled. Otherwise it's just noop.
>>
>> This is also helpful if some drivers can be built w/ or w/o
>> oftree support.
>
> This should go to OF folks, please.
hmm, they should be CCed, if my script works right.
This one is only needed for the 4th patch (skip oftree...).
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 19.04.19 09:48, Dmitry Torokhov wrote:
> I would prefer if gpiod API could parse static board data/gpio lookup
> tables for child nodes, instead of adding this to gpio-keys. Now that
> Heikki Krogerus work on software nodes has landed I need to resurrect
> my patch to gpiolib.
Of course, a more generic approach would be better.
I did it that way I wasn't that aware that a more generic approach is
in the pipeline, and I needed something usable now.
The patch is already used in the field and seems to work quite well,
but if I'm alway open for better solutions.
I figure that the generic solution will still take some time, so can we
take this one for the next merge window and rework it in the next one ?
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287
On 4/16/19 12:57 PM, Enrico Weigelt, metux IT consult wrote:
> we don't need to build in oftree probing stuff when oftree isn't
> enabled at all.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
> ---
> drivers/input/keyboard/gpio_keys_polled.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
> index 3f773b2..fbccb89 100644
> --- a/drivers/input/keyboard/gpio_keys_polled.c
> +++ b/drivers/input/keyboard/gpio_keys_polled.c
> @@ -147,6 +147,7 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
> static struct gpio_keys_platform_data *
> gpio_keys_polled_get_devtree_pdata(struct device *dev)
> {
> +#ifdef CONFIG_OF
> struct gpio_keys_platform_data *pdata;
> struct gpio_keys_button *button;
> struct fwnode_handle *child;
> @@ -200,6 +201,9 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
> }
>
> return pdata;
> +#else /* CONFIG_OF */
> + return ERR_PTR(-ENOENT);
> +#endif /* CONFIG_OF */
> }
>
> static void gpio_keys_polled_set_abs_params(struct input_dev *input,
> @@ -226,7 +230,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
> { .compatible = "gpio-keys-polled", },
> { },
> };
> -MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
> +MODULE_DEVICE_TABLE_OF(gpio_keys_polled_of_match);
Not needed, when you use of_match_ptr() -- see below.
>
> static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
> struct device *dev,
> @@ -452,7 +456,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
> .probe = gpio_keys_polled_probe,
> .driver = {
> .name = DRV_NAME,
> +#ifdef CONFIG_OF
> .of_match_table = gpio_keys_polled_of_match,
> +#endif /* CONFIG_OF */
No need for the #ifdef, use of_match_ptr():
.of_match_table = of_match_ptr(gpio_keys_polled_of_match),
> },
> };
> module_platform_driver(gpio_keys_polled_driver);
>
On 4/24/19 3:48 AM, Enrico Weigelt, metux IT consult wrote:
> On 19.04.19 09:40, Dmitry Torokhov wrote:
>> Hi Enrico,
>>
>> On Tue, Apr 16, 2019 at 09:57:22PM +0200, Enrico Weigelt, metux IT consult wrote:
>>> Little helper macro that declares an oftree module device table,
>>> if CONFIG_OF is enabled. Otherwise it's just noop.
>>>
>>> This is also helpful if some drivers can be built w/ or w/o
>>> oftree support.
>>
>> This should go to OF folks, please.
>
> hmm, they should be CCed, if my script works right.
> This one is only needed for the 4th patch (skip oftree...).
Your script did not work (BTDT). I just happened to notice this on lkml.
-Frank
>
>
> --mtx
>
On 29.04.19 21:44, Frank Rowand wrote:
> On 4/16/19 12:57 PM, Enrico Weigelt, metux IT consult wrote:
>> we don't need to build in oftree probing stuff when oftree isn't
>> enabled at all.
>>
>> Signed-off-by: Enrico Weigelt, metux IT consult <[email protected]>
>> ---
>> drivers/input/keyboard/gpio_keys_polled.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
>> index 3f773b2..fbccb89 100644
>> --- a/drivers/input/keyboard/gpio_keys_polled.c
>> +++ b/drivers/input/keyboard/gpio_keys_polled.c
>> @@ -147,6 +147,7 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
>> static struct gpio_keys_platform_data *
>> gpio_keys_polled_get_devtree_pdata(struct device *dev)
>> {
>> +#ifdef CONFIG_OF
>> struct gpio_keys_platform_data *pdata;
>> struct gpio_keys_button *button;
>> struct fwnode_handle *child;
>> @@ -200,6 +201,9 @@ static void gpio_keys_polled_close(struct input_polled_dev *dev)
>> }
>>
>> return pdata;
>> +#else /* CONFIG_OF */
>> + return ERR_PTR(-ENOENT);
>> +#endif /* CONFIG_OF */
>> }
>>
>> static void gpio_keys_polled_set_abs_params(struct input_dev *input,
>> @@ -226,7 +230,7 @@ static void gpio_keys_polled_set_abs_params(struct input_dev *input,
>> { .compatible = "gpio-keys-polled", },
>> { },
>> };
>
>
>> -MODULE_DEVICE_TABLE(of, gpio_keys_polled_of_match);
>> +MODULE_DEVICE_TABLE_OF(gpio_keys_polled_of_match);
>
> Not needed, when you use of_match_ptr() -- see below.
Shall I remove the MODULE_DEVICE_TABLE... line completely ?
I'd like to have nothing of-related compiled in, when oftree isn't
enabled.
>> static struct gpio_desc *gpio_keys_polled_get_gpiod_fwnode(
>> struct device *dev,
>> @@ -452,7 +456,9 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
>> .probe = gpio_keys_polled_probe,
>> .driver = {
>> .name = DRV_NAME,
>
>> +#ifdef CONFIG_OF
>> .of_match_table = gpio_keys_polled_of_match,
>> +#endif /* CONFIG_OF */
>
> No need for the #ifdef, use of_match_ptr():
>
> .of_match_table = of_match_ptr(gpio_keys_polled_of_match),
Ok, thanks.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287