Re: [PATCH v2 01/13] gpiolib: add gpiod_multi_set_value_cansleep()

[Geert Uytterhoeven]

Hi David,

On Thu, 6 Feb 2025 at 23:48, David Lechner <dlechner@xxxxxxxxxxxx> wrote:
> Add a new gpiod_multi_set_value_cansleep() helper function with fewer
> parameters than gpiod_set_array_value_cansleep().
>
> Calling gpiod_set_array_value_cansleep() can get quite verbose. In many
> cases, the first arguments all come from the same struct gpio_descs, so
> having a separate function where we can just pass that cuts down on the
> boilerplate.
>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Signed-off-by: David Lechner <dlechner@xxxxxxxxxxxx>

Thanks for your patch!

> --- a/include/linux/gpio/consumer.h
> +++ b/include/linux/gpio/consumer.h
> @@ -655,4 +655,11 @@ static inline void gpiod_unexport(struct gpio_desc *desc)
>
>  #endif /* CONFIG_GPIOLIB && CONFIG_GPIO_SYSFS */
>
> +static inline int gpiod_multi_set_value_cansleep(struct gpio_descs *descs,
> +                                                unsigned long *value_bitmap)
> +{
> +       return gpiod_set_array_value_cansleep(descs->ndescs, descs->desc,
> +                                             descs->info, value_bitmap);

I am wondering whether this needs a check for !IS_ERR_OR_NULL(descs),
to handle the !CONFIG_GPIOLIB and gpiod_get_array_optional() cases?

Slightly related: shouldn't gpiod_put_array() (both the implementation
and the !CONFIG_GPIOLIB dummy) allow the caller to pass NULL, to
streamline the gpiod_get_array_optional() case?

> +}
> +
>  #endif

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds