[PATCH v2 0/2] gpiolib: Make code more robust by using for_each_if()

[PATCH v2 0/2] gpiolib: Make code more robust by using for_each_if()

[Andy Shevchenko]

Instead of opencoding with long lines, use for_each_if() macro
which makes intention clearer and less error prone.

In v2:
- moved original for_each_if() implementation to the global header (Bart)

Andy Shevchenko (2):
  drm: Move for_each_if() to util_macros.h for wider use
  gpiolib: Switch to use for_each_if() helper

 include/drm/drm_util.h      | 16 +---------------
 include/linux/gpio/driver.h |  7 ++++---
 include/linux/util_macros.h | 15 +++++++++++++++
 3 files changed, 20 insertions(+), 18 deletions(-)

-- 
2.45.1.3035.g276e886db78b

[PATCH v2 1/2] drm: Move for_each_if() to util_macros.h for wider use

[Andy Shevchenko]

Other subsystem(s) may want to reuse the for_each_if() macro.
Move it to util_macros.h to make it globally available.

Suggested-by: Bartosz Golaszewski <brgl@bgdev.pl>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/drm/drm_util.h      | 16 +---------------
 include/linux/util_macros.h | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
index 79952d8c4bba..440199618620 100644
--- a/include/drm/drm_util.h
+++ b/include/drm/drm_util.h
@@ -36,6 +36,7 @@
 #include <linux/kgdb.h>
 #include <linux/preempt.h>
 #include <linux/smp.h>
+#include <linux/util_macros.h>
 
 /*
  * Use EXPORT_SYMBOL_FOR_TESTS_ONLY() for functions that shall
@@ -47,21 +48,6 @@
 #define EXPORT_SYMBOL_FOR_TESTS_ONLY(x)
 #endif
 
-/**
- * for_each_if - helper for handling conditionals in various for_each macros
- * @condition: The condition to check
- *
- * Typical use::
- *
- *	#define for_each_foo_bar(x, y) \'
- *		list_for_each_entry(x, y->list, head) \'
- *			for_each_if(x->something == SOMETHING)
- *
- * The for_each_if() macro makes the use of for_each_foo_bar() less error
- * prone.
- */
-#define for_each_if(condition) if (!(condition)) {} else
-
 /**
  * drm_can_sleep - returns true if currently okay to sleep
  *
diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
index 825487fb66fa..3b570b765b75 100644
--- a/include/linux/util_macros.h
+++ b/include/linux/util_macros.h
@@ -4,6 +4,21 @@
 
 #include <linux/math.h>
 
+/**
+ * for_each_if - helper for handling conditionals in various for_each macros
+ * @condition: The condition to check
+ *
+ * Typical use::
+ *
+ *	#define for_each_foo_bar(x, y) \'
+ *		list_for_each_entry(x, y->list, head) \'
+ *			for_each_if(x->something == SOMETHING)
+ *
+ * The for_each_if() macro makes the use of for_each_foo_bar() less error
+ * prone.
+ */
+#define for_each_if(condition) if (!(condition)) {} else
+
 /**
  * find_closest - locate the closest element in a sorted array
  * @x: The reference value.
-- 
2.45.1.3035.g276e886db78b

[PATCH v2 2/2] gpiolib: Switch to use for_each_if() helper

[Andy Shevchenko]

The for_each_*() APIs that are conditional can be written shorter and
less error prone with for_each_if() helper in use. Switch them to use
this helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/gpio/driver.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index ae93f75170f2..af046f7fd4f5 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -14,6 +14,7 @@
 #include <linux/property.h>
 #include <linux/spinlock_types.h>
 #include <linux/types.h>
+#include <linux/util_macros.h>
 
 #ifdef CONFIG_GENERIC_MSI_IRQ
 #include <asm/msi.h>
@@ -561,7 +562,7 @@ DEFINE_CLASS(_gpiochip_for_each_data,
 	for (CLASS(_gpiochip_for_each_data, _data)(&_label, &_i);			\
 	     _i < _size;								\
 	     _i++, kfree(_label), _label = NULL)					\
-		if (IS_ERR(_label = gpiochip_dup_line_label(_chip, _base + _i))) {} else
+		for_each_if(!IS_ERR(_label = gpiochip_dup_line_label(_chip, _base + _i)))
 
 /**
  * for_each_hwgpio - Iterates over all GPIOs for given chip.
@@ -583,7 +584,7 @@ DEFINE_CLASS(_gpiochip_for_each_data,
  */
 #define for_each_requested_gpio_in_range(_chip, _i, _base, _size, _label)		\
 	for_each_hwgpio_in_range(_chip, _i, _base, _size, _label)			\
-		if (_label == NULL) {} else
+		for_each_if(_label)
 
 /* Iterates over all requested GPIO of the given @chip */
 #define for_each_requested_gpio(chip, i, label)						\
@@ -869,7 +870,7 @@ static inline void gpiochip_unlock_as_irq(struct gpio_chip *gc,
 
 #define for_each_gpiochip_node(dev, child)					\
 	device_for_each_child_node(dev, child)					\
-		if (!fwnode_property_present(child, "gpio-controller")) {} else
+		for_each_if(fwnode_property_present(child, "gpio-controller"))
 
 static inline unsigned int gpiochip_node_count(struct device *dev)
 {
-- 
2.45.1.3035.g276e886db78b

Re: [PATCH v2 1/2] drm: Move for_each_if() to util_macros.h for wider use

[Bartosz Golaszewski]

On Thu, Feb 13, 2025 at 7:25 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Other subsystem(s) may want to reuse the for_each_if() macro.
> Move it to util_macros.h to make it globally available.
>
> Suggested-by: Bartosz Golaszewski <brgl@bgdev.pl>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Who would pick it up? Should I take it via the GPIO tree and provide
an immutable branch for the DRM tree?

Bart

Re: [PATCH v2 1/2] drm: Move for_each_if() to util_macros.h for wider use

[Jani Nikula]

On Thu, 13 Feb 2025, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> Other subsystem(s) may want to reuse the for_each_if() macro.
> Move it to util_macros.h to make it globally available.
>
> Suggested-by: Bartosz Golaszewski <brgl@bgdev.pl>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Yay!

Acked-by: Jani Nikula <jani.nikula@intel.com>

If you want to go down the memory lane, see [1]. ;)


[1] https://lore.kernel.org/r/20180709083650.23549-1-daniel.vetter@ffwll.ch

> ---
>  include/drm/drm_util.h      | 16 +---------------
>  include/linux/util_macros.h | 15 +++++++++++++++
>  2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/include/drm/drm_util.h b/include/drm/drm_util.h
> index 79952d8c4bba..440199618620 100644
> --- a/include/drm/drm_util.h
> +++ b/include/drm/drm_util.h
> @@ -36,6 +36,7 @@
>  #include <linux/kgdb.h>
>  #include <linux/preempt.h>
>  #include <linux/smp.h>
> +#include <linux/util_macros.h>
>  
>  /*
>   * Use EXPORT_SYMBOL_FOR_TESTS_ONLY() for functions that shall
> @@ -47,21 +48,6 @@
>  #define EXPORT_SYMBOL_FOR_TESTS_ONLY(x)
>  #endif
>  
> -/**
> - * for_each_if - helper for handling conditionals in various for_each macros
> - * @condition: The condition to check
> - *
> - * Typical use::
> - *
> - *	#define for_each_foo_bar(x, y) \'
> - *		list_for_each_entry(x, y->list, head) \'
> - *			for_each_if(x->something == SOMETHING)
> - *
> - * The for_each_if() macro makes the use of for_each_foo_bar() less error
> - * prone.
> - */
> -#define for_each_if(condition) if (!(condition)) {} else
> -
>  /**
>   * drm_can_sleep - returns true if currently okay to sleep
>   *
> diff --git a/include/linux/util_macros.h b/include/linux/util_macros.h
> index 825487fb66fa..3b570b765b75 100644
> --- a/include/linux/util_macros.h
> +++ b/include/linux/util_macros.h
> @@ -4,6 +4,21 @@
>  
>  #include <linux/math.h>
>  
> +/**
> + * for_each_if - helper for handling conditionals in various for_each macros
> + * @condition: The condition to check
> + *
> + * Typical use::
> + *
> + *	#define for_each_foo_bar(x, y) \'
> + *		list_for_each_entry(x, y->list, head) \'
> + *			for_each_if(x->something == SOMETHING)
> + *
> + * The for_each_if() macro makes the use of for_each_foo_bar() less error
> + * prone.
> + */
> +#define for_each_if(condition) if (!(condition)) {} else
> +
>  /**
>   * find_closest - locate the closest element in a sorted array
>   * @x: The reference value.

-- 
Jani Nikula, Intel

Re: [PATCH v2 1/2] drm: Move for_each_if() to util_macros.h for wider use

[Andy Shevchenko]

On Thu, Feb 13, 2025 at 07:36:38PM +0100, Bartosz Golaszewski wrote:
> On Thu, Feb 13, 2025 at 7:25 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Other subsystem(s) may want to reuse the for_each_if() macro.
> > Move it to util_macros.h to make it globally available.

> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Who would pick it up? Should I take it via the GPIO tree and provide
> an immutable branch for the DRM tree?

I believe it makes sense as the first user outside drm is GPIO.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/2] drm: Move for_each_if() to util_macros.h for wider use

[Andy Shevchenko]

On Thu, Feb 13, 2025 at 08:48:02PM +0200, Jani Nikula wrote:
> On Thu, 13 Feb 2025, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > Other subsystem(s) may want to reuse the for_each_if() macro.
> > Move it to util_macros.h to make it globally available.

> Acked-by: Jani Nikula <jani.nikula@intel.com>

Thanks!

> If you want to go down the memory lane, see [1]. ;)
> 
> [1] https://lore.kernel.org/r/20180709083650.23549-1-daniel.vetter@ffwll.ch

Yeah, I have read it a few hours ago to refresh my memories.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] gpiolib: Make code more robust by using for_each_if()

[Linus Walleij]

On Thu, Feb 13, 2025 at 7:25 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> Instead of opencoding with long lines, use for_each_if() macro
> which makes intention clearer and less error prone.

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

Yours,
Linus Walleij

Re: [PATCH v2 0/2] gpiolib: Make code more robust by using for_each_if()

[Bartosz Golaszewski]

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


On Thu, 13 Feb 2025 20:23:59 +0200, Andy Shevchenko wrote:
> Instead of opencoding with long lines, use for_each_if() macro
> which makes intention clearer and less error prone.
> 
> In v2:
> - moved original for_each_if() implementation to the global header (Bart)
> 
> Andy Shevchenko (2):
>   drm: Move for_each_if() to util_macros.h for wider use
>   gpiolib: Switch to use for_each_if() helper
> 
> [...]

Applied, thanks!

[1/2] drm: Move for_each_if() to util_macros.h for wider use
      commit: b82e90c6f987e8f930523313eb803209cf9c6c97
[2/2] gpiolib: Switch to use for_each_if() helper
      commit: c60fa3ba030a4d95b55fcca8945dacac89c542a6

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