| Message ID | 20170109140228.47613-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Make fwnode_get_named_gpiod() consistent with the rest of gpiod_get() like API, > i.e. configure GPIO pin immediately after request. > > Besides obvious clean up it will help to configure pins based on firmware > provided resources. > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> This is overall fine. However I have another patch in the works from Alexander Stein to pass down the label to the core properly, can you look at this patch too, so we don't have to mess around too much? http://marc.info/?l=linux-gpio&m=148179211709228&w=2 Which patch should we base on which patch? Or can they be combined into one? That patch has been held back awaiting ACKs and already has an ACK from Jacek so I would merge it first. Could you rebase this patch on top of Alexander's patch? Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > Make fwnode_get_named_gpiod() consistent with the rest of > > gpiod_get() like API, > > i.e. configure GPIO pin immediately after request. > > > > Besides obvious clean up it will help to configure pins based on > > firmware > > provided resources. > > > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > This is overall fine. However I have another patch in the works from > Alexander Stein to pass down the label to the core properly, can you > look > at this patch too, so we don't have to mess around too much? > http://marc.info/?l=linux-gpio&m=148179211709228&w=2 > > Which patch should we base on which patch? Or can they be > combined into one? > That patch has been held back awaiting ACKs and already > has an ACK from Jacek so I would merge it first. Could you > rebase this patch on top of Alexander's patch? > At the first glance looks okay to just merge it. Means I don't see it might affect functionality of what I'm trying to do. So, please, go ahead and thanks for pointing out. I will prepare mine on top of it.
On Wed, 2017-01-11 at 15:35 +0200, Andy Shevchenko wrote: > On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > This is overall fine. However I have another patch in the works from > > Alexander Stein to pass down the label to the core properly, can you > > look > > at this patch too, so we don't have to mess around too much? > > http://marc.info/?l=linux-gpio&m=148179211709228&w=2 > > > > Which patch should we base on which patch? Or can they be > > combined into one? > > That patch has been held back awaiting ACKs and already > > has an ACK from Jacek so I would merge it first. Could you > > rebase this patch on top of Alexander's patch? > > > > At the first glance looks okay to just merge it. > Means I don't see it might affect functionality of what I'm trying to > do. > > So, please, go ahead and thanks for pointing out. I will prepare mine > on > top of it. Alas, it doesn't apply clearly. gpio_keys_polled.c has been changed a lot from that time. So, would you like to fix it yourself? Since mine is applied clearly I may do other way around, rebase and fix that patch on top of mine. How does it sound?
On Wednesday 11 January 2017 16:14:57, Andy Shevchenko wrote: > On Wed, 2017-01-11 at 15:35 +0200, Andy Shevchenko wrote: > > On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > > > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > This is overall fine. However I have another patch in the works from > > > Alexander Stein to pass down the label to the core properly, can you > > > look > > > at this patch too, so we don't have to mess around too much? > > > http://marc.info/?l=linux-gpio&m=148179211709228&w=2 > > > > > > Which patch should we base on which patch? Or can they be > > > combined into one? > > > > > > That patch has been held back awaiting ACKs and already > > > has an ACK from Jacek so I would merge it first. Could you > > > rebase this patch on top of Alexander's patch? > > > > At the first glance looks okay to just merge it. > > Means I don't see it might affect functionality of what I'm trying to > > do. > > > > So, please, go ahead and thanks for pointing out. I will prepare mine > > on > > top of it. > > Alas, it doesn't apply clearly. gpio_keys_polled.c has been changed a > lot from that time. So, would you like to fix it yourself? > > Since mine is applied clearly I may do other way around, rebase and fix > that patch on top of mine. > How does it sound? Yeah, I noticed that gpio_keys_polled.c has changed meanwhile, so my patch has to be modifed anyway. If your patch is applied somewhere I can rebase my change on that and resubmit. Best regards, Alexander
On Thu, 2017-01-12 at 08:27 +0100, Alexander Stein wrote: > On Wednesday 11 January 2017 16:14:57, Andy Shevchenko wrote: > > On Wed, 2017-01-11 at 15:35 +0200, Andy Shevchenko wrote: > > > On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > > > > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > Alas, it doesn't apply clearly. gpio_keys_polled.c has been changed > > a > > lot from that time. So, would you like to fix it yourself? > > > > Since mine is applied clearly I may do other way around, rebase and > > fix > > that patch on top of mine. > > How does it sound? > > Yeah, I noticed that gpio_keys_polled.c has changed meanwhile, so my > patch has > to be modifed anyway. If your patch is applied somewhere I can rebase > my > change on that and resubmit. First of all, how do you feel if Linus applies my change first? Please, keep me in Cc list for new version if you are going to re- base/re-send.
On Thursday 12 January 2017 14:15:12, Andy Shevchenko wrote: > On Thu, 2017-01-12 at 08:27 +0100, Alexander Stein wrote: > > On Wednesday 11 January 2017 16:14:57, Andy Shevchenko wrote: > > > On Wed, 2017-01-11 at 15:35 +0200, Andy Shevchenko wrote: > > > > On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > > > > > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > > > > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > Alas, it doesn't apply clearly. gpio_keys_polled.c has been changed > > > a > > > lot from that time. So, would you like to fix it yourself? > > > > > > Since mine is applied clearly I may do other way around, rebase and > > > fix > > > that patch on top of mine. > > > How does it sound? > > > > Yeah, I noticed that gpio_keys_polled.c has changed meanwhile, so my > > patch has > > to be modifed anyway. If your patch is applied somewhere I can rebase > > my > > change on that and resubmit. > > First of all, how do you feel if Linus applies my change first? I don't mind. Just go ahead and point me somewhere I can base on. > Please, keep me in Cc list for new version if you are going to re- > base/re-send. Will do. Best regards, Alexander
On Thu, 2017-01-12 at 13:51 +0100, Alexander Stein wrote: > On Thursday 12 January 2017 14:15:12, Andy Shevchenko wrote: > > On Thu, 2017-01-12 at 08:27 +0100, Alexander Stein wrote: > > > On Wednesday 11 January 2017 16:14:57, Andy Shevchenko wrote: > > > > On Wed, 2017-01-11 at 15:35 +0200, Andy Shevchenko wrote: > > > > > On Wed, 2017-01-11 at 13:21 +0100, Linus Walleij wrote: > > > > > > On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko > > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > Alas, it doesn't apply clearly. gpio_keys_polled.c has been > > > > changed > > > > a > > > > lot from that time. So, would you like to fix it yourself? > > > > > > > > Since mine is applied clearly I may do other way around, rebase > > > > and > > > > fix > > > > that patch on top of mine. > > > > How does it sound? > > > Yeah, I noticed that gpio_keys_polled.c has changed meanwhile, so > > > my > > > patch has > > > to be modifed anyway. If your patch is applied somewhere I can > > > rebase > > > my > > > change on that and resubmit. > > First of all, how do you feel if Linus applies my change first? > I don't mind. Just go ahead and point me somewhere I can base on. For now the only my topic/uart/rpm branch [1] on bitbucket contains it. (It also has many other patches, meaning you need to base your branch on top of gpio-next + cherry-picked the only one from my branch) [1] https://bitbucket.org/andy-shev/linux/branch/topic%2Fuart%2frpm
On Mon, Jan 9, 2017 at 3:02 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Make fwnode_get_named_gpiod() consistent with the rest of gpiod_get() like API, > i.e. configure GPIO pin immediately after request. > > Besides obvious clean up it will help to configure pins based on firmware > provided resources. > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Patch applied. Will apply Alex' patch on top. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/devres.c b/drivers/gpio/devres.c index 54da61112752..3da9c39fed04 100644 --- a/drivers/gpio/devres.c +++ b/drivers/gpio/devres.c @@ -129,13 +129,18 @@ EXPORT_SYMBOL(devm_gpiod_get_index); * @dev: GPIO consumer * @con_id: function within the GPIO consumer * @child: firmware node (child of @dev) + * @flags: GPIO initialization flags * * GPIO descriptors returned from this function are automatically disposed on * driver detach. + * + * On successfull request the GPIO pin is configured in accordance with + * provided @flags. */ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, const char *con_id, - struct fwnode_handle *child) + struct fwnode_handle *child, + enum gpiod_flags flags) { char prop_name[32]; /* 32 is max size of property name */ struct gpio_desc **dr; @@ -155,7 +160,7 @@ struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, snprintf(prop_name, sizeof(prop_name), "%s", gpio_suffixes[i]); - desc = fwnode_get_named_gpiod(child, prop_name); + desc = fwnode_get_named_gpiod(child, prop_name, flags); if (!IS_ERR(desc) || (PTR_ERR(desc) != -ENOENT)) break; } diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 7f51c9bf5533..38e415642e03 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3303,6 +3303,7 @@ EXPORT_SYMBOL_GPL(gpiod_get_index); * fwnode_get_named_gpiod - obtain a GPIO from firmware node * @fwnode: handle of the firmware node * @propname: name of the firmware property representing the GPIO + * @dflags: GPIO initialization flags * * This function can be used for drivers that get their configuration * from firmware. @@ -3311,12 +3312,17 @@ EXPORT_SYMBOL_GPL(gpiod_get_index); * underlying firmware interface and then makes sure that the GPIO * descriptor is requested before it is returned to the caller. * + * On successfull request the GPIO pin is configured in accordance with + * provided @dflags. + * * In case of error an ERR_PTR() is returned. */ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, - const char *propname) + const char *propname, + enum gpiod_flags dflags) { struct gpio_desc *desc = ERR_PTR(-ENODEV); + unsigned long lflags = 0; bool active_low = false; bool single_ended = false; int ret; @@ -3349,13 +3355,19 @@ struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, return ERR_PTR(ret); if (active_low) - set_bit(FLAG_ACTIVE_LOW, &desc->flags); + lflags |= GPIO_ACTIVE_LOW; if (single_ended) { if (active_low) - set_bit(FLAG_OPEN_DRAIN, &desc->flags); + lflags |= GPIO_OPEN_DRAIN; else - set_bit(FLAG_OPEN_SOURCE, &desc->flags); + lflags |= GPIO_OPEN_SOURCE; + } + + ret = gpiod_configure_flags(desc, propname, lflags, dflags); + if (ret < 0) { + gpiod_put(desc); + return ERR_PTR(ret); } return desc; diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c index 582462d0af75..9de4b876100a 100644 --- a/drivers/input/keyboard/gpio_keys.c +++ b/drivers/input/keyboard/gpio_keys.c @@ -481,7 +481,7 @@ static int gpio_keys_setup_key(struct platform_device *pdev, spin_lock_init(&bdata->lock); if (child) { - bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child); + bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, child, GPIOD_IN); if (IS_ERR(bdata->gpiod)) { error = PTR_ERR(bdata->gpiod); if (error == -ENOENT) { @@ -496,13 +496,6 @@ static int gpio_keys_setup_key(struct platform_device *pdev, error); return error; } - } else { - error = gpiod_direction_input(bdata->gpiod); - if (error) { - dev_err(dev, "Failed to configure GPIO %d as input: %d\n", - desc_to_gpio(bdata->gpiod), error); - return error; - } } } else if (gpio_is_valid(button->gpio)) { /* diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c index bed4f2086158..fd7ab4dad87b 100644 --- a/drivers/input/keyboard/gpio_keys_polled.c +++ b/drivers/input/keyboard/gpio_keys_polled.c @@ -304,7 +304,8 @@ static int gpio_keys_polled_probe(struct platform_device *pdev) } bdata->gpiod = devm_get_gpiod_from_child(dev, NULL, - child); + child, + GPIOD_IN); if (IS_ERR(bdata->gpiod)) { error = PTR_ERR(bdata->gpiod); if (error != -EPROBE_DEFER) @@ -314,14 +315,6 @@ static int gpio_keys_polled_probe(struct platform_device *pdev) fwnode_handle_put(child); return error; } - - error = gpiod_direction_input(bdata->gpiod); - if (error) { - dev_err(dev, "Failed to configure GPIO %d as input: %d\n", - desc_to_gpio(bdata->gpiod), error); - fwnode_handle_put(child); - return error; - } } else if (gpio_is_valid(button->gpio)) { /* * Legacy GPIO number so request the GPIO here and diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c index d400dcaf4d29..00cc671cddcc 100644 --- a/drivers/leds/leds-gpio.c +++ b/drivers/leds/leds-gpio.c @@ -174,7 +174,7 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev) const char *state = NULL; struct device_node *np = to_of_node(child); - led.gpiod = devm_get_gpiod_from_child(dev, NULL, child); + led.gpiod = devm_get_gpiod_from_child(dev, NULL, child, GPIOD_ASIS); if (IS_ERR(led.gpiod)) { fwnode_handle_put(child); return ERR_CAST(led.gpiod); diff --git a/drivers/video/fbdev/amba-clcd-nomadik.c b/drivers/video/fbdev/amba-clcd-nomadik.c index 0c06fcaaa6e8..2e800d70b0e5 100644 --- a/drivers/video/fbdev/amba-clcd-nomadik.c +++ b/drivers/video/fbdev/amba-clcd-nomadik.c @@ -184,32 +184,27 @@ static void tpg110_init(struct device *dev, struct device_node *np, { dev_info(dev, "TPG110 display init\n"); - grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode); + /* This asserts the GRESTB signal, putting the display into reset */ + grestb = devm_get_gpiod_from_child(dev, "grestb", &np->fwnode, GPIOD_OUT_HIGH); if (IS_ERR(grestb)) { dev_err(dev, "no GRESTB GPIO\n"); return; } - /* This asserts the GRESTB signal, putting the display into reset */ - gpiod_direction_output(grestb, 1); - - scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode); + scen = devm_get_gpiod_from_child(dev, "scen", &np->fwnode, GPIOD_OUT_LOW); if (IS_ERR(scen)) { dev_err(dev, "no SCEN GPIO\n"); return; } - gpiod_direction_output(scen, 0); - scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode); + scl = devm_get_gpiod_from_child(dev, "scl", &np->fwnode, GPIOD_OUT_LOW); if (IS_ERR(scl)) { dev_err(dev, "no SCL GPIO\n"); return; } - gpiod_direction_output(scl, 0); - sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode); + sda = devm_get_gpiod_from_child(dev, "sda", &np->fwnode, GPIOD_OUT_LOW); if (IS_ERR(sda)) { dev_err(dev, "no SDA GPIO\n"); return; } - gpiod_direction_output(sda, 0); board->enable = tpg110_enable; board->disable = tpg110_disable; } diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index fb0fde686cb1..930d10049d8d 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -135,10 +135,12 @@ int desc_to_gpio(const struct gpio_desc *desc); struct fwnode_handle; struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, - const char *propname); + const char *propname, + enum gpiod_flags dflags); struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, const char *con_id, - struct fwnode_handle *child); + struct fwnode_handle *child, + enum gpiod_flags flags); #else /* CONFIG_GPIOLIB */ static inline int gpiod_count(struct device *dev, const char *con_id) @@ -411,14 +413,19 @@ static inline int desc_to_gpio(const struct gpio_desc *desc) /* Child properties interface */ struct fwnode_handle; -static inline struct gpio_desc *fwnode_get_named_gpiod( - struct fwnode_handle *fwnode, const char *propname) +static inline +struct gpio_desc *fwnode_get_named_gpiod(struct fwnode_handle *fwnode, + const char *propname, + enum gpiod_flags dflags) { return ERR_PTR(-ENOSYS); } -static inline struct gpio_desc *devm_get_gpiod_from_child( - struct device *dev, const char *con_id, struct fwnode_handle *child) +static inline +struct gpio_desc *devm_get_gpiod_from_child(struct device *dev, + const char *con_id, + struct fwnode_handle *child, + enum gpiod_flags flags) { return ERR_PTR(-ENOSYS); }