Re: [PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
On 4/15/21 4:35 PM, Alexandre TORGUE wrote: On 4/15/21 4:30 PM, Marek Vasut wrote: On 4/15/21 3:34 PM, Alexandre TORGUE wrote: Hi Marek Hello Alexandre, diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 2bc92ef3aeb9..19ef475a48fc 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -82,9 +82,15 @@ };; + #size-cells = <0>; + + ltdc_ep0_out: endpoint@0 { + reg = <0>; + remote-endpoint = <&sii9022_in>; + }; + ltdc_ep1_out: endpoint@1 { reg = <1>; remote-endpoint = <&dsi_in>; [...] diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi index 64dca5b7f748..e7f10975cacf 100644 --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi @@ -277,11 +277,7 @@ status = "okay"; port { - #address-cells = <1>; - #size-cells = <0>; - - ltdc_ep0_out: endpoint@0 { - reg = <0>; + ltdc_ep0_out: endpoint { remote-endpoint = <&adv7513_in>; }; }; I think this is wrong, the AV96 can have two displays connected to two ports of the LTDC, just like DK2 for example. As for dk2 address/size cells are added only if there are 2 endpoints. It is for this reason I moved endpoint0 definition from stm32mp15xx-dkx to stm32mp151a-dk1.dts (dk1 has only one endpoint). Here it's the same, if you have second endpoint then adress/size will have to be added. That's a bit problematic. Consider either the use case of DTO which adds the other display, or even a custom board DTS. Without your patch, this works: arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi ; }; }; }; board-with-display.dts or board-overlay.dts ; }; }; }; With your patch, the DTS would have to modify the "endpoint" node to be "endpoint@0" probably with a whole lot of /detele-node/ etc. magic (DTO cannot do that, so that's a problem, and I do use DTOs on AV96 extensively for the various expansion cards) and then add the endpoint@1. That becomes real complicated in custom board DT, and impossible with DTO. Yes I agree that it'll be problematic. So maybe so solution would be to not detect a warning for the initial case (only one endpoint with a reg) That looks OK. Or even better, if the checker warned only on IPs which cannot have more than one endpoint, but have endpoint@N in DT (where N in 0..+inf) . On IPs which can have one or more endpoints, the warning should not be emitted.
Re: [PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
On 4/15/21 4:30 PM, Marek Vasut wrote: On 4/15/21 3:34 PM, Alexandre TORGUE wrote: Hi Marek Hello Alexandre, diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 2bc92ef3aeb9..19ef475a48fc 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -82,9 +82,15 @@ };; + #size-cells = <0>; + + ltdc_ep0_out: endpoint@0 { + reg = <0>; + remote-endpoint = <&sii9022_in>; + }; + ltdc_ep1_out: endpoint@1 { reg = <1>; remote-endpoint = <&dsi_in>; [...] diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi index 64dca5b7f748..e7f10975cacf 100644 --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi @@ -277,11 +277,7 @@ status = "okay"; port { - #address-cells = <1>; - #size-cells = <0>; - - ltdc_ep0_out: endpoint@0 { - reg = <0>; + ltdc_ep0_out: endpoint { remote-endpoint = <&adv7513_in>; }; }; I think this is wrong, the AV96 can have two displays connected to two ports of the LTDC, just like DK2 for example. As for dk2 address/size cells are added only if there are 2 endpoints. It is for this reason I moved endpoint0 definition from stm32mp15xx-dkx to stm32mp151a-dk1.dts (dk1 has only one endpoint). Here it's the same, if you have second endpoint then adress/size will have to be added. That's a bit problematic. Consider either the use case of DTO which adds the other display, or even a custom board DTS. Without your patch, this works: arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi ; }; }; }; board-with-display.dts or board-overlay.dts ; }; }; }; With your patch, the DTS would have to modify the "endpoint" node to be "endpoint@0" probably with a whole lot of /detele-node/ etc. magic (DTO cannot do that, so that's a problem, and I do use DTOs on AV96 extensively for the various expansion cards) and then add the endpoint@1. That becomes real complicated in custom board DT, and impossible with DTO. Yes I agree that it'll be problematic. So maybe so solution would be to not detect a warning for the initial case (only one endpoint with a reg)
Re: [PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
On 4/15/21 3:34 PM, Alexandre TORGUE wrote: Hi Marek Hello Alexandre, diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 2bc92ef3aeb9..19ef475a48fc 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -82,9 +82,15 @@ };; + #size-cells = <0>; + + ltdc_ep0_out: endpoint@0 { + reg = <0>; + remote-endpoint = <&sii9022_in>; + }; + ltdc_ep1_out: endpoint@1 { reg = <1>; remote-endpoint = <&dsi_in>; [...] diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi index 64dca5b7f748..e7f10975cacf 100644 --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi @@ -277,11 +277,7 @@ status = "okay"; port { - #address-cells = <1>; - #size-cells = <0>; - - ltdc_ep0_out: endpoint@0 { - reg = <0>; + ltdc_ep0_out: endpoint { remote-endpoint = <&adv7513_in>; }; }; I think this is wrong, the AV96 can have two displays connected to two ports of the LTDC, just like DK2 for example. As for dk2 address/size cells are added only if there are 2 endpoints. It is for this reason I moved endpoint0 definition from stm32mp15xx-dkx to stm32mp151a-dk1.dts (dk1 has only one endpoint). Here it's the same, if you have second endpoint then adress/size will have to be added. That's a bit problematic. Consider either the use case of DTO which adds the other display, or even a custom board DTS. Without your patch, this works: arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi ; }; }; }; board-with-display.dts or board-overlay.dts ; }; }; }; With your patch, the DTS would have to modify the "endpoint" node to be "endpoint@0" probably with a whole lot of /detele-node/ etc. magic (DTO cannot do that, so that's a problem, and I do use DTOs on AV96 extensively for the various expansion cards) and then add the endpoint@1. That becomes real complicated in custom board DT, and impossible with DTO.
Re: [PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
Hi Marek On 4/15/21 3:21 PM, Marek Vasut wrote: On 4/15/21 12:10 PM, Alexandre Torgue wrote: Running "make dtbs_check W=1", some warnings are reported concerning LTDC port subnode: /soc/display-controller@5a001000/port: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property /soc/display-controller@5a001000/port: graph node has single child node 'endpoint', #address-cells/#size-cells are not necessary btw could you retain diffstat on your patches ? It's useful to see which files changed right away. [...] diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 2bc92ef3aeb9..19ef475a48fc 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -82,9 +82,15 @@ };; + #size-cells = <0>; + + ltdc_ep0_out: endpoint@0 { + reg = <0>; + remote-endpoint = <&sii9022_in>; + }; + ltdc_ep1_out: endpoint@1 { reg = <1>; remote-endpoint = <&dsi_in>; [...] diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi index 64dca5b7f748..e7f10975cacf 100644 --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi @@ -277,11 +277,7 @@ status = "okay"; port { - #address-cells = <1>; - #size-cells = <0>; - - ltdc_ep0_out: endpoint@0 { - reg = <0>; + ltdc_ep0_out: endpoint { remote-endpoint = <&adv7513_in>; }; }; I think this is wrong, the AV96 can have two displays connected to two ports of the LTDC, just like DK2 for example. As for dk2 address/size cells are added only if there are 2 endpoints. It is for this reason I moved endpoint0 definition from stm32mp15xx-dkx to stm32mp151a-dk1.dts (dk1 has only one endpoint). Here it's the same, if you have second endpoint then adress/size will have to be added. alex
Re: [PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
On 4/15/21 12:10 PM, Alexandre Torgue wrote: Running "make dtbs_check W=1", some warnings are reported concerning LTDC port subnode: /soc/display-controller@5a001000/port: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property /soc/display-controller@5a001000/port: graph node has single child node 'endpoint', #address-cells/#size-cells are not necessary btw could you retain diffstat on your patches ? It's useful to see which files changed right away. [...] diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts b/arch/arm/boot/dts/stm32mp157c-dk2.dts index 2bc92ef3aeb9..19ef475a48fc 100644 --- a/arch/arm/boot/dts/stm32mp157c-dk2.dts +++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts @@ -82,9 +82,15 @@ };; + #size-cells = <0>; + + ltdc_ep0_out: endpoint@0 { + reg = <0>; + remote-endpoint = <&sii9022_in>; + }; + ltdc_ep1_out: endpoint@1 { reg = <1>; remote-endpoint = <&dsi_in>; [...] diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi index 64dca5b7f748..e7f10975cacf 100644 --- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi +++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi @@ -277,11 +277,7 @@ status = "okay"; port { - #address-cells = <1>; - #size-cells = <0>; - - ltdc_ep0_out: endpoint@0 { - reg = <0>; + ltdc_ep0_out: endpoint { remote-endpoint = <&adv7513_in>; }; }; I think this is wrong, the AV96 can have two displays connected to two ports of the LTDC, just like DK2 for example.
[PATCH 11/13] ARM: dts: stm32: fix LTDC port node on STM32 MCU ad MPU
Running "make dtbs_check W=1", some warnings are reported concerning
LTDC port subnode:
/soc/display-controller@5a001000/port:
unnecessary #address-cells/#size-cells without "ranges" or child "reg"
property
/soc/display-controller@5a001000/port: graph node has single child node
'endpoint', #address-cells/#size-cells are not necessary
Signed-off-by: Alexandre Torgue
diff --git a/arch/arm/boot/dts/stm32f469-disco.dts
b/arch/arm/boot/dts/stm32f469-disco.dts
index 8c982ae79f43..f530f84474ea 100644
--- a/arch/arm/boot/dts/stm32f469-disco.dts
+++ b/arch/arm/boot/dts/stm32f469-disco.dts
@@ -175,7 +175,7 @@
status = "okay";
port {
- ltdc_out_dsi: endpoint@0 {
+ ltdc_out_dsi: endpoint {
remote-endpoint = <&dsi_in>;
};
};
diff --git a/arch/arm/boot/dts/stm32mp151.dtsi
b/arch/arm/boot/dts/stm32mp151.dtsi
index 8aa87cb86821..98a703d1c3a0 100644
--- a/arch/arm/boot/dts/stm32mp151.dtsi
+++ b/arch/arm/boot/dts/stm32mp151.dtsi
@@ -1471,11 +1471,7 @@
resets = <&rcc LTDC_R>;
status = "disabled";
- port {
- #address-cells = <1>;
- #size-cells = <0>;
};
- };
iwdg2: watchdog@5a002000 {
compatible = "st,stm32mp1-iwdg";
diff --git a/arch/arm/boot/dts/stm32mp157a-dk1.dts
b/arch/arm/boot/dts/stm32mp157a-dk1.dts
index 4c8be9c8eb20..c06763d24890 100644
--- a/arch/arm/boot/dts/stm32mp157a-dk1.dts
+++ b/arch/arm/boot/dts/stm32mp157a-dk1.dts
@@ -26,3 +26,11 @@
stdout-path = "serial0:115200n8";
};
};
+
+;
+ };
+ };
+};
diff --git
a/arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dts
b/arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dts
index 674b2d330dc4..ba1e2d7f06bf 100644
--- a/arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dts
+++ b/arch/arm/boot/dts/stm32mp157a-microgea-stm32mp1-microdev2.0-of7.dts
@@ -81,8 +81,7 @@
status = "okay";
port {
- ltdc_ep0_out: endpoint@0 {
- reg = <0>;
+ ltdc_ep0_out: endpoint {
remote-endpoint = <&panel_in>;
};
};
diff --git a/arch/arm/boot/dts/stm32mp157c-dk2.dts
b/arch/arm/boot/dts/stm32mp157c-dk2.dts
index 2bc92ef3aeb9..19ef475a48fc 100644
--- a/arch/arm/boot/dts/stm32mp157c-dk2.dts
+++ b/arch/arm/boot/dts/stm32mp157c-dk2.dts
@@ -82,9 +82,15 @@
};
;
+ #size-cells = <0>;
+
+ ltdc_ep0_out: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&sii9022_in>;
+ };
+
ltdc_ep1_out: endpoint@1 {
reg = <1>;
remote-endpoint = <&dsi_in>;
diff --git a/arch/arm/boot/dts/stm32mp157c-ev1.dts
b/arch/arm/boot/dts/stm32mp157c-ev1.dts
index 5c5b1ddf7bfd..6fe5b0fee7c4 100644
--- a/arch/arm/boot/dts/stm32mp157c-ev1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-ev1.dts
@@ -239,8 +239,7 @@
status = "okay";
port {
- ltdc_ep0_out: endpoint@0 {
- reg = <0>;
+ ltdc_ep0_out: endpoint {
remote-endpoint = <&dsi_in>;
};
};
diff --git a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
index 1e9bf7eea0f1..70202ef5267c 100644
--- a/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
+++ b/arch/arm/boot/dts/stm32mp157c-lxa-mc1.dts
@@ -161,8 +161,7 @@
status = "okay";
port {
- ltdc_ep0_out: endpoint@0 {
- reg = <0>;
+ ltdc_ep0_out: endpoint {
remote-endpoint = <&panel_input>;
};
};
diff --git a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
index 64dca5b7f748..e7f10975cacf 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dhcor-avenger96.dtsi
@@ -277,11 +277,7 @@
status = "okay";
port {
- #address-cells = <1>;
- #size-cells = <0>;
-
- ltdc_ep0_out: endpoint@0 {
- reg = <0>;
+ ltdc_ep0_out: endpoint {
remote-endpoint = <&adv7513_in>;
};
};
diff --git a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
index 59f18846cf5d..ca2d21689ecc 100644
--- a/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
+++ b/arch/arm/boot/dts/stm32mp15xx-dkx.dtsi
@@ -458,13 +458,6 @@
