| Message ID | 20250303160341.1322640-3-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: Reduce 'gpio' namespace when operate over GPIOd | expand |
On Tue, Mar 04, 2025 at 11:18:04AM +0200, Mika Westerberg wrote: > On Mon, Mar 03, 2025 at 06:00:33PM +0200, Andy Shevchenko wrote: > > In order to reduce the 'gpio' namespace when operate over GPIO descriptor > > rename gpio_set_debounce_timeout() to gpiod_do_set_debounce(). > > To me anything that has '_do_' in their name sounds like an internal static > function that gets wrapped by the actual API function(s). > > For instance it could be > > int gpio_set_debounce_timeout() > { > ... > gpiod_do_set_debounce() > ... > > However, gpiod_set_debounce_timeout() or gpiod_set_debounce() sounds good > to me. Then please propose the second name for gpiod_set_config_XXX to follow the same pattern. The series unifies naming and reduces the current inconsistency.
On Tue, Mar 04, 2025 at 01:16:54PM +0200, Andy Shevchenko wrote: > On Tue, Mar 04, 2025 at 01:11:57PM +0200, Mika Westerberg wrote: > > On Tue, Mar 04, 2025 at 12:59:25PM +0200, Andy Shevchenko wrote: > > > On Tue, Mar 04, 2025 at 11:18:04AM +0200, Mika Westerberg wrote: > > > > On Mon, Mar 03, 2025 at 06:00:33PM +0200, Andy Shevchenko wrote: > > > > > In order to reduce the 'gpio' namespace when operate over GPIO descriptor > > > > > rename gpio_set_debounce_timeout() to gpiod_do_set_debounce(). > > > > > > > > To me anything that has '_do_' in their name sounds like an internal static > > > > function that gets wrapped by the actual API function(s). > > > > > > > > For instance it could be > > > > > > > > int gpio_set_debounce_timeout() > > > > { > > > > ... > > > > gpiod_do_set_debounce() > > > > ... > > > > > > > > However, gpiod_set_debounce_timeout() or gpiod_set_debounce() sounds good > > > > to me. > > > > > > Then please propose the second name for gpiod_set_config_XXX to follow > > > the same pattern. The series unifies naming and reduces the current > > > inconsistency. > > > gpiod_set_config()? > > The problem is that > > gpiod_set_debounce() and gpiod_set_config() are _existing_ public APIs. > That's why I considered "_do_" fitting the purpose. I see. Hmm, we have: int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce) { unsigned long config; config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce); return gpiod_set_config(desc, config); } and int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce) { int ret; ret = gpio_set_config_with_argument_optional(desc, PIN_CONFIG_INPUT_DEBOUNCE, debounce); if (!ret) gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG); return ret; } I wonder if there is an opportunity to consolidate? ;-)
On Tue, Mar 04, 2025 at 01:31:35PM +0200, Mika Westerberg wrote: > On Tue, Mar 04, 2025 at 01:16:54PM +0200, Andy Shevchenko wrote: > > On Tue, Mar 04, 2025 at 01:11:57PM +0200, Mika Westerberg wrote: > > > On Tue, Mar 04, 2025 at 12:59:25PM +0200, Andy Shevchenko wrote: > > > > On Tue, Mar 04, 2025 at 11:18:04AM +0200, Mika Westerberg wrote: > > > > > On Mon, Mar 03, 2025 at 06:00:33PM +0200, Andy Shevchenko wrote: > > > > > > In order to reduce the 'gpio' namespace when operate over GPIO descriptor > > > > > > rename gpio_set_debounce_timeout() to gpiod_do_set_debounce(). > > > > > > > > > > To me anything that has '_do_' in their name sounds like an internal static > > > > > function that gets wrapped by the actual API function(s). > > > > > > > > > > For instance it could be > > > > > > > > > > int gpio_set_debounce_timeout() > > > > > { > > > > > ... > > > > > gpiod_do_set_debounce() > > > > > ... > > > > > > > > > > However, gpiod_set_debounce_timeout() or gpiod_set_debounce() sounds good > > > > > to me. > > > > > > > > Then please propose the second name for gpiod_set_config_XXX to follow > > > > the same pattern. The series unifies naming and reduces the current > > > > inconsistency. > > > > > gpiod_set_config()? > > > > The problem is that > > > > gpiod_set_debounce() and gpiod_set_config() are _existing_ public APIs. > > That's why I considered "_do_" fitting the purpose. > > I see. > > Hmm, we have: > > int gpiod_set_debounce(struct gpio_desc *desc, unsigned int debounce) > { > unsigned long config; > > config = pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE, debounce); > return gpiod_set_config(desc, config); > } > > and > > int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce) > { > int ret; > > ret = gpio_set_config_with_argument_optional(desc, > PIN_CONFIG_INPUT_DEBOUNCE, > debounce); > if (!ret) > gpiod_line_state_notify(desc, GPIO_V2_LINE_CHANGED_CONFIG); > > return ret; > } > > I wonder if there is an opportunity to consolidate? ;-) Send a patch! I would be glad to see less functions and internal APIs in GPIOLIB.
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 2aa88ace5868..3f442127222d 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -340,7 +340,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_do_set_debounce(desc, agpio->debounce_timeout * 10); if (ret) dev_warn(chip->parent, "Failed to set debounce-timeout for pin 0x%04X, err %d\n", @@ -1098,7 +1098,7 @@ int acpi_dev_gpio_irq_wake_get_by(struct acpi_device *adev, const char *con_id, return ret; /* ACPI uses hundredths of milliseconds units */ - ret = gpio_set_debounce_timeout(desc, info.debounce * 10); + ret = gpiod_do_set_debounce(desc, info.debounce * 10); if (ret) return ret; diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index df5b85284788..8980eef6802c 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -2697,7 +2697,7 @@ static int gpio_set_bias(struct gpio_desc *desc) } /** - * gpio_set_debounce_timeout() - Set debounce timeout + * gpiod_do_set_debounce() - Set debounce timeout * @desc: GPIO descriptor to set the debounce timeout * @debounce: Debounce timeout in microseconds * @@ -2707,7 +2707,7 @@ static int gpio_set_bias(struct gpio_desc *desc) * Returns: * 0 on success, or negative errno on failure. */ -int gpio_set_debounce_timeout(struct gpio_desc *desc, unsigned int debounce) +int gpiod_do_set_debounce(struct gpio_desc *desc, unsigned int debounce) { int ret; diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h index 65db879d1c74..b3ea7b710995 100644 --- a/drivers/gpio/gpiolib.h +++ b/drivers/gpio/gpiolib.h @@ -263,7 +263,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer, int gpio_do_set_config(struct gpio_desc *desc, unsigned long config); 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_do_set_debounce(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);
In order to reduce the 'gpio' namespace when operate over GPIO descriptor rename gpio_set_debounce_timeout() to gpiod_do_set_debounce(). Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib-acpi.c | 4 ++-- drivers/gpio/gpiolib.c | 4 ++-- drivers/gpio/gpiolib.h | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)