diff mbox series

[v6,06/12] gpiolib: Add gpio_get_property_name_length()

Message ID 20240904090016.2841572-7-wenst@chromium.org
State Not Applicable
Delegated to: Wolfram Sang
Headers show
Series platform/chrome: Introduce DT hardware prober | expand

Commit Message

Chen-Yu Tsai Sept. 4, 2024, 9 a.m. UTC
The I2C device tree component prober needs to get and toggle GPIO lines
for the components it intends to probe. These components may not use the
same name for their GPIO lines, so the prober must go through the device
tree, check each property to see it is a GPIO property, and get the GPIO
line.

Instead of duplicating the GPIO suffixes, or exporting them to the
prober to do pattern matching, simply add and export a new function that
does the pattern matching and returns the length of the GPIO name. The
caller can then use that to copy out the name if it needs to.

Andy suggested a much shorter implementation.

Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

---
Changes since v5:
- Changed function name to "gpio_get_property_name_length()"
- Changed argument name to "propname"
- Clarified return value for "*-<GPIO suffix>" case
- Reworked according to Andy's suggestion
- Added stub function

Changes since v4:
- new patch
---
 drivers/gpio/gpiolib.c        | 25 +++++++++++++++++++++++++
 include/linux/gpio/consumer.h |  7 +++++++
 2 files changed, 32 insertions(+)

Comments

Andy Shevchenko Sept. 4, 2024, 1:40 p.m. UTC | #1
On Wed, Sep 04, 2024 at 05:00:08PM +0800, Chen-Yu Tsai wrote:
> The I2C device tree component prober needs to get and toggle GPIO lines
> for the components it intends to probe. These components may not use the
> same name for their GPIO lines, so the prober must go through the device
> tree, check each property to see it is a GPIO property, and get the GPIO
> line.
> 
> Instead of duplicating the GPIO suffixes, or exporting them to the
> prober to do pattern matching, simply add and export a new function that
> does the pattern matching and returns the length of the GPIO name. The
> caller can then use that to copy out the name if it needs to.

> Andy suggested a much shorter implementation.

No need to have this sentence in the commit message, changelog area is fine.
But if you wish... :-)

> Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>

...

> +/**
> + * gpio_get_property_name_length - Returns the GPIO name length from a property name
> + * @propname:	name of the property to check
> + *
> + * This function checks if the given property name matches the GPIO property
> + * patterns, and returns the length of the name of the GPIO. The pattern is
> + * "*-<GPIO suffix>" or just "<GPIO suffix>".
> + *
> + * Returns:
> + * The length of the string before '-<GPIO suffix>' if it matches
> + * "*-<GPIO suffix>", or 0 if no name part, just the suffix, or
> + * -EINVAL if the string doesn't match the pattern.

Should be %-EINVAL as we agreed with Bart when I updated GPIOLIB kernel-doc.

> + */
Chen-Yu Tsai Sept. 9, 2024, 2:45 a.m. UTC | #2
On Wed, Sep 4, 2024 at 9:40 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Sep 04, 2024 at 05:00:08PM +0800, Chen-Yu Tsai wrote:
> > The I2C device tree component prober needs to get and toggle GPIO lines
> > for the components it intends to probe. These components may not use the
> > same name for their GPIO lines, so the prober must go through the device
> > tree, check each property to see it is a GPIO property, and get the GPIO
> > line.
> >
> > Instead of duplicating the GPIO suffixes, or exporting them to the
> > prober to do pattern matching, simply add and export a new function that
> > does the pattern matching and returns the length of the GPIO name. The
> > caller can then use that to copy out the name if it needs to.
>
> > Andy suggested a much shorter implementation.
>
> No need to have this sentence in the commit message, changelog area is fine.
> But if you wish... :-)

It does seem out of place without any context. I'll move it to the
changelog area. :D

> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
>
> ...
>
> > +/**
> > + * gpio_get_property_name_length - Returns the GPIO name length from a property name
> > + * @propname:        name of the property to check
> > + *
> > + * This function checks if the given property name matches the GPIO property
> > + * patterns, and returns the length of the name of the GPIO. The pattern is
> > + * "*-<GPIO suffix>" or just "<GPIO suffix>".
> > + *
> > + * Returns:
> > + * The length of the string before '-<GPIO suffix>' if it matches
> > + * "*-<GPIO suffix>", or 0 if no name part, just the suffix, or
> > + * -EINVAL if the string doesn't match the pattern.
>
> Should be %-EINVAL as we agreed with Bart when I updated GPIOLIB kernel-doc.

Ack.

In the regulator cleanups I did, I used -%EINVAL instead. But then I
realized that constants aren't really cross-referenced. I probably
have to go through all of them to fix those up.


ChenYu
Chen-Yu Tsai Sept. 11, 2024, 7:37 a.m. UTC | #3
On Mon, Sep 9, 2024 at 10:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> On Wed, Sep 4, 2024 at 9:40 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > On Wed, Sep 04, 2024 at 05:00:08PM +0800, Chen-Yu Tsai wrote:
> > > The I2C device tree component prober needs to get and toggle GPIO lines
> > > for the components it intends to probe. These components may not use the
> > > same name for their GPIO lines, so the prober must go through the device
> > > tree, check each property to see it is a GPIO property, and get the GPIO
> > > line.
> > >
> > > Instead of duplicating the GPIO suffixes, or exporting them to the
> > > prober to do pattern matching, simply add and export a new function that
> > > does the pattern matching and returns the length of the GPIO name. The
> > > caller can then use that to copy out the name if it needs to.
> >
> > > Andy suggested a much shorter implementation.
> >
> > No need to have this sentence in the commit message, changelog area is fine.
> > But if you wish... :-)
>
> It does seem out of place without any context. I'll move it to the
> changelog area. :D
>
> > > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> >
> > ...
> >
> > > +/**
> > > + * gpio_get_property_name_length - Returns the GPIO name length from a property name
> > > + * @propname:        name of the property to check
> > > + *
> > > + * This function checks if the given property name matches the GPIO property
> > > + * patterns, and returns the length of the name of the GPIO. The pattern is
> > > + * "*-<GPIO suffix>" or just "<GPIO suffix>".
> > > + *
> > > + * Returns:
> > > + * The length of the string before '-<GPIO suffix>' if it matches
> > > + * "*-<GPIO suffix>", or 0 if no name part, just the suffix, or
> > > + * -EINVAL if the string doesn't match the pattern.
> >
> > Should be %-EINVAL as we agreed with Bart when I updated GPIOLIB kernel-doc.
>
> Ack.
>
> In the regulator cleanups I did, I used -%EINVAL instead. But then I
> realized that constants aren't really cross-referenced. I probably
> have to go through all of them to fix those up.

FTR this patch ended up getting dropped from the series as it was no
longer needed. However if folks think there is still value in this patch,
I can still send a new version.


ChenYu
Andy Shevchenko Sept. 11, 2024, 2:39 p.m. UTC | #4
On Wed, Sep 11, 2024 at 03:37:35PM +0800, Chen-Yu Tsai wrote:
> On Mon, Sep 9, 2024 at 10:45 AM Chen-Yu Tsai <wenst@chromium.org> wrote:
> > On Wed, Sep 4, 2024 at 9:40 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Wed, Sep 04, 2024 at 05:00:08PM +0800, Chen-Yu Tsai wrote:

...

> > > > +/**
> > > > + * gpio_get_property_name_length - Returns the GPIO name length from a property name
> > > > + * @propname:        name of the property to check
> > > > + *
> > > > + * This function checks if the given property name matches the GPIO property
> > > > + * patterns, and returns the length of the name of the GPIO. The pattern is
> > > > + * "*-<GPIO suffix>" or just "<GPIO suffix>".
> > > > + *
> > > > + * Returns:
> > > > + * The length of the string before '-<GPIO suffix>' if it matches
> > > > + * "*-<GPIO suffix>", or 0 if no name part, just the suffix, or
> > > > + * -EINVAL if the string doesn't match the pattern.
> > >
> > > Should be %-EINVAL as we agreed with Bart when I updated GPIOLIB kernel-doc.
> >
> > Ack.
> >
> > In the regulator cleanups I did, I used -%EINVAL instead. But then I
> > realized that constants aren't really cross-referenced. I probably
> > have to go through all of them to fix those up.
> 
> FTR this patch ended up getting dropped from the series as it was no
> longer needed.

That's great news!

> However if folks think there is still value in this patch,
> I can still send a new version.

Not for me, thanks.
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 3903d0a75304..86527cc7991b 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4295,6 +4295,31 @@  struct gpio_desc *fwnode_gpiod_get_index(struct fwnode_handle *fwnode,
 }
 EXPORT_SYMBOL_GPL(fwnode_gpiod_get_index);
 
+/**
+ * gpio_get_property_name_length - Returns the GPIO name length from a property name
+ * @propname:	name of the property to check
+ *
+ * This function checks if the given property name matches the GPIO property
+ * patterns, and returns the length of the name of the GPIO. The pattern is
+ * "*-<GPIO suffix>" or just "<GPIO suffix>".
+ *
+ * Returns:
+ * The length of the string before '-<GPIO suffix>' if it matches
+ * "*-<GPIO suffix>", or 0 if no name part, just the suffix, or
+ * -EINVAL if the string doesn't match the pattern.
+ */
+int gpio_get_property_name_length(const char *propname)
+{
+	const char *dash = strrchr(propname, '-');
+
+	for (const char *const *p = gpio_suffixes; *p; p++)
+		if (!strcmp(dash ? dash + 1 : propname, *p))
+			return dash ? dash - propname : 0;
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(gpio_get_property_name_length);
+
 /**
  * gpiod_count - return the number of GPIOs associated with a device / function
  *		or -ENOENT if no GPIO has been assigned to the requested function
diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h
index db2dfbae8edb..494dde33ca44 100644
--- a/include/linux/gpio/consumer.h
+++ b/include/linux/gpio/consumer.h
@@ -56,6 +56,8 @@  enum gpiod_flags {
 
 #ifdef CONFIG_GPIOLIB
 
+int gpio_get_property_name_length(const char *propname);
+
 /* Return the number of GPIOs associated with a device / function */
 int gpiod_count(struct device *dev, const char *con_id);
 
@@ -188,6 +190,11 @@  struct gpio_desc *devm_fwnode_gpiod_get_index(struct device *dev,
 
 #include <asm/bug.h>
 
+static inline int gpio_get_property_name_length(const char *propname)
+{
+	return ERR_PTR(-ENOSYS);
+}
+
 static inline int gpiod_count(struct device *dev, const char *con_id)
 {
 	return 0;