[PATCH v1 00/16] gpio: Use string_choices.h

[PATCH v1 00/16] gpio: Use string_choices.h

[Andy Shevchenko]

Use string_choices.h in the GPIO drivers and library.
It has been tested on x86_64 and (semi-)compile tested
over all.

Andy Shevchenko (16):
  lib/string_helpers: Add missing header files to MAINTAINERS database
  lib/string_helpers: Split out string_choices.h
  lib/string_choices: Add str_high_low() helper
  lib/string_choices: Add str_input_output() helper
  gpiolib: Utilize helpers from string_choices.h
  gpio: adnp: Utilize helpers from string_choices.h
  gpio: brcmstb: Utilize helpers from string_choices.h
  gpio: crystalcove: Utilize helpers from string_choices.h
  gpio: grgpio: Utilize helpers from string_choices.h
  gpio: mvebu: Utilize helpers from string_choices.h
  gpio: pl061: Utilize helpers from string_choices.h
  gpio: stmpe: Utilize helpers from string_choices.h
  gpio: wcove: Utilize helpers from string_choices.h
  gpio: wm831x: Utilize helpers from string_choices.h
  gpio: wm8994: Utilize helpers from string_choices.h
  gpio: xra1403: Utilize helpers from string_choices.h

 MAINTAINERS                     |  3 ++
 drivers/gpio/gpio-adnp.c        | 24 ++++----------
 drivers/gpio/gpio-brcmstb.c     |  3 +-
 drivers/gpio/gpio-crystalcove.c | 17 +++++-----
 drivers/gpio/gpio-grgpio.c      |  3 +-
 drivers/gpio/gpio-mvebu.c       | 27 +++++++---------
 drivers/gpio/gpio-pl061.c       |  4 +--
 drivers/gpio/gpio-stmpe.c       | 19 +++++------
 drivers/gpio/gpio-wcove.c       | 15 ++++-----
 drivers/gpio/gpio-wm831x.c      |  5 +--
 drivers/gpio/gpio-wm8994.c      |  6 ++--
 drivers/gpio/gpio-xra1403.c     |  5 +--
 drivers/gpio/gpiolib-sysfs.c    |  3 +-
 drivers/gpio/gpiolib.c          | 13 ++++----
 include/linux/string_choices.h  | 56 +++++++++++++++++++++++++++++++++
 include/linux/string_helpers.h  | 26 +--------------
 16 files changed, 125 insertions(+), 104 deletions(-)
 create mode 100644 include/linux/string_choices.h

-- 
2.39.1

[PATCH v1 01/16] lib/string_helpers: Add missing header files to MAINTAINERS database

[Andy Shevchenko]

The header files string.h and string_helpers.h are missing in
the MAINTAINERS. Add them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 20544cbdace9..ee157981c4cf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8615,6 +8615,8 @@ F:	drivers/input/touchscreen/resistive-adc-touch.c
 GENERIC STRING LIBRARY
 R:	Andy Shevchenko <andy@kernel.org>
 S:	Maintained
+F:	include/linux/string.h
+F:	include/linux/string_helpers.h
 F:	lib/string.c
 F:	lib/string_helpers.c
 F:	lib/test_string.c
-- 
2.39.1

[PATCH v1 02/16] lib/string_helpers: Split out string_choices.h

[Andy Shevchenko]

Some users may only need the string choice APIs. Split
the respective header, i.e. string_choices.h. Include
it in the string_helpers.h for backward compatibility.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 MAINTAINERS                    |  1 +
 include/linux/string_choices.h | 32 ++++++++++++++++++++++++++++++++
 include/linux/string_helpers.h | 26 +-------------------------
 3 files changed, 34 insertions(+), 25 deletions(-)
 create mode 100644 include/linux/string_choices.h

diff --git a/MAINTAINERS b/MAINTAINERS
index ee157981c4cf..64f19e200191 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8616,6 +8616,7 @@ GENERIC STRING LIBRARY
 R:	Andy Shevchenko <andy@kernel.org>
 S:	Maintained
 F:	include/linux/string.h
+F:	include/linux/string_choices.h
 F:	include/linux/string_helpers.h
 F:	lib/string.c
 F:	lib/string_helpers.c
diff --git a/include/linux/string_choices.h b/include/linux/string_choices.h
new file mode 100644
index 000000000000..b7e7b9fd098c
--- /dev/null
+++ b/include/linux/string_choices.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_STRING_CHOICES_H_
+#define _LINUX_STRING_CHOICES_H_
+
+#include <linux/types.h>
+
+static inline const char *str_enable_disable(bool v)
+{
+	return v ? "enable" : "disable";
+}
+
+static inline const char *str_enabled_disabled(bool v)
+{
+	return v ? "enabled" : "disabled";
+}
+
+static inline const char *str_read_write(bool v)
+{
+	return v ? "read" : "write";
+}
+
+static inline const char *str_on_off(bool v)
+{
+	return v ? "on" : "off";
+}
+
+static inline const char *str_yes_no(bool v)
+{
+	return v ? "yes" : "no";
+}
+
+#endif
diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index fae6beaaa217..0f2255738940 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -5,6 +5,7 @@
 #include <linux/bits.h>
 #include <linux/ctype.h>
 #include <linux/string.h>
+#include <linux/string_choices.h>
 #include <linux/types.h>
 
 struct device;
@@ -113,29 +114,4 @@ void kfree_strarray(char **array, size_t n);
 
 char **devm_kasprintf_strarray(struct device *dev, const char *prefix, size_t n);
 
-static inline const char *str_yes_no(bool v)
-{
-	return v ? "yes" : "no";
-}
-
-static inline const char *str_on_off(bool v)
-{
-	return v ? "on" : "off";
-}
-
-static inline const char *str_enable_disable(bool v)
-{
-	return v ? "enable" : "disable";
-}
-
-static inline const char *str_enabled_disabled(bool v)
-{
-	return v ? "enabled" : "disabled";
-}
-
-static inline const char *str_read_write(bool v)
-{
-	return v ? "read" : "write";
-}
-
 #endif
-- 
2.39.1

[PATCH v1 03/16] lib/string_choices: Add str_high_low() helper

[Andy Shevchenko]

Add str_high_low() helper to return 'high' or 'low' string literal.
Also add an inversed variant, i.e. str_low_high().

All the same for str_hi_low().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string_choices.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/string_choices.h b/include/linux/string_choices.h
index b7e7b9fd098c..48120222b9b2 100644
--- a/include/linux/string_choices.h
+++ b/include/linux/string_choices.h
@@ -14,6 +14,18 @@ static inline const char *str_enabled_disabled(bool v)
 	return v ? "enabled" : "disabled";
 }
 
+static inline const char *str_hi_lo(bool v)
+{
+	return v ? "hi" : "lo";
+}
+#define str_lo_hi(v)		str_hi_lo(!(v))
+
+static inline const char *str_high_low(bool v)
+{
+	return v ? "high" : "low";
+}
+#define str_low_high(v)		str_high_low(!(v))
+
 static inline const char *str_read_write(bool v)
 {
 	return v ? "read" : "write";
-- 
2.39.1

[PATCH v1 04/16] lib/string_choices: Add str_input_output() helper

[Andy Shevchenko]

Add str_input_output() helper to return 'input' or 'output' string literal.
Also add an inversed variant, i.e. str_output_input().

All the same for str_in_out().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string_choices.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/linux/string_choices.h b/include/linux/string_choices.h
index 48120222b9b2..ad345df325ec 100644
--- a/include/linux/string_choices.h
+++ b/include/linux/string_choices.h
@@ -26,6 +26,18 @@ static inline const char *str_high_low(bool v)
 }
 #define str_low_high(v)		str_high_low(!(v))
 
+static inline const char *str_in_out(bool v)
+{
+	return v ? "in" : "out";
+}
+#define str_out_in(v)		str_in_out(!(v))
+
+static inline const char *str_input_output(bool v)
+{
+	return v ? "input" : "output";
+}
+#define str_output_input(v)	str_input_output(!(v))
+
 static inline const char *str_read_write(bool v)
 {
 	return v ? "read" : "write";
-- 
2.39.1

[PATCH v1 05/16] gpiolib: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

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

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index c1cbf71329f0..54dc53b74dc4 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -13,6 +13,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/string.h>
+#include <linux/string_choices.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 
@@ -82,7 +83,7 @@ static ssize_t direction_show(struct device *dev,
 
 	mutex_unlock(&data->mutex);
 
-	return sysfs_emit(buf, "%s\n", value ? "out" : "in");
+	return sysfs_emit(buf, "%s\n", str_out_in(value));
 }
 
 static ssize_t direction_store(struct device *dev,
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 700195a1faae..b554ad435245 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -20,6 +20,7 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/string_choices.h>
 
 #include <linux/gpio.h>
 #include <linux/gpio/driver.h>
@@ -4198,6 +4199,8 @@ EXPORT_SYMBOL_GPL(gpiod_get_index_optional);
 int gpiod_hog(struct gpio_desc *desc, const char *name,
 	      unsigned long lflags, enum gpiod_flags dflags)
 {
+	bool out = dflags & GPIOD_FLAGS_BIT_DIR_OUT;
+	bool hi = dflags & GPIOD_FLAGS_BIT_DIR_VAL;
 	struct gpio_chip *gc;
 	struct gpio_desc *local_desc;
 	int hwnum;
@@ -4218,10 +4221,7 @@ int gpiod_hog(struct gpio_desc *desc, const char *name,
 	/* Mark GPIO as hogged so it can be identified and removed later */
 	set_bit(FLAG_IS_HOGGED, &desc->flags);
 
-	gpiod_info(desc, "hogged as %s%s\n",
-		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ? "output" : "input",
-		(dflags & GPIOD_FLAGS_BIT_DIR_OUT) ?
-		  (dflags & GPIOD_FLAGS_BIT_DIR_VAL) ? "/high" : "/low" : "");
+	gpiod_info(desc, "hogged as %s/%s\n", str_output_input(out), out ? str_high_low(hi) : "?");
 
 	return 0;
 }
@@ -4509,10 +4509,9 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
 			value = gpio_chip_get_value(gc, desc);
 			is_irq = test_bit(FLAG_USED_AS_IRQ, &desc->flags);
 			active_low = test_bit(FLAG_ACTIVE_LOW, &desc->flags);
-			seq_printf(s, " gpio-%-3d (%-20.20s|%-20.20s) %s %s %s%s\n",
+			seq_printf(s, " gpio-%-3d (%-20.20s|%-20.20s) %-3.3s %-2.2s %s%s\n",
 				   gpio, desc->name ?: "", desc->label,
-				   is_out ? "out" : "in ",
-				   value >= 0 ? (value ? "hi" : "lo") : "?  ",
+				   str_out_in(is_out), value < 0 ? "?" : str_hi_lo(value),
 				   is_irq ? "IRQ " : "",
 				   active_low ? "ACTIVE LOW" : "");
 		} else if (desc->name) {
-- 
2.39.1

[PATCH v1 06/16] gpio: adnp: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

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

diff --git a/drivers/gpio/gpio-adnp.c b/drivers/gpio/gpio-adnp.c
index a6439e3daff0..823cad855ee8 100644
--- a/drivers/gpio/gpio-adnp.c
+++ b/drivers/gpio/gpio-adnp.c
@@ -11,6 +11,7 @@
 #include <linux/property.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
+#include <linux/string_choices.h>
 
 #define GPIO_DDR(gpio) (0x00 << (gpio)->reg_shift)
 #define GPIO_PLR(gpio) (0x01 << (gpio)->reg_shift)
@@ -211,25 +212,12 @@ static void adnp_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 
 		for (j = 0; j < 8; j++) {
 			unsigned int bit = (i << adnp->reg_shift) + j;
-			const char *direction = "input ";
-			const char *level = "low ";
-			const char *interrupt = "disabled";
-			const char *pending = "";
 
-			if (ddr & BIT(j))
-				direction = "output";
-
-			if (plr & BIT(j))
-				level = "high";
-
-			if (ier & BIT(j))
-				interrupt = "enabled ";
-
-			if (isr & BIT(j))
-				pending = "pending";
-
-			seq_printf(s, "%2u: %s %s IRQ %s %s\n", bit,
-				   direction, level, interrupt, pending);
+			seq_printf(s, "%2u: %-6.6s %-4.4s IRQ %-8.8s %s\n", bit,
+				   str_output_input(ddr & BIT(j)),
+				   str_high_low(plr & BIT(j)),
+				   str_enabled_disabled(ier & BIT(j)),
+				   (isr & BIT(j)) ? "pending" : "");
 		}
 	}
 
-- 
2.39.1

[PATCH v1 07/16] gpio: brcmstb: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-brcmstb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-brcmstb.c b/drivers/gpio/gpio-brcmstb.c
index c55b35da61a0..a0535be1201c 100644
--- a/drivers/gpio/gpio-brcmstb.c
+++ b/drivers/gpio/gpio-brcmstb.c
@@ -9,6 +9,7 @@
 #include <linux/irqdomain.h>
 #include <linux/irqchip/chained_irq.h>
 #include <linux/interrupt.h>
+#include <linux/string_choices.h>
 
 enum gio_reg_index {
 	GIO_REG_ODEN = 0,
@@ -225,7 +226,7 @@ static int brcmstb_gpio_priv_set_wake(struct brcmstb_gpio_priv *priv,
 		ret = disable_irq_wake(priv->parent_wake_irq);
 	if (ret)
 		dev_err(&priv->pdev->dev, "failed to %s wake-up interrupt\n",
-				enable ? "enable" : "disable");
+			str_enable_disable(enable));
 	return ret;
 }
 
-- 
2.39.1

Re: [PATCH v1 07/16] gpio: brcmstb: Utilize helpers from string_choices.h

[Florian Fainelli]

On 3/6/23 11:55, Andy Shevchenko wrote:
> There are a few helpers available to convert a boolean variable
> to the dedicated string literals depending on the application.
> Use them in the driver.
>  > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

[PATCH v1 08/16] gpio: crystalcove: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-crystalcove.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpio/gpio-crystalcove.c b/drivers/gpio/gpio-crystalcove.c
index 1ee62cd58582..56d628b18397 100644
--- a/drivers/gpio/gpio-crystalcove.c
+++ b/drivers/gpio/gpio-crystalcove.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/seq_file.h>
+#include <linux/string_choices.h>
 #include <linux/types.h>
 
 #define CRYSTALCOVE_GPIO_NUM	16
@@ -315,15 +316,15 @@ static void crystalcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip
 			    &irq);
 
 		offset = gpio % 8;
-		seq_printf(s, " gpio-%-2d %s %s %s %s ctlo=%2x,%s %s %s\n",
-			   gpio, ctlo & CTLO_DIR_OUT ? "out" : "in ",
-			   ctli & 0x1 ? "hi" : "lo",
-			   ctli & CTLI_INTCNT_NE ? "fall" : "    ",
-			   ctli & CTLI_INTCNT_PE ? "rise" : "    ",
+		seq_printf(s, " gpio-%-2d %-3.3s %-2.2s %-4.4s %-4.4s ctlo=%2x,%-9.9s %-9.9s %s\n",
+			   gpio, str_out_in(ctlo & CTLO_DIR_OUT),
+			   str_hi_lo(ctli & BIT(0)),
+			   ctli & CTLI_INTCNT_NE ? "fall" : "",
+			   ctli & CTLI_INTCNT_PE ? "rise" : "",
 			   ctlo,
-			   mirqs0 & BIT(offset) ? "s0 mask  " : "s0 unmask",
-			   mirqsx & BIT(offset) ? "sx mask  " : "sx unmask",
-			   irq & BIT(offset) ? "pending" : "       ");
+			   mirqs0 & BIT(offset) ? "s0 mask" : "s0 unmask",
+			   mirqsx & BIT(offset) ? "sx mask" : "sx unmask",
+			   irq & BIT(offset) ? "pending" : "");
 	}
 }
 
-- 
2.39.1

[PATCH v1 09/16] gpio: grgpio: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-grgpio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-grgpio.c b/drivers/gpio/gpio-grgpio.c
index bea0e32c195d..ccd942109f6a 100644
--- a/drivers/gpio/gpio-grgpio.c
+++ b/drivers/gpio/gpio-grgpio.c
@@ -30,6 +30,7 @@
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/bitops.h>
+#include <linux/string_choices.h>
 
 #define GRGPIO_MAX_NGPIO 32
 
@@ -426,7 +427,7 @@ static int grgpio_probe(struct platform_device *ofdev)
 	}
 
 	dev_info(&ofdev->dev, "regs=0x%p, base=%d, ngpio=%d, irqs=%s\n",
-		 priv->regs, gc->base, gc->ngpio, priv->domain ? "on" : "off");
+		 priv->regs, gc->base, gc->ngpio, str_on_off(priv->domain));
 
 	return 0;
 }
-- 
2.39.1

[PATCH v1 10/16] gpio: mvebu: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-mvebu.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index a68f682aec01..f2bbe928577c 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -48,6 +48,7 @@
 #include <linux/pwm.h>
 #include <linux/regmap.h>
 #include <linux/slab.h>
+#include <linux/string_choices.h>
 
 /*
  * GPIO unit register offsets.
@@ -897,32 +898,28 @@ static void mvebu_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	lvl_msk	= mvebu_gpio_read_level_mask(mvchip);
 
 	for_each_requested_gpio(chip, i, label) {
-		u32 msk;
-		bool is_out;
+		u32 msk = BIT(i);
 
-		msk = BIT(i);
-		is_out = !(io_conf & msk);
+		seq_printf(s, " gpio-%-3d (%-20.20s) %-3.3s ", chip->base + i, label,
+			   str_in_out(io_conf & msk));
 
-		seq_printf(s, " gpio-%-3d (%-20.20s)", chip->base + i, label);
-
-		if (is_out) {
-			seq_printf(s, " out %s %s\n",
-				   out & msk ? "hi" : "lo",
+		if (!(io_conf & msk)) {
+			seq_printf(s, "%-2.2s %s\n", str_hi_lo(out & msk),
 				   blink & msk ? "(blink )" : "");
 			continue;
 		}
 
-		seq_printf(s, " in  %s (act %s) - IRQ",
-			   (data_in ^ in_pol) & msk  ? "hi" : "lo",
-			   in_pol & msk ? "lo" : "hi");
+		seq_printf(s, "%-2.2s (act %-2.2s) - IRQ ",
+			   str_hi_lo((data_in ^ in_pol) & msk),
+			   str_lo_hi(in_pol & msk));
 		if (!((edg_msk | lvl_msk) & msk)) {
-			seq_puts(s, " disabled\n");
+			seq_puts(s, "disabled\n");
 			continue;
 		}
 		if (edg_msk & msk)
-			seq_puts(s, " edge ");
+			seq_puts(s, "edge ");
 		if (lvl_msk & msk)
-			seq_puts(s, " level");
+			seq_puts(s, "level");
 		seq_printf(s, " (%s)\n", cause & msk ? "pending" : "clear  ");
 	}
 }
-- 
2.39.1

[PATCH v1 11/16] gpio: pl061: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-pl061.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 9fc1f3dd4190..99e71b4490a1 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -25,6 +25,7 @@
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/string_choices.h>
 
 #define GPIODIR 0x400
 #define GPIOIS  0x404
@@ -165,8 +166,7 @@ static int pl061_irq_type(struct irq_data *d, unsigned trigger)
 			gpioiev &= ~bit;
 		irq_set_handler_locked(d, handle_level_irq);
 		dev_dbg(gc->parent, "line %d: IRQ on %s level\n",
-			offset,
-			polarity ? "HIGH" : "LOW");
+			offset, str_high_low(polarity));
 	} else if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) {
 		/* Disable level detection */
 		gpiois &= ~bit;
-- 
2.39.1

[PATCH v1 12/16] gpio: stmpe: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-stmpe.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpio-stmpe.c b/drivers/gpio/gpio-stmpe.c
index 0fa4f0a93378..ca8a98b252c2 100644
--- a/drivers/gpio/gpio-stmpe.c
+++ b/drivers/gpio/gpio-stmpe.c
@@ -14,6 +14,7 @@
 #include <linux/mfd/stmpe.h>
 #include <linux/seq_file.h>
 #include <linux/bitops.h>
+#include <linux/string_choices.h>
 
 /*
  * These registers are modified under the irq bus lock and cached to avoid
@@ -264,12 +265,12 @@ static void stmpe_dbg_show_one(struct seq_file *s,
 	ret = stmpe_reg_read(stmpe, dir_reg);
 	if (ret < 0)
 		return;
-	dir = !!(ret & mask);
 
+	dir = !!(ret & mask);
+	seq_printf(s, " gpio-%-3d (%-20.20s) %-3.3s %-2.2s ", gpio,
+		   label ?: "(none)", str_out_in(dir), str_hi_lo(val));
 	if (dir) {
-		seq_printf(s, " gpio-%-3d (%-20.20s) out %s",
-			   gpio, label ?: "(none)",
-			   val ? "hi" : "lo");
+		seq_putc(s, '\n');
 	} else {
 		u8 edge_det_reg;
 		u8 rise_reg;
@@ -336,11 +337,9 @@ static void stmpe_dbg_show_one(struct seq_file *s,
 			return;
 		irqen = !!(ret & mask);
 
-		seq_printf(s, " gpio-%-3d (%-20.20s) in  %s %13s %13s %25s %25s",
-			   gpio, label ?: "(none)",
-			   val ? "hi" : "lo",
+		seq_printf(s, "%13s IRQ-%9s %25s %25s\n",
 			   edge_det_values[edge_det],
-			   irqen ? "IRQ-enabled" : "IRQ-disabled",
+			   str_enabled_disabled(irqen),
 			   rise_values[rise],
 			   fall_values[fall]);
 	}
@@ -351,10 +350,8 @@ static void stmpe_dbg_show(struct seq_file *s, struct gpio_chip *gc)
 	unsigned i;
 	unsigned gpio = gc->base;
 
-	for (i = 0; i < gc->ngpio; i++, gpio++) {
+	for (i = 0; i < gc->ngpio; i++, gpio++)
 		stmpe_dbg_show_one(s, gc, i, gpio);
-		seq_putc(s, '\n');
-	}
 }
 
 static struct irq_chip stmpe_gpio_irq_chip = {
-- 
2.39.1

[PATCH v1 13/16] gpio: wcove: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

While at, utilize specifier field for padding the strings where
it's required.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-wcove.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
index c18b6b47384f..8d30fd04dede 100644
--- a/drivers/gpio/gpio-wcove.c
+++ b/drivers/gpio/gpio-wcove.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 #include <linux/seq_file.h>
+#include <linux/string_choices.h>
 
 /*
  * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
@@ -391,14 +392,14 @@ static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 			break;
 		}
 
-		seq_printf(s, " gpio-%-2d %s %s %s %s ctlo=%2x,%s %s\n",
-			   gpio, ctlo & CTLO_DIR_OUT ? "out" : "in ",
-			   ctli & 0x1 ? "hi" : "lo",
-			   ctli & CTLI_INTCNT_NE ? "fall" : "    ",
-			   ctli & CTLI_INTCNT_PE ? "rise" : "    ",
+		seq_printf(s, " gpio-%-2d %-3.3s %-2.2s %-4.4s %-4.4s ctlo=%2x,%-6.6s %s\n",
+			   gpio, str_out_in(ctlo & CTLO_DIR_OUT),
+			   str_hi_lo(ctli & BIT(0)),
+			   ctli & CTLI_INTCNT_NE ? "fall" : "",
+			   ctli & CTLI_INTCNT_PE ? "rise" : "",
 			   ctlo,
-			   irq_mask & mask ? "mask  " : "unmask",
-			   irq_status & mask ? "pending" : "       ");
+			   irq_mask & mask ? "mask" : "unmask",
+			   irq_status & mask ? "pending" : "");
 	}
 }
 
-- 
2.39.1

Re: [PATCH v1 13/16] gpio: wcove: Utilize helpers from string_choices.h

[Sathyanarayanan Kuppuswamy]

Hi,

On 3/6/23 11:55 AM, Andy Shevchenko wrote:
> There are a few helpers available to convert a boolean variable
> to the dedicated string literals depending on the application.
> Use them in the driver.
> 
> While at, utilize specifier field for padding the strings where
> it's required.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Looks good to me.

Acked-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

> ---
>  drivers/gpio/gpio-wcove.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpio/gpio-wcove.c b/drivers/gpio/gpio-wcove.c
> index c18b6b47384f..8d30fd04dede 100644
> --- a/drivers/gpio/gpio-wcove.c
> +++ b/drivers/gpio/gpio-wcove.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/regmap.h>
>  #include <linux/seq_file.h>
> +#include <linux/string_choices.h>
>  
>  /*
>   * Whiskey Cove PMIC has 13 physical GPIO pins divided into 3 banks:
> @@ -391,14 +392,14 @@ static void wcove_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
>  			break;
>  		}
>  
> -		seq_printf(s, " gpio-%-2d %s %s %s %s ctlo=%2x,%s %s\n",
> -			   gpio, ctlo & CTLO_DIR_OUT ? "out" : "in ",
> -			   ctli & 0x1 ? "hi" : "lo",
> -			   ctli & CTLI_INTCNT_NE ? "fall" : "    ",
> -			   ctli & CTLI_INTCNT_PE ? "rise" : "    ",
> +		seq_printf(s, " gpio-%-2d %-3.3s %-2.2s %-4.4s %-4.4s ctlo=%2x,%-6.6s %s\n",
> +			   gpio, str_out_in(ctlo & CTLO_DIR_OUT),
> +			   str_hi_lo(ctli & BIT(0)),
> +			   ctli & CTLI_INTCNT_NE ? "fall" : "",
> +			   ctli & CTLI_INTCNT_PE ? "rise" : "",
>  			   ctlo,
> -			   irq_mask & mask ? "mask  " : "unmask",
> -			   irq_status & mask ? "pending" : "       ");
> +			   irq_mask & mask ? "mask" : "unmask",
> +			   irq_status & mask ? "pending" : "");
>  	}
>  }
>  

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

[PATCH v1 14/16] gpio: wm831x: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-wm831x.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-wm831x.c b/drivers/gpio/gpio-wm831x.c
index 7eaf8a28638c..5456485ce801 100644
--- a/drivers/gpio/gpio-wm831x.c
+++ b/drivers/gpio/gpio-wm831x.c
@@ -15,6 +15,7 @@
 #include <linux/mfd/core.h>
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
+#include <linux/string_choices.h>
 
 #include <linux/mfd/wm831x/core.h>
 #include <linux/mfd/wm831x/pdata.h>
@@ -229,8 +230,8 @@ static void wm831x_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 
 		seq_printf(s, " %s %s %s %s%s\n"
 			   "                                  %s%s (0x%4x)\n",
-			   reg & WM831X_GPN_DIR ? "in" : "out",
-			   wm831x_gpio_get(chip, i) ? "high" : "low",
+			   str_in_out(reg & WM831X_GPN_DIR),
+			   str_high_low(wm831x_gpio_get(chip, i)),
 			   pull,
 			   powerdomain,
 			   reg & WM831X_GPN_POL ? "" : " inverted",
-- 
2.39.1

Re: [PATCH v1 14/16] gpio: wm831x: Utilize helpers from string_choices.h

[Charles Keepax]

On Mon, Mar 06, 2023 at 09:55:54PM +0200, Andy Shevchenko wrote:
> There are a few helpers available to convert a boolean variable
> to the dedicated string literals depending on the application.
> Use them in the driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 15/16] gpio: wm8994: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-wm8994.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-wm8994.c b/drivers/gpio/gpio-wm8994.c
index f4a474cef32d..15757d94448a 100644
--- a/drivers/gpio/gpio-wm8994.c
+++ b/drivers/gpio/gpio-wm8994.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
 #include <linux/regmap.h>
+#include <linux/string_choices.h>
 
 #include <linux/mfd/wm8994/core.h>
 #include <linux/mfd/wm8994/pdata.h>
@@ -215,10 +216,7 @@ static void wm8994_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 			continue;
 		}
 
-		if (reg & WM8994_GPN_DIR)
-			seq_printf(s, "in ");
-		else
-			seq_printf(s, "out ");
+		seq_printf(s, "%s ", str_in_out(reg & WM8994_GPN_DIR));
 
 		if (reg & WM8994_GPN_PU)
 			seq_printf(s, "pull up ");
-- 
2.39.1

Re: [PATCH v1 15/16] gpio: wm8994: Utilize helpers from string_choices.h

[Charles Keepax]

On Mon, Mar 06, 2023 at 09:55:55PM +0200, Andy Shevchenko wrote:
> There are a few helpers available to convert a boolean variable
> to the dedicated string literals depending on the application.
> Use them in the driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 16/16] gpio: xra1403: Utilize helpers from string_choices.h

[Andy Shevchenko]

There are a few helpers available to convert a boolean variable
to the dedicated string literals depending on the application.
Use them in the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-xra1403.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-xra1403.c b/drivers/gpio/gpio-xra1403.c
index 49c878cfd5c6..bc17beaba9e7 100644
--- a/drivers/gpio/gpio-xra1403.c
+++ b/drivers/gpio/gpio-xra1403.c
@@ -15,6 +15,7 @@
 #include <linux/seq_file.h>
 #include <linux/spi/spi.h>
 #include <linux/regmap.h>
+#include <linux/string_choices.h>
 
 /* XRA1403 registers */
 #define XRA_GSR   0x00 /* GPIO State */
@@ -140,8 +141,8 @@ static void xra1403_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 	for_each_requested_gpio(chip, i, label) {
 		seq_printf(s, " gpio-%-3d (%-12s) %s %s\n",
 			   chip->base + i, label,
-			   (gcr & BIT(i)) ? "in" : "out",
-			   (gsr & BIT(i)) ? "hi" : "lo");
+			   str_in_out(gcr & BIT(i)),
+			   str_hi_lo(gsr & BIT(i)));
 	}
 }
 #else
-- 
2.39.1

Re: [PATCH v1 00/16] gpio: Use string_choices.h

[Andy Shevchenko]

On Mon, Mar 06, 2023 at 09:55:40PM +0200, Andy Shevchenko wrote:
> Use string_choices.h in the GPIO drivers and library.
> It has been tested on x86_64 and (semi-)compile tested
> over all.

Forgot to mention that this is based on top of the immutable tag I sent PR with
earlier today.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 00/16] gpio: Use string_choices.h

[Bartosz Golaszewski]

On Mon, Mar 6, 2023 at 8:55 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Use string_choices.h in the GPIO drivers and library.
> It has been tested on x86_64 and (semi-)compile tested
> over all.
>
> Andy Shevchenko (16):
>   lib/string_helpers: Add missing header files to MAINTAINERS database
>   lib/string_helpers: Split out string_choices.h
>   lib/string_choices: Add str_high_low() helper
>   lib/string_choices: Add str_input_output() helper
>   gpiolib: Utilize helpers from string_choices.h
>   gpio: adnp: Utilize helpers from string_choices.h
>   gpio: brcmstb: Utilize helpers from string_choices.h
>   gpio: crystalcove: Utilize helpers from string_choices.h
>   gpio: grgpio: Utilize helpers from string_choices.h
>   gpio: mvebu: Utilize helpers from string_choices.h
>   gpio: pl061: Utilize helpers from string_choices.h
>   gpio: stmpe: Utilize helpers from string_choices.h
>   gpio: wcove: Utilize helpers from string_choices.h
>   gpio: wm831x: Utilize helpers from string_choices.h
>   gpio: wm8994: Utilize helpers from string_choices.h
>   gpio: xra1403: Utilize helpers from string_choices.h
>
>  MAINTAINERS                     |  3 ++
>  drivers/gpio/gpio-adnp.c        | 24 ++++----------
>  drivers/gpio/gpio-brcmstb.c     |  3 +-
>  drivers/gpio/gpio-crystalcove.c | 17 +++++-----
>  drivers/gpio/gpio-grgpio.c      |  3 +-
>  drivers/gpio/gpio-mvebu.c       | 27 +++++++---------
>  drivers/gpio/gpio-pl061.c       |  4 +--
>  drivers/gpio/gpio-stmpe.c       | 19 +++++------
>  drivers/gpio/gpio-wcove.c       | 15 ++++-----
>  drivers/gpio/gpio-wm831x.c      |  5 +--
>  drivers/gpio/gpio-wm8994.c      |  6 ++--
>  drivers/gpio/gpio-xra1403.c     |  5 +--
>  drivers/gpio/gpiolib-sysfs.c    |  3 +-
>  drivers/gpio/gpiolib.c          | 13 ++++----
>  include/linux/string_choices.h  | 56 +++++++++++++++++++++++++++++++++
>  include/linux/string_helpers.h  | 26 +--------------
>  16 files changed, 125 insertions(+), 104 deletions(-)
>  create mode 100644 include/linux/string_choices.h
>
> --
> 2.39.1
>

I've been thinking about this and I must say it doesn't make much
sense to me. Not only does it NOT reduce the code size (even if we
assume the unlikely case where we'd build all those modules that use
the helpers) but also decreases the readability for anyone not
familiar with the new interfaces (meaning time spent looking up the
new function). The "%s", x ? "if" : "else" statement is concise and
clear already, I don't see much improvement with this series. And I'm
saying it from the position of someone who loves factoring out common
code. :)

I'll wait to hear what others have to say but if it were up to me, I'd
politely say no.

(I mean: I guess, in the end it is up to me, but I'm open to arguments.) :)

Bart

Re: [PATCH v1 00/16] gpio: Use string_choices.h

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1443 bytes --]

Hi Bart, hi Andy,

On Thu, Mar 09, 2023 at 04:22:19PM +0100, Bartosz Golaszewski wrote:
> I've been thinking about this and I must say it doesn't make much
> sense to me. Not only does it NOT reduce the code size (even if we
> assume the unlikely case where we'd build all those modules that use
> the helpers) but also decreases the readability for anyone not
> familiar with the new interfaces (meaning time spent looking up the
> new function). The "%s", x ? "if" : "else" statement is concise and
> clear already, I don't see much improvement with this series. And I'm
> saying it from the position of someone who loves factoring out common
> code. :)
> 
> I'll wait to hear what others have to say but if it were up to me, I'd
> politely say no.

Interpreting this as request to share my view: I'm having the same
doubts. While I'm not a big fan of the ?: operator, it's semantic is
more obvious here.

What I find most difficult about

	str_high_low(plr & BIT(j))

(from patch #6) is: Does this give me "high" or "low" if the argument is
zero? You could tell me, and judging from the patch I'd hope that it
would give me "low". But if I stumble over this code in two weeks I
have probably forgotten and have to look it up again.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]