| Message ID | 20230904073410.5880-1-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] gpiolib: rename gpio_set_debounce_timeout() for consistency | expand |
On Mon, Sep 4, 2023 at 11:25 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Sep 04, 2023 at 09:34:09AM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > All other functions that manipulate a struct gpio_desc use the gpiod_ > > prefix. Follow this convention and rename gpio_set_debounce_timeout() to > > gpiod_set_debounce_timeout(). > > No, that's not true. This one is inline with the other gpio_set() _internal_ > APIs. If renamed, should be done consistently. > All the other ones are static to gpiolib.c. With static symbols the naming convention is a bit more relaxed throughout the kernel. But I do agree and I will get to them in time. :) Bart
On Mon, Sep 4, 2023 at 11:27 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, Sep 04, 2023 at 09:34:10AM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > All other functions that manipulate a struct gpio_desc use the gpiod_ > > prefix. Follow this convention and rename gpio_chip_hwgpio() to > > gpiod_get_hwgpio(). > > Same comment. Also, I don't think it's good idea as it steps on the exported > API's toes. I.o.w. I won't mix those two. > Even if I agreed with your other comment, gpio_chip_hwgpio() is a terrible name and if I didn't know, I couldn't tell you what it does just from looking at the name. Bart
On Tue, Sep 5, 2023 at 10:37 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > gpio_chip_hwgpio() is a > terrible name and if I didn't know, I couldn't tell you what it does > just from looking at the name. Probably my fault. "For the chip containing this desc obtain the corresponding hardware GPIO number/offset" is what I would put in kerneldoc. (Which is by the way also horrible.) Let's rename it to something that says clearly what it does. (Rusty Russell's API design hierarchy.) I guess I would just name it something like *_get_chip_hw_offset() or *_get_chip_hwgpio_offset() but it gets a bit long, maybe it is a bit talkative but easy to understand. (I stay off the prefix discussion.) Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index fbda452fb4d6..4a390d8f6544 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -341,7 +341,7 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip, return desc; /* ACPI uses hundredths of milliseconds units */ - ret = gpio_set_debounce_timeout(desc, agpio->debounce_timeout * 10); + ret = gpiod_set_debounce_timeout(desc, agpio->debounce_timeout * 10); if (ret) dev_warn(chip->parent, "Failed to set debounce-timeout for pin 0x%04X, err %d\n", @@ -1087,7 +1087,8 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *name, in return ret; /* ACPI uses hundredths of milliseconds units */ - ret = gpio_set_debounce_timeout(desc, info.debounce * 10); + ret = gpiod_set_debounce_timeout(desc, + info.debounce * 10); if (ret) return ret; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index edffa0d2acaa..6fab0f211e67 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2377,7 +2377,7 @@ static int gpio_set_bias(struct gpio_desc *desc) } /** - * gpio_set_debounce_timeout() - Set debounce timeout + * gpiod_set_debounce_timeout() - Set debounce timeout * @desc: GPIO descriptor to set the debounce timeout * @debounce: Debounce timeout in microseconds * @@ -2386,7 +2386,7 @@ static int gpio_set_bias(struct gpio_desc *desc) * * Returns 0 on success, or negative error code otherwise. */ -int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce) +int gpiod_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce) { return gpio_set_config_with_argument_optional(desc, PIN_CONFIG_INPUT_DEBOUNCE, diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 9bff5c2cf720..9ea5b88ad304 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -219,7 +219,7 @@ static inline int gpiod_request_user(struct gpio_desc *desc, const char *label) int gpiod_configure_flags(struct gpio_desc *desc, const char *con_id, unsigned long lflags, enum gpiod_flags dflags); -int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce); +int gpiod_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce); int gpiod_hog(struct gpio_desc *desc, const char *name, unsigned long lflags, enum gpiod_flags dflags); int gpiochip_get_ngpios(struct gpio_chip *gc, struct device *dev);