Davinci gpio devicetree

classic Classic list List threaded Threaded
35 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Davinci gpio devicetree

Alexander Holler
Hello,

I've seen kernel 3.14-rc contains support for gpios using devicetree.

I've two comments:

1. #gpio-cells seems to be missing,
2. a small usage example (e.g. with gpio-leds or gpio-keys) would be nice.

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Sekhar Nori
+ Prabhakar

On Tuesday 25 February 2014 08:54 PM, Alexander Holler wrote:

> Hello,
>
> I've seen kernel 3.14-rc contains support for gpios using devicetree.
>
> I've two comments:
>
> 1. #gpio-cells seems to be missing,
> 2. a small usage example (e.g. with gpio-leds or gpio-keys) would be nice.
>
> Regards,
>
> Alexander Holler
> _______________________________________________
> Davinci-linux-open-source mailing list
> [hidden email]
> http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Lad, Prabhakar
Hi Alexander,

On Fri, Feb 28, 2014 at 2:56 PM, Sekhar Nori <[hidden email]> wrote:

> + Prabhakar
>
> On Tuesday 25 February 2014 08:54 PM, Alexander Holler wrote:
>> Hello,
>>
>> I've seen kernel 3.14-rc contains support for gpios using devicetree.
>>
>> I've two comments:
>>
>> 1. #gpio-cells seems to be missing,
>> 2. a small usage example (e.g. with gpio-leds or gpio-keys) would be nice.
>>
Yes #gpio-cells is missing, you can refer following patch fixing it,

Regards,
--Prabhakar Lad

-----X------------X

>From e8e96492926fe74012bb8ae8163411405a12057c Mon Sep 17 00:00:00 2001
From: "Lad, Prabhakar" <[hidden email]>
Date: Fri, 28 Feb 2014 19:15:22 +0530
Subject: [PATCH] devicetree: bindings: gpio-davinci: fix documentation

This patch adds missing #gpio-cells and also adds a
usage example.

Reported-by: Alexander Holler <[hidden email]>
Signed-off-by: Lad, Prabhakar <[hidden email]>
---
 .../devicetree/bindings/gpio/gpio-davinci.txt      |   18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index a2e839d..fb96b94 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -8,6 +8,10 @@ Required Properties:

 - gpio-controller : Marks the device node as a gpio controller.

+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+
 - interrupt-parent: phandle of the parent interrupt controller.

 - interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are
@@ -27,6 +31,7 @@ Example:
 gpio: gpio@1e26000 {
     compatible = "ti,dm6441-gpio";
     gpio-controller;
+    #gpio-cells = <2>;
     reg = <0x226000 0x1000>;
     interrupt-parent = <&intc>;
     interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
@@ -39,3 +44,16 @@ gpio: gpio@1e26000 {
     interrupt-controller;
     #interrupt-cells = <2>;
 };
+
+leds {
+    compatible = "gpio-leds";
+    led1 {
+        label = "davinci:green:usr1";
+        gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
+    };
+
+    led2 {
+        label = "davinci:red:debug1";
+        gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
+    };
+};
--
1.7.9.5
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Alexander Holler
Am 28.02.2014 14:51, schrieb Prabhakar Lad:

> Hi Alexander,
>
> On Fri, Feb 28, 2014 at 2:56 PM, Sekhar Nori <[hidden email]> wrote:
>> + Prabhakar
>>
>> On Tuesday 25 February 2014 08:54 PM, Alexander Holler wrote:
>>> Hello,
>>>
>>> I've seen kernel 3.14-rc contains support for gpios using devicetree.
>>>
>>> I've two comments:
>>>
>>> 1. #gpio-cells seems to be missing,
>>> 2. a small usage example (e.g. with gpio-leds or gpio-keys) would be nice.
>>>
> Yes #gpio-cells is missing, you can refer following patch fixing it,

Thanks, thats just what I've used.

I've mentioned the usage example because there should be already at
least one example, the one used to test the patch. ;)

Maybe it makes sense to add the example to one of the provided dts too.
I've used the da850-evm.dts as a template, and just had to add

#include <dt-bindings/gpio/gpio.h>

besides that the &gpio0 in your binding description there just has to be
&gpio.

Regards,

Alexander Holler


>
> Regards,
> --Prabhakar Lad
>
> -----X------------X
>
>  From e8e96492926fe74012bb8ae8163411405a12057c Mon Sep 17 00:00:00 2001
> From: "Lad, Prabhakar" <[hidden email]>
> Date: Fri, 28 Feb 2014 19:15:22 +0530
> Subject: [PATCH] devicetree: bindings: gpio-davinci: fix documentation
>
> This patch adds missing #gpio-cells and also adds a
> usage example.
>
> Reported-by: Alexander Holler <[hidden email]>
> Signed-off-by: Lad, Prabhakar <[hidden email]>
> ---
>   .../devicetree/bindings/gpio/gpio-davinci.txt      |   18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> index a2e839d..fb96b94 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -8,6 +8,10 @@ Required Properties:
>
>   - gpio-controller : Marks the device node as a gpio controller.
>
> +- #gpio-cells : Should be two.
> +  - first cell is the pin number
> +  - second cell is used to specify optional parameters (unused)
> +
>   - interrupt-parent: phandle of the parent interrupt controller.
>
>   - interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are
> @@ -27,6 +31,7 @@ Example:
>   gpio: gpio@1e26000 {
>       compatible = "ti,dm6441-gpio";
>       gpio-controller;
> +    #gpio-cells = <2>;
>       reg = <0x226000 0x1000>;
>       interrupt-parent = <&intc>;
>       interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
> @@ -39,3 +44,16 @@ gpio: gpio@1e26000 {
>       interrupt-controller;
>       #interrupt-cells = <2>;
>   };
> +
> +leds {
> +    compatible = "gpio-leds";
> +    led1 {
> +        label = "davinci:green:usr1";
> +        gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
> +    };
> +
> +    led2 {
> +        label = "davinci:red:debug1";
> +        gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
> +    };
> +};
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Alexander Holler
Hello,

Having had a second look at your example (comparing with what I've used
here), I think it might make sense to change it a bit:

Am 01.03.2014 14:10, schrieb Alexander Holler:
 > Am 28.02.2014 14:51, schrieb Prabhakar Lad:

 >> +leds {
         pinctrl-names = "default";
         pinctrl-0 = <&led_pins>;
 >> +    compatible = "gpio-leds";
 >> +    led1 {
 >> +        label = "davinci:green:usr1";
 >> +        gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
             linux,default-trigger = "heartbeat";
 >> +    };
 >> +
 >> +    led2 {
 >> +        label = "davinci:red:debug1";
 >> +        gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
 >> +    };
 >> +};

or just add "..." to denote that there should/might be some additional
stuff which doesn't really belong to the description of the gpio-binding
(like pinctrl).

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Lad, Prabhakar
Hi Alexander,

On Sat, Mar 1, 2014 at 6:58 PM, Alexander Holler <[hidden email]> wrote:

> Hello,
>
> Having had a second look at your example (comparing with what I've used
> here), I think it might make sense to change it a bit:
>
> Am 01.03.2014 14:10, schrieb Alexander Holler:
>
>> Am 28.02.2014 14:51, schrieb Prabhakar Lad:
>
>>> +leds {
>         pinctrl-names = "default";
>         pinctrl-0 = <&led_pins>;
>
I think this can be dropped or else one might also feel led_pins are missing.

>>> +    compatible = "gpio-leds";
>>> +    led1 {
>>> +        label = "davinci:green:usr1";
>>> +        gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
>             linux,default-trigger = "heartbeat";
>
>>> +    };
>>> +
>>> +    led2 {
>>> +        label = "davinci:red:debug1";
>>> +        gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
>>> +    };
>>> +};
>
> or just add "..." to denote that there should/might be some additional stuff
> which doesn't really belong to the description of the gpio-binding (like
> pinctrl).
>
I would prefer "..." instead


Sekhar If you are OK with the above changes I'll post a updated patch to DT list
aswell let me know your comments on this.

Thanks,
--Prabhakar Lad
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Sekhar Nori
On Monday 03 March 2014 03:53 PM, Prabhakar Lad wrote:

> Hi Alexander,
>
> On Sat, Mar 1, 2014 at 6:58 PM, Alexander Holler <[hidden email]> wrote:
>> Hello,
>>
>> Having had a second look at your example (comparing with what I've used
>> here), I think it might make sense to change it a bit:
>>
>> Am 01.03.2014 14:10, schrieb Alexander Holler:
>>
>>> Am 28.02.2014 14:51, schrieb Prabhakar Lad:
>>
>>>> +leds {
>>         pinctrl-names = "default";
>>         pinctrl-0 = <&led_pins>;
>>
> I think this can be dropped or else one might also feel led_pins are missing.
>
>>>> +    compatible = "gpio-leds";
>>>> +    led1 {
>>>> +        label = "davinci:green:usr1";
>>>> +        gpios = <&gpio0 10 GPIO_ACTIVE_HIGH>;
>>             linux,default-trigger = "heartbeat";
>>
>>>> +    };
>>>> +
>>>> +    led2 {
>>>> +        label = "davinci:red:debug1";
>>>> +        gpios = <&gpio0 11 GPIO_ACTIVE_HIGH>;
>>>> +    };
>>>> +};
>>
>> or just add "..." to denote that there should/might be some additional stuff
>> which doesn't really belong to the description of the gpio-binding (like
>> pinctrl).
>>
> I would prefer "..." instead
>
>
> Sekhar If you are OK with the above changes I'll post a updated patch to DT list
> aswell let me know your comments on this.

Yes, please post a formal patch.

Thanks,
Sekhar
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Alexander Holler
In reply to this post by Lad, Prabhakar
BTW., I wonder how to use e.g. GP6_7 (which I calc as no 103 by using
6*16+7)?

Using e.g.

        gpios = <&gpio 103 GPIO_ACTIVE_HIGH>;

in a led definition (for da850-evm) fails because 103 is greater than
the max. limit of 32 as set for every chip in davinci_gpio_probe() in
gpio-davinci.c.

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Alexander Holler
Am 03.03.2014 23:49, schrieb Alexander Holler:

> BTW., I wonder how to use e.g. GP6_7 (which I calc as no 103 by using
> 6*16+7)?
>
> Using e.g.
>
>     gpios = <&gpio 103 GPIO_ACTIVE_HIGH>;
>
> in a led definition (for da850-evm) fails because 103 is greater than
> the max. limit of 32 as set for every chip in davinci_gpio_probe() in
> gpio-davinci.c.

To be a bit more precise, I wonder how one is able to use one of those 4
other gpio_chips gpio-davinci registers through an entry with ti,ngpio
set to 144, like it's done in da850.dtsi. With such an entry,
gpio-davinci registers 5 gpio_chips (4 with 32 gpios and one with 16
gpios), but I think currently one is only able to use the first 32 gpios
from within a dt.

Maybe I'm missing some magic somewhere.

I've added two other people to cc from which I think they might be
interested in that topic.

Regards,

Alexander Holler

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Grygorii Strashko
On 03/04/2014 01:54 PM, Alexander Holler wrote:

> Am 03.03.2014 23:49, schrieb Alexander Holler:
>> BTW., I wonder how to use e.g. GP6_7 (which I calc as no 103 by using
>> 6*16+7)?
>>
>> Using e.g.
>>
>>      gpios = <&gpio 103 GPIO_ACTIVE_HIGH>;
>>
>> in a led definition (for da850-evm) fails because 103 is greater than
>> the max. limit of 32 as set for every chip in davinci_gpio_probe() in
>> gpio-davinci.c.
>
> To be a bit more precise, I wonder how one is able to use one of those 4
> other gpio_chips gpio-davinci registers through an entry with ti,ngpio
> set to 144, like it's done in da850.dtsi. With such an entry,
> gpio-davinci registers 5 gpio_chips (4 with 32 gpios and one with 16
> gpios), but I think currently one is only able to use the first 32 gpios
> from within a dt.
>
> Maybe I'm missing some magic somewhere.
>
> I've added two other people to cc from which I think they might be
> interested in that topic.

The gpio_chip of_xlate staff has to be added to handle this.
(gpio-pxa.c can be used as ref)

>
> Regards,
>
> Alexander Holler
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Alexander Holler
Am 04.03.2014 16:12, schrieb Grygorii Strashko:

> On 03/04/2014 01:54 PM, Alexander Holler wrote:
>> Am 03.03.2014 23:49, schrieb Alexander Holler:
>>> BTW., I wonder how to use e.g. GP6_7 (which I calc as no 103 by using
>>> 6*16+7)?
>>>
>>> Using e.g.
>>>
>>>      gpios = <&gpio 103 GPIO_ACTIVE_HIGH>;
>>>
>>> in a led definition (for da850-evm) fails because 103 is greater than
>>> the max. limit of 32 as set for every chip in davinci_gpio_probe() in
>>> gpio-davinci.c.
>>
>> To be a bit more precise, I wonder how one is able to use one of those 4
>> other gpio_chips gpio-davinci registers through an entry with ti,ngpio
>> set to 144, like it's done in da850.dtsi. With such an entry,
>> gpio-davinci registers 5 gpio_chips (4 with 32 gpios and one with 16
>> gpios), but I think currently one is only able to use the first 32 gpios
>> from within a dt.
>>
>> Maybe I'm missing some magic somewhere.
>>
>> I've added two other people to cc from which I think they might be
>> interested in that topic.
>
> The gpio_chip of_xlate staff has to be added to handle this.
> (gpio-pxa.c can be used as ref)

Sounds like what I've thought. I've debugged it yesterday evening down
to of_gpio_simple_xlate() where it ends because the gpio number 103 is
greater than ngpio which is 32. The result is an always defered probe.

I haven't noticed it before, because I wasn't sure until when it is
supposed to be defered, as I first had to solve some other problems
while trying to boot a dt-davinci-kernel here.

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Davinci gpio devicetree

Lad, Prabhakar
In reply to this post by Alexander Holler
Hi Alexander,

On Tue, Mar 4, 2014 at 5:24 PM, Alexander Holler <[hidden email]> wrote:

> Am 03.03.2014 23:49, schrieb Alexander Holler:
>> BTW., I wonder how to use e.g. GP6_7 (which I calc as no 103 by using
>> 6*16+7)?
>>
>> Using e.g.
>>
>>     gpios = <&gpio 103 GPIO_ACTIVE_HIGH>;
>>
>> in a led definition (for da850-evm) fails because 103 is greater than
>> the max. limit of 32 as set for every chip in davinci_gpio_probe() in
>> gpio-davinci.c.
>
> To be a bit more precise, I wonder how one is able to use one of those 4
> other gpio_chips gpio-davinci registers through an entry with ti,ngpio
> set to 144, like it's done in da850.dtsi. With such an entry,
> gpio-davinci registers 5 gpio_chips (4 with 32 gpios and one with 16
> gpios), but I think currently one is only able to use the first 32 gpios
> from within a dt.
>
You got it correct it creates 5 gpio chips, with each gpio group containing
32 gpio as per  Davinci GPIO register architecture (GPIO control register)
Currently from all the GPIO's can be used, for example the dip switch (S7-8 gpio
pin 116 ) you can use the patch at [1] for testing on da850 evm.

[1]  http://git.linuxtv.org/mhadli/v4l-dvb-davinci_devices.git/commitdiff/75575f7b34d094677546f7ed094af107ebdc1e7d

> Maybe I'm missing some magic somewhere.
>
> I've added two other people to cc from which I think they might be
> interested in that topic.
>
> Regards,
>
> Alexander Holler
>
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH] gpio: davinci: fix gpio selection for OF

Alexander Holler
In reply to this post by Grygorii Strashko
The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.

While there I've set #gpio_cells to a fixed value of 2.

I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.

Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.

Signed-off-by: Alexander Holler <[hidden email]>
---
 drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..79f45c4 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -44,6 +44,9 @@ struct davinci_gpio_regs {
 
 static void __iomem *gpio_base;
 
+static struct davinci_gpio_controller *davinci_gpio_chips;
+static int davinci_last_gpio;
+
 static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
 {
  void __iomem *ptr;
@@ -172,6 +175,24 @@ of_err:
  return NULL;
 }
 
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+     const struct of_phandle_args *gpiospec,
+     u32 *flags)
+{
+ if (gpiospec->args[0] > davinci_last_gpio)
+ return -EINVAL;
+
+ if (gc != &davinci_gpio_chips[gpiospec->args[0] / 32].chip)
+ return -EINVAL;
+
+ if (flags)
+ *flags = gpiospec->args[1];
+
+ return gpiospec->args[0] % 32;
+}
+#endif
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
  int i, base;
@@ -236,6 +257,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
  chips[i].chip.ngpio = 32;
 
 #ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_gpio_n_cells = 2;
+ chips[i].chip.of_xlate = davinci_gpio_of_xlate;
  chips[i].chip.of_node = dev->of_node;
 #endif
  spin_lock_init(&chips[i].lock);
@@ -251,6 +274,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 
  platform_set_drvdata(pdev, chips);
  davinci_gpio_irq_setup(pdev);
+
+ davinci_gpio_chips = chips;
+ davinci_last_gpio = ngpio;
+
  return 0;
 }
 
--
1.8.3.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] gpio: davinci: fix gpio selection for OF

Linus Walleij-2
On Wed, Mar 5, 2014 at 6:06 AM, Alexander Holler <[hidden email]> wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <[hidden email]>

Grygorii, can I have your review tag on this patch so I can apply it?

Yours,
Linus Walleij
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH] gpio: davinci: fix gpio selection for OF

Grygorii Strashko
In reply to this post by Alexander Holler
On 03/05/2014 12:06 AM, Alexander Holler wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <[hidden email]>
> ---
>   drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 7629b4f..79f45c4 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -44,6 +44,9 @@ struct davinci_gpio_regs {
>
>   static void __iomem *gpio_base;
>
> +static struct davinci_gpio_controller *davinci_gpio_chips;
> +static int davinci_last_gpio;
> +
>   static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
>   {
>   void __iomem *ptr;
> @@ -172,6 +175,24 @@ of_err:
>   return NULL;
>   }
>
> +#ifdef CONFIG_OF_GPIO
> +static int davinci_gpio_of_xlate(struct gpio_chip *gc,
> +     const struct of_phandle_args *gpiospec,
> +     u32 *flags)
> +{
> + if (gpiospec->args[0] > davinci_last_gpio)
> + return -EINVAL;
> +
> + if (gc != &davinci_gpio_chips[gpiospec->args[0] / 32].chip)
> + return -EINVAL;
> +
> + if (flags)
> + *flags = gpiospec->args[1];

Just one question here - Could we use gpio_chip-dev and hence, drop
static variables (davinci_gpio_chips, davinci_last_gpio)?

The Davinci device holds davinci_gpio_platform_data
in dev->platform_data and pointer on davinci_gpio_controller array
in dev->p->driver_data.

And looks like, dev_get_platdata(gc->dev) and
dev_get_drvdata(gc->dev) can be used here.

> +
> + return gpiospec->args[0] % 32;
> +}
> +#endif
> +
>   static int davinci_gpio_probe(struct platform_device *pdev)
>   {
>   int i, base;
> @@ -236,6 +257,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>   chips[i].chip.ngpio = 32;
>

add here chips[i].chip.dev = dev;

>   #ifdef CONFIG_OF_GPIO
> + chips[i].chip.of_gpio_n_cells = 2;
> + chips[i].chip.of_xlate = davinci_gpio_of_xlate;
>   chips[i].chip.of_node = dev->of_node;
>   #endif
>   spin_lock_init(&chips[i].lock);
> @@ -251,6 +274,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>
>   platform_set_drvdata(pdev, chips);
>   davinci_gpio_irq_setup(pdev);
> +
> + davinci_gpio_chips = chips;
> + davinci_last_gpio = ngpio;
> +
>   return 0;
>   }

regards,
-grygorii


_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH v2] gpio: davinci: fix gpio selection for OF

Alexander Holler
The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.

While there I've set #gpio_cells to a fixed value of 2.

I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.

Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.

Signed-off-by: Alexander Holler <[hidden email]>
Signed-off-by: Grygorii Strashko <[hidden email]>
---
 drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

 Changes in v2: replaced static variables by indirections.

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..92574a0 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -172,6 +172,27 @@ of_err:
  return NULL;
 }
 
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+     const struct of_phandle_args *gpiospec,
+     u32 *flags)
+{
+ struct davinci_gpio_controller *chips = dev_get_drvdata(gc->dev);
+ struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc->dev);
+
+ if (gpiospec->args[0] > pdata->ngpio)
+ return -EINVAL;
+
+ if (gc != &chips[gpiospec->args[0] / 32].chip)
+ return -EINVAL;
+
+ if (flags)
+ *flags = gpiospec->args[1];
+
+ return gpiospec->args[0] % 32;
+}
+#endif
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
  int i, base;
@@ -236,6 +257,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
  chips[i].chip.ngpio = 32;
 
 #ifdef CONFIG_OF_GPIO
+ chips[i].chip.of_gpio_n_cells = 2;
+ chips[i].chip.of_xlate = davinci_gpio_of_xlate;
+ chips[i].chip.dev = dev;
  chips[i].chip.of_node = dev->of_node;
 #endif
  spin_lock_init(&chips[i].lock);
--
1.8.3.1

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] gpio: davinci: fix gpio selection for OF

Grygorii Strashko
Hi Alexander,

On 03/05/2014 01:21 PM, Alexander Holler wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <[hidden email]>
> Signed-off-by: Grygorii Strashko <[hidden email]>

Looks good to me now. Thanks.

> ---
>   drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
>   Changes in v2: replaced static variables by indirections.
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 7629b4f..92574a0 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -172,6 +172,27 @@ of_err:
>   return NULL;
>   }
>
> +#ifdef CONFIG_OF_GPIO
> +static int davinci_gpio_of_xlate(struct gpio_chip *gc,
> +     const struct of_phandle_args *gpiospec,
> +     u32 *flags)
> +{
> + struct davinci_gpio_controller *chips = dev_get_drvdata(gc->dev);
> + struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc->dev);
> +
> + if (gpiospec->args[0] > pdata->ngpio)
> + return -EINVAL;
> +
> + if (gc != &chips[gpiospec->args[0] / 32].chip)
> + return -EINVAL;
> +
> + if (flags)
> + *flags = gpiospec->args[1];
> +
> + return gpiospec->args[0] % 32;
> +}
> +#endif
> +
>   static int davinci_gpio_probe(struct platform_device *pdev)
>   {
>   int i, base;
> @@ -236,6 +257,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>   chips[i].chip.ngpio = 32;
>
>   #ifdef CONFIG_OF_GPIO
> + chips[i].chip.of_gpio_n_cells = 2;
> + chips[i].chip.of_xlate = davinci_gpio_of_xlate;
> + chips[i].chip.dev = dev;
>   chips[i].chip.of_node = dev->of_node;
>   #endif
>   spin_lock_init(&chips[i].lock);
>

_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] gpio: davinci: fix gpio selection for OF

Linus Walleij-2
In reply to this post by Alexander Holler
On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <[hidden email]> wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <[hidden email]>
> Signed-off-by: Grygorii Strashko <[hidden email]>

This v2 version applied, thanks!

Yours,
Linus Walleij
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] gpio: davinci: fix gpio selection for OF

Alexander Holler
Am 11.03.2014 11:15, schrieb Linus Walleij:

> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <[hidden email]> wrote:
>
>> The driver missed an of_xlate function to translate gpio numbers
>> as found in the DT to the correct chip and number.
>>
>> While there I've set #gpio_cells to a fixed value of 2.
>>
>> I've used gpio-pxa.c as template for those changes and tested my changes
>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>> reinvent the wheel but just copied and tested stuff.
>>
>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>
>> Signed-off-by: Alexander Holler <[hidden email]>
>> Signed-off-by: Grygorii Strashko <[hidden email]>
>
> This v2 version applied, thanks!

Thanks, but actually that should have been a fix for 3.14 with which the
OF functionality for davinci gpio gets introduced. I assum with the
patch in for-next, 3.14 will appear with that functionality broken and
it will become a candidate for -stable.

Regards,

Alexander Holler
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH v2] gpio: davinci: fix gpio selection for OF

Linus Walleij-2
On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler <[hidden email]> wrote:

> Am 11.03.2014 11:15, schrieb Linus Walleij:
>> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <[hidden email]> wrote:
>>
>>> The driver missed an of_xlate function to translate gpio numbers
>>> as found in the DT to the correct chip and number.
>>>
>>> While there I've set #gpio_cells to a fixed value of 2.
>>>
>>> I've used gpio-pxa.c as template for those changes and tested my changes
>>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>>> reinvent the wheel but just copied and tested stuff.
>>>
>>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>>
>>> Signed-off-by: Alexander Holler <[hidden email]>
>>> Signed-off-by: Grygorii Strashko <[hidden email]>
>>
>> This v2 version applied, thanks!
>
> Thanks, but actually that should have been a fix for 3.14 with which the
> OF functionality for davinci gpio gets introduced. I assum with the
> patch in for-next, 3.14 will appear with that functionality broken and
> it will become a candidate for -stable.

I just get the impression that DT support for DaVinci in v3.14 is so risky
and unstable that noone except those implementing it (i.e. you) is really
using it, is that correct?

In that case it is hardly a fix that we need to rush out to the entire world.

But if you have bug reports from DaVinci developers doing advanced DT
stuff and scratching their heads, then we can push this to fixes.

Yours,
Linus Walleij
_______________________________________________
Davinci-linux-open-source mailing list
[hidden email]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
12
Loading...