Re: [PATCH] pinctrl: qcom: add get_direction function

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Tue 14 Mar 17:12 PDT 2017, Timur Tabi wrote:

> Stephen Boyd wrote:
> 
> > > > The idea is to notify drivers with an error code when they make a
> > > > mistake. Perhaps the device tree or the ACPI table has an error?
> 
> > In general the kernel isn't a firmware validator. At least that's
> > the way I view it. Others may have different opinions here.
> 
> I would be okay with wrapping that check around #ifdef CONFIG_DEBUG_GPIO.
> 

This will, more or less, only be useful for system integrators - whom
likely won't enable DEBUG_GPIO. But I'm generally fine with failing
gpio_set if we're not in function 0.

My question is if we should wrap that check in a WARN(), just to make it
easy for said system integrator to spot the issue - or if that will just
leave another chunk of printouts that people will ignore in their
products.

> > > > I could add that, but I still think returning an error code is
> > > > appropriate.  On the TLMM, we know for sure that the pin must be set
> > > > to function 0 in order for the read/write routines to operate
> > > > correctly.
> 
> > On ACPI we could make the gpio_get() path fail if the pin isn't
> > in GPIO mode?
> 
> Did you mean the gpio_chip.request callback?  Currently that points to
> gpiochip_generic_request in pinctrl-msm.c.
> 

It might be useful to fail gpio_get, as that gives a much nicer error
path in the client. But I'm slightly concerned about the few cases where
one of the pinmux states is gpio and that this would force the
gpio_get() only to be called after switching to that particular mode.


@Linus, have there been any discussion around this in other drivers?

Regards,
Bjorn