| Message ID | 20180906075902.31240-3-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] gpio: fwnode: Obtains descs from machine tables | expand |
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@gmail.com> > Cc: Andy Shevchenko <andy@infradead.org> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > 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
On Mon, Sep 17, 2018 at 11:15 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > The issue I see here is that gpio-keys still > needs platform data to deal with per-button config. Yes I was just thinking inside the GPIO box and solving my own little problem... >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. I love it. I will look closely at it. I think also Andy has some of these boardfile type deployments on Intel when no ACPI description is available. Yours, Linus Walleij
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)) { /*
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@gmail.com> Cc: Andy Shevchenko <andy@infradead.org> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- 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. --- drivers/input/keyboard/gpio_keys.c | 47 ++++++++++++++++++------------ 1 file changed, 28 insertions(+), 19 deletions(-) -- 2.17.1