Re: [PATCH v4 0/5] gpio: expose line bias flags to userspace

[Kent Gibson <warthog618@xxxxxxxxx>]

On Mon, Nov 04, 2019 at 12:48:33PM +0100, Bartosz Golaszewski wrote:
> pon., 4 lis 2019 o 12:12 Kent Gibson <warthog618@xxxxxxxxx> napisał(a):
> >
> > On Mon, Nov 04, 2019 at 11:14:56AM +0100, Bartosz Golaszewski wrote:
> > > pon., 4 lis 2019 o 02:07 Kent Gibson <warthog618@xxxxxxxxx> napisał(a):
> > > >
> > > > On Mon, Nov 04, 2019 at 01:26:54AM +0100, Linus Walleij wrote:
> > > > > On Thu, Oct 31, 2019 at 8:10 AM Bartosz Golaszewski
> > > > > <bgolaszewski@xxxxxxxxxxxx> wrote:
> > > > >
> > > > > > [Kent]
> > > > > > > This series adds gross control of pull-up/pull-down to the GPIO uAPI.
> > > > > > > Gross control means enabling and disabling of bias functionality,
> > > > > > > not finer grained control such as setting biasing impedances.
> > > > >
> > > > > Right, excellent and persistent work here, much appreciated!
> > > > >
> > > >
> > > > No problem - hopefully I haven't irritated too many people in the process.
> > > >
> > > > > As long as I get Bartosz's blanket ACK on v5 I think it is ready
> > > > > to merge. His consent is required for this.
> > > > >
> > > >
> > > > I'm still waiting on open questions from v4 before submitting v5:
> > > >
> > > > One, handling of errors when setting bias, Bart has referred to Thomas,
> > > > so waiting for feedback on that.
> > > >
> > >
> > > If we can get it merged for v5.5, then I don't want to block it
> > > waiting for answers. Looking at the code I think we should only ignore
> > > the EOPNOTSUPP error and propagate all other codes. Can you add a
> > > patch changing that and then fix the other nits I pointed out? Also:
> > > please Cc Thomas Petazzoni so that he gets the chance to yell at us if
> > > it breaks something.
> > >
> >

Can you just confirm if it is EOPNOTSUPP or ENOTSUPP that you want ignored?

> > Will do.
> >
> > > > The other, where gpio_set_bias is hooked into gpiod_direction_output,
> > > > is fine as is for the UAPI - it can always be relocated subsequently if
> > > > other APIs need to set bias.  On the other hand, if decoupling setting
> > > > direction and bias is in order then that really should be done now.
> > > > Can I get an an ACK on that either way?
> > > >
> > >
> > > This is in line with what you did for input. I don't think it should
> > > be decoupled (any particular reason for that? There is none mentioned
> > > in the cover letter), so I propose to leave it as it is in patch 5/5.
> > >
> >
> > Wrt decoupling, I was thinking of both input and output, not just output,
> > though it was the output that got me onto that line of thought as
> > gpiod_direction_output sets bias, but gpiod_direction_output_raw doesn't.
> > That seemed totally arbitrary.
> >
> > That lead to thinking that the gpiod_direction_output (and _input) is now
> > doing more than implied by the name, and by the documentation for that
> > matter.  So perhaps it should be split out and promote gpio_set_bias to
> > gpiod_set_bias?  Anyway, that was the line of thought.
> > The problem there being some callers of gpiod_direction_input already
> > expect it to set bias, at least on a best effort basis, and they would
> > have to be updated to call gpiod_set_bias.
> >
> 
> I see. I think that in this case, the _raw variants should stay as
> simple as possible (hence the name) while the bias *should* be set in
> the regular gpiod_direction_intput()/output(). For now I don't think
> we need an exported gpiod_set_bias(), but if someone should request it
> in the future it will be straightforward to add.
> 

OK.

> > Maybe just update the documentation for exported functions that do set
> > bias?
> 
> Sure, sounds good. You can even extend the doc to include other flags
> these functions handle.
> 

OK to add this later, or does it need to be in this series?

> >
> > > One more thing - since we all want this to make it for v5.5 - can you
> > > make the set config patches part of this series (simply bunch it all
> > > together)? This will make it easy to review and merge everything.
> > >
> >
> > May as well - I've got it all in the one branch anyway.
> >
> > > Thanks in advance and great job!
> >
> > I was about to start updating libgpiod to add set_config, after adding
> > the equivalent to my gpiod library (nearly finished writing the tests
> > for that), but I'll put all that on the back burner and get v5,
> > including in the set_config patches, out as soon as I can.
> >
> 
> Let me know if you still want to do it and you'll have patches ready
> before v5.5 is released. Otherwise I can do it myself too if needed.
> 

When does the window close for v5.5?
I've got an updated series ready - other than the doc updates mentioned
above.

Cheers,
Kent.