Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Friday 29 November 2013 01:16 PM, Linus Walleij wrote: > On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori wrote: >> On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: > >>> Actually, the same was proposed by Linus, but we've tried avoid such huge >>> rework - >>> by switching to one irq_domain per all banks for example. >> >> I didn't really read that proposal from Linus so if two people >> independently suggested the same thing, there must be something worth >> considering there :) > > From a GPIO POV it's not such a big deal really, this approach is fine > and the important thing is that we progress toward a more standard > driver... it's more a question for the DT people IMO. I really like the > current patch set. Rob has acked v5 of this patch. So no objection from DT people too. My concern was a bit futuristic. Since everyone is happy I think we can go with the current patch itself. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Friday 29 November 2013 01:16 PM, Linus Walleij wrote: On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori nsek...@ti.com wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. I didn't really read that proposal from Linus so if two people independently suggested the same thing, there must be something worth considering there :) From a GPIO POV it's not such a big deal really, this approach is fine and the important thing is that we progress toward a more standard driver... it's more a question for the DT people IMO. I really like the current patch set. Rob has acked v5 of this patch. So no objection from DT people too. My concern was a bit futuristic. Since everyone is happy I think we can go with the current patch itself. Thanks, Sekhar -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori wrote: > On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: >> Actually, the same was proposed by Linus, but we've tried avoid such huge >> rework - >> by switching to one irq_domain per all banks for example. > > I didn't really read that proposal from Linus so if two people > independently suggested the same thing, there must be something worth > considering there :) >From a GPIO POV it's not such a big deal really, this approach is fine and the important thing is that we progress toward a more standard driver... it's more a question for the DT people IMO. I really like the current patch set. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Tue, Nov 26, 2013 at 6:12 PM, Sekhar Nori nsek...@ti.com wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. I didn't really read that proposal from Linus so if two people independently suggested the same thing, there must be something worth considering there :) From a GPIO POV it's not such a big deal really, this approach is fine and the important thing is that we progress toward a more standard driver... it's more a question for the DT people IMO. I really like the current patch set. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Wednesday 27 November 2013 01:11 AM, Grygorii Strashko wrote: > On 11/26/2013 07:12 PM, Sekhar Nori wrote: >> On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: >>> On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. > > Acked-by: Linus Walleij > Acked-by: Rob Herring > Signed-off-by: KV Sujith > Signed-off-by: Philip Avinash > [prabhakar.cse...@gmail.com: 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 | 41 > ++ >drivers/gpio/gpio-davinci.c| 57 > ++-- >2 files changed, 95 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..a2e839d > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt > @@ -0,0 +1,41 @@ > +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. > + > +- interrupt-parent: phandle of the parent interrupt controller. > + > +- interrupts: Array of GPIO interrupt number. Only banked or > unbanked IRQs are > + supported at a time. If this is true.. > + > +- ti,ngpio: The number of GPIO pins supported. > + > +- ti,davinci-gpio-unbanked: The number of GPIOs that have an > individual interrupt > + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). >>> >>> Number of IRQ can't be simply used to determine type of IRQ - need to >>> handle IRQ names, >>> because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 >>> GPIO). >> >> Okay. That's why I inserted that comment in parenthesis :) >> >>> >>> Few things here: >>> - The mixed banked/unbanked functionality has never been supported >>> before. >> >> True. I actually misread the driver before. >> >>> - The Davinci GPIO IP is different from OMAP and has common >>>control registers for all banks. >> >> Well the only common register I can see is BINTEN - bank interrupt >> enable. This register can simply be initialized to enable interrupts >> from all banks possible as until the rising and falling edge triggers >> are programmed, there wont be any actual interrupts generated. >> >>> - The proposed approach is more less easy to implement for DT case, >>> but for not-DT >>>case - the platform data will need to be changed significantly (. >>>So, from this point of view, that would be a big change (actually >>> the total driver rewriting). >> >> Well, I certainly don't think its a complete driver re-write. It will >> take a bit of effort agreed, but I think the driver will also come out a >> lot cleaner. >> >> Honestly, I am not so much worried about the kernel code here. Its the >> bindings I am worried about. Once the bindings go in assuming there will >> never be banked and unbanked GPIO IRQs on the same SoC, changing them to >> do something else will be very painful with the need to keep backward >> compatibility and support for both semantics. >> >> That said, because there is no present hardware which needs both banked >> and unbanked at the same time, I wont press for this to be done >> endlessly. >> >>> Do you have any thoughts about how it can be done in a simpler way? >> >> I don't know if there is a "simpler" way, but I don't think there is too >> much
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/26/2013 07:12 PM, Sekhar Nori wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. Acked-by: Linus Walleij Acked-by: Rob Herring Signed-off-by: KV Sujith Signed-off-by: Philip Avinash [prabhakar.cse...@gmail.com: 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 | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt +line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) Few things here: - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a "simpler" way, but I don't think there is too much effort too. I leave it to those implementing it though. Oh. I see no problem to implement it for DT, but this change require to convert one device to the tree of devices: GPIO controller |- GPIO bank1 ... |- GPIO bankX And that's will need to be handled somehow from platform code (which is non-DT and which I can't verify and which I don't want to touch actually ;). Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. I didn't really read that proposal from Linus
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: > On 11/25/2013 01:00 PM, Sekhar Nori wrote: >> On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. >>> >>> Acked-by: Linus Walleij >>> Acked-by: Rob Herring >>> Signed-off-by: KV Sujith >>> Signed-off-by: Philip Avinash >>> [prabhakar.cse...@gmail.com: 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 | 41 ++ >>> drivers/gpio/gpio-davinci.c| 57 >>> ++-- >>> 2 files changed, 95 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..a2e839d >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >>> @@ -0,0 +1,41 @@ >>> +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. >>> + >>> +- interrupt-parent: phandle of the parent interrupt controller. >>> + >>> +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs >>> are >>> + supported at a time. >> >> If this is true.. >> >>> + >>> +- ti,ngpio: The number of GPIO pins supported. >>> + >>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual >>> interrupt >>> +line to processor. >> >> .. then why do you need to maintain this separately? Number of elements >> in interrupts property should give you this answer, no? >> >> There can certainly be devices (past and future) which use a mixture of >> banked and unbanked IRQs. So a binding which does not take care of this >> is likely to change in future and that is a problem since it brings in >> backward compatibility of the binding into picture. >> >> The right thing would be to define the DT node per-bank similar to what >> is done on OMAP rather than for all banks together. That way there can >> be a separate property which determines whether that bank supports >> direct-mapped or banked IRQs (or that could be inferred if the number of >> tuples in the interrupts property is more than one). > > Number of IRQ can't be simply used to determine type of IRQ - need to handle > IRQ names, > because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) > > Few things here: > - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. > - The Davinci GPIO IP is different from OMAP and has common > control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. > - The proposed approach is more less easy to implement for DT case, but for > not-DT > case - the platform data will need to be changed significantly (. > So, from this point of view, that would be a big change (actually the total > driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. > Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a "simpler" way, but I don't think there is too much effort too. I leave it to those implementing it though. > > Actually, the same was proposed by Linus, but we've tried avoid such huge > rework - > by switching to one irq_domain per all banks for example. I didn't really read that proposal from Linus so if two people independently suggested the same thing, there must be something worth considering there :) Thanks,
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/25/2013 01:00 PM, Sekhar Nori wrote: > On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. >> >> Acked-by: Linus Walleij >> Acked-by: Rob Herring >> Signed-off-by: KV Sujith >> Signed-off-by: Philip Avinash >> [prabhakar.cse...@gmail.com: 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 | 41 ++ >> drivers/gpio/gpio-davinci.c| 57 >> ++-- >> 2 files changed, 95 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..a2e839d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> @@ -0,0 +1,41 @@ >> +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. >> + >> +- interrupt-parent: phandle of the parent interrupt controller. >> + >> +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs >> are >> + supported at a time. > > If this is true.. > >> + >> +- ti,ngpio: The number of GPIO pins supported. >> + >> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual >> interrupt >> + line to processor. > > .. then why do you need to maintain this separately? Number of elements > in interrupts property should give you this answer, no? > > There can certainly be devices (past and future) which use a mixture of > banked and unbanked IRQs. So a binding which does not take care of this > is likely to change in future and that is a problem since it brings in > backward compatibility of the binding into picture. > > The right thing would be to define the DT node per-bank similar to what > is done on OMAP rather than for all banks together. That way there can > be a separate property which determines whether that bank supports > direct-mapped or banked IRQs (or that could be inferred if the number of > tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Few things here: - The mixed banked/unbanked functionality has never been supported before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Do you have any thoughts about how it can be done in a simpler way? Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. Regards, - grygorii -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
Hi Sekhar, Thanks for the review. On Mon, Nov 25, 2013 at 4:30 PM, Sekhar Nori wrote: > On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. >> >> Acked-by: Linus Walleij >> Acked-by: Rob Herring >> Signed-off-by: KV Sujith >> Signed-off-by: Philip Avinash >> [prabhakar.cse...@gmail.com: 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 | 41 ++ >> drivers/gpio/gpio-davinci.c| 57 >> ++-- >> 2 files changed, 95 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..a2e839d >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt >> @@ -0,0 +1,41 @@ >> +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. >> + >> +- interrupt-parent: phandle of the parent interrupt controller. >> + >> +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs >> are >> + supported at a time. > > If this is true.. > >> + >> +- ti,ngpio: The number of GPIO pins supported. >> + >> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual >> interrupt >> + line to processor. > > .. then why do you need to maintain this separately? Number of elements > in interrupts property should give you this answer, no? > > There can certainly be devices (past and future) which use a mixture of > banked and unbanked IRQs. So a binding which does not take care of this > is likely to change in future and that is a problem since it brings in > backward compatibility of the binding into picture. > > The right thing would be to define the DT node per-bank similar to what > is done on OMAP rather than for all banks together. That way there can > be a separate property which determines whether that bank supports > direct-mapped or banked IRQs (or that could be inferred if the number of > tuples in the interrupts property is more than one). > Can you point me to the OMAP implementation. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
Hi Sekhar, Thanks for the review. On Mon, Nov 25, 2013 at 4:30 PM, Sekhar Nori nsek...@ti.com wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Can you point me to the OMAP implementation. Regards, --Prabhakar Lad -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Few things here: - The mixed banked/unbanked functionality has never been supported before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Do you have any thoughts about how it can be done in a simpler way? Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. Regards, - grygorii -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt +line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) Few things here: - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a simpler way, but I don't think there is too much effort too. I leave it to those implementing it though. Actually, the same was proposed by Linus, but we've tried avoid such huge rework - by switching to one irq_domain per all banks for example. I didn't really read that proposal from Linus so if two people independently suggested the same thing, there must be something worth considering there :) Thanks, Sekhar -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On 11/26/2013 07:12 PM, Sekhar Nori wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt +line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) Few things here: - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a simpler way, but I don't think there is too much effort too. I leave it to those implementing it though. Oh. I see no problem to implement it for DT, but this change require to convert one device to the tree of devices: GPIO controller |- GPIO bank1 ... |- GPIO bankX And that's will need to be handled somehow from platform code (which is non-DT and which I can't verify and which I don't want to touch actually ;). Actually, the same was proposed by Linus, but we've tried avoid such
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Wednesday 27 November 2013 01:11 AM, Grygorii Strashko wrote: On 11/26/2013 07:12 PM, Sekhar Nori wrote: On Tuesday 26 November 2013 06:03 PM, Grygorii Strashko wrote: On 11/25/2013 01:00 PM, Sekhar Nori wrote: On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Number of IRQ can't be simply used to determine type of IRQ - need to handle IRQ names, because each bank(32 gpios) may have up to 2 banked IRQs (one per 16 GPIO). Okay. That's why I inserted that comment in parenthesis :) Few things here: - The mixed banked/unbanked functionality has never been supported before. True. I actually misread the driver before. - The Davinci GPIO IP is different from OMAP and has common control registers for all banks. Well the only common register I can see is BINTEN - bank interrupt enable. This register can simply be initialized to enable interrupts from all banks possible as until the rising and falling edge triggers are programmed, there wont be any actual interrupts generated. - The proposed approach is more less easy to implement for DT case, but for not-DT case - the platform data will need to be changed significantly (. So, from this point of view, that would be a big change (actually the total driver rewriting). Well, I certainly don't think its a complete driver re-write. It will take a bit of effort agreed, but I think the driver will also come out a lot cleaner. Honestly, I am not so much worried about the kernel code here. Its the bindings I am worried about. Once the bindings go in assuming there will never be banked and unbanked GPIO IRQs on the same SoC, changing them to do something else will be very painful with the need to keep backward compatibility and support for both semantics. That said, because there is no present hardware which needs both banked and unbanked at the same time, I wont press for this to be done endlessly. Do you have any thoughts about how it can be done in a simpler way? I don't know if there is a simpler way, but I don't think there is too much effort too. I leave it to those implementing it though. Oh. I see no problem to implement it for DT, but this change require to convert one device to the tree of devices: GPIO controller |- GPIO bank1 ... |- GPIO bankX And that's will need to be handled somehow from platform code (which is non-DT and which I can't verify
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad 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/. > > Acked-by: Linus Walleij > Acked-by: Rob Herring > Signed-off-by: KV Sujith > Signed-off-by: Philip Avinash > [prabhakar.cse...@gmail.com: 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 | 41 ++ > drivers/gpio/gpio-davinci.c| 57 > ++-- > 2 files changed, 95 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..a2e839d > --- /dev/null > +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt > @@ -0,0 +1,41 @@ > +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. > + > +- interrupt-parent: phandle of the parent interrupt controller. > + > +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs > are > + supported at a time. If this is true.. > + > +- ti,ngpio: The number of GPIO pins supported. > + > +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual > interrupt > + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 4/6] gpio: davinci: add OF support
On Thursday 21 November 2013 11:45 PM, Prabhakar Lad wrote: From: KV Sujith sujit...@ti.com This patch adds OF parser support for davinci gpio driver and also appropriate documentation in gpio-davinci.txt located at Documentation/devicetree/bindings/gpio/. Acked-by: Linus Walleij linus.wall...@linaro.org Acked-by: Rob Herring rob.herr...@calxeda.com Signed-off-by: KV Sujith sujit...@ti.com Signed-off-by: Philip Avinash avinashphi...@ti.com [prabhakar.cse...@gmail.com: simplified the OF code, removed unnecessary DT property and also simplified the commit message] Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- .../devicetree/bindings/gpio/gpio-davinci.txt | 41 ++ drivers/gpio/gpio-davinci.c| 57 ++-- 2 files changed, 95 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..a2e839d --- /dev/null +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt @@ -0,0 +1,41 @@ +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. + +- interrupt-parent: phandle of the parent interrupt controller. + +- interrupts: Array of GPIO interrupt number. Only banked or unbanked IRQs are + supported at a time. If this is true.. + +- ti,ngpio: The number of GPIO pins supported. + +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt + line to processor. .. then why do you need to maintain this separately? Number of elements in interrupts property should give you this answer, no? There can certainly be devices (past and future) which use a mixture of banked and unbanked IRQs. So a binding which does not take care of this is likely to change in future and that is a problem since it brings in backward compatibility of the binding into picture. The right thing would be to define the DT node per-bank similar to what is done on OMAP rather than for all banks together. That way there can be a separate property which determines whether that bank supports direct-mapped or banked IRQs (or that could be inferred if the number of tuples in the interrupts property is more than one). Thanks, Sekhar -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
