Re: [libgpiod v2.0][PATCH] core: extend config objects

[Kent Gibson <warthog618@gmail.com>]

On Fri, Aug 13, 2021 at 02:59:43PM +0200, Bartosz Golaszewski wrote:
> On Thu, Aug 12, 2021 at 5:02 PM Kent Gibson <warthog618@gmail.com> wrote:
> >
> > On Thu, Aug 12, 2021 at 04:43:46PM +0200, Bartosz Golaszewski wrote:
> > > On Thu, Aug 12, 2021 at 4:23 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > >
> > > > On Thu, Aug 12, 2021 at 02:51:02PM +0200, Bartosz Golaszewski wrote:
> > > > > On Thu, Aug 12, 2021 at 12:29 PM Kent Gibson <warthog618@gmail.com> wrote:
> > > > > >
> > > > >
> > > >
> > > > <snip>
> > > >
> > > > > >
> > > > > > Sure, it's a trade-off, but the alternative is requiring a 2-3k block
> > > > > > even for a one line request, which seems a wee bit excessive.
> > > > > >
> > > > >
> > > > > As you said - it's on the heap, so who cares. But this is also an
> > > > > internal structure and so we can use bit fields. That should reduce
> > > > > the memory footprint significantly as we now don't require more than 3
> > > > > bits for any given enum. That would leave us with the debounce period
> > > > > and offset as full size variables.
> > > > >
> > > >
> > > > Rather than introducing a new collection of enums and bitfields, why not
> > > > just store the v2 flags for the line?
> > > >
> > >
> > > Why a new collection of enums? It wouldn't change anything, we'd just
> > > make sure in the setters we never overflow.
> > >
> >
> > Yeah, my bad - you already have the enums.
> >
> > > I would prefer to limit the use of kernel symbols (and types!) to the
> > > minimum for clarity.
> > >
> >
> > OK, but you need to map the libgpiod config to kernel flags at some
> > point...
> 
> Yep, right before passing the arguments to the kernel. Having separate
> fields for each setting is much clearer IMO than storing flags.
> 
> > And this is all internal to line-config.c...
> >
> 
> This code still needs to be maintained and using enums makes it easier.
> 

Fair enough.

> Anyway, this is implementation detail really as with bit fields we'll
> fit in an 32-bit integer anyway with all those enums.

Agreed - I thought of that after I posted.

> Does the general idea sound good? If so, then I'll rework it.
> 

Sounds like a plan - rework away.

Cheers,
Kent.