On Thu, Sep 06, 2018 at 09:59:01AM +0200, Linus Walleij wrote:
> This makes the gpio_keys input driver try fwnode first when
> looking up GPIO descriptors, even if no fwnode was passed.
>
> With the changes to the gpiolib, if NULL us passed as
> fwnode, the gpiolib will attempt to look up the GPIO
> descriptor from the machine descriptor tables.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Cc: Andy Shevchenko <andy@xxxxxxxxxxxxx>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Cc: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> Dmitry: I'm looking for your ACK if you agree with this
> approach overall so I can apply this with the changes to
> gpiolib to the GPIO tree.
Linus, sorry for the delay. The issue I see here is that gpio-keys still
needs platform data to deal with per-button config. I will be sending a
patch set in a minute that introduces notion of "children" to legacy
device properties (expressed via property_entry/property_set) and wiring
up gpiolib lookup tables to work with them. Hopefully you and Rafael
would be OK with it, and then we could do proper cleanup of gpio-keys
and similar drivers.
Thanks!
> ---
> drivers/input/keyboard/gpio_keys.c | 47 ++++++++++++++++++------------
> 1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 492a971b95b5..eef2dcbc9185 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -499,25 +499,34 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> bdata->button = button;
> spin_lock_init(&bdata->lock);
>
> - if (child) {
> - bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
> - child,
> - GPIOD_IN,
> - desc);
> - 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;
> - }
> + /*
> + * We try this first as it will find GPIOs even from board
> + * files if properly done.
> + */
> + bdata->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL,
> + child,
> + GPIOD_IN,
> + desc);
> + /*
> + * If we have a valid fwnode and this lookup fails, we need
> + * to give up. Otherwise we try to use the GPIO from the
> + * platform data.
> + */
> + if (!IS_ERR(bdata->gpiod)) {
> + /* All is good */
> + } else if (child) {
> + 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)) {
> /*
> --
> 2.17.1
>
--
Dmitry
