Re: [libgpiod v2][PATCH 2/6] API: rename gpiod_request_config_get_num_offsets to gpiod_request_config_get_num_lines to match line_request pattern

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 15, 2022 at 11:52:21AM +0100, Bartosz Golaszewski wrote:
> On Fri, Mar 11, 2022 at 8:40 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
> >
> > Both gpiod_request_config and gpiod_line_request contain a number of
> > lines, but the former has a get_num_offsets accessor, while the latter
> > has get_num_lines.  Make them consistent by switching request_config to
> > get_num_lines.
> >
> > Signed-off-by: Kent Gibson <warthog618@xxxxxxxxx>
> > ---
> 
> IMO this doesn't make much sense because we still have
> gpiod_request_config_set_offsets(). So now you have set_offsets but
> get_lines. At the time of preparing the request_config we're still
> talking about offsets of lines to request, while once the request has
> been made, we're talking about requested lines.
> 

Oh FFS we are always talking about lines.  Whether you have requested
them yet or not is irrelevant.  You WANT to request lines.
If we had globally unique line names we wouldn't give a rats about the
offset.

And take another look - you have actually have get_offsets and
get_num_lines functions.

Offsets is just one of the attributes of the lines, and this approach
still works if there is another fields of interest. e.g. values:

int gpiod_line_request_set_values_subset(struct gpiod_line_request *request,
					 size_t num_lines,
					 const unsigned int *offsets,
					 const int *values);

which you then complain about in patch 4 as I'm writing this.... <sigh>.

You could equally argue that one should be num_values.

While we are still preparing the configuration, we are preparing the
config for LINES, not offsets.  Using num_lines is a reminder that you
need to provide the offset for each line - the two are inextricably
linked.  Using num_offsets could be taken to imply that
gpiod_request_config_set_offsets() can be called multiple times to add
offsets.

> I would leave it as it is personally.
> 

I know, I know :-|.

Cheers,
Kent.



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux