Re: [PATCH v3 1/3] gpio: davinci: add OF support
On Fri, Oct 4, 2013 at 6:03 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com Acked-by: Linus Walleij linus.wall...@linaro.org ^Don't trust this guy. [prabhakar.cse...@gmail.com: simplified the OF code and also the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 34 +++ drivers/gpio/gpio-davinci.c| 60 +++- 2 files changed, 91 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 000..87abd3b --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,34 @@ +Davinci GPIO controller bindings + +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. + +- ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-irq-base: Base from where GPIO interrupt numbering starts. What is this? If I have ever ACKed this I have been drunk. I take it back. This base is a Linux-specific thing and has no place in the device tree, and shall not be there. You have to find some way to avoid this, what do you think some other OS should do with this value... All IRQs in Linux are assumed to be dynamically assigned numbers nowadays, with a property like this you can never switch on SPARSE_IRQ for the DaVinci. Yours, Linus Walleij ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 1/3] gpio: davinci: add OF support
On Fri, Oct 11, 2013 at 4:59 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: On 10/11/13, Linus Walleij linus.wall...@linaro.org wrote: On Fri, Oct 4, 2013 at 6:03 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: +- ti,davinci-gpio-irq-base: Base from where GPIO interrupt numbering starts. What is this? If I have ever ACKed this I have been drunk. I take it back. here is the ACK https://patchwork.kernel.org/patch/2721181/ And as suspected that version of the patch did not contain this strange node property. Don't keep my ACK on patches if you change basic stuff like that, they need to be re-acked, this runs the risk of abusing my trust amongst other subsystem maintainers who might go and merge this because aha the GPIO maintainer thinks that this is OK. This base is a Linux-specific thing and has no place in the device tree, and shall not be there. You have to find some way to avoid this, what do you think some other OS should do with this value... All IRQs in Linux are assumed to be dynamically assigned numbers nowadays, with a property like this you can never switch on SPARSE_IRQ for the DaVinci. Can you point to any alternative solution if you have any ? First convert this GPIO driver to use an irqdomain to map HW IRQs to Linux IRQs, and grab a few IRQ descriptors dynamically off the irq descriptor heap. Example: commit a6c45b99a658521291cfb66ecf035cc58b38f206 pinctrl/coh901: use irqdomain, allocate irqdescs Then on a longer term convert DaVinci to use dynamically allocated IRQs for all interrupt controllers, and move it over to SPARSE_IRQ so you know this works. Yours, Linus Walleij ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH v3 1/3] gpio: davinci: add OF support
On Fri, Oct 11, 2013 at 6:18 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: On 10/11/13, Linus Walleij linus.wall...@linaro.org wrote: On Fri, Oct 11, 2013 at 4:59 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: On 10/11/13, Linus Walleij linus.wall...@linaro.org wrote: On Fri, Oct 4, 2013 at 6:03 PM, Prabhakar Lad prabhakar.cse...@gmail.com wrote: +- ti,davinci-gpio-irq-base: Base from where GPIO interrupt numbering starts. What is this? If I have ever ACKed this I have been drunk. I take it back. here is the ACK https://patchwork.kernel.org/patch/2721181/ And as suspected that version of the patch did not contain this strange node property. The property did exist in the patch 'intc_irq_num', I just renamed it and gave a proper description to it. Hm yeah you're right ... I didn't understand what it was actually doing until I saw the revised documentation, I though it was stating the number of (hardware) IRQs, but it was stating the Linux-internal offset. Yours, Linus Walleij ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
