Re: [PATCH 00/13] gpiolib: add gpiods_set_array_value_cansleep
From: Andy Shevchenko
Date: Sat Feb 01 2025 - 10:11:02 EST
On Sat, Feb 1, 2025 at 12:36 PM Bartosz Golaszewski <brgl@xxxxxxxx> wrote:
>
> On Fri, 31 Jan 2025 21:24:40 +0100, David Lechner <dlechner@xxxxxxxxxxxx> said:
> > This series was inspired by some minor annoyance I have experienced a
> > few times in recent reviews.
> >
> > Calling gpiod_set_array_value_cansleep() can be quite verbose due to
> > having so many parameters. In most cases, we already have a struct
> > gpio_descs that contains the first 3 parameters so we end up with 3 (or
> > often even 6) pointer indirections at each call site. Also, people have
> > a tendency to want to hard-code the first argument instead of using
> > struct gpio_descs.ndescs, often without checking that ndescs >= the
> > hard-coded value.
> >
> > So I'm proposing that we add a gpiods_set_array_value_cansleep()
> > function that is a wrapper around gpiod_set_array_value_cansleep()
> > that has struct gpio_descs as the first parameter to make it a bit
> > easier to read the code and avoid the hard-coding temptation.
> >
> > I've just done gpiods_set_array_value_cansleep() for now since there
> > were over 10 callers of this one. There aren't as many callers of
> > the get and atomic variants, but we can add those too if this seems
> > like a useful thing to do.
> This looks good to me except for one thing: the function prefix. I would
> really appreciate it if we could stay within the existing gpiod_ namespace and
> not add a new one in the form of gpiods_.
>
> Maybe: gpiod_multiple_set_ or gpiod_collected_set...?
+1 here, i.e. I like the idea, but the naming needs to be amended.
--
With Best Regards,
Andy Shevchenko