Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
On 14/11/20 9:45 AM, Grygorii Strashko wrote: > Hi > > On 13/11/2020 22:55, Nishanth Menon wrote: >> On 00:39-20201114, Sekhar Nori wrote: >>> >>> I was using the latest schema from master. But I changed to 2020.08.1 >>> also, and still don't see the warning. >>> >>> $ dt-doc-validate --version >>> 2020.12.dev1+gab5a73fcef26 >>> >>> I dont have a system-wide dtc installed. One in kernel tree is updated. >>> >>> $ scripts/dtc/dtc --version >>> Version: DTC 1.6.0-gcbca977e >>> >>> Looking at your logs, it looks like you have more patches than just this >>> applied. I wonder if thats making a difference. Can you check with just >>> these patches applied to linux-next or share your tree which includes >>> other patches? >>> >>> In your logs, you have such error for other interrupt controller nodes >>> as well. For example: >>> >>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: >>> /bus@10/bus@3000/interrupt-controller1: Missing #address-cells >>> in interrupt provider >>> >>> Which I don't see in my logs. My guess is some other patch(es) in your >>> patch stack either uncovers this warning or causes it. >> >> Oh boy! I sent you and myself on wild goose chase! Really sorry about >> messing up in the report of bug. >> >> It is not dtbs_check, it is building dtbs with W=2 that generates this >> warning. dtc 1.6.0 is sufficient to reproduce this behavior. >> >> Using v5.10-rc1 as baseline (happens the same with next-20201113 as >> well. >> >> v5.10-rc1: https://pastebin.ubuntu.com/p/Pn9HDqRjQ4/ (recording: >> https://asciinema.org/a/55YVpql9Bq8rh8fePTxI2xObO) >> >> v5.10-rc1 + 1st patch in the series(since we are testing): >> https://pastebin.ubuntu.com/p/QWQRMSv565/ (recording: >> https://asciinema.org/a/ZSKZkOY13l4lmZ2xWH34jMlM1) >> >> Diff: https://pastebin.ubuntu.com/p/239sYYT2QY/ >> > > This warning come from scripts/dtc/checks.c > and was introduced by commit 3eb619b2f7d8 ("scripts/dtc: Update to > upstream version v1.6.0-11-g9d7888cbf19c"). > > In my opinion it's false warning as there is no requirement to have > #address-cells in interrupt provider node. > by the way, above commit description says: "The interrupt_provider check > is noisy, so turn it off for now." Adding Andre who adding this check in upstream dtc for guidance. It looks like address-cells makes sense only if there is an interrupt-map specified as well. Since we don't use it, I can add #address-cells = <0>; to silence the warning. Let me know if there is a better way to deal with this. Thanks, Sekhar
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
Hi On 13/11/2020 22:55, Nishanth Menon wrote: On 00:39-20201114, Sekhar Nori wrote: I was using the latest schema from master. But I changed to 2020.08.1 also, and still don't see the warning. $ dt-doc-validate --version 2020.12.dev1+gab5a73fcef26 I dont have a system-wide dtc installed. One in kernel tree is updated. $ scripts/dtc/dtc --version Version: DTC 1.6.0-gcbca977e Looking at your logs, it looks like you have more patches than just this applied. I wonder if thats making a difference. Can you check with just these patches applied to linux-next or share your tree which includes other patches? In your logs, you have such error for other interrupt controller nodes as well. For example: arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/bus@3000/interrupt-controller1: Missing #address-cells in interrupt provider Which I don't see in my logs. My guess is some other patch(es) in your patch stack either uncovers this warning or causes it. Oh boy! I sent you and myself on wild goose chase! Really sorry about messing up in the report of bug. It is not dtbs_check, it is building dtbs with W=2 that generates this warning. dtc 1.6.0 is sufficient to reproduce this behavior. Using v5.10-rc1 as baseline (happens the same with next-20201113 as well. v5.10-rc1: https://pastebin.ubuntu.com/p/Pn9HDqRjQ4/ (recording: https://asciinema.org/a/55YVpql9Bq8rh8fePTxI2xObO) v5.10-rc1 + 1st patch in the series(since we are testing): https://pastebin.ubuntu.com/p/QWQRMSv565/ (recording: https://asciinema.org/a/ZSKZkOY13l4lmZ2xWH34jMlM1) Diff: https://pastebin.ubuntu.com/p/239sYYT2QY/ This warning come from scripts/dtc/checks.c and was introduced by commit 3eb619b2f7d8 ("scripts/dtc: Update to upstream version v1.6.0-11-g9d7888cbf19c"). In my opinion it's false warning as there is no requirement to have #address-cells in interrupt provider node. by the way, above commit description says: "The interrupt_provider check is noisy, so turn it off for now." -- Best regards, grygorii
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
On 00:39-20201114, Sekhar Nori wrote: > > I was using the latest schema from master. But I changed to 2020.08.1 > also, and still don't see the warning. > > $ dt-doc-validate --version > 2020.12.dev1+gab5a73fcef26 > > I dont have a system-wide dtc installed. One in kernel tree is updated. > > $ scripts/dtc/dtc --version > Version: DTC 1.6.0-gcbca977e > > Looking at your logs, it looks like you have more patches than just this > applied. I wonder if thats making a difference. Can you check with just > these patches applied to linux-next or share your tree which includes > other patches? > > In your logs, you have such error for other interrupt controller nodes > as well. For example: > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: > /bus@10/bus@3000/interrupt-controller1: Missing #address-cells > in interrupt provider > > Which I don't see in my logs. My guess is some other patch(es) in your > patch stack either uncovers this warning or causes it. Oh boy! I sent you and myself on wild goose chase! Really sorry about messing up in the report of bug. It is not dtbs_check, it is building dtbs with W=2 that generates this warning. dtc 1.6.0 is sufficient to reproduce this behavior. Using v5.10-rc1 as baseline (happens the same with next-20201113 as well. v5.10-rc1: https://pastebin.ubuntu.com/p/Pn9HDqRjQ4/ (recording: https://asciinema.org/a/55YVpql9Bq8rh8fePTxI2xObO) v5.10-rc1 + 1st patch in the series(since we are testing): https://pastebin.ubuntu.com/p/QWQRMSv565/ (recording: https://asciinema.org/a/ZSKZkOY13l4lmZ2xWH34jMlM1) Diff: https://pastebin.ubuntu.com/p/239sYYT2QY/ -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
On 14/11/20 12:10 AM, Nishanth Menon wrote: > On 23:59-20201113, Sekhar Nori wrote: > [..] >>> dtbs_check: we added: >>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@60: Missing >>> #address-cells in interrupt provider >>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@61: Missing >>> #address-cells in interrupt provider >>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@62: Missing >>> #address-cells in interrupt provider >>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@63: Missing >>> #address-cells in interrupt provider >> >> Hmm, running dtbs_check, I did not really see this. These are all the >> warnings I see for TI platforms: https://pastebin.ubuntu.com/p/m2my62mjQq/ > > Here is the full list of checks I ran through with kernel_patch_verify > (docker) > https://pastebin.ubuntu.com/p/tcnWw89CMD/ > > See lines 128 onwards for this series. kernel_patch_verify does'nt > complain on existing warnings, but just prints when there are additional > ones added in. Also make sure we have the right dtc as well > dtc 1.6.0 and dt_schema 2020.8.1 was used. I was using the latest schema from master. But I changed to 2020.08.1 also, and still don't see the warning. $ dt-doc-validate --version 2020.12.dev1+gab5a73fcef26 I dont have a system-wide dtc installed. One in kernel tree is updated. $ scripts/dtc/dtc --version Version: DTC 1.6.0-gcbca977e Looking at your logs, it looks like you have more patches than just this applied. I wonder if thats making a difference. Can you check with just these patches applied to linux-next or share your tree which includes other patches? In your logs, you have such error for other interrupt controller nodes as well. For example: arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/bus@3000/interrupt-controller1: Missing #address-cells in interrupt provider Which I don't see in my logs. My guess is some other patch(es) in your patch stack either uncovers this warning or causes it. > >> >> The tree I am testing is linux-next of 12th Nov + these three patches >> applied. >> >> Also, #address-cells for interrupt provider being compulsory does not >> make full sense to me. Nothing in >> Documentation/devicetree/bindings/interrupt-controller/interrupts.txt or >> Documentation/devicetree/bindings/gpio/gpio-davinci.txt suggests that as >> well. >> >> Existing GPIO nodes for AM654 or J721E does not have #address-cells as well. >> >> Adding Grygorii as well, in case he knows more about this. > > > Yes - we need to have this conversation in the community :) I had > tagged this internally already during the 5.10 merge cycle that we > need to clean up the #address-cells warning and in some cases, maybe > the bindings are probably not accurate to attempt an enforcement. > I'd really like a conclusion on the topic as I recollect Lokesh and > Grygorii had a debate internally, but reached no conclusion, lets get > the wisdom of the community to help us here. Adding Lokesh to cc as well. Thanks, Sekhar
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
On 23:59-20201113, Sekhar Nori wrote: [..] > > dtbs_check: we added: > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@60: Missing > > #address-cells in interrupt provider > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@61: Missing > > #address-cells in interrupt provider > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@62: Missing > > #address-cells in interrupt provider > > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@63: Missing > > #address-cells in interrupt provider > > Hmm, running dtbs_check, I did not really see this. These are all the > warnings I see for TI platforms: https://pastebin.ubuntu.com/p/m2my62mjQq/ Here is the full list of checks I ran through with kernel_patch_verify (docker) https://pastebin.ubuntu.com/p/tcnWw89CMD/ See lines 128 onwards for this series. kernel_patch_verify does'nt complain on existing warnings, but just prints when there are additional ones added in. Also make sure we have the right dtc as well dtc 1.6.0 and dt_schema 2020.8.1 was used. > > The tree I am testing is linux-next of 12th Nov + these three patches > applied. > > Also, #address-cells for interrupt provider being compulsory does not > make full sense to me. Nothing in > Documentation/devicetree/bindings/interrupt-controller/interrupts.txt or > Documentation/devicetree/bindings/gpio/gpio-davinci.txt suggests that as > well. > > Existing GPIO nodes for AM654 or J721E does not have #address-cells as well. > > Adding Grygorii as well, in case he knows more about this. Yes - we need to have this conversation in the community :) I had tagged this internally already during the 5.10 merge cycle that we need to clean up the #address-cells warning and in some cases, maybe the bindings are probably not accurate to attempt an enforcement. I'd really like a conclusion on the topic as I recollect Lokesh and Grygorii had a debate internally, but reached no conclusion, lets get the wisdom of the community to help us here. [1] https://github.com/nmenon/kernel_patch_verify/blob/master/kpv -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
Hi Nishanth, On 12/11/20 10:09 PM, Nishanth Menon wrote: > On 00:41-20201103, Faiz Abbas wrote: >> There are 4 instances of gpio modules in main domain: >> gpio0, gpio2, gpio4 and gpio6 >> >> Groups are created to provide protection between different processor virtual >> worlds. Each of these modules I/O pins are muxed within the group. Exactly >> one module can be selected to control the corresponding pin by selecting it >> in the pad mux configuration registers. > Could you check with checkpatch --strict please? > > I see: > > WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per > line) > >> >> This group pins out 69 lines (5 banks). >> >> Add DT modes for each module instance in the main domain. >> >> Signed-off-by: Faiz Abbas >> --- >> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 68 +++ > > dtbs_check: we added: > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@60: Missing > #address-cells in interrupt provider > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@61: Missing > #address-cells in interrupt provider > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@62: Missing > #address-cells in interrupt provider > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@63: Missing > #address-cells in interrupt provider Hmm, running dtbs_check, I did not really see this. These are all the warnings I see for TI platforms: https://pastebin.ubuntu.com/p/m2my62mjQq/ The tree I am testing is linux-next of 12th Nov + these three patches applied. Also, #address-cells for interrupt provider being compulsory does not make full sense to me. Nothing in Documentation/devicetree/bindings/interrupt-controller/interrupts.txt or Documentation/devicetree/bindings/gpio/gpio-davinci.txt suggests that as well. Existing GPIO nodes for AM654 or J721E does not have #address-cells as well. Adding Grygorii as well, in case he knows more about this. Thanks, Sekhar
Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
On 00:41-20201103, Faiz Abbas wrote: > There are 4 instances of gpio modules in main domain: > gpio0, gpio2, gpio4 and gpio6 > > Groups are created to provide protection between different processor virtual > worlds. Each of these modules I/O pins are muxed within the group. Exactly > one module can be selected to control the corresponding pin by selecting it > in the pad mux configuration registers. Could you check with checkpatch --strict please? I see: WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) > > This group pins out 69 lines (5 banks). > > Add DT modes for each module instance in the main domain. > > Signed-off-by: Faiz Abbas > --- > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 68 +++ dtbs_check: we added: arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@60: Missing #address-cells in interrupt provider arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@61: Missing #address-cells in interrupt provider arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@62: Missing #address-cells in interrupt provider arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@10/gpio@63: Missing #address-cells in interrupt provider > 1 file changed, 68 insertions(+) > > diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > index 72d6496e88dd..c22ef2efa531 100644 > --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi > @@ -446,4 +446,72 @@ > dr_mode = "otg"; > }; > }; > + > + main_gpio0: gpio@60 { > + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; > + reg = <0x00 0x0060 0x00 0x100>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-parent = <&main_gpio_intr>; > + interrupts = <145>, <146>, <147>, <148>, > + <149>; > + interrupt-controller; > + #interrupt-cells = <2>; > + ti,ngpio = <69>; > + ti,davinci-gpio-unbanked = <0>; > + power-domains = <&k3_pds 105 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 105 0>; > + clock-names = "gpio"; > + }; > + > + main_gpio2: gpio@61 { > + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; > + reg = <0x00 0x0061 0x00 0x100>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-parent = <&main_gpio_intr>; > + interrupts = <154>, <155>, <156>, <157>, > + <158>; > + interrupt-controller; > + #interrupt-cells = <2>; > + ti,ngpio = <69>; > + ti,davinci-gpio-unbanked = <0>; > + power-domains = <&k3_pds 107 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 107 0>; > + clock-names = "gpio"; > + }; > + > + main_gpio4: gpio@62 { > + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; > + reg = <0x00 0x0062 0x00 0x100>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-parent = <&main_gpio_intr>; > + interrupts = <163>, <164>, <165>, <166>, > + <167>; > + interrupt-controller; > + #interrupt-cells = <2>; > + ti,ngpio = <69>; > + ti,davinci-gpio-unbanked = <0>; > + power-domains = <&k3_pds 109 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 109 0>; > + clock-names = "gpio"; > + }; > + > + main_gpio6: gpio@63 { > + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; > + reg = <0x00 0x0063 0x00 0x100>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-parent = <&main_gpio_intr>; > + interrupts = <172>, <173>, <174>, <175>, > + <176>; > + interrupt-controller; > + #interrupt-cells = <2>; > + ti,ngpio = <69>; > + ti,davinci-gpio-unbanked = <0>; > + power-domains = <&k3_pds 111 TI_SCI_PD_EXCLUSIVE>; > + clocks = <&k3_clks 111 0>; > + clock-names = "gpio"; > + }; > }; > -- > 2.17.1 > > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Regards, Nishanth Menon Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
[PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
There are 4 instances of gpio modules in main domain: gpio0, gpio2, gpio4 and gpio6 Groups are created to provide protection between different processor virtual worlds. Each of these modules I/O pins are muxed within the group. Exactly one module can be selected to control the corresponding pin by selecting it in the pad mux configuration registers. This group pins out 69 lines (5 banks). Add DT modes for each module instance in the main domain. Signed-off-by: Faiz Abbas --- arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 68 +++ 1 file changed, 68 insertions(+) diff --git a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi index 72d6496e88dd..c22ef2efa531 100644 --- a/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi +++ b/arch/arm64/boot/dts/ti/k3-j7200-main.dtsi @@ -446,4 +446,72 @@ dr_mode = "otg"; }; }; + + main_gpio0: gpio@60 { + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; + reg = <0x00 0x0060 0x00 0x100>; + gpio-controller; + #gpio-cells = <2>; + interrupt-parent = <&main_gpio_intr>; + interrupts = <145>, <146>, <147>, <148>, +<149>; + interrupt-controller; + #interrupt-cells = <2>; + ti,ngpio = <69>; + ti,davinci-gpio-unbanked = <0>; + power-domains = <&k3_pds 105 TI_SCI_PD_EXCLUSIVE>; + clocks = <&k3_clks 105 0>; + clock-names = "gpio"; + }; + + main_gpio2: gpio@61 { + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; + reg = <0x00 0x0061 0x00 0x100>; + gpio-controller; + #gpio-cells = <2>; + interrupt-parent = <&main_gpio_intr>; + interrupts = <154>, <155>, <156>, <157>, +<158>; + interrupt-controller; + #interrupt-cells = <2>; + ti,ngpio = <69>; + ti,davinci-gpio-unbanked = <0>; + power-domains = <&k3_pds 107 TI_SCI_PD_EXCLUSIVE>; + clocks = <&k3_clks 107 0>; + clock-names = "gpio"; + }; + + main_gpio4: gpio@62 { + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; + reg = <0x00 0x0062 0x00 0x100>; + gpio-controller; + #gpio-cells = <2>; + interrupt-parent = <&main_gpio_intr>; + interrupts = <163>, <164>, <165>, <166>, +<167>; + interrupt-controller; + #interrupt-cells = <2>; + ti,ngpio = <69>; + ti,davinci-gpio-unbanked = <0>; + power-domains = <&k3_pds 109 TI_SCI_PD_EXCLUSIVE>; + clocks = <&k3_clks 109 0>; + clock-names = "gpio"; + }; + + main_gpio6: gpio@63 { + compatible = "ti,j721e-gpio", "ti,keystone-gpio"; + reg = <0x00 0x0063 0x00 0x100>; + gpio-controller; + #gpio-cells = <2>; + interrupt-parent = <&main_gpio_intr>; + interrupts = <172>, <173>, <174>, <175>, +<176>; + interrupt-controller; + #interrupt-cells = <2>; + ti,ngpio = <69>; + ti,davinci-gpio-unbanked = <0>; + power-domains = <&k3_pds 111 TI_SCI_PD_EXCLUSIVE>; + clocks = <&k3_clks 111 0>; + clock-names = "gpio"; + }; }; -- 2.17.1