* [PATCH 0/2] ARM: IMX: fix ability to use pcie interrupt-map @ 2014-03-13 17:40 Tim Harvey 2014-03-13 17:40 ` [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation Tim Harvey 2014-03-13 17:40 ` [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value Tim Harvey 0 siblings, 2 replies; 9+ messages in thread From: Tim Harvey @ 2014-03-13 17:40 UTC (permalink / raw) To: linux-pci; +Cc: linux-arm-kernel The #address-cells of the arm-cortext-a9 gic in the imx pcie host controller node is invalid which breaks the ability to user interrupt-map for legacy pci interrupts. Tim Harvey (2): ARM: DT: fix gic interrupt controller documentation ARM: dts: imx: fix invalid #address-cells value Documentation/devicetree/bindings/arm/gic.txt | 2 +- arch/arm/boot/dts/imx6qdl.dtsi | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 17:40 [PATCH 0/2] ARM: IMX: fix ability to use pcie interrupt-map Tim Harvey @ 2014-03-13 17:40 ` Tim Harvey 2014-03-13 17:44 ` Stephen Warren 2014-03-13 17:40 ` [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value Tim Harvey 1 sibling, 1 reply; 9+ messages in thread From: Tim Harvey @ 2014-03-13 17:40 UTC (permalink / raw) To: linux-pci Cc: linux-arm-kernel, Jason Gunthorpe, Jingoo Han, Lucas Stach, Mark Rutland, linux-samsung-soc, Richard Zhu, Sascha Hauer, Arnd Bergmann, Stephen Warren, Bjorn Helgaas, Simon Horman, Thierry Reding, Ben Dooks, linux-tegra, Kukjin Kim, Shawn Guo, Grant Likely When using interrupt-maps, the size of a map entry is #address-cells + #interrupt-cells for the parent interrupt controller. For the ARM GIC address-cells should be 0 as this is not used. This patch fixes the example by correctly specifying #address-cells = 0. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Mark Rutland <mark.rutland@arm.com> Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org> Cc: Richard Zhu <r65037@freescale.com> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Simon Horman <horms@verge.net.au> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Ben Dooks <ben-linux@fluff.org> Cc: linux-tegra <linux-tegra@vger.kernel.org> Cc: Kukjin Kim <kgene.kim@samsung.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Grant Likely <grant.likely@linaro.org> --- Documentation/devicetree/bindings/arm/gic.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt index bae0d87..b59ff1f 100644 --- a/Documentation/devicetree/bindings/arm/gic.txt +++ b/Documentation/devicetree/bindings/arm/gic.txt @@ -55,7 +55,7 @@ Example: intc: interrupt-controller@fff11000 { compatible = "arm,cortex-a9-gic"; #interrupt-cells = <3>; - #address-cells = <1>; + #address-cells = <0>; interrupt-controller; reg = <0xfff11000 0x1000>, <0xfff10100 0x100>; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 17:40 ` [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation Tim Harvey @ 2014-03-13 17:44 ` Stephen Warren 2014-03-13 18:25 ` Jason Gunthorpe 0 siblings, 1 reply; 9+ messages in thread From: Stephen Warren @ 2014-03-13 17:44 UTC (permalink / raw) To: Tim Harvey, linux-pci Cc: linux-arm-kernel, Jason Gunthorpe, Jingoo Han, Lucas Stach, Mark Rutland, linux-samsung-soc, Richard Zhu, Sascha Hauer, Arnd Bergmann, Bjorn Helgaas, Simon Horman, Thierry Reding, Ben Dooks, linux-tegra, Kukjin Kim, Shawn Guo, Grant Likely On 03/13/2014 11:40 AM, Tim Harvey wrote: > When using interrupt-maps, the size of a map entry is #address-cells + > #interrupt-cells for the parent interrupt controller. For the ARM GIC > address-cells should be 0 as this is not used. > > This patch fixes the example by correctly specifying #address-cells = 0. If the #address-cells property is required (well, or even optional...) in the node, shouldn't it be included in the list of required/optional properties above, and not solely included in the example? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 17:44 ` Stephen Warren @ 2014-03-13 18:25 ` Jason Gunthorpe 2014-03-13 22:25 ` Tim Harvey 2014-03-14 13:05 ` Grant Likely 0 siblings, 2 replies; 9+ messages in thread From: Jason Gunthorpe @ 2014-03-13 18:25 UTC (permalink / raw) To: Stephen Warren Cc: Tim Harvey, linux-pci, linux-arm-kernel, Jingoo Han, Lucas Stach, Mark Rutland, linux-samsung-soc, Richard Zhu, Sascha Hauer, Arnd Bergmann, Bjorn Helgaas, Simon Horman, Thierry Reding, Ben Dooks, linux-tegra, Kukjin Kim, Shawn Guo, Grant Likely On Thu, Mar 13, 2014 at 11:44:33AM -0600, Stephen Warren wrote: > On 03/13/2014 11:40 AM, Tim Harvey wrote: > > When using interrupt-maps, the size of a map entry is #address-cells + > > #interrupt-cells for the parent interrupt controller. For the ARM GIC > > address-cells should be 0 as this is not used. > > > > This patch fixes the example by correctly specifying #address-cells = 0. > > If the #address-cells property is required (well, or even optional...) > in the node, shouldn't it be included in the list of required/optional > properties above, and not solely included in the example? AFAIK, #address-cells = 0 is the same as not including a #address-cells at all. Omitting entirely is what other interrupt-controller bindings are doing, so I'd just drop mention of #address-cells completely. Jason ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 18:25 ` Jason Gunthorpe @ 2014-03-13 22:25 ` Tim Harvey 2014-03-14 12:14 ` Shawn Guo 2014-03-14 13:05 ` Grant Likely 1 sibling, 1 reply; 9+ messages in thread From: Tim Harvey @ 2014-03-13 22:25 UTC (permalink / raw) To: Jason Gunthorpe Cc: Stephen Warren, linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Jingoo Han, Lucas Stach, Mark Rutland, linux-samsung-soc, Richard Zhu, Sascha Hauer, Arnd Bergmann, Bjorn Helgaas, Simon Horman, Thierry Reding, Ben Dooks, linux-tegra, Kukjin Kim, Shawn Guo, Grant Likely On Thu, Mar 13, 2014 at 11:25 AM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Thu, Mar 13, 2014 at 11:44:33AM -0600, Stephen Warren wrote: >> On 03/13/2014 11:40 AM, Tim Harvey wrote: >> > When using interrupt-maps, the size of a map entry is #address-cells + >> > #interrupt-cells for the parent interrupt controller. For the ARM GIC >> > address-cells should be 0 as this is not used. >> > >> > This patch fixes the example by correctly specifying #address-cells = 0. >> >> If the #address-cells property is required (well, or even optional...) >> in the node, shouldn't it be included in the list of required/optional >> properties above, and not solely included in the example? > > AFAIK, #address-cells = 0 is the same as not including a > #address-cells at all. > > Omitting entirely is what other interrupt-controller bindings are > doing, so I'd just drop mention of #address-cells completely. > > Jason Jason / Stephen, Yes, it does indeed default to 0 if not specified. For arm-gic it would appear #address-cells must be 0, so should we remove all places its set explicitly to 0 in dts as wel as the example in Documentation/devicetree/bindings? Looking through arch/arm/boot/dts I'm seeing other dts configs using 'arm,cortex-a9-gic' which I'm guessing are equally as wrong as the imx6 situation: - arch/arm/boot/dts/highbank.dts - arch/arm/boot/dts/sh73a0.dtsi I'm also seeing a number of places where its set explicitly to 0: - Documentation/devicetree/bindings/arm/gic.txt - Documentation/devicetree/bindings/arm/vexpress.txt - arch/arm/boot/dts/bcm11351.dtsi - arch/arm/boot/dts/hi3620.dtsi - arch/arm/boot/dts/imx6qdl.dtsi - arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts If I extend this search to the other compatible gic interrupt controllers implemented in drivers/irqchip/irq-gic.c (arm,cortex-a15-gic, qcom,msm-8660-qgic, qcom,msm-qgic2) I find even more examples where #address-cells is set explicitly to 0 or to 1 (and I would believe a value of 1 is invalid, just as it is in my case). I'm happy to broaden my patch, but I certainly want to make sure I'm doing the right thing with it, as I don't have the ability to test all the other devicetree configs and am by no means a devicetree expert. Recommendations? Tim ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 22:25 ` Tim Harvey @ 2014-03-14 12:14 ` Shawn Guo 0 siblings, 0 replies; 9+ messages in thread From: Shawn Guo @ 2014-03-14 12:14 UTC (permalink / raw) To: Tim Harvey Cc: Jason Gunthorpe, Mark Rutland, Richard Zhu, linux-samsung-soc, Grant Likely, Ben Dooks, Arnd Bergmann, Stephen Warren, linux-pci@vger.kernel.org, Jingoo Han, linux-tegra, Simon Horman, Thierry Reding, Sascha Hauer, Bjorn Helgaas, Kukjin Kim, Shawn Guo, linux-arm-kernel@lists.infradead.org, Lucas Stach, Rob Herring On Thu, Mar 13, 2014 at 03:25:50PM -0700, Tim Harvey wrote: > On Thu, Mar 13, 2014 at 11:25 AM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > On Thu, Mar 13, 2014 at 11:44:33AM -0600, Stephen Warren wrote: > >> On 03/13/2014 11:40 AM, Tim Harvey wrote: > >> > When using interrupt-maps, the size of a map entry is #address-cells + > >> > #interrupt-cells for the parent interrupt controller. For the ARM GIC > >> > address-cells should be 0 as this is not used. > >> > > >> > This patch fixes the example by correctly specifying #address-cells = 0. > >> > >> If the #address-cells property is required (well, or even optional...) > >> in the node, shouldn't it be included in the list of required/optional > >> properties above, and not solely included in the example? > > > > AFAIK, #address-cells = 0 is the same as not including a > > #address-cells at all. > > > > Omitting entirely is what other interrupt-controller bindings are > > doing, so I'd just drop mention of #address-cells completely. > > > > Jason > > Jason / Stephen, > > Yes, it does indeed default to 0 if not specified. For arm-gic it > would appear #address-cells must be 0, so should we remove all places > its set explicitly to 0 in dts as wel as the example in > Documentation/devicetree/bindings? > > Looking through arch/arm/boot/dts I'm seeing other dts configs using > 'arm,cortex-a9-gic' which I'm guessing are equally as wrong as the > imx6 situation: > - arch/arm/boot/dts/highbank.dts > - arch/arm/boot/dts/sh73a0.dtsi > > I'm also seeing a number of places where its set explicitly to 0: > - Documentation/devicetree/bindings/arm/gic.txt > - Documentation/devicetree/bindings/arm/vexpress.txt > - arch/arm/boot/dts/bcm11351.dtsi > - arch/arm/boot/dts/hi3620.dtsi > - arch/arm/boot/dts/imx6qdl.dtsi > - arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts > > If I extend this search to the other compatible gic interrupt > controllers implemented in drivers/irqchip/irq-gic.c > (arm,cortex-a15-gic, qcom,msm-8660-qgic, qcom,msm-qgic2) I find even > more examples where #address-cells is set explicitly to 0 or to 1 (and > I would believe a value of 1 is invalid, just as it is in my case). > > I'm happy to broaden my patch, but I certainly want to make sure I'm > doing the right thing with it, as I don't have the ability to test all > the other devicetree configs and am by no means a devicetree expert. > > Recommendations? Copy Rob, who might want to comment a bit. My understanding is that #address-cells and #size-cells are only valid for a node that may have child nodes. Will gic node possibly get any child node? Shawn ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation 2014-03-13 18:25 ` Jason Gunthorpe 2014-03-13 22:25 ` Tim Harvey @ 2014-03-14 13:05 ` Grant Likely 1 sibling, 0 replies; 9+ messages in thread From: Grant Likely @ 2014-03-14 13:05 UTC (permalink / raw) To: Jason Gunthorpe Cc: Stephen Warren, Mark Rutland, Richard Zhu, linux-samsung-soc, Ben Dooks, Arnd Bergmann, linux-pci@vger.kernel.org, Tim Harvey, Jingoo Han, linux-tegra, Simon Horman, Thierry Reding, Sascha Hauer, Bjorn Helgaas, Kukjin Kim, Shawn Guo, linux-arm-kernel@lists.infradead.org, Lucas Stach On Thu, Mar 13, 2014 at 6:25 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Thu, Mar 13, 2014 at 11:44:33AM -0600, Stephen Warren wrote: >> On 03/13/2014 11:40 AM, Tim Harvey wrote: >> > When using interrupt-maps, the size of a map entry is #address-cells + >> > #interrupt-cells for the parent interrupt controller. For the ARM GIC >> > address-cells should be 0 as this is not used. >> > >> > This patch fixes the example by correctly specifying #address-cells = 0. >> >> If the #address-cells property is required (well, or even optional...) >> in the node, shouldn't it be included in the list of required/optional >> properties above, and not solely included in the example? > > AFAIK, #address-cells = 0 is the same as not including a > #address-cells at all. > > Omitting entirely is what other interrupt-controller bindings are > doing, so I'd just drop mention of #address-cells completely. Correct. Just drop it. g. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value 2014-03-13 17:40 [PATCH 0/2] ARM: IMX: fix ability to use pcie interrupt-map Tim Harvey 2014-03-13 17:40 ` [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation Tim Harvey @ 2014-03-13 17:40 ` Tim Harvey 2014-03-14 13:31 ` Shawn Guo 1 sibling, 1 reply; 9+ messages in thread From: Tim Harvey @ 2014-03-13 17:40 UTC (permalink / raw) To: linux-pci Cc: linux-arm-kernel, Jason Gunthorpe, Jingoo Han, Lucas Stach, Mark Rutland, linux-samsung-soc, Richard Zhu, Sascha Hauer, Arnd Bergmann, Stephen Warren, Bjorn Helgaas, Simon Horman, Thierry Reding, Ben Dooks, linux-tegra, Kukjin Kim, Shawn Guo, Grant Likely The invalid value of #address-cells in the imx6 pcie host controller node causes of_irq_parse_raw() to incorrectly advance through an interrupt-map table of more than one interrupt. This patch resolves this issue and allows proper interrupt mapping for an imx6 pcie host connected to a P2P bridge when using legacy interrupts. Signed-off-by: Tim Harvey <tharvey@gateworks.com> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Jingoo Han <jg1.han@samsung.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Mark Rutland <mark.rutland@arm.com> Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org> Cc: Richard Zhu <r65037@freescale.com> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Stephen Warren <swarren@wwwdotorg.org> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Simon Horman <horms@verge.net.au> Cc: Thierry Reding <thierry.reding@gmail.com> Cc: Ben Dooks <ben-linux@fluff.org> Cc: linux-tegra <linux-tegra@vger.kernel.org> Cc: Kukjin Kim <kgene.kim@samsung.com> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Grant Likely <grant.likely@linaro.org> --- arch/arm/boot/dts/imx6qdl.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi index af8d28d..2285299 100644 --- a/arch/arm/boot/dts/imx6qdl.dtsi +++ b/arch/arm/boot/dts/imx6qdl.dtsi @@ -49,7 +49,7 @@ intc: interrupt-controller@00a01000 { compatible = "arm,cortex-a9-gic"; #interrupt-cells = <3>; - #address-cells = <1>; + #address-cells = <0>; #size-cells = <1>; interrupt-controller; reg = <0x00a01000 0x1000>, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value 2014-03-13 17:40 ` [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value Tim Harvey @ 2014-03-14 13:31 ` Shawn Guo 0 siblings, 0 replies; 9+ messages in thread From: Shawn Guo @ 2014-03-14 13:31 UTC (permalink / raw) To: Tim Harvey Cc: linux-pci, Mark Rutland, Richard Zhu, linux-samsung-soc, Grant Likely, Ben Dooks, Arnd Bergmann, Stephen Warren, Jingoo Han, Jason Gunthorpe, linux-tegra, Simon Horman, Thierry Reding, Sascha Hauer, Bjorn Helgaas, Kukjin Kim, Shawn Guo, linux-arm-kernel, Lucas Stach On Thu, Mar 13, 2014 at 10:40:05AM -0700, Tim Harvey wrote: > The invalid value of #address-cells in the imx6 pcie host controller node > causes of_irq_parse_raw() to incorrectly advance through an interrupt-map > table of more than one interrupt. > > This patch resolves this issue and allows proper interrupt mapping for an > imx6 pcie host connected to a P2P bridge when using legacy interrupts. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Jingoo Han <jg1.han@samsung.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: linux-samsung-soc <linux-samsung-soc@vger.kernel.org> > Cc: Richard Zhu <r65037@freescale.com> > Cc: Sascha Hauer <kernel@pengutronix.de> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Stephen Warren <swarren@wwwdotorg.org> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Simon Horman <horms@verge.net.au> > Cc: Thierry Reding <thierry.reding@gmail.com> > Cc: Ben Dooks <ben-linux@fluff.org> > Cc: linux-tegra <linux-tegra@vger.kernel.org> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Shawn Guo <shawn.guo@linaro.org> > Cc: Grant Likely <grant.likely@linaro.org> > --- > arch/arm/boot/dts/imx6qdl.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi > index af8d28d..2285299 100644 > --- a/arch/arm/boot/dts/imx6qdl.dtsi > +++ b/arch/arm/boot/dts/imx6qdl.dtsi > @@ -49,7 +49,7 @@ > intc: interrupt-controller@00a01000 { > compatible = "arm,cortex-a9-gic"; > #interrupt-cells = <3>; > - #address-cells = <1>; > + #address-cells = <0>; > #size-cells = <1>; So we should probably just drop both #address-cells and #size-cells? If so, please help patch imx6sl.dtsi as well. Shawn > interrupt-controller; > reg = <0x00a01000 0x1000>, > -- > 1.8.3.2 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-03-14 13:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-13 17:40 [PATCH 0/2] ARM: IMX: fix ability to use pcie interrupt-map Tim Harvey 2014-03-13 17:40 ` [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation Tim Harvey 2014-03-13 17:44 ` Stephen Warren 2014-03-13 18:25 ` Jason Gunthorpe 2014-03-13 22:25 ` Tim Harvey 2014-03-14 12:14 ` Shawn Guo 2014-03-14 13:05 ` Grant Likely 2014-03-13 17:40 ` [PATCH 2/2] ARM: dts: imx: fix invalid #address-cells value Tim Harvey 2014-03-14 13:31 ` Shawn Guo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).