| Message ID | 20200324135653.6676-5-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gpio: Add GPIO Aggregator | expand |
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > The GPIO Aggregator will need a method to forward a .set_config() call > to its parent gpiochip. This requires obtaining the gpio_chip and > offset for a given gpio_desc. While gpiod_to_chip() is public, > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > needed GPIO offset parameter. > > Hence introduce a public gpiod_set_config() helper, which invokes the > .set_config() callback through a gpio_desc pointer, like is done for > most other gpio_chip callbacks. > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > gpiod_set_config(), to avoid duplication. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > v6: > - New. This is nice, I tried to actually just apply this (you also sent some two cleanups that I tried to apply) byt Yue's cleanup patch commit d18fddff061d2796525e6d4a958cb3d30aed8efd "gpiolib: Remove duplicated function gpio_do_set_config()" makes none of them apply :/ Yours, Linus Walleij
Hi Linus, On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij <linus.walleij@linaro.org> wrote: > On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven > <geert+renesas@glider.be> wrote: > > The GPIO Aggregator will need a method to forward a .set_config() call > > to its parent gpiochip. This requires obtaining the gpio_chip and > > offset for a given gpio_desc. While gpiod_to_chip() is public, > > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > > needed GPIO offset parameter. > > > > Hence introduce a public gpiod_set_config() helper, which invokes the > > .set_config() callback through a gpio_desc pointer, like is done for > > most other gpio_chip callbacks. > > > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > > gpiod_set_config(), to avoid duplication. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- > > v6: > > - New. > > This is nice, I tried to actually just apply this (you also sent some > two cleanups that I tried to apply) byt Yue's cleanup patch > commit d18fddff061d2796525e6d4a958cb3d30aed8efd > "gpiolib: Remove duplicated function gpio_do_set_config()" > makes none of them apply :/ /me confused. That commit was reverted later, so it shouldn't matter. I have just verified, and both my full series and just this single patch, do apply fine to all of current gpio/for-next, linus/master, and next-20200327. They even apply fine to gpio/for-next before or after the two cleanups I sent, too. What am I missing? Thanks! Gr{oetje,eeting}s, Geert
On Fri, Mar 27, 2020 at 9:45 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Mar 26, 2020 at 10:26 PM Linus Walleij <linus.walleij@linaro.org> wrote: > > On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven > > <geert+renesas@glider.be> wrote: > > > The GPIO Aggregator will need a method to forward a .set_config() call > > > to its parent gpiochip. This requires obtaining the gpio_chip and > > > offset for a given gpio_desc. While gpiod_to_chip() is public, > > > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > > > needed GPIO offset parameter. > > > > > > Hence introduce a public gpiod_set_config() helper, which invokes the > > > .set_config() callback through a gpio_desc pointer, like is done for > > > most other gpio_chip callbacks. > > > > > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > > > gpiod_set_config(), to avoid duplication. > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- > > > v6: > > > - New. > > > > This is nice, I tried to actually just apply this (you also sent some > > two cleanups that I tried to apply) byt Yue's cleanup patch > > commit d18fddff061d2796525e6d4a958cb3d30aed8efd > > "gpiolib: Remove duplicated function gpio_do_set_config()" > > makes none of them apply :/ > > /me confused. > > That commit was reverted later, so it shouldn't matter. > > I have just verified, and both my full series and just this single > patch, do apply fine to all of current gpio/for-next, linus/master, and > next-20200327. They even apply fine to gpio/for-next before or after > the two cleanups I sent, too. > > What am I missing? Ah I see, it is because my development branch is based on v5.6-rc1. So I have to merge in a later -rc where this revert is applied so that this applies. Yours, Linus Walleij
On Tue, Mar 24, 2020 at 2:57 PM Geert Uytterhoeven <geert+renesas@glider.be> wrote: > The GPIO Aggregator will need a method to forward a .set_config() call > to its parent gpiochip. This requires obtaining the gpio_chip and > offset for a given gpio_desc. While gpiod_to_chip() is public, > gpio_chip_hwgpio() is not, so there is currently no method to obtain the > needed GPIO offset parameter. > > Hence introduce a public gpiod_set_config() helper, which invokes the > .set_config() callback through a gpio_desc pointer, like is done for > most other gpio_chip callbacks. > > Rewrite the existing gpiod_set_debounce() helper as a wrapper around > gpiod_set_config(), to avoid duplication. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > v6: > - New. Patch applied in preparation for the next kernel cycle so we get Geert's patch stack down. Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index c756602e249c052e..30ea75e972b5a3b1 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -3478,6 +3478,26 @@ int gpiod_direction_output(struct gpio_desc *desc, int value) } EXPORT_SYMBOL_GPL(gpiod_direction_output); +/** + * gpiod_set_config - sets @config for a GPIO + * @desc: descriptor of the GPIO for which to set the configuration + * @config: Same packed config format as generic pinconf + * + * Returns: + * 0 on success, %-ENOTSUPP if the controller doesn't support setting the + * configuration. + */ +int gpiod_set_config(struct gpio_desc *desc, unsigned long config) +{ + struct gpio_chip *chip; + + VALIDATE_DESC(desc); + chip = desc->gdev->chip; + + return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config); +} +EXPORT_SYMBOL_GPL(gpiod_set_config); + /** * gpiod_set_debounce - sets @debounce time for a GPIO * @desc: descriptor of the GPIO for which to set debounce time @@ -3489,14 +3509,10 @@ EXPORT_SYMBOL_GPL(gpiod_direction_output); */ int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) { - struct gpio_chip *chip; - unsigned long config; - - VALIDATE_DESC(desc); - chip = desc->gdev->chip; + unsigned long config; config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce); - return gpio_do_set_config(chip, gpio_chip_hwgpio(desc), config); + return gpiod_set_config(desc, config); } EXPORT_SYMBOL_GPL(gpiod_set_debounce); diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index 0a72fccf60fff230..901aab89d025f3ff 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -157,6 +157,7 @@ int gpiod_set_raw_array_value_cansleep(unsigned int array_size, struct gpio_array *array_info, unsigned long *value_bitmap); +int gpiod_set_config(struct gpio_desc *desc, unsigned long config); int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce); int gpiod_set_transitory(struct gpio_desc *desc, bool transitory); void gpiod_toggle_active_low(struct gpio_desc *desc); @@ -473,6 +474,13 @@ static inline int gpiod_set_raw_array_value_cansleep(unsigned int array_size, return 0; } +static inline int gpiod_set_config(struct gpio_desc *desc, unsigned long config) +{ + /* GPIO can never have been requested */ + WARN_ON(desc); + return -ENOSYS; +} + static inline int gpiod_set_debounce(struct gpio_desc *desc, unsigned debounce) { /* GPIO can never have been requested */
The GPIO Aggregator will need a method to forward a .set_config() call to its parent gpiochip. This requires obtaining the gpio_chip and offset for a given gpio_desc. While gpiod_to_chip() is public, gpio_chip_hwgpio() is not, so there is currently no method to obtain the needed GPIO offset parameter. Hence introduce a public gpiod_set_config() helper, which invokes the .set_config() callback through a gpio_desc pointer, like is done for most other gpio_chip callbacks. Rewrite the existing gpiod_set_debounce() helper as a wrapper around gpiod_set_config(), to avoid duplication. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- v6: - New. --- drivers/gpio/gpiolib.c | 28 ++++++++++++++++++++++------ include/linux/gpio/consumer.h | 8 ++++++++ 2 files changed, 30 insertions(+), 6 deletions(-)