Re: [RFC/PATCH] gpio/pinctrl: baytrail: move gpio driver from pinctrl to gpio directory

[Mika Westerberg]

On Wed, Oct 15, 2014 at 09:55:42AM -0700, David Cohen wrote:
> Hi Mika,
> 
> Thanks for your feedback. See below my reply.
> 
> On Wed, Oct 15, 2014 at 10:08:12AM +0300, Mika Westerberg wrote:
> > On Tue, Oct 14, 2014 at 10:45:35AM -0700, David Cohen wrote:
> > > Hi Mathias,
> > > 
> > > On Tue, Oct 14, 2014 at 01:35:43PM +0300, Mathias Nyman wrote:
> > > > On 13.10.2014 22:17, David Cohen wrote:
> > > > > Even though GPIO module on Intel Bay Trail is able to control pin
> > > > > functionality, it's unlikely Linux kernel driver will ever support it
> > > > > since BIOS should handle all pin muxing itself.
> > > > > 
> > > > > Currently this driver does not register any pinctrl interface and
> > > > > doesn't call any pinctrl interface. It just uses on internal functions
> > > > > the 'struct pinctrl_gpio_range', which is a weak justification to not be
> > > > > under gpio directory.
> > > > > 
> > > > 
> > > > This discussion was held when gpio-baytrail was first submitted.
> > > > These threads explain the gpio/pinctrl-baytrail history:
> > > > 
> > > > http://marc.info/?l=linux-kernel&m=136981432427668&w=2
> > > > http://marc.info/?l=linux-kernel&m=137113578604763&w=2
> > > > http://marc.info/?l=linux-kernel&m=137155497023054&w=2
> > > 
> > > Thanks for pointing that out.
> > > 
> > > > 
> > > > A proper pinctrl driver for baytrail is still not yet ruled out
> > > 
> > > Having it inside pinctrl directory is creating some confusion because
> > > ppl expect it to implement the actual pinctrl interface.
> > 
> > A typical pinctrl driver can implement both a pinctrl interface and a
> > GPIO interface. So it is not uncommon to look the GPIO drivers under
> > drivers/pinctrl/*.
> 
> Agreed :)
> But pinctrl-baytrail has no pinctrl interface, just gpio.

The hardware is capable of muxing pins etc. it just has not been
implemented because we currently don't have need for anything else.
I have seen that in some of our internal trees the driver is also muxing
alternate functions to the pins (using non-pinctrl interface). So maybe
it will get a full pinctrl support in the future.

We certainly don't want to move it from pinctrl to gpio and then back.

> > Furthermore the driver announces that it is a GPIO driver in its Kconfig
> > entry:
> > 
> > config PINCTRL_BAYTRAIL
> >         bool "Intel Baytrail GPIO pin control"
> > 
> > so I don't quite get why this would confuse people.
> 
> How come? You just wrote the inconsistence :)

Well, it says it is a "GPIO pin control" :-)

> We're enabling PINCTRL_BAYTRAIL to build a gpio only driver.
> 
> > 
> > We are also planning to add more Intel pinctrl (real) drivers in the
> > future. drivers/pinctrl/intel/* should be the place where people find
> > the pinctrl/GPIO drivers for newer Intel hardware.
> 
> I fully agree pinctrl/gpio drivers should stay under drivers/pinctrl.
> But we're talking about a gpio driver. IMHO we should at least mention
> in the driver it lacks pinctrl interface currently but it will come in
> future.

OK, why not.

> A side discussion would be why a distro would want to have the pinctrl
> interface on Bay Trail. I'd assume any driver being the consumer of it
> is likely to be wrong. When Linux boots, all pins' functions should be
> already set.

In an ideal world, yes. However, the reality has shown that BIOS/FW gets
these wrong and we need to work it around in the OS.

Also for devices like MinnowBoard MAX (which has Baytrail SoC), you
actually might want to mux some other function out of certain pins.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/