Re: [PATCH v2 0/1] gpio driver for Intel Baytrail platforms
On Fri, Jun 14, 2013 at 2:14 PM, Mathias Nyman wrote: > [Me] >> 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. Christian's patch is merged into the pinctrl tree for v3.11. > any naming preference? > pinctrl-baytrail.c Bingo! Because the hardware is a pin controller after all. I know there is some BIOS or ROM very much wanting it to look simpler than it is, but it sure is a pin controller :-) Thanks, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/1] gpio driver for Intel Baytrail platforms
On Thu, Jun 13, 2013 at 5:06 PM, Mathias Nyman 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? 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. 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. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH v2 0/1] gpio driver for Intel Baytrail platforms
This is the second version of the Intel baytrail GPIO driver. 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) Changes since v1: - generic cleanups suggested by Andy S. - removed text about pin muxing - added missing 44:th pin to SUS controller - added level triggering option - prevent "forever loop" in case pin is stuck in "interrupt triggered" status, Mathias Nyman (1): gpio: add Intel BayTrail gpio driver drivers/gpio/Kconfig | 12 + drivers/gpio/Makefile|1 + drivers/gpio/gpio-baytrail.c | 547 ++ 3 files changed, 560 insertions(+), 0 deletions(-) create mode 100644 drivers/gpio/gpio-baytrail.c -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/