| Message ID | CA+V-a8sncfVq6x4iQxAo=te+rXJ25-mOD6rU+CFpbcypdPA_oA@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 28.02.2014 14:51, schrieb Prabhakar Lad: > Hi Alexander, > > On Fri, Feb 28, 2014 at 2:56 PM, Sekhar Nori <nsekhar@ti.com> 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" <prabhakar.csengg@gmail.com> > 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 <holler@ahsoftware.de> > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com> > --- > .../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>; > + }; > +}; >
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
Hi Alexander, On Sat, Mar 1, 2014 at 6:58 PM, Alexander Holler <holler@ahsoftware.de> 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
On Monday 03 March 2014 03:53 PM, Prabhakar Lad wrote: > Hi Alexander, > > On Sat, Mar 1, 2014 at 6:58 PM, Alexander Holler <holler@ahsoftware.de> 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
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
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
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
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 >
Hi Alexander, On Tue, Mar 4, 2014 at 5:24 PM, Alexander Holler <holler@ahsoftware.de> 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 >
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>; + }; +};