From mboxrd@z Thu Jan 1 00:00:00 1970 From: Grygorii Strashko Subject: Re: [PATCH v4 4/6] gpio: davinci: add OF support Date: Tue, 5 Nov 2013 18:59:27 +0200 Message-ID: <527923EF.4070602@ti.com> References: <1383406775-14902-1-git-send-email-prabhakar.csenng@gmail.com> <1383406775-14902-5-git-send-email-prabhakar.csenng@gmail.com> <5277E752.8090005@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: Errors-To: davinci-linux-open-source-bounces-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org To: Prabhakar Lad Cc: Mark Rutland , "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , dlos , Russell King , LDOC , Pawel Moll , Stephen Warren , Linus Walleij , Ian Campbell , LKML , Rob Herring , "linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , Rob Landley , Alex Elder , Grant Likely , LAK List-Id: devicetree@vger.kernel.org On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii, > > Thanks for the review. > > On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko > wrote: >> Hi Prabhakar Lad, >> >> >> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote: >>> >>> From: KV Sujith >>> >>> This patch adds OF parser support for davinci gpio >>> driver and also appropriate documentation in gpio-davinci.txt >>> located at Documentation/devicetree/bindings/gpio/. >> >> >> I worry, do we need to have gpio_chip.of_xlate() callback implemented? > > I looked for the other OF GPIO implementations with same "ngpio" > property (marvel, msm) but I don=92t see of_xlate() callback implemented. The question: will below definitions in DT work or not after this series? Will of_get_gpio()/of_get_named_gpio() work? Example1 - leds: leds { compatible =3D "gpio-leds"; debug0 { label =3D "green:debug0"; gpios =3D <&gpio 29 GPIO_ACTIVE_HIGH>; }; }; Example2 - any dev: devA { compatible =3D "devA"; gpios =3D <&gpio 120 GPIO_ACTIVE_LOW>; } > >> - From one side, Davinci GPIO controller in DT described by one entry >> which defines number of supported GPIOs as "ti,ngpio =3D <144>;" >> >> - From other side, on Linux level more than one gpio_chip objects are >> instantiated (one per each 32 GPIO). >> >> How the standard GPIO biding will work in this case? .. And will they? >> >> Linus, I'd very appreciate if you will be able to clarify this point. >> >> >>> >>> Signed-off-by: KV Sujith >>> Signed-off-by: Philip Avinash >>> [prabhakar.csengg-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org: simplified the OF code, removed >>> unnecessary DT property and also simplified >>> the commit message] >>> Signed-off-by: Lad, Prabhakar >>> --- >>> .../devicetree/bindings/gpio/gpio-davinci.txt | 32 = ++++++++++++ >>> drivers/gpio/gpio-davinci.c | 54 >>> ++++++++++++++++++-- >>> 2 files changed, 83 insertions(+), 3 deletions(-) >>> create mode 100644 >>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> new file mode 100644 >>> index 0000000..55aae1c >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> @@ -0,0 +1,32 @@ >>> +Davinci GPIO controller bindings29 >>> + >>> +Required Properties: >>> +- compatible: should be "ti,dm6441-gpio" >>> + >>> +- reg: Physical base address of the controller and the size of memory >>> mapped >>> + registers. >>> + >>> +- gpio-controller : Marks the device node as a gpio controller. >>> + >>> +- interrupts: Array of GPIO interrupt number. >> >> >> May be meaning of property need to be extended, because, >> as of now, only banked or unbanked IRQs are supported - and not both. >> >> > OK > >>> + >>> +- ti,ngpio: The number of GPIO pins supported. >>> + >>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an = individual >>> interrupt >>> + line to processor. >> >> >> Should interrupt-controller; specifier be added here? >> > No So, it would be impossible to map GPIO IRQ to device through DT. Right? Like: devX@0 { compatible =3D "devX"; interrupt-parent =3D <&gpio>; interrupts =3D <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */ }; > >> >>> + >>> +Example: >>> + >>> +gpio: gpio@1e26000 { >>> + compatible =3D "ti,dm6441-gpio"; >>> + gpio-controller; >>> + reg =3D <0x226000 0x1000>; >>> + interrupts =3D <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH >>> + 44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH >>> + 46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH >>> + 48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH >>> + 50 IRQ_TYPE_EDGE_BOTH>; >>> + ti,ngpio =3D <144>; >>> + ti,davinci-gpio-irq-base =3D <101>; >> >> >> ^^ Is it still needed? >> > OOps missed to remove that. > Regards, -grygorii