Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
On Thu, Mar 13, 2014 at 6:25 PM, Jason Gunthorpe 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. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
On Thu, Mar 13, 2014 at 03:25:50PM -0700, Tim Harvey wrote: > On Thu, Mar 13, 2014 at 11:25 AM, Jason Gunthorpe > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
On Thu, Mar 13, 2014 at 11:25 AM, Jason Gunthorpe 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
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? -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ARM: DT: fix gic interrupt controller documentation
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
Cc: Jason Gunthorpe
Cc: Jingoo Han
Cc: Lucas Stach
Cc: Mark Rutland
Cc: linux-samsung-soc
Cc: Richard Zhu
Cc: Sascha Hauer
Cc: Arnd Bergmann
Cc: Stephen Warren
Cc: Bjorn Helgaas
Cc: Simon Horman
Cc: Thierry Reding
Cc: Ben Dooks
Cc: linux-tegra
Cc: Kukjin Kim
Cc: Shawn Guo
Cc: Grant Likely
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
