Re: [PATCH v1 1/1] gpiolib: Deduplicate some code in for_each_requested_gpio_in_range()

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Tue, Feb 4, 2025 at 6:57 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> Refactor for_each_requested_gpio_in_range() to deduplicate some code
> which is basically repeats the for_each_hwgpio(). In order to achieve
> this, split the latter to two, for_each_hwgpio_in_range() and
> for_each_hwgpio().
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  include/linux/gpio/driver.h | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
> index a307d8ed9c51..24890ecfa0fb 100644
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -549,20 +549,25 @@ DEFINE_CLASS(_gpiochip_for_each_data,
>              const char **label, int *i)
>
>  /**
> - * for_each_hwgpio - Iterates over all GPIOs for given chip.
> + * for_each_hwgpio_in_range - Iterates over all GPIOs in a given range
>   * @_chip: Chip to iterate over.
>   * @_i: Loop counter.
> + * @_base: First GPIO in the ranger.
> + * @_size: Amount of GPIOs to check starting from @base.
>   * @_label: Place to store the address of the label if the GPIO is requested.
>   *          Set to NULL for unused GPIOs.
>   */
> -#define for_each_hwgpio(_chip, _i, _label) \
> +#define for_each_hwgpio_in_range(_chip, _i, _base, _size, _label)                      \
>         for (CLASS(_gpiochip_for_each_data, _data)(&_label, &_i); \
> -            *_data.i < _chip->ngpio; \
> +            *_data.i < _size;                                                          \
>              (*_data.i)++, kfree(*(_data.label)), *_data.label = NULL) \
>                 if (IS_ERR(*_data.label = \
> -                       gpiochip_dup_line_label(_chip, *_data.i))) {} \
> +                       gpiochip_dup_line_label(_chip, _base + *_data.i))) {}           \
>                 else
>

Can you add a kerneldoc here as well, please?

Bart

> +#define for_each_hwgpio(_chip, _i, _label)                                             \
> +       for_each_hwgpio_in_range(_chip, _i, 0, _chip->ngpio, _label)
> +
>  /**
>   * for_each_requested_gpio_in_range - iterates over requested GPIOs in a given range
>   * @_chip:     the chip to query
> @@ -572,12 +577,8 @@ DEFINE_CLASS(_gpiochip_for_each_data,
>   * @_label:    label of current GPIO
>   */
>  #define for_each_requested_gpio_in_range(_chip, _i, _base, _size, _label)              \
> -       for (CLASS(_gpiochip_for_each_data, _data)(&_label, &_i);                       \
> -            *_data.i < _size;                                                          \
> -            (*_data.i)++, kfree(*(_data.label)), *_data.label = NULL)                  \
> -               if ((*_data.label =                                                     \
> -                       gpiochip_dup_line_label(_chip, _base + *_data.i)) == NULL) {}   \
> -               else if (IS_ERR(*_data.label)) {}                                       \
> +       for_each_hwgpio_in_range(_chip, _i, _base, _size, _label)                       \
> +               if (*_data.label == NULL) {}                                            \
>                 else
>
>  /* Iterates over all requested GPIO of the given @chip */
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>