| Message ID | 1434718697-31335-2-git-send-email-michael@smart-africa.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jun 19, 2015 at 2:58 PM, Michael van der Westhuizen <michael@smart-africa.com> wrote: > The basic MMIO gpio driver is unaware of the pinctrl subsystem, > and this lack of pinctrl awareness extends to drivers using bgpio > unless the drivers handle pinctrl interaction by overriding the > request and free methods of gpiochip. > > Since pinctrl consumer interfaces are stubbed when pinctrl is not > enabled this implementation is very simple, and there is no > behaviour change when pinctrl is not enabled. > > Tested on picoXcell pc3x3 using gpio-dwapb. > > Signed-off-by: Michael van der Westhuizen <michael@smart-africa.com> > static int bgpio_request(struct gpio_chip *chip, unsigned gpio_pin) > { > if (gpio_pin < chip->ngpio) > - return 0; > + return pinctrl_request_gpio(chip->base + gpio_pin); > > return -EINVAL; > } > > +static void bgpio_free(struct gpio_chip *chip, unsigned gpio_pin) > +{ > + if (gpio_pin < chip->ngpio) > + pinctrl_free_gpio(chip->base + gpio_pin); > +} I found a problem with this patch unfortunately. If you have several MMIO controllers on the same system, some that use a backing pin controller and some that doesn't, this will not work, because all instances will call pinctrl_[request|free]_gpio() regardless if there is a pin controller behind it or not. Can you please augment the driver to handle this... I don't know the best way for boardfiles, but I guess (see drivers/gpio/gpio-pl061.c for exampe): - In struct bgpio_chip, introduce bool uses_pinctrl - Assume no backing pin controller so this will be false by default (propbably no code needed) - Only call pinctrl_[request|free]_gpio if uses_pinctrl is true - Explicitly set it to true for drivers that have backing pin control (like Jonas' system) or pass that flag from platform data. - A quirk like this for DT enabled systems: if (of_property_read_bool(dev->of_node, "gpio-ranges")) chip->uses_pinctrl = true; I need to move the BGPIO header to include/linux/gpio too... Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 20 Jul 2015, at 10:46 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > On Fri, Jun 19, 2015 at 2:58 PM, Michael van der Westhuizen > <michael@smart-africa.com> wrote: > >> The basic MMIO gpio driver is unaware of the pinctrl subsystem, >> and this lack of pinctrl awareness extends to drivers using bgpio >> unless the drivers handle pinctrl interaction by overriding the >> request and free methods of gpiochip. >> >> Since pinctrl consumer interfaces are stubbed when pinctrl is not >> enabled this implementation is very simple, and there is no >> behaviour change when pinctrl is not enabled. >> >> Tested on picoXcell pc3x3 using gpio-dwapb. >> >> Signed-off-by: Michael van der Westhuizen <michael@smart-africa.com> > >> static int bgpio_request(struct gpio_chip *chip, unsigned gpio_pin) >> { >> if (gpio_pin < chip->ngpio) >> - return 0; >> + return pinctrl_request_gpio(chip->base + gpio_pin); >> >> return -EINVAL; >> } >> >> +static void bgpio_free(struct gpio_chip *chip, unsigned gpio_pin) >> +{ >> + if (gpio_pin < chip->ngpio) >> + pinctrl_free_gpio(chip->base + gpio_pin); >> +} > > I found a problem with this patch unfortunately. > > If you have several MMIO controllers on the same system, some > that use a backing pin controller and some that doesn't, this > will not work, because all instances will call > pinctrl_[request|free]_gpio() regardless if there is a pin > controller behind it or not. > > Can you please augment the driver to handle this... I don't > know the best way for boardfiles, but I guess (see > drivers/gpio/gpio-pl061.c for exampe): > > - In struct bgpio_chip, introduce bool uses_pinctrl > > - Assume no backing pin controller so this will be false > by default (propbably no code needed) > > - Only call pinctrl_[request|free]_gpio if uses_pinctrl > is true > > - Explicitly set it to true for drivers that have backing pin > control (like Jonas' system) or pass that flag from > platform data. > > - A quirk like this for DT enabled systems: > if (of_property_read_bool(dev->of_node, "gpio-ranges")) > chip->uses_pinctrl = true; > > I need to move the BGPIO header to include/linux/gpio too… I will update the series. Michael > > Yours, > Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-generic.c b/drivers/gpio/gpio-generic.c index b92a690..be28ba2 100644 --- a/drivers/gpio/gpio-generic.c +++ b/drivers/gpio/gpio-generic.c @@ -58,6 +58,7 @@ o ` ~~~~\___/~~~~ ` controller in FPGA is ,.` #include <linux/io.h> #include <linux/gpio.h> #include <linux/slab.h> +#include <linux/pinctrl/consumer.h> #include <linux/platform_device.h> #include <linux/mod_devicetable.h> #include <linux/basic_mmio_gpio.h> @@ -467,11 +468,17 @@ static int bgpio_setup_direction(struct bgpio_chip *bgc, static int bgpio_request(struct gpio_chip *chip, unsigned gpio_pin) { if (gpio_pin < chip->ngpio) - return 0; + return pinctrl_request_gpio(chip->base + gpio_pin); return -EINVAL; } +static void bgpio_free(struct gpio_chip *chip, unsigned gpio_pin) +{ + if (gpio_pin < chip->ngpio) + pinctrl_free_gpio(chip->base + gpio_pin); +} + int bgpio_remove(struct bgpio_chip *bgc) { gpiochip_remove(&bgc->gc); @@ -499,6 +506,7 @@ int bgpio_init(struct bgpio_chip *bgc, struct device *dev, bgc->gc.base = -1; bgc->gc.ngpio = bgc->bits; bgc->gc.request = bgpio_request; + bgc->gc.free = bgpio_free; ret = bgpio_setup_io(bgc, dat, set, clr); if (ret)
The basic MMIO gpio driver is unaware of the pinctrl subsystem, and this lack of pinctrl awareness extends to drivers using bgpio unless the drivers handle pinctrl interaction by overriding the request and free methods of gpiochip. Since pinctrl consumer interfaces are stubbed when pinctrl is not enabled this implementation is very simple, and there is no behaviour change when pinctrl is not enabled. Tested on picoXcell pc3x3 using gpio-dwapb. Signed-off-by: Michael van der Westhuizen <michael@smart-africa.com> --- drivers/gpio/gpio-generic.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)