Re: [PATCH] pinctrl: baytrail: show output gpio state correctly on Intel Baytrail

[David Cohen <david.a.cohen@linux.intel.com>]

On Tue, Nov 04, 2014 at 09:34:24PM +0200, Mika Westerberg wrote:
> On Tue, Nov 04, 2014 at 11:11:16AM -0800, David Cohen wrote:
> > > It is not implicit at all.
> > > 
> > > The user of the GPIO in ACPI DSDT table says something like:
> > > 
> > > 	Name (_DEP, Package () { \_SB.GPO2 })
> > > 
> > > or similar. That is *explicit* dependency. Here \_SB.GPO2 is one of the
> > > GPIO banks.
> > 
> > Either kernel knows on-the-fly or statically the required dependency.
> > The static dependency is well described by Kconfig. An on-the-fly
> > dependency could be a probe execution failing because it couldn't access
> > part of required resources. If the dependency is temporarily not
> > described this way, it would still be acceptable a documentation
> > somewhere explaining that we do have this hidden thing going on.
> 
> The only thing kernel knows about this is when it finds that the
> device in question has _DEP object. Once that happens and it evaluates

That's all kernel needs to know.

> to a list of devices we depend on, we can defer this particular driver
> going further in probe until all the dependencies listed in _DEP are
> resolved.

That's the best way to prevent this issue IMHO, but looks like it's
already being addressed:
https://lkml.org/lkml/2014/10/27/455

> 
> The documentation you are after is ACPI 5.1 specification downloadable
> freely at uefi.org/acpi.

Nope. The documentation I am referring to is the one that doesn't exist.
It supposed to be a simple comment on pinctrl_baytrail.c's file, or
maybe a comment on Kconfig. Wherever it could tell ppl not involved with
original development that there might be an implicit dependency to this
gpio driver by other drivers. And because of that, it's recommended to
let this driver probed as soon as possible on Bay Trail platforms.

BTW if we can't find the dependency either on Kconfig or on kernel
codes, if no dependent is checking for this resource when probing, if
all we have to define that this dependency exists is by looking random
ACPI tables (which are neither generic, nor part of kernel), it is then
implicit WRT kernel. Kernel cannot find it, nor developers can foresee
it.

> 
> > > > But IMHO all dependency to a driver should be explicitly described
> > > > (e.g. on Kconfigs, or maybe failing probe). With current situation if we
> > > > do not select pinctrl_baytrail, instead of affecting just the drivers
> > > > that explicitly depend on that, it affects others which we are unable to
> > > > easily identify.
> > > 
> > > So how do you propose we describe the dependency? It is completely in
> > > firmware. Should we make i2c-hid.c dependent on pinctrl-baytrail.c just
> > > because some underlying firmware method (_PSx for example) needs the
> > > GPIO but the driver itself does not?
> > 
> > i2c-hid.c should fail, WARN, yell, scream or whatever :)
> > This way one could say: hey, we needed GPIO.
> 
> But i2c-hid.c does not know anything about GPIOS in the first place.
> Like I said the dependency is in the firmware level. It may need GPIOs
> to do something or not but the driver never sees those GPIOs.

AFAIU any gpio usage by FW would start from a call from i2c-hid.c
driver. Maybe this call could fail, and then nothing implicit happens.
But i2c-hid.c does know about the dependency if checks its ACPI table
during the probe. But that's being addressed by the thread I posted
above.

Br, David