| Message ID | 1412847735-498-1-git-send-email-haojian.zhuang@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Xinwei, Do I miss anything? At here, .request pointer isn't null. It always points to pl061_gpio_request(). Best Regards Haojian On 9 October 2014 18:38, k00278426 <kong.kongxinwei@hisilicon.com> wrote: > On 2014/10/9 17:42, Haojian Zhuang wrote: > > gpio-ranges property could binds gpio to pinctrl. But there may be some > gpios without pinctrl operation. So check whether gpio-ranges property > exists in device node first. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > drivers/gpio/gpio-pl061.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c > index 84b49cf..d1813f0 100644 > --- a/drivers/gpio/gpio-pl061.c > +++ b/drivers/gpio/gpio-pl061.c > @@ -52,28 +52,34 @@ struct pl061_gpio { > > void __iomem *base; > struct gpio_chip gc; > + int uses_pinctrl; > > #ifdef CONFIG_PM > struct pl061_context_save_regs csave_regs; > #endif > }; > > -static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset) > +static int pl061_gpio_request(struct gpio_chip *gc, unsigned offset) > { > /* > * Map back to global GPIO space and request muxing, the direction > * parameter does not matter for this controller. > */ > - int gpio = chip->base + offset; > + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + int gpio = gc->base + offset; > > - return pinctrl_request_gpio(gpio); > + if (chip->uses_pinctrl) > + return pinctrl_request_gpio(gpio); > + return 0; > } > > In the request_gpio process,if the .request point is null,the return > of request_gpio process will value 0. the request_gpio process will not > enter the pintrcl system. > so the request_gpio have deal with the "if" branch. > > > -static void pl061_gpio_free(struct gpio_chip *chip, unsigned offset) > +static void pl061_gpio_free(struct gpio_chip *gc, unsigned offset) > { > - int gpio = chip->base + offset; > + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + int gpio = gc->base + offset; > > - pinctrl_free_gpio(gpio); > + if (chip->uses_pinctrl) > + pinctrl_free_gpio(gpio); > } > > static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) > @@ -264,6 +270,9 @@ static int pl061_probe(struct amba_device *adev, const > struct amba_id *id) > > spin_lock_init(&chip->lock); > > + /* Hook the request()/free() for pinctrl operation */ > + if (of_property_read_bool(dev->of_node, "gpio-ranges")) > + chip->uses_pinctrl = true; > chip->gc.request = pl061_gpio_request; > chip->gc.free = pl061_gpio_free; > chip->gc.direction_input = pl061_direction_input; > >
hi jian
In the afternoon,we submit a patch about gpio-ranges item,we slove this issue by setting the .request pointer which is null. before coming true the
pl061_gpio_request function, we slove the issue.you can study it.
Best Regards
xinwei
On 2014/10/9 18:56, Haojian Zhuang wrote:
> Hi Xinwei,
>
> Do I miss anything? At here, .request pointer isn't null. It always
> points to pl061_gpio_request().
>
> Best Regards
> Haojian
>
> On 9 October 2014 18:38, k00278426 <kong.kongxinwei@hisilicon.com> wrote:
>> On 2014/10/9 17:42, Haojian Zhuang wrote:
>>
>> gpio-ranges property could binds gpio to pinctrl. But there may be some
>> gpios without pinctrl operation. So check whether gpio-ranges property
>> exists in device node first.
>>
>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org>
>> ---
>> drivers/gpio/gpio-pl061.c | 21 +++++++++++++++------
>> 1 file changed, 15 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
>> index 84b49cf..d1813f0 100644
>> --- a/drivers/gpio/gpio-pl061.c
>> +++ b/drivers/gpio/gpio-pl061.c
>> @@ -52,28 +52,34 @@ struct pl061_gpio {
>>
>> void __iomem *base;
>> struct gpio_chip gc;
>> + int uses_pinctrl;
>>
>> #ifdef CONFIG_PM
>> struct pl061_context_save_regs csave_regs;
>> #endif
>> };
>>
>> -static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset)
>> +static int pl061_gpio_request(struct gpio_chip *gc, unsigned offset)
>> {
>> /*
>> * Map back to global GPIO space and request muxing, the direction
>> * parameter does not matter for this controller.
>> */
>> - int gpio = chip->base + offset;
>> + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
>> + int gpio = gc->base + offset;
>>
>> - return pinctrl_request_gpio(gpio);
>> + if (chip->uses_pinctrl)
>> + return pinctrl_request_gpio(gpio);
>> + return 0;
>> }
>>
>> In the request_gpio process,if the .request point is null,the return
>> of request_gpio process will value 0. the request_gpio process will not
>> enter the pintrcl system.
>> so the request_gpio have deal with the "if" branch.
>>
>>
>> -static void pl061_gpio_free(struct gpio_chip *chip, unsigned offset)
>> +static void pl061_gpio_free(struct gpio_chip *gc, unsigned offset)
>> {
>> - int gpio = chip->base + offset;
>> + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
>> + int gpio = gc->base + offset;
>>
>> - pinctrl_free_gpio(gpio);
>> + if (chip->uses_pinctrl)
>> + pinctrl_free_gpio(gpio);
>> }
>>
>> static int pl061_direction_input(struct gpio_chip *gc, unsigned offset)
>> @@ -264,6 +270,9 @@ static int pl061_probe(struct amba_device *adev, const
>> struct amba_id *id)
>>
>> spin_lock_init(&chip->lock);
>>
>> + /* Hook the request()/free() for pinctrl operation */
>> + if (of_property_read_bool(dev->of_node, "gpio-ranges"))
>> + chip->uses_pinctrl = true;
>> chip->gc.request = pl061_gpio_request;
>> chip->gc.free = pl061_gpio_free;
>> chip->gc.direction_input = pl061_direction_input;
>>
>>
> .
>
It's good. But it's similar the one that I sent in last year. You can check this link in below. http://marc.info/?l=linux-gpio&m=138300092720593&w=2 My patch is used to fix this issue according to the comments. Best Regards Haojian On 9 October 2014 19:15, k00278426 <kong.kongxinwei@hisilicon.com> wrote: > hi jian > > In the afternoon,we submit a patch about gpio-ranges item,we slove this issue by setting the .request pointer which is null. before coming true the > > pl061_gpio_request function, we slove the issue.you can study it. > > Best Regards > xinwei > > > On 2014/10/9 18:56, Haojian Zhuang wrote: >> Hi Xinwei, >> >> Do I miss anything? At here, .request pointer isn't null. It always >> points to pl061_gpio_request(). >> >> Best Regards >> Haojian >> >> On 9 October 2014 18:38, k00278426 <kong.kongxinwei@hisilicon.com> wrote: >>> On 2014/10/9 17:42, Haojian Zhuang wrote: >>> >>> gpio-ranges property could binds gpio to pinctrl. But there may be some >>> gpios without pinctrl operation. So check whether gpio-ranges property >>> exists in device node first. >>> >>> Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> >>> --- >>> drivers/gpio/gpio-pl061.c | 21 +++++++++++++++------ >>> 1 file changed, 15 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c >>> index 84b49cf..d1813f0 100644 >>> --- a/drivers/gpio/gpio-pl061.c >>> +++ b/drivers/gpio/gpio-pl061.c >>> @@ -52,28 +52,34 @@ struct pl061_gpio { >>> >>> void __iomem *base; >>> struct gpio_chip gc; >>> + int uses_pinctrl; >>> >>> #ifdef CONFIG_PM >>> struct pl061_context_save_regs csave_regs; >>> #endif >>> }; >>> >>> -static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset) >>> +static int pl061_gpio_request(struct gpio_chip *gc, unsigned offset) >>> { >>> /* >>> * Map back to global GPIO space and request muxing, the direction >>> * parameter does not matter for this controller. >>> */ >>> - int gpio = chip->base + offset; >>> + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); >>> + int gpio = gc->base + offset; >>> >>> - return pinctrl_request_gpio(gpio); >>> + if (chip->uses_pinctrl) >>> + return pinctrl_request_gpio(gpio); >>> + return 0; >>> } >>> >>> In the request_gpio process,if the .request point is null,the return >>> of request_gpio process will value 0. the request_gpio process will not >>> enter the pintrcl system. >>> so the request_gpio have deal with the "if" branch. >>> >>> >>> -static void pl061_gpio_free(struct gpio_chip *chip, unsigned offset) >>> +static void pl061_gpio_free(struct gpio_chip *gc, unsigned offset) >>> { >>> - int gpio = chip->base + offset; >>> + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); >>> + int gpio = gc->base + offset; >>> >>> - pinctrl_free_gpio(gpio); >>> + if (chip->uses_pinctrl) >>> + pinctrl_free_gpio(gpio); >>> } >>> >>> static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) >>> @@ -264,6 +270,9 @@ static int pl061_probe(struct amba_device *adev, const >>> struct amba_id *id) >>> >>> spin_lock_init(&chip->lock); >>> >>> + /* Hook the request()/free() for pinctrl operation */ >>> + if (of_property_read_bool(dev->of_node, "gpio-ranges")) >>> + chip->uses_pinctrl = true; >>> chip->gc.request = pl061_gpio_request; >>> chip->gc.free = pl061_gpio_free; >>> chip->gc.direction_input = pl061_direction_input; >>> >>> >> . >> > >
On Thu, Oct 9, 2014 at 6:42 PM, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: > gpio-ranges property could binds gpio to pinctrl. But there may be some > gpios without pinctrl operation. So check whether gpio-ranges property > exists in device node first. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > --- > drivers/gpio/gpio-pl061.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c > index 84b49cf..d1813f0 100644 > --- a/drivers/gpio/gpio-pl061.c > +++ b/drivers/gpio/gpio-pl061.c > @@ -52,28 +52,34 @@ struct pl061_gpio { > > void __iomem *base; > struct gpio_chip gc; > + int uses_pinctrl; > > #ifdef CONFIG_PM > struct pl061_context_save_regs csave_regs; > #endif > }; > > -static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset) > +static int pl061_gpio_request(struct gpio_chip *gc, unsigned offset) > { > /* > * Map back to global GPIO space and request muxing, the direction > * parameter does not matter for this controller. > */ > - int gpio = chip->base + offset; > + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + int gpio = gc->base + offset; > > - return pinctrl_request_gpio(gpio); > + if (chip->uses_pinctrl) > + return pinctrl_request_gpio(gpio); > + return 0; > } > > -static void pl061_gpio_free(struct gpio_chip *chip, unsigned offset) > +static void pl061_gpio_free(struct gpio_chip *gc, unsigned offset) > { > - int gpio = chip->base + offset; > + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + int gpio = gc->base + offset; > > - pinctrl_free_gpio(gpio); > + if (chip->uses_pinctrl) > + pinctrl_free_gpio(gpio); > } > > static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) > @@ -264,6 +270,9 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id) > > spin_lock_init(&chip->lock); > > + /* Hook the request()/free() for pinctrl operation */ > + if (of_property_read_bool(dev->of_node, "gpio-ranges")) > + chip->uses_pinctrl = true; You probably want to document this property in Documentation/devicetree/bindings/gpio/pl061-gpio.txt.
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c index 84b49cf..d1813f0 100644 --- a/drivers/gpio/gpio-pl061.c +++ b/drivers/gpio/gpio-pl061.c @@ -52,28 +52,34 @@ struct pl061_gpio { void __iomem *base; struct gpio_chip gc; + int uses_pinctrl; #ifdef CONFIG_PM struct pl061_context_save_regs csave_regs; #endif }; -static int pl061_gpio_request(struct gpio_chip *chip, unsigned offset) +static int pl061_gpio_request(struct gpio_chip *gc, unsigned offset) { /* * Map back to global GPIO space and request muxing, the direction * parameter does not matter for this controller. */ - int gpio = chip->base + offset; + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); + int gpio = gc->base + offset; - return pinctrl_request_gpio(gpio); + if (chip->uses_pinctrl) + return pinctrl_request_gpio(gpio); + return 0; } -static void pl061_gpio_free(struct gpio_chip *chip, unsigned offset) +static void pl061_gpio_free(struct gpio_chip *gc, unsigned offset) { - int gpio = chip->base + offset; + struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); + int gpio = gc->base + offset; - pinctrl_free_gpio(gpio); + if (chip->uses_pinctrl) + pinctrl_free_gpio(gpio); } static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) @@ -264,6 +270,9 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id) spin_lock_init(&chip->lock); + /* Hook the request()/free() for pinctrl operation */ + if (of_property_read_bool(dev->of_node, "gpio-ranges")) + chip->uses_pinctrl = true; chip->gc.request = pl061_gpio_request; chip->gc.free = pl061_gpio_free; chip->gc.direction_input = pl061_direction_input;
gpio-ranges property could binds gpio to pinctrl. But there may be some gpios without pinctrl operation. So check whether gpio-ranges property exists in device node first. Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- drivers/gpio/gpio-pl061.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)