[PATCH v1 0/3] gpiolib: Reduce 'gpio' namespace when operate over GPIOd

[PATCH v1 0/3] gpiolib: Reduce 'gpio' namespace when operate over GPIOd

[Andy Shevchenko]

In order to reduce the 'gpio' namespace when operate over GPIO descriptor
rename a couple of functions.

The choice of the name in patch 2 is inspired by the existing
gpio_do_set_config() versus gpiod_set_config(). The patch 3
also fixes it to be gpiod_do_set_config(), so we establish
two namespaces here:
- gpiod_do_foo() for the internal APIs
- gpiod_foo() for the external APIs
for whatever foo that makes sense.

While at it, the ad-hoc amendment to the FLAG_* definitions to increase
readability. No functional changes intended nor made.

Andy Shevchenko (3):
  gpiolib: Align FLAG_* definitions in the struct gpio_desc
  gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()
  gpiolib: Rename gpio_do_set_config() --> gpiod_do_set_config()

 drivers/gpio/gpiolib-acpi.c |  4 ++--
 drivers/gpio/gpiolib-cdev.c |  2 +-
 drivers/gpio/gpiolib.c      | 10 +++++-----
 drivers/gpio/gpiolib.h      | 40 ++++++++++++++++++-------------------
 4 files changed, 28 insertions(+), 28 deletions(-)

-- 
2.47.2

[PATCH v1 1/3] gpiolib: Align FLAG_* definitions in the struct gpio_desc

[Andy Shevchenko]

Align FLAG_* definitions in the struct gpio_desc for better readability.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib.h | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 7663c9caebaf..65db879d1c74 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -183,24 +183,24 @@ struct gpio_desc {
 	struct gpio_device	*gdev;
 	unsigned long		flags;
 /* flag symbols are bit numbers */
-#define FLAG_REQUESTED	0
-#define FLAG_IS_OUT	1
-#define FLAG_EXPORT	2	/* protected by sysfs_lock */
-#define FLAG_SYSFS	3	/* exported via /sys/class/gpio/control */
-#define FLAG_ACTIVE_LOW	6	/* value has active low */
-#define FLAG_OPEN_DRAIN	7	/* Gpio is open drain type */
-#define FLAG_OPEN_SOURCE 8	/* Gpio is open source type */
-#define FLAG_USED_AS_IRQ 9	/* GPIO is connected to an IRQ */
-#define FLAG_IRQ_IS_ENABLED 10	/* GPIO is connected to an enabled IRQ */
-#define FLAG_IS_HOGGED	11	/* GPIO is hogged */
-#define FLAG_TRANSITORY 12	/* GPIO may lose value in sleep or reset */
-#define FLAG_PULL_UP    13	/* GPIO has pull up enabled */
-#define FLAG_PULL_DOWN  14	/* GPIO has pull down enabled */
-#define FLAG_BIAS_DISABLE    15	/* GPIO has pull disabled */
-#define FLAG_EDGE_RISING     16	/* GPIO CDEV detects rising edge events */
-#define FLAG_EDGE_FALLING    17	/* GPIO CDEV detects falling edge events */
-#define FLAG_EVENT_CLOCK_REALTIME	18 /* GPIO CDEV reports REALTIME timestamps in events */
-#define FLAG_EVENT_CLOCK_HTE		19 /* GPIO CDEV reports hardware timestamps in events */
+#define FLAG_REQUESTED			0
+#define FLAG_IS_OUT			1
+#define FLAG_EXPORT			2	/* protected by sysfs_lock */
+#define FLAG_SYSFS			3	/* exported via /sys/class/gpio/control */
+#define FLAG_ACTIVE_LOW			6	/* value has active low */
+#define FLAG_OPEN_DRAIN			7	/* Gpio is open drain type */
+#define FLAG_OPEN_SOURCE		8	/* Gpio is open source type */
+#define FLAG_USED_AS_IRQ		9	/* GPIO is connected to an IRQ */
+#define FLAG_IRQ_IS_ENABLED		10	/* GPIO is connected to an enabled IRQ */
+#define FLAG_IS_HOGGED			11	/* GPIO is hogged */
+#define FLAG_TRANSITORY			12	/* GPIO may lose value in sleep or reset */
+#define FLAG_PULL_UP			13	/* GPIO has pull up enabled */
+#define FLAG_PULL_DOWN			14	/* GPIO has pull down enabled */
+#define FLAG_BIAS_DISABLE		15	/* GPIO has pull disabled */
+#define FLAG_EDGE_RISING		16	/* GPIO CDEV detects rising edge events */
+#define FLAG_EDGE_FALLING		17	/* GPIO CDEV detects falling edge events */
+#define FLAG_EVENT_CLOCK_REALTIME	18	/* GPIO CDEV reports REALTIME timestamps in events */
+#define FLAG_EVENT_CLOCK_HTE		19	/* GPIO CDEV reports hardware timestamps in events */
 
 	/* Connection label */
 	struct gpio_desc_label __rcu *label;
-- 
2.47.2

[PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Andy Shevchenko]

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(-)

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);
-- 
2.47.2

[PATCH v1 3/3] gpiolib: Rename gpio_do_set_config() --> gpiod_do_set_config()

[Andy Shevchenko]

In order to reduce the 'gpio' namespace when operate over GPIO descriptor
rename gpio_do_set_config() to gpiod_do_set_config().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-cdev.c | 2 +-
 drivers/gpio/gpiolib.c      | 6 +++---
 drivers/gpio/gpiolib.h      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index c8b7499a1e6e..68b6e27ce6ef 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -927,7 +927,7 @@ static int debounce_setup(struct line *line, unsigned int debounce_period_us)
 	 * Try hardware. Skip gpiod_set_config() to avoid emitting two
 	 * CHANGED_CONFIG line state events.
 	 */
-	ret = gpio_do_set_config(line->desc,
+	ret = gpiod_do_set_config(line->desc,
 			pinconf_to_config_packed(PIN_CONFIG_INPUT_DEBOUNCE,
 						 debounce_period_us));
 	if (ret != -ENOTSUPP)
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 8980eef6802c..efaf34a30c20 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2599,7 +2599,7 @@ EXPORT_SYMBOL_GPL(gpiochip_free_own_desc);
  * rely on gpio_request() having been called beforehand.
  */
 
-int gpio_do_set_config(struct gpio_desc *desc, unsigned long config)
+int gpiod_do_set_config(struct gpio_desc *desc, unsigned long config)
 {
 	int ret;
 
@@ -2634,7 +2634,7 @@ static int gpio_set_config_with_argument(struct gpio_desc *desc,
 	unsigned long config;
 
 	config = pinconf_to_config_packed(mode, argument);
-	return gpio_do_set_config(desc, config);
+	return gpiod_do_set_config(desc, config);
 }
 
 static int gpio_set_config_with_argument_optional(struct gpio_desc *desc,
@@ -3104,7 +3104,7 @@ int gpiod_set_config(struct gpio_desc *desc, unsigned long config)
 
 	VALIDATE_DESC(desc);
 
-	ret = gpio_do_set_config(desc, config);
+	ret = gpiod_do_set_config(desc, config);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index b3ea7b710995..4f603a530d38 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -260,7 +260,7 @@ struct gpio_desc *gpiod_find_and_request(struct device *consumer,
 					 const char *label,
 					 bool platform_lookup_allowed);
 
-int gpio_do_set_config(struct gpio_desc *desc, unsigned long config);
+int gpiod_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 gpiod_do_set_debounce(struct gpio_desc *desc, unsigned int debounce);
-- 
2.47.2

Re: [PATCH v1 3/3] gpiolib: Rename gpio_do_set_config() --> gpiod_do_set_config()

[Andy Shevchenko]

On Mon, Mar 03, 2025 at 06:00:34PM +0200, Andy Shevchenko wrote:
> In order to reduce the 'gpio' namespace when operate over GPIO descriptor
> rename gpio_do_set_config() to gpiod_do_set_config().

This change was made against my custom tree and I forgot about that.
I will wait for the overall response to this series and if okay I
may issue the correct patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 3/3] gpiolib: Rename gpio_do_set_config() --> gpiod_do_set_config()

[Andy Shevchenko]

On Mon, Mar 03, 2025 at 06:09:21PM +0200, Andy Shevchenko wrote:
> On Mon, Mar 03, 2025 at 06:00:34PM +0200, Andy Shevchenko wrote:
> > In order to reduce the 'gpio' namespace when operate over GPIO descriptor
> > rename gpio_do_set_config() to gpiod_do_set_config().
> 
> This change was made against my custom tree and I forgot about that.
> I will wait for the overall response to this series and if okay I
> may issue the correct patch.

Or I can send that custom change separately as a prerequisite...
Tell me what you prefer.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Linus Walleij]

On Mon, Mar 3, 2025 at 5:03 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> 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>

Fair enough,
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 1/3] gpiolib: Align FLAG_* definitions in the struct gpio_desc

[Linus Walleij]

On Mon, Mar 3, 2025 at 5:03 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Align FLAG_* definitions in the struct gpio_desc for better readability.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 3/3] gpiolib: Rename gpio_do_set_config() --> gpiod_do_set_config()

[Linus Walleij]

On Mon, Mar 3, 2025 at 5:09 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, Mar 03, 2025 at 06:00:34PM +0200, Andy Shevchenko wrote:
> > In order to reduce the 'gpio' namespace when operate over GPIO descriptor
> > rename gpio_do_set_config() to gpiod_do_set_config().
>
> This change was made against my custom tree and I forgot about that.
> I will wait for the overall response to this series and if okay I
> may issue the correct patch.

For the correct patch, with the same idea:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Mika Westerberg]

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.

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Mika Westerberg]

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()?

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Mika Westerberg]

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? ;-)

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Bartosz Golaszewski]

On Tue, Mar 4, 2025 at 1:00 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> 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.
>

I'm definitely in favor of consolidation instead of renaming to
gpiod_go_set_debounce(). If anything a better name would be:
gpiod_set_debounce_nocheck() to indicate the actual functionality.

How about first extending gpio_set_config_with_argument() to take a
boolean "optional" argument and removing
gpio_set_config_with_argument_optional() altogether? Both are internal
to drivers/gpio/ so it would have no effect on consumers.

Bart

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Andy Shevchenko]

On Tue, Mar 04, 2025 at 01:15:02PM +0100, Bartosz Golaszewski wrote:
> On Tue, Mar 4, 2025 at 1:00 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > 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.
> >
> 
> I'm definitely in favor of consolidation instead of renaming to
> gpiod_go_set_debounce(). If anything a better name would be:
> gpiod_set_debounce_nocheck() to indicate the actual functionality.
> 
> How about first extending gpio_set_config_with_argument() to take a
> boolean "optional" argument and removing
> gpio_set_config_with_argument_optional() altogether? Both are internal
> to drivers/gpio/ so it would have no effect on consumers.

Consider this series as a report then, I am not going to spend time on it.
Thank you for the review.

-- 
With Best Regards,
Andy Shevchenko

Re: (subset) [PATCH v1 0/3] gpiolib: Reduce 'gpio' namespace when operate over GPIOd

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 03 Mar 2025 18:00:31 +0200, Andy Shevchenko wrote:
> In order to reduce the 'gpio' namespace when operate over GPIO descriptor
> rename a couple of functions.
> 
> The choice of the name in patch 2 is inspired by the existing
> gpio_do_set_config() versus gpiod_set_config(). The patch 3
> also fixes it to be gpiod_do_set_config(), so we establish
> two namespaces here:
> - gpiod_do_foo() for the internal APIs
> - gpiod_foo() for the external APIs
> for whatever foo that makes sense.
> 
> [...]

Applied, thanks!

[1/3] gpiolib: Align FLAG_* definitions in the struct gpio_desc
      commit: a45faa2aba2cb2b12ad4c732c9f5692db1f7f12f

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH v1 2/3] gpiolib: Rename gpio_set_debounce_timeout() to gpiod_do_set_debounce()

[Bartosz Golaszewski]

On Tue, Mar 4, 2025 at 2:38 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> >
> > I'm definitely in favor of consolidation instead of renaming to
> > gpiod_go_set_debounce(). If anything a better name would be:
> > gpiod_set_debounce_nocheck() to indicate the actual functionality.
> >
> > How about first extending gpio_set_config_with_argument() to take a
> > boolean "optional" argument and removing
> > gpio_set_config_with_argument_optional() altogether? Both are internal
> > to drivers/gpio/ so it would have no effect on consumers.
>
> Consider this series as a report then, I am not going to spend time on it.
> Thank you for the review.
>

No worries. I applied patch 1/3. 3/3 doesn't apply on its own so feel
free to resend it if you still want it.

Bartosz