Re: Documentation for line_config PULL_UP, effect on line_event edges and line_request values?

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, Jun 03, 2024 at 11:39:16PM -0500, David C. Rankin wrote:
> On 6/3/24 21:43, Kent Gibson wrote:
> > Does highlighting that line values are logical help?
>
> Thank you Kent for the reply!
>

Btw, when replying to the list, reply-all so all included in the thread
get a copy directly rather than having to check the list - I only just
noticed this one now, some eight hours after you posted.

>   Chuckling... yes, flashing attributes too and a sledge-hammer waiting to
> fall on my head may also help.
>
>   All kidding aside, the reason I bring it up, is if this is something that
> I scratched my head about after reading the documentation and the header,
> then I'm not alone. It's always the case with documenting code, when you are
> the author (or closely involved in creating the code), everything seems very
> clear and obvious -- you've lived with the code for months or years...
>
>   However, on the other end of the understanding continuum, is the person
> looking to use the v2 uABI for the first time, without any familiarity with
> the code, that reads the chardev.html document will likely never appreciate
> the careful use of the words active/inactive. The comments in gpio.h header
> really do seem to just make that point more clear despite having the same
> text. I know it was that way for me.
>
>   It may not take much of an addition at all to emphasize the logical edge
> and value relationship. In fact, just what you explained in your reply would
> make a perfect addition to help clarify and bridge the gap between those who
> know the uABI inside and out and those who just start working with it.
>
>   Without looking at the code (or isolating the PULL and ACTIVE_LOW) it
> wasn't immediately clear which one was flipping the logical relationship.
> From a hardware standpoint it would make sense that either one could do it.
> Your explanation of bias being physical and the ACTIVE_LOW flag being the
> one that sets the logic makes that point clear. That would be a great
> addition to the doc as well.
>

Agreed - I'm looking into adding some clarification to the docs.

> > >
> > >  * @GPIO_V2_LINE_FLAG_EDGE_RISING: line detects rising (inactive to active)
> > >  * edges
> > >  * @GPIO_V2_LINE_FLAG_EDGE_FALLING: line detects falling (active to
> > >  * inactive) edges
> > >  ...
> >
> > So that does not makes it clear that the edge definitions are based on
> > logical values?
> >
>
>   That does make it clear, but for whatever human-factors reason, it is not
> as apparent in the enum gpio_v2_line_flag section of the chardev.html web
> page. Maybe it just has to do with the way the web-page puts the explanation
> on a separate line below in smaller serif font? But it almost seems the
> header screams "Pay attention to this!" while the doc just reads "Here is
> some other info too". Your idea of highlighting or at least bolding the
> "(inactive to active)" and "(active to inactive)" would certainly help
> there.
>
>   In chardev.html, adding your explanation on the logical/physical
> difference would work great as a "Note: ...." right before the enum
> gpio_v2_line_flag block (or right after whichever you prefer)
>

I'm not sure that is possible, given the way the documentation is generated
from reST.

And, given the way the html documentation is structured, my preference would
be to expand the documentation of the relevant ioctls and reads, as that is
where a reader trying to understand what the function is doing would be
looking.

>   Anyway, all just good ideas intended to improve the ease of initial
> understanding for what is a great improvement over the v1 uABI. I'll leave
> the rest in your hands, you provided a great, short and concise explanation
> of the logical verses physical implementation of edge detection and value
> behavior. I'd only ask that you give serious thought to adding a few
> sentences or a paragraph precisely as you did in the reply. That really can
> make all the difference in understanding for someone coming anew to the
> gpio_v2 ABI.
>
>   Thanks again for your reply.

Thanks for the feedback.

Oh, and are you ok with me adding you as suggesting the patch?

Cheers,
Kent.