Re: [PATCH] gpio: pl061: hook request if gpio-ranges avaiable

[Linus Walleij <linus.walleij@xxxxxxxxxx>]

On Fri, Nov 21, 2014 at 7:43 AM, Yunlei He <heyunlei@xxxxxxxxxx> wrote:

> Gpio-ranges property is useful to represent which GPIOs correspond
> to which pins on which pin controllers. But there may be some gpios
> without pinctrl operation. So check whether gpio-ranges property
> exists in device node first.
>
> Signed-off-by: Yunlei He <heyunlei@xxxxxxxxxx>
> Signed-off-by: Xinwei Kong <kong.kongxinwei@xxxxxxxxxxxxx>
> Signed-off-by: Haojian Zhuang <haojian.zhuang@xxxxxxxxxx>

(...)

>  Documentation/devicetree/bindings/gpio/pl061-gpio.txt | 2 +-
>  drivers/gpio/gpio-pl061.c                             | 7 +++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> index a2c416b..577bcf7 100644
> --- a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> @@ -7,4 +7,4 @@ Required properties:
>    - bit 0 specifies polarity (0 for normal, 1 for inverted)
>  - gpio-controller : Marks the device node as a GPIO controller.
>  - interrupts : Interrupt mapping for GPIO IRQ.
> -
> +- gpio-ranges : Interaction with the PINCTRL subsystem

This can be a separate patch. Please send it as such.

> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index 84b49cf..01875d1 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
> @@ -24,6 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/pm.h>
> +#include <linux/of_address.h>

What you're checking for is not an address. This would be just
#include <linux/of.h>

>         spin_lock_init(&chip->lock);
> +       if (of_get_property(dev->of_node, "gpio-ranges", NULL)) {
> +               chip->gc.request = pl061_gpio_request;
> +               chip->gc.free = pl061_gpio_free;
> +       }
>
> -       chip->gc.request = pl061_gpio_request;
> -       chip->gc.free = pl061_gpio_free;

NAK.

No this does not work. GPIO ranges doe not *have* to come from
the device tree, it is more common that a GPIO driver adds it by
way of gpiochip_add_pin_range().

Haojian has already solved this problem in the pinctrl core.
Inspect commit 51e13c2475913d45a3ec546dee647538a9341d6a
"pinctrl: check pinctrl ready for gpio range"

The call(s) to pinctrl_request_gpio() from
pl061_gpio_request() should already return silently with 0
AFAICT, Haojian do you agree?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html