[PATCH 0/3] Add gpio support for TI's J7200 platform

[PATCH 0/3] Add gpio support for TI's J7200 platform

[Faiz Abbas]

The following patches add gpio support for TI's J7200 platform.

These patches were posted as a part of an older series but have now
been split into three parts. The 3 parts add configs, gpios and MMC/SD
related dts patches respectively.

Older series is here:
https://lore.kernel.org/linux-arm-kernel/20201001190541.6364-1-faiz_abbas@ti.com/

Series adding configs to arm64 defconfig is here:
https://lore.kernel.org/linux-arm-kernel/20201102183005.14174-1-faiz_abbas@ti.com/

Faiz Abbas (3):
  arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
  arm64: dts: ti: k3-j7200: Add gpio nodes in wakeup domain
  arm64: dts: ti: k3-j7200-common-proc-board: Disable unused gpio
    modules

 .../dts/ti/k3-j7200-common-proc-board.dts     | 16 +++++
 arch/arm64/boot/dts/ti/k3-j7200-main.dtsi     | 68 +++++++++++++++++++
 .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 32 +++++++++
 3 files changed, 116 insertions(+)

-- 
2.17.1

[PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain

[Faiz Abbas]

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 <faiz_abbas@ti.com>
---
 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@600000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x00600000 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@610000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x00610000 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@620000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x00620000 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@630000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x00630000 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

[PATCH 2/3] arm64: dts: ti: k3-j7200: Add gpio nodes in wakeup domain

[Faiz Abbas]

Similar to the gpio groups in main domain, there is one gpio group in
wakeup domain with 2 mdoules instances in it. The gpio group pins out
73 pins (5 banks). Add DT nodes for these 2 gpio module instances.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 .../boot/dts/ti/k3-j7200-mcu-wakeup.dtsi      | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)

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..5ee64d4ee41f 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j7200-mcu-wakeup.dtsi
@@ -107,6 +107,38 @@
 		ti,interrupt-ranges = <16 960 16>;
 	};
 
+	wkup_gpio0: gpio@42110000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x42110000 0x00 0x100>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-parent = <&wkup_gpio_intr>;
+		interrupts = <103>, <104>, <105>, <106>, <107>, <108>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		ti,ngpio = <73>;
+		ti,davinci-gpio-unbanked = <0>;
+		power-domains = <&k3_pds 113 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 113 0>;
+		clock-names = "gpio";
+	};
+
+	wkup_gpio1: gpio@42100000 {
+		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
+		reg = <0x00 0x42100000 0x00 0x100>;
+		gpio-controller;
+		#gpio-cells = <2>;
+		interrupt-parent = <&wkup_gpio_intr>;
+		interrupts = <112>, <113>, <114>, <115>, <116>, <117>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		ti,ngpio = <73>;
+		ti,davinci-gpio-unbanked = <0>;
+		power-domains = <&k3_pds 114 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 114 0>;
+		clock-names = "gpio";
+	};
+
 	mcu_navss: bus@28380000 {
 		compatible = "simple-mfd";
 		#address-cells = <2>;
-- 
2.17.1

[PATCH 3/3] arm64: dts: ti: k3-j7200-common-proc-board: Disable unused gpio modules

[Faiz Abbas]

There are 6 gpio instances inside SoC with 2 groups as show below:
	Group one: wkup_gpio0, wkup_gpio1
	Group two: main_gpio0, main_gpio2, main_gpio4, main_gpio6

Only one instance from each group can be used at a time. So use main_gpio0
and wkup_gpio0 in current linux context and disable the rest of the nodes.

Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
 .../boot/dts/ti/k3-j7200-common-proc-board.dts   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
index ef03e7636b66..0bc4170225d5 100644
--- a/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-j7200-common-proc-board.dts
@@ -127,6 +127,22 @@
 	status = "disabled";
 };
 
+&main_gpio2 {
+	status = "disabled";
+};
+
+&main_gpio4 {
+	status = "disabled";
+};
+
+&main_gpio6 {
+	status = "disabled";
+};
+
+&wkup_gpio1 {
+	status = "disabled";
+};
+
 &mcu_cpsw {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mcu_cpsw_pins_default &mcu_mdio_pins_default>;
-- 
2.17.1

Re: [PATCH 0/3] Add gpio support for TI's J7200 platform

[Lokesh Vutla]

On 03/11/20 12:41 am, Faiz Abbas wrote:
> The following patches add gpio support for TI's J7200 platform.
> 
> These patches were posted as a part of an older series but have now
> been split into three parts. The 3 parts add configs, gpios and MMC/SD
> related dts patches respectively.

Series looks good to me.

Reviewed-by: Lokesh Vutla <lokeshvutla@ti.com>

Thanks and regards,
Lokesh

Re: [PATCH 1/3] arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain

[Nishanth Menon]

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 <faiz_abbas@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 68 +++++++++++++++++++++++

dtbs_check: we added:
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@600000: Missing #address-cells in interrupt provider
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@610000: Missing #address-cells in interrupt provider
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@620000: Missing #address-cells in interrupt provider
arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@630000: 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@600000 {
> +		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
> +		reg = <0x00 0x00600000 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@610000 {
> +		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
> +		reg = <0x00 0x00610000 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@620000 {
> +		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
> +		reg = <0x00 0x00620000 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@630000 {
> +		compatible = "ti,j721e-gpio", "ti,keystone-gpio";
> +		reg = <0x00 0x00630000 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-kernel@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

Re: [PATCH 0/3] Add gpio support for TI's J7200 platform

[Nishanth Menon]

On 00:41-20201103, Faiz Abbas wrote:
> The following patches add gpio support for TI's J7200 platform.
> 
> These patches were posted as a part of an older series but have now
> been split into three parts. The 3 parts add configs, gpios and MMC/SD
> related dts patches respectively.
> 
> Older series is here:
> https://lore.kernel.org/linux-arm-kernel/20201001190541.6364-1-faiz_abbas@ti.com/
> 
> Series adding configs to arm64 defconfig is here:
> https://lore.kernel.org/linux-arm-kernel/20201102183005.14174-1-faiz_abbas@ti.com/
> 
> Faiz Abbas (3):
>   arm64: dts: ti: k3-j7200-main: Add gpio nodes in main domain
>   arm64: dts: ti: k3-j7200: Add gpio nodes in wakeup domain

I am not sure why we are splitting patches per domain here. We
should just have a single patch that introduces the nodes, I dont see a
specific benefit. In addition, series also introduces additional
 Missing #address-cells in interrupt provider

Which we need a conclusion for and the comments already provided.


-- 
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

[Sekhar Nori]

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 <faiz_abbas@ti.com>
>> ---
>>  arch/arm64/boot/dts/ti/k3-j7200-main.dtsi | 68 +++++++++++++++++++++++
> 
> dtbs_check: we added:
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@600000: Missing #address-cells in interrupt provider
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@610000: Missing #address-cells in interrupt provider
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@620000: Missing #address-cells in interrupt provider
> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@630000: 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

[Nishanth Menon]

On 23:59-20201113, Sekhar Nori wrote:
[..]
> > dtbs_check: we added:
> > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@600000: Missing #address-cells in interrupt provider
> > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@610000: Missing #address-cells in interrupt provider
> > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@620000: Missing #address-cells in interrupt provider
> > arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@630000: 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

[Sekhar Nori]

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@100000/gpio@600000: Missing #address-cells in interrupt provider
>>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@610000: Missing #address-cells in interrupt provider
>>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@620000: Missing #address-cells in interrupt provider
>>> arch/arm64/boot/dts/ti/k3-j7200-main.dtsi: /bus@100000/gpio@630000: 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@100000/bus@30000000/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

[Nishanth Menon]

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@100000/bus@30000000/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

[Grygorii Strashko]

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@100000/bus@30000000/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

[Sekhar Nori]

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@100000/bus@30000000/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