Re: [PATCH v2 0/1] gpio driver for Intel Baytrail platforms

[Mathias Nyman]

On 06/13/2013 06:45 PM, Linus Walleij wrote:
> On Thu, Jun 13, 2013 at 5:06 PM, Mathias Nyman
> <mathias.nyman@xxxxxxxxxxxxxxx>  wrote:
>
> > After looking at the pinctrl subsystem that Linus W. suggested I think
> > pinctrl suits platforms that don't have firmware configuring the pins
> > before the operating system is started, or when pins need to be configured
> > on the fly.
> >
> > I'd still keep this driver under GPIO. Adding it to pinctrl
> > feels like adding more complexity without any bigger use for the features.
> >
> > We expect BIOS to set all pin configurations correctly.
> > The comments about pin muxing capabilities are removed from the driver.
> > The same firmware is anyway listing gpio resources in ACPI tables, so pin
> > configurations should be correct. (The previous indication in the driver
> > about the need to configure pins was mostly because we're working with early
> > develpment stage firmwares which still have small hickups)
>
> This does not address the issue that you're reimplementing
> the GPIO ranges from the pinctrl subsystem, and just hours ago
> on the mailing list Christian Ruppert sent a patch making these
> more flexible I think.
>
> Subject "Add pin-list based GPIO ranges", please check this
> patch, isn't that exactly the helper infrastructure you need?

It fits better yes, with that patch I could use
struct pinctrl_gpio_range instead of the custom struct gpio_bank.
The .name entry can be used for acpi_id to identify the range.

Also the gpio_to_pad() function is usable.

> Of course you can make an argument that is is a good idea to
> duplicate this, but I want that to be explicit. To me it is still
> quite obvious that these gpio to pad mappings are laid out
> according to the actual hardware registers, and that the actual
> hardware registers pertain to pads rather than what we call
> "GPIOs", which in kernel terms are only some line.

That is true, it's about mapping between layout of hw registers.
I guess it's a tradeoff between more

> I would still vote to put the thing in drivers/pinctrl anyway,
> I am perfectly happy to house pure GPIO drivers there,
> especially if they're obviously masking something more
> pinctrl-like in reality, it will be way more flexible the day that
> you just want to add "this one little quirk for this pin config
> thing", then it'll fit just fine.

I'm fine with having it under drivers/pinctrl as a GPIO driver, either 
just as it is, or by using the pinctrl_gpio_range structure and helper 
functions such as gpio_to_pad(), once Christian Rupperts patch is accepted.

any naming preference?
gpio-baytrail.c
pinctrl-gpio-baytrail.c
pinctrl-baytrail.c

-Mathias
--
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/