With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
like below for all interrupt controllers.
/bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
Fix these by adding #address-cells = <0>; for all interrupt controllers in
TI device-tree files. Any other #address-cells value is really only needed
if interrupt-map property is being used (which is not the case for existing
TI device-tree files)
Signed-off-by: Sekhar Nori <[email protected]>
---
arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
8 files changed, 27 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
index aa8725db0187..55aaa1404d7d 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
@@ -440,6 +440,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <100>;
ti,interrupt-ranges = <0 392 32>;
@@ -461,6 +462,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <182>;
ti,interrupt-ranges = <0 64 64>,
@@ -474,6 +476,7 @@
interrupt-parent = <&intr_main_navss>;
msi-controller;
#interrupt-cells = <0>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <179>;
ti,interrupt-ranges = <0 0 256>;
@@ -670,6 +673,7 @@
interrupts = <192>, <193>, <194>, <195>, <196>, <197>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <96>;
ti,davinci-gpio-unbanked = <0>;
clocks = <&k3_clks 57 0>;
@@ -685,6 +689,7 @@
interrupts = <200>, <201>, <202>, <203>, <204>, <205>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <90>;
ti,davinci-gpio-unbanked = <0>;
clocks = <&k3_clks 58 0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
index ed42f13e7663..7fe5782a1f79 100644
--- a/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi
@@ -75,6 +75,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <156>;
ti,interrupt-ranges = <0 712 16>;
@@ -89,6 +90,7 @@
interrupts = <60>, <61>, <62>, <63>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <56>;
ti,davinci-gpio-unbanked = <0>;
clocks = <&k3_clks 59 0>;
diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index d12dd89f3405..376de272cb4e 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
@@ -236,6 +236,7 @@
interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
};
};
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
index 72d6496e88dd..d07081b20aee 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi
@@ -67,6 +67,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <131>;
ti,interrupt-ranges = <8 392 56>;
@@ -85,6 +86,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <213>;
ti,interrupt-ranges = <0 64 64>,
@@ -97,6 +99,7 @@
reg = <0x00 0x33d00000 0x00 0x100000>;
interrupt-controller;
#interrupt-cells = <0>;
+ #address-cells = <0>;
interrupt-parent = <&main_navss_intr>;
msi-controller;
ti,sci = <&dmsc>;
diff --git a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
index eb2a78a53512..4801876bd107 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -102,6 +102,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <137>;
ti,interrupt-ranges = <16 960 16>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
index 52e121155563..0490cb15f0c9 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts
@@ -442,6 +442,7 @@
interrupts = <11 IRQ_TYPE_EDGE_FALLING>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
};
};
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
index ffedd9531362..7f44692e15ec 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-main.dtsi
@@ -114,6 +114,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <131>;
ti,interrupt-ranges = <8 392 56>;
@@ -135,6 +136,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <213>;
ti,interrupt-ranges = <0 64 64>,
@@ -149,6 +151,7 @@
interrupt-parent = <&main_navss_intr>;
msi-controller;
#interrupt-cells = <0>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <209>;
ti,interrupt-ranges = <0 0 256>;
@@ -948,6 +951,7 @@
<260>, <261>, <262>, <263>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <128>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 105 TI_SCI_PD_EXCLUSIVE>;
@@ -964,6 +968,7 @@
interrupts = <288>, <289>, <290>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <36>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 106 TI_SCI_PD_EXCLUSIVE>;
@@ -981,6 +986,7 @@
<268>, <269>, <270>, <271>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <128>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 107 TI_SCI_PD_EXCLUSIVE>;
@@ -997,6 +1003,7 @@
interrupts = <292>, <293>, <294>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <36>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 108 TI_SCI_PD_EXCLUSIVE>;
@@ -1014,6 +1021,7 @@
<276>, <277>, <278>, <279>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <128>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 109 TI_SCI_PD_EXCLUSIVE>;
@@ -1030,6 +1038,7 @@
interrupts = <296>, <297>, <298>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <36>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 110 TI_SCI_PD_EXCLUSIVE>;
@@ -1047,6 +1056,7 @@
<284>, <285>, <286>, <287>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <128>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 111 TI_SCI_PD_EXCLUSIVE>;
@@ -1063,6 +1073,7 @@
interrupts = <300>, <301>, <302>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <36>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 112 TI_SCI_PD_EXCLUSIVE>;
diff --git a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
index e581cb1d87ee..ed3098ed7b56 100644
--- a/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi
@@ -102,6 +102,7 @@
interrupt-controller;
interrupt-parent = <&gic500>;
#interrupt-cells = <1>;
+ #address-cells = <0>;
ti,sci = <&dmsc>;
ti,sci-dev-id = <137>;
ti,interrupt-ranges = <16 960 16>;
@@ -116,6 +117,7 @@
interrupts = <103>, <104>, <105>, <106>, <107>, <108>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <84>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 113 TI_SCI_PD_EXCLUSIVE>;
@@ -132,6 +134,7 @@
interrupts = <112>, <113>, <114>, <115>, <116>, <117>;
interrupt-controller;
#interrupt-cells = <2>;
+ #address-cells = <0>;
ti,ngpio = <84>;
ti,davinci-gpio-unbanked = <0>;
power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
--
2.17.1
Hi Rob,
On 17/11/2020 18:19, Sekhar Nori wrote:
> With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
> like below for all interrupt controllers.
>
> /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
>
> Fix these by adding #address-cells = <0>; for all interrupt controllers in
> TI device-tree files. Any other #address-cells value is really only needed
> if interrupt-map property is being used (which is not the case for existing
> TI device-tree files)
>
> Signed-off-by: Sekhar Nori <[email protected]>
> ---
> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
> arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
> arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
> arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
> arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
> 8 files changed, 27 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> index aa8725db0187..55aaa1404d7d 100644
> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> @@ -440,6 +440,7 @@
> interrupt-controller;
> interrupt-parent = <&gic500>;
> #interrupt-cells = <1>;
> + #address-cells = <0>;
Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
do not have child nodes and no "interrupt-map"?
--
Best regards,
grygorii
On 13:38-20201118, Grygorii Strashko wrote:
> Hi Rob,
>
> On 17/11/2020 18:19, Sekhar Nori wrote:
> > With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
> > like below for all interrupt controllers.
> >
> > /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
> >
> > Fix these by adding #address-cells = <0>; for all interrupt controllers in
> > TI device-tree files. Any other #address-cells value is really only needed
> > if interrupt-map property is being used (which is not the case for existing
> > TI device-tree files)
> >
> > Signed-off-by: Sekhar Nori <[email protected]>
> > ---
> > arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
> > arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
> > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
> > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
> > arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
> > arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
> > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
> > arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
> > 8 files changed, 27 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > index aa8725db0187..55aaa1404d7d 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > @@ -440,6 +440,7 @@
> > interrupt-controller;
> > interrupt-parent = <&gic500>;
> > #interrupt-cells = <1>;
> > + #address-cells = <0>;
> Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
> do not have child nodes and no "interrupt-map"?
Just to help clarify (I could be mistaken as well): is'nt the
interrupt map for user interrupt map nodes that refer to this
interrupt controller node to state they dont need a parent address
specifier - so are we claiming none of the users will have an
interrupt-map (now and never in the future as well) - we we might want
to explain why we think that is the case, and if we are expecting dtc
to deduce that (if so how?)?
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 18/11/2020 17:12, Nishanth Menon wrote:
> On 13:38-20201118, Grygorii Strashko wrote:
>> Hi Rob,
>>
>> On 17/11/2020 18:19, Sekhar Nori wrote:
>>> With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
>>> like below for all interrupt controllers.
>>>
>>> /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
>>>
>>> Fix these by adding #address-cells = <0>; for all interrupt controllers in
>>> TI device-tree files. Any other #address-cells value is really only needed
>>> if interrupt-map property is being used (which is not the case for existing
>>> TI device-tree files)
>>>
>>> Signed-off-by: Sekhar Nori <[email protected]>
>>> ---
>>> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
>>> arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
>>> arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
>>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
>>> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
>>> arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
>>> arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
>>> 8 files changed, 27 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> index aa8725db0187..55aaa1404d7d 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>> @@ -440,6 +440,7 @@
>>> interrupt-controller;
>>> interrupt-parent = <&gic500>;
>>> #interrupt-cells = <1>;
>>> + #address-cells = <0>;
>> Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
>> do not have child nodes and no "interrupt-map"?
>
> Just to help clarify (I could be mistaken as well): is'nt the
> interrupt map for user interrupt map nodes that refer to this
> interrupt controller node to state they dont need a parent address
> specifier - so are we claiming none of the users will have an
> interrupt-map (now and never in the future as well) - we we might want
> to explain why we think that is the case, and if we are expecting dtc
> to deduce that (if so how?)?
>
The main reason I commented - is hope to get some clarification from DT maintainers.
90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
(most often is present in PCI and GIC nodes).
and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
And there is no "never" here - #address-cells always can be added if really required.
--
Best regards,
grygorii
Punting over to Rob and DT team's wisdom..
On 13:17-20201119, Grygorii Strashko wrote:
>
>
> On 18/11/2020 17:12, Nishanth Menon wrote:
> > On 13:38-20201118, Grygorii Strashko wrote:
> > > Hi Rob,
> > >
> > > On 17/11/2020 18:19, Sekhar Nori wrote:
> > > > With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
> > > > like below for all interrupt controllers.
> > > >
> > > > /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
> > > >
> > > > Fix these by adding #address-cells = <0>; for all interrupt controllers in
> > > > TI device-tree files. Any other #address-cells value is really only needed
> > > > if interrupt-map property is being used (which is not the case for existing
> > > > TI device-tree files)
> > > >
> > > > Signed-off-by: Sekhar Nori <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
> > > > arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
> > > > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
> > > > arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
> > > > arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
> > > > 8 files changed, 27 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > index aa8725db0187..55aaa1404d7d 100644
> > > > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > @@ -440,6 +440,7 @@
> > > > interrupt-controller;
> > > > interrupt-parent = <&gic500>;
> > > > #interrupt-cells = <1>;
> > > > + #address-cells = <0>;
> > > Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
> > > do not have child nodes and no "interrupt-map"?
> >
> > Just to help clarify (I could be mistaken as well): is'nt the
> > interrupt map for user interrupt map nodes that refer to this
> > interrupt controller node to state they dont need a parent address
> > specifier - so are we claiming none of the users will have an
> > interrupt-map (now and never in the future as well) - we we might want
> > to explain why we think that is the case, and if we are expecting dtc
> > to deduce that (if so how?)?
> >
>
> The main reason I commented - is hope to get some clarification from DT maintainers.
> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
> (most often is present in PCI and GIC nodes).
> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
>
> And there is no "never" here - #address-cells always can be added if really required.
OK - as a GPIO node, but as an interrupt-controller node, I was
looking at [1] and wondering if that was the precedence.
Yes, will be good to get direction from the DT maintainers on this
topic.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/interrupt-controller/open-pic.txt
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 19/11/20 6:58 PM, Nishanth Menon wrote:
> Punting over to Rob and DT team's wisdom..
>
> On 13:17-20201119, Grygorii Strashko wrote:
>>
>>
>> On 18/11/2020 17:12, Nishanth Menon wrote:
>>> On 13:38-20201118, Grygorii Strashko wrote:
>>>> Hi Rob,
>>>>
>>>> On 17/11/2020 18:19, Sekhar Nori wrote:
>>>>> With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
>>>>> like below for all interrupt controllers.
>>>>>
>>>>> /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
>>>>>
>>>>> Fix these by adding #address-cells = <0>; for all interrupt controllers in
>>>>> TI device-tree files. Any other #address-cells value is really only needed
>>>>> if interrupt-map property is being used (which is not the case for existing
>>>>> TI device-tree files)
>>>>>
>>>>> Signed-off-by: Sekhar Nori <[email protected]>
>>>>> ---
>>>>> arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
>>>>> arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
>>>>> arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
>>>>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
>>>>> arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
>>>>> arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
>>>>> arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
>>>>> arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
>>>>> 8 files changed, 27 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>>>> index aa8725db0187..55aaa1404d7d 100644
>>>>> --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>>>> +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
>>>>> @@ -440,6 +440,7 @@
>>>>> interrupt-controller;
>>>>> interrupt-parent = <&gic500>;
>>>>> #interrupt-cells = <1>;
>>>>> + #address-cells = <0>;
>>>> Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
>>>> do not have child nodes and no "interrupt-map"?
>>>
>>> Just to help clarify (I could be mistaken as well): is'nt the
>>> interrupt map for user interrupt map nodes that refer to this
>>> interrupt controller node to state they dont need a parent address
>>> specifier - so are we claiming none of the users will have an
>>> interrupt-map (now and never in the future as well) - we we might want
>>> to explain why we think that is the case, and if we are expecting dtc
>>> to deduce that (if so how?)?
>>>
>>
>> The main reason I commented - is hope to get some clarification from DT maintainers.
>> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
>> (most often is present in PCI and GIC nodes).
>> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
>>
>> And there is no "never" here - #address-cells always can be added if really required.
>
>
> OK - as a GPIO node, but as an interrupt-controller node, I was
> looking at [1] and wondering if that was the precedence.
>
> Yes, will be good to get direction from the DT maintainers on this
> topic.
Shall I respin this series with 2/4 dropped while we wait for decision
on this?
#address-cells warnings on interrupt controller can perhaps be handled
all at once (there are many of those in existing DT anyway).
GPIO is basic support and holds up many other modules (like MMC/SD).
Thanks,
Sekhar
On 09:45-20201123, Sekhar Nori wrote:
> >> The main reason I commented - is hope to get some clarification from DT maintainers.
> >> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
> >> (most often is present in PCI and GIC nodes).
> >> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
> >>
> >> And there is no "never" here - #address-cells always can be added if really required.
> >
> >
> > OK - as a GPIO node, but as an interrupt-controller node, I was
> > looking at [1] and wondering if that was the precedence.
> >
> > Yes, will be good to get direction from the DT maintainers on this
> > topic.
>
> Shall I respin this series with 2/4 dropped while we wait for decision
> on this?
>
> #address-cells warnings on interrupt controller can perhaps be handled
> all at once (there are many of those in existing DT anyway).
>
> GPIO is basic support and holds up many other modules (like MMC/SD).
There are'nt too many new patches in my queue that depends on GPIO, I'd
rather not introduce new warnings unless we are completely at a
stalemate. I'd rather use this opportunity to understand where what we
need to be doing.
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
On 24/11/20 6:51 AM, Nishanth Menon wrote:
> On 09:45-20201123, Sekhar Nori wrote:
>>>> The main reason I commented - is hope to get some clarification from DT maintainers.
>>>> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
>>>> (most often is present in PCI and GIC nodes).
>>>> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
>>>>
>>>> And there is no "never" here - #address-cells always can be added if really required.
>>>
>>>
>>> OK - as a GPIO node, but as an interrupt-controller node, I was
>>> looking at [1] and wondering if that was the precedence.
>>>
>>> Yes, will be good to get direction from the DT maintainers on this
>>> topic.
>>
>> Shall I respin this series with 2/4 dropped while we wait for decision
>> on this?
>>
>> #address-cells warnings on interrupt controller can perhaps be handled
>> all at once (there are many of those in existing DT anyway).
>>
>> GPIO is basic support and holds up many other modules (like MMC/SD).
>
>
> There are'nt too many new patches in my queue that depends on GPIO, I'd
> rather not introduce new warnings unless we are completely at a
> stalemate. I'd rather use this opportunity to understand where what we
> need to be doing.
GPIO was originally submitted as part of 8 patch series titled "[PATCH
0/8] Add support for UHS modes in TI's J721e and J7200 boards"
Rest of those patches need to be resubmitted after GPIO is accepted.
Can you apply patch 1/4 at least. Its fairly non-controversial. It will
help reduce patch backlog and fix some warnings too.
Thanks,
Sekhar
On 09:46-20201124, Sekhar Nori wrote:
> On 24/11/20 6:51 AM, Nishanth Menon wrote:
> > On 09:45-20201123, Sekhar Nori wrote:
> >>>> The main reason I commented - is hope to get some clarification from DT maintainers.
> >>>> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
> >>>> (most often is present in PCI and GIC nodes).
> >>>> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
> >>>>
> >>>> And there is no "never" here - #address-cells always can be added if really required.
> >>>
> >>>
> >>> OK - as a GPIO node, but as an interrupt-controller node, I was
> >>> looking at [1] and wondering if that was the precedence.
> >>>
> >>> Yes, will be good to get direction from the DT maintainers on this
> >>> topic.
> >>
> >> Shall I respin this series with 2/4 dropped while we wait for decision
> >> on this?
> >>
> >> #address-cells warnings on interrupt controller can perhaps be handled
> >> all at once (there are many of those in existing DT anyway).
> >>
> >> GPIO is basic support and holds up many other modules (like MMC/SD).
> >
> >
> > There are'nt too many new patches in my queue that depends on GPIO, I'd
> > rather not introduce new warnings unless we are completely at a
> > stalemate. I'd rather use this opportunity to understand where what we
> > need to be doing.
> GPIO was originally submitted as part of 8 patch series titled "[PATCH
> 0/8] Add support for UHS modes in TI's J721e and J7200 boards"
>
> Rest of those patches need to be resubmitted after GPIO is accepted.
>
> Can you apply patch 1/4 at least. Its fairly non-controversial. It will
> help reduce patch backlog and fix some warnings too.
I see that Grygorii is suggesting 1,3,4 to be pulled in. can you repost
with just the required patches alone and pick up the reviewed-bys?
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Hi Rob, Grygorii,
On 27/11/20 7:53 pm, Nishanth Menon wrote:
> On 09:46-20201124, Sekhar Nori wrote:
>> On 24/11/20 6:51 AM, Nishanth Menon wrote:
>>> On 09:45-20201123, Sekhar Nori wrote:
>>>>>> The main reason I commented - is hope to get some clarification from DT maintainers.
>>>>>> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
>>>>>> (most often is present in PCI and GIC nodes).
>>>>>> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
>>>>>>
>>>>>> And there is no "never" here - #address-cells always can be added if really required.
>>>>>
>>>>>
>>>>> OK - as a GPIO node, but as an interrupt-controller node, I was
>>>>> looking at [1] and wondering if that was the precedence.
>>>>>
>>>>> Yes, will be good to get direction from the DT maintainers on this
>>>>> topic.
>>>>
Is there a conclusion on this topic? Without adding address-cells for interrupt
controller we will be introducing new warning for all the new nodes we are adding.
Thanks and regards,
Lokesh
On Thu, Nov 19, 2020 at 01:17:36PM +0200, Grygorii Strashko wrote:
>
>
> On 18/11/2020 17:12, Nishanth Menon wrote:
> > On 13:38-20201118, Grygorii Strashko wrote:
> > > Hi Rob,
> > >
> > > On 17/11/2020 18:19, Sekhar Nori wrote:
> > > > With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
> > > > like below for all interrupt controllers.
> > > >
> > > > /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
> > > >
> > > > Fix these by adding #address-cells = <0>; for all interrupt controllers in
> > > > TI device-tree files. Any other #address-cells value is really only needed
> > > > if interrupt-map property is being used (which is not the case for existing
> > > > TI device-tree files)
> > > >
> > > > Signed-off-by: Sekhar Nori <[email protected]>
> > > > ---
> > > > arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
> > > > arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
> > > > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
> > > > arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
> > > > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
> > > > arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
> > > > 8 files changed, 27 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > index aa8725db0187..55aaa1404d7d 100644
> > > > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > @@ -440,6 +440,7 @@
> > > > interrupt-controller;
> > > > interrupt-parent = <&gic500>;
> > > > #interrupt-cells = <1>;
> > > > + #address-cells = <0>;
> > > Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
> > > do not have child nodes and no "interrupt-map"?
> >
> > Just to help clarify (I could be mistaken as well): is'nt the
> > interrupt map for user interrupt map nodes that refer to this
> > interrupt controller node to state they dont need a parent address
> > specifier - so are we claiming none of the users will have an
> > interrupt-map (now and never in the future as well) - we we might want
> > to explain why we think that is the case, and if we are expecting dtc
> > to deduce that (if so how?)?
> >
>
> The main reason I commented - is hope to get some clarification from DT maintainers.
> 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
> (most often is present in PCI and GIC nodes).
> and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
>
> And there is no "never" here - #address-cells always can be added if really required.
Once required, how does one figure that out? It's not obvious and
requires booting. So we need something at build time. I'm okay with
loosening the check as long as it warns when a interrupt parent phandle
in an interrupt-map is missing '#address-cells'.
Now that I look back at the dtc change, I'm now confused why this
check got applied. Both David and I wanted changes in regards to
#address-cells. Either a separate check or part of interrupt-map checks.
And the interrupt-map check never got applied. Andre?
Rob
On Mon, 25 Jan 2021 18:01:08 -0600
Rob Herring <[email protected]> wrote:
Hi,
> On Thu, Nov 19, 2020 at 01:17:36PM +0200, Grygorii Strashko wrote:
> >
> >
> > On 18/11/2020 17:12, Nishanth Menon wrote:
> > > On 13:38-20201118, Grygorii Strashko wrote:
> > > > Hi Rob,
> > > >
> > > > On 17/11/2020 18:19, Sekhar Nori wrote:
> > > > > With dtc 1.6.0, building TI device-tree files with W=2 results in warnings
> > > > > like below for all interrupt controllers.
> > > > >
> > > > > /bus@100000/bus@30000000/interrupt-controller1: Missing #address-cells in interrupt provider
> > > > >
> > > > > Fix these by adding #address-cells = <0>; for all interrupt controllers in
> > > > > TI device-tree files. Any other #address-cells value is really only needed
> > > > > if interrupt-map property is being used (which is not the case for existing
> > > > > TI device-tree files)
> > > > >
> > > > > Signed-off-by: Sekhar Nori <[email protected]>
> > > > > ---
> > > > > arch/arm64/boot/dts/ti/k3-am65-main.dtsi | 5 +++++
> > > > > arch/arm64/boot/dts/ti/k3-am65-wakeup.dtsi | 2 ++
> > > > > arch/arm64/boot/dts/ti/k3-am654-base-board.dts | 1 +
> > > > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 3 +++
> > > > > arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi | 1 +
> > > > > arch/arm64/boot/dts/ti/k3-j721e-common-proc-board.dts | 1 +
> > > > > arch/arm64/boot/dts/ti/k3-j721e-main.dtsi | 11 +++++++++++
> > > > > arch/arm64/boot/dts/ti/k3-j721e-mcu-wakeup.dtsi | 3 +++
> > > > > 8 files changed, 27 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > index aa8725db0187..55aaa1404d7d 100644
> > > > > --- a/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > +++ b/arch/arm64/boot/dts/ti/k3-am65-main.dtsi
> > > > > @@ -440,6 +440,7 @@
> > > > > interrupt-controller;
> > > > > interrupt-parent = <&gic500>;
> > > > > #interrupt-cells = <1>;
> > > > > + #address-cells = <0>;
> > > > Does it really required or mandatory to have #address-cells = <0>; defined for interrupt-controller DT nodes which
> > > > do not have child nodes and no "interrupt-map"?
> > >
> > > Just to help clarify (I could be mistaken as well): is'nt the
> > > interrupt map for user interrupt map nodes that refer to this
> > > interrupt controller node to state they dont need a parent address
> > > specifier - so are we claiming none of the users will have an
> > > interrupt-map (now and never in the future as well) - we we might want
> > > to explain why we think that is the case, and if we are expecting dtc
> > > to deduce that (if so how?)?
> > >
> >
> > The main reason I commented - is hope to get some clarification from DT maintainers.
> > 90% of interrupt-controller nodes do not have #address-cells and I never seen in in GPIO nodes
> > (most often is present in PCI and GIC nodes).
> > and nobody seems fixing it. So, if we are going to move this direction it's reasonable to get clarification to be sure.
> >
> > And there is no "never" here - #address-cells always can be added if really required.
>
> Once required, how does one figure that out? It's not obvious and
> requires booting. So we need something at build time. I'm okay with
> loosening the check as long as it warns when a interrupt parent phandle
> in an interrupt-map is missing '#address-cells'.
So I think the rationale for requiring #address-cells is that the usage
of an interrupt in an interrupt-map can be totally disconnected from
the actual interrupt controller node. Typically the controller is in
the .dtsi, but an interrupt map could be anywhere, down in some
board .dts, or even some "common peripherals" intermediate .dts.
Possibly even in an overlay (I2C IRQ lines?).
So while not having this property works today, for your board, it might
surprisingly break for someone else. And those things are hard to find
(unless you know what you are looking for).
Been there, done that with the VExpress DTs, and that was the reason I
pushed for more tests.
On top of that is the standard's default value of "2" for
#address-cells, which Linux observes in this case. That leads to
somewhat surprising results when interpreting interrupt-maps without an
explicit #address-cells (cost me a few hours to figure out back then!)
So given the already somewhat complicated nature of interrupt-maps I
think it's comparably little to ask for explicit #address-cells
properties, even though you might not immediately benefit from it.
> Now that I look back at the dtc change, I'm now confused why this
> check got applied. Both David and I wanted changes in regards to
> #address-cells. Either a separate check or part of interrupt-map checks.
> And the interrupt-map check never got applied. Andre?
Yeah, I somewhat dropped the ball on this, after some iterations and a
partial merge. Will put it on my list to revive this.
Cheers,
Andre.
Andre, Rob,
On 16:38-20210126, Andre Przywara wrote:
> > Now that I look back at the dtc change, I'm now confused why this
> > check got applied. Both David and I wanted changes in regards to
> > #address-cells. Either a separate check or part of interrupt-map checks.
> > And the interrupt-map check never got applied. Andre?
>
> Yeah, I somewhat dropped the ball on this, after some iterations and a
> partial merge. Will put it on my list to revive this.
I was hoping we made some steps, but I did see [1] as well and it is
possible that I am missing some discussion, but it is starting to get
W=2 builds warnings noisy enough to start interfering with discovering
real problems as we keep adding new stuff in.. Just wondering...
[1] https://lore.kernel.org/linux-devicetree/CAL_Jsq++DyiKG9smQGx9FAPDJnVrezcXNb0Y5uh-5_2GBzTQpQ@mail.gmail.com/
[2] https://pastebin.ubuntu.com/p/ns6hPCBxVM/
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D