Messages in this thread | | | Date | Fri, 14 Jun 2013 15:14:07 +0300 | From | Mathias Nyman <> | Subject | Re: [PATCH v2 0/1] gpio driver for Intel Baytrail platforms |
| |
On 06/13/2013 06:45 PM, Linus Walleij wrote: > On Thu, Jun 13, 2013 at 5:06 PM, Mathias Nyman > <mathias.nyman@linux.intel.com> 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
| |