[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot
issues when the bootloader does not create a 'chosen' node if such node
is not present in the dtb.

The reason for the boot failure is well explained by Javier Martinez
Canillas: "the decompressor relies on a pre-existing chosen node to be
available to insert the command line and merge other ATAGS info."

, so pass an empty 'chosen' node to fix the boot problem.

This issue has been seen in the kernelci reports with Barebox as
bootloader.

Also pass the 'memory' node in order to fix boot issues on the SolidRun
iMX6 platforms.

Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")
Reported-by: kernelci.org bot <bot-ssFOTAMYnuFg9hUCZPvPmw@public.gmane.org>
Reported-by: Russell King <rmk+kernel-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
Signed-off-by: Fabio Estevam <fabio.estevam-3arQi8VN3Tc@public.gmane.org>
---
Changes since v1:
- Also pass the memory node.

 arch/arm/boot/dts/imx1.dtsi    | 2 ++
 arch/arm/boot/dts/imx23.dtsi   | 2 ++
 arch/arm/boot/dts/imx25.dtsi   | 2 ++
 arch/arm/boot/dts/imx27.dtsi   | 2 ++
 arch/arm/boot/dts/imx28.dtsi   | 2 ++
 arch/arm/boot/dts/imx31.dtsi   | 2 ++
 arch/arm/boot/dts/imx35.dtsi   | 2 ++
 arch/arm/boot/dts/imx50.dtsi   | 2 ++
 arch/arm/boot/dts/imx51.dtsi   | 2 ++
 arch/arm/boot/dts/imx53.dtsi   | 2 ++
 arch/arm/boot/dts/imx6qdl.dtsi | 2 ++
 arch/arm/boot/dts/imx6sl.dtsi  | 2 ++
 arch/arm/boot/dts/imx6sx.dtsi  | 2 ++
 arch/arm/boot/dts/imx6ul.dtsi  | 2 ++
 arch/arm/boot/dts/imx7s.dtsi   | 2 ++
 15 files changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi
index dd3de38..7ef0d36 100644
--- a/arch/arm/boot/dts/imx1.dtsi
+++ b/arch/arm/boot/dts/imx1.dtsi
@@ -18,6 +18,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio1;
diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index 96eae64..7bb662c 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -16,6 +16,8 @@
 	#size-cells = <1>;
 
 	interrupt-parent = <&icoll>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio0;
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 213d86e..f328ecc 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -14,6 +14,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index 6a7cb9e..3271ec49 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 905bdb5..5f47558 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -17,6 +17,8 @@
 	#size-cells = <1>;
 
 	interrupt-parent = <&icoll>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &mac0;
diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi
index c0a5d5f..afd4106 100644
--- a/arch/arm/boot/dts/imx31.dtsi
+++ b/arch/arm/boot/dts/imx31.dtsi
@@ -12,6 +12,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		serial0 = &uart1;
diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 6f7b943..e08e574 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -13,6 +13,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index fe0221e..46329ff 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -17,6 +17,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 33526ca..c6ffc7b 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index ca51dc0..2d56f26 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f..c88a373 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -16,6 +16,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 19cbd87..89e8315 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -14,6 +14,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 10f3330..f52d017 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -15,6 +15,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		can0 = &flexcan1;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 0f69a3f..1d93601 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -15,6 +15,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec1;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8db1eb9..1757789 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -50,6 +50,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio1;
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot
issues when the bootloader does not create a 'chosen' node if such node
is not present in the dtb.

The reason for the boot failure is well explained by Javier Martinez
Canillas: "the decompressor relies on a pre-existing chosen node to be
available to insert the command line and merge other ATAGS info."

, so pass an empty 'chosen' node to fix the boot problem.

This issue has been seen in the kernelci reports with Barebox as
bootloader.

Also pass the 'memory' node in order to fix boot issues on the SolidRun
iMX6 platforms.

Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")
Reported-by: kernelci.org bot <bot@kernelci.org>
Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
Changes since v1:
- Also pass the memory node.

 arch/arm/boot/dts/imx1.dtsi    | 2 ++
 arch/arm/boot/dts/imx23.dtsi   | 2 ++
 arch/arm/boot/dts/imx25.dtsi   | 2 ++
 arch/arm/boot/dts/imx27.dtsi   | 2 ++
 arch/arm/boot/dts/imx28.dtsi   | 2 ++
 arch/arm/boot/dts/imx31.dtsi   | 2 ++
 arch/arm/boot/dts/imx35.dtsi   | 2 ++
 arch/arm/boot/dts/imx50.dtsi   | 2 ++
 arch/arm/boot/dts/imx51.dtsi   | 2 ++
 arch/arm/boot/dts/imx53.dtsi   | 2 ++
 arch/arm/boot/dts/imx6qdl.dtsi | 2 ++
 arch/arm/boot/dts/imx6sl.dtsi  | 2 ++
 arch/arm/boot/dts/imx6sx.dtsi  | 2 ++
 arch/arm/boot/dts/imx6ul.dtsi  | 2 ++
 arch/arm/boot/dts/imx7s.dtsi   | 2 ++
 15 files changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi
index dd3de38..7ef0d36 100644
--- a/arch/arm/boot/dts/imx1.dtsi
+++ b/arch/arm/boot/dts/imx1.dtsi
@@ -18,6 +18,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio1;
diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index 96eae64..7bb662c 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -16,6 +16,8 @@
 	#size-cells = <1>;
 
 	interrupt-parent = <&icoll>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio0;
diff --git a/arch/arm/boot/dts/imx25.dtsi b/arch/arm/boot/dts/imx25.dtsi
index 213d86e..f328ecc 100644
--- a/arch/arm/boot/dts/imx25.dtsi
+++ b/arch/arm/boot/dts/imx25.dtsi
@@ -14,6 +14,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx27.dtsi b/arch/arm/boot/dts/imx27.dtsi
index 6a7cb9e..3271ec49 100644
--- a/arch/arm/boot/dts/imx27.dtsi
+++ b/arch/arm/boot/dts/imx27.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 905bdb5..5f47558 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -17,6 +17,8 @@
 	#size-cells = <1>;
 
 	interrupt-parent = <&icoll>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &mac0;
diff --git a/arch/arm/boot/dts/imx31.dtsi b/arch/arm/boot/dts/imx31.dtsi
index c0a5d5f..afd4106 100644
--- a/arch/arm/boot/dts/imx31.dtsi
+++ b/arch/arm/boot/dts/imx31.dtsi
@@ -12,6 +12,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		serial0 = &uart1;
diff --git a/arch/arm/boot/dts/imx35.dtsi b/arch/arm/boot/dts/imx35.dtsi
index 6f7b943..e08e574 100644
--- a/arch/arm/boot/dts/imx35.dtsi
+++ b/arch/arm/boot/dts/imx35.dtsi
@@ -13,6 +13,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx50.dtsi b/arch/arm/boot/dts/imx50.dtsi
index fe0221e..46329ff 100644
--- a/arch/arm/boot/dts/imx50.dtsi
+++ b/arch/arm/boot/dts/imx50.dtsi
@@ -17,6 +17,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx51.dtsi b/arch/arm/boot/dts/imx51.dtsi
index 33526ca..c6ffc7b 100644
--- a/arch/arm/boot/dts/imx51.dtsi
+++ b/arch/arm/boot/dts/imx51.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index ca51dc0..2d56f26 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -19,6 +19,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 89b834f..c88a373 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -16,6 +16,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6sl.dtsi b/arch/arm/boot/dts/imx6sl.dtsi
index 19cbd87..89e8315 100644
--- a/arch/arm/boot/dts/imx6sl.dtsi
+++ b/arch/arm/boot/dts/imx6sl.dtsi
@@ -14,6 +14,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec;
diff --git a/arch/arm/boot/dts/imx6sx.dtsi b/arch/arm/boot/dts/imx6sx.dtsi
index 10f3330..f52d017 100644
--- a/arch/arm/boot/dts/imx6sx.dtsi
+++ b/arch/arm/boot/dts/imx6sx.dtsi
@@ -15,6 +15,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		can0 = &flexcan1;
diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
index 0f69a3f..1d93601 100644
--- a/arch/arm/boot/dts/imx6ul.dtsi
+++ b/arch/arm/boot/dts/imx6ul.dtsi
@@ -15,6 +15,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		ethernet0 = &fec1;
diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 8db1eb9..1757789 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -50,6 +50,8 @@
 / {
 	#address-cells = <1>;
 	#size-cells = <1>;
+	chosen {};
+	memory { device_type = "memory"; reg = <0 0>; };
 
 	aliases {
 		gpio0 = &gpio1;
-- 
2.7.4

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 12:02:46PM -0200, Fabio Estevam wrote:
> Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot
> issues when the bootloader does not create a 'chosen' node if such node
> is not present in the dtb.
> 
> The reason for the boot failure is well explained by Javier Martinez
> Canillas: "the decompressor relies on a pre-existing chosen node to be
> available to insert the command line and merge other ATAGS info."
> 
> , so pass an empty 'chosen' node to fix the boot problem.
> 
> This issue has been seen in the kernelci reports with Barebox as
> bootloader.
> 
> Also pass the 'memory' node in order to fix boot issues on the SolidRun
> iMX6 platforms.
> 
> Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")
> Reported-by: kernelci.org bot <bot@kernelci.org>
> Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Also pass the memory node.
> 
>  arch/arm/boot/dts/imx1.dtsi    | 2 ++
>  arch/arm/boot/dts/imx23.dtsi   | 2 ++
>  arch/arm/boot/dts/imx25.dtsi   | 2 ++
>  arch/arm/boot/dts/imx27.dtsi   | 2 ++
>  arch/arm/boot/dts/imx28.dtsi   | 2 ++
>  arch/arm/boot/dts/imx31.dtsi   | 2 ++
>  arch/arm/boot/dts/imx35.dtsi   | 2 ++
>  arch/arm/boot/dts/imx50.dtsi   | 2 ++
>  arch/arm/boot/dts/imx51.dtsi   | 2 ++
>  arch/arm/boot/dts/imx53.dtsi   | 2 ++
>  arch/arm/boot/dts/imx6qdl.dtsi | 2 ++
>  arch/arm/boot/dts/imx6sl.dtsi  | 2 ++
>  arch/arm/boot/dts/imx6sx.dtsi  | 2 ++
>  arch/arm/boot/dts/imx6ul.dtsi  | 2 ++
>  arch/arm/boot/dts/imx7s.dtsi   | 2 ++
>  15 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi
> index dd3de38..7ef0d36 100644
> --- a/arch/arm/boot/dts/imx1.dtsi
> +++ b/arch/arm/boot/dts/imx1.dtsi
> @@ -18,6 +18,8 @@
>  / {
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> +	chosen {};
> +	memory { device_type = "memory"; reg = <0 0>; };

Would it be nice to add a comment about why this was added? Something to
prevent a cleanup like "remove empty nodes and invalid memory
configurations".

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 12:02:46PM -0200, Fabio Estevam wrote:
> Commit 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi") causes boot
> issues when the bootloader does not create a 'chosen' node if such node
> is not present in the dtb.
> 
> The reason for the boot failure is well explained by Javier Martinez
> Canillas: "the decompressor relies on a pre-existing chosen node to be
> available to insert the command line and merge other ATAGS info."
> 
> , so pass an empty 'chosen' node to fix the boot problem.
> 
> This issue has been seen in the kernelci reports with Barebox as
> bootloader.
> 
> Also pass the 'memory' node in order to fix boot issues on the SolidRun
> iMX6 platforms.
> 
> Fixes: 7f107887d199 ("ARM: dts: imx: Remove skeleton.dtsi")
> Reported-by: kernelci.org bot <bot@kernelci.org>
> Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> Changes since v1:
> - Also pass the memory node.
> 
>  arch/arm/boot/dts/imx1.dtsi    | 2 ++
>  arch/arm/boot/dts/imx23.dtsi   | 2 ++
>  arch/arm/boot/dts/imx25.dtsi   | 2 ++
>  arch/arm/boot/dts/imx27.dtsi   | 2 ++
>  arch/arm/boot/dts/imx28.dtsi   | 2 ++
>  arch/arm/boot/dts/imx31.dtsi   | 2 ++
>  arch/arm/boot/dts/imx35.dtsi   | 2 ++
>  arch/arm/boot/dts/imx50.dtsi   | 2 ++
>  arch/arm/boot/dts/imx51.dtsi   | 2 ++
>  arch/arm/boot/dts/imx53.dtsi   | 2 ++
>  arch/arm/boot/dts/imx6qdl.dtsi | 2 ++
>  arch/arm/boot/dts/imx6sl.dtsi  | 2 ++
>  arch/arm/boot/dts/imx6sx.dtsi  | 2 ++
>  arch/arm/boot/dts/imx6ul.dtsi  | 2 ++
>  arch/arm/boot/dts/imx7s.dtsi   | 2 ++
>  15 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/imx1.dtsi b/arch/arm/boot/dts/imx1.dtsi
> index dd3de38..7ef0d36 100644
> --- a/arch/arm/boot/dts/imx1.dtsi
> +++ b/arch/arm/boot/dts/imx1.dtsi
> @@ -18,6 +18,8 @@
>  / {
>  	#address-cells = <1>;
>  	#size-cells = <1>;
> +	chosen {};
> +	memory { device_type = "memory"; reg = <0 0>; };

Would it be nice to add a comment about why this was added? Something to
prevent a cleanup like "remove empty nodes and invalid memory
configurations".

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Hi Uwe,

On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> wrote:

> Would it be nice to add a comment about why this was added? Something to
> prevent a cleanup like "remove empty nodes and invalid memory
> configurations".

Do you mean something like this?

/* "chosen" and "memory" nodes are mandatory */
chosen {};
memory { device_type = "memory"; reg = <0 0>; };

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Hi Uwe,

On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
<u.kleine-koenig@pengutronix.de> wrote:

> Would it be nice to add a comment about why this was added? Something to
> prevent a cleanup like "remove empty nodes and invalid memory
> configurations".

Do you mean something like this?

/* "chosen" and "memory" nodes are mandatory */
chosen {};
memory { device_type = "memory"; reg = <0 0>; };

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> Hi Uwe,
> 
> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
> 
> > Would it be nice to add a comment about why this was added? Something to
> > prevent a cleanup like "remove empty nodes and invalid memory
> > configurations".
> 
> Do you mean something like this?
> 
> /* "chosen" and "memory" nodes are mandatory */
> chosen {};
> memory { device_type = "memory"; reg = <0 0>; };

Not very helpful comment. Something like:

	/*
	 * The decompressor relies on a pre-existing chosen node to be                                                          
	 * available to insert the command line and merge other ATAGS
	 * info.
	 */

Is it difficult to fix the decompressor?

I didn't understood the breakage regarding the memory node good enough
to suggest a comment for that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> Hi Uwe,
> 
> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
> <u.kleine-koenig@pengutronix.de> wrote:
> 
> > Would it be nice to add a comment about why this was added? Something to
> > prevent a cleanup like "remove empty nodes and invalid memory
> > configurations".
> 
> Do you mean something like this?
> 
> /* "chosen" and "memory" nodes are mandatory */
> chosen {};
> memory { device_type = "memory"; reg = <0 0>; };

Not very helpful comment. Something like:

	/*
	 * The decompressor relies on a pre-existing chosen node to be                                                          
	 * available to insert the command line and merge other ATAGS
	 * info.
	 */

Is it difficult to fix the decompressor?

I didn't understood the breakage regarding the memory node good enough
to suggest a comment for that.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Russell King - ARM Linux]

On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote:
> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> > Hi Uwe,
> > 
> > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
> > <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > Would it be nice to add a comment about why this was added? Something to
> > > prevent a cleanup like "remove empty nodes and invalid memory
> > > configurations".
> > 
> > Do you mean something like this?
> > 
> > /* "chosen" and "memory" nodes are mandatory */
> > chosen {};
> > memory { device_type = "memory"; reg = <0 0>; };
> 
> Not very helpful comment. Something like:
> 
> 	/*
> 	 * The decompressor relies on a pre-existing chosen node to be
> 	 * available to insert the command line and merge other ATAGS
> 	 * info.
> 	 */
> 
> Is it difficult to fix the decompressor?

... and that comment would be wrong.  Yes, the decompressor relies on it,
as do some uboot versions.

> I didn't understood the breakage regarding the memory node good enough
> to suggest a comment for that.

A missing memory node appears to prevent some uboot versions supplying
any kind of memory layout to the kernel, which then causes the kernel to
crash very early during boot.

Again, this is not using appended DTB - this is using a separately loaded
DTB in uboot.  uboot fails to update the dtb if these nodes are missing.

Frankly, I think the original change (removing the skeleton.dtsi include)
was misguided and needs to be reverted - the change is imho built upon an
incorrect assumption that nothing in skeleton.dtsi is required.  That's
clearly false.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Russell King - ARM Linux]

On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-K?nig wrote:
> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> > Hi Uwe,
> > 
> > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
> > <u.kleine-koenig@pengutronix.de> wrote:
> > 
> > > Would it be nice to add a comment about why this was added? Something to
> > > prevent a cleanup like "remove empty nodes and invalid memory
> > > configurations".
> > 
> > Do you mean something like this?
> > 
> > /* "chosen" and "memory" nodes are mandatory */
> > chosen {};
> > memory { device_type = "memory"; reg = <0 0>; };
> 
> Not very helpful comment. Something like:
> 
> 	/*
> 	 * The decompressor relies on a pre-existing chosen node to be
> 	 * available to insert the command line and merge other ATAGS
> 	 * info.
> 	 */
> 
> Is it difficult to fix the decompressor?

... and that comment would be wrong.  Yes, the decompressor relies on it,
as do some uboot versions.

> I didn't understood the breakage regarding the memory node good enough
> to suggest a comment for that.

A missing memory node appears to prevent some uboot versions supplying
any kind of memory layout to the kernel, which then causes the kernel to
crash very early during boot.

Again, this is not using appended DTB - this is using a separately loaded
DTB in uboot.  uboot fails to update the dtb if these nodes are missing.

Frankly, I think the original change (removing the skeleton.dtsi include)
was misguided and needs to be reverted - the change is imho built upon an
incorrect assumption that nothing in skeleton.dtsi is required.  That's
clearly false.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

On Thu, Jan 19, 2017 at 12:56 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Frankly, I think the original change (removing the skeleton.dtsi include)
> was misguided and needs to be reverted - the change is imho built upon an
> incorrect assumption that nothing in skeleton.dtsi is required.  That's
> clearly false.

Wouldn't this v2 patch restore things back to normal?

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

On Thu, Jan 19, 2017 at 12:56 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> Frankly, I think the original change (removing the skeleton.dtsi include)
> was misguided and needs to be reverted - the change is imho built upon an
> incorrect assumption that nothing in skeleton.dtsi is required.  That's
> clearly false.

Wouldn't this v2 patch restore things back to normal?

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote:
> > On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> > > Hi Uwe,
> > > 
> > > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
> > > <u.kleine-koenig@pengutronix.de> wrote:
> > > 
> > > > Would it be nice to add a comment about why this was added? Something to
> > > > prevent a cleanup like "remove empty nodes and invalid memory
> > > > configurations".
> > > 
> > > Do you mean something like this?
> > > 
> > > /* "chosen" and "memory" nodes are mandatory */
> > > chosen {};
> > > memory { device_type = "memory"; reg = <0 0>; };
> > 
> > Not very helpful comment. Something like:
> > 
> > 	/*
> > 	 * The decompressor relies on a pre-existing chosen node to be
> > 	 * available to insert the command line and merge other ATAGS
> > 	 * info.
> > 	 */
> > 
> > Is it difficult to fix the decompressor?
> 
> ... and that comment would be wrong.  Yes, the decompressor relies on it,
> as do some uboot versions.

Good that we talked about this, otherwise I would have removed that once
the decompressor is fixed. So the comment must be

	/*
	 * The decompressor and also some versions of U-Boot rely on a
	 * pre-existing /chosen node to be available to insert the
	 * command line and merge other ATAGS info.
	 * Also for U-Boot there must be a pre-existing /memory node.
	 */
 
> > I didn't understood the breakage regarding the memory node good enough
> > to suggest a comment for that.
> 
> A missing memory node appears to prevent some uboot versions supplying
> any kind of memory layout to the kernel, which then causes the kernel to
> crash very early during boot.
> 
> Again, this is not using appended DTB - this is using a separately loaded
> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
> 
> Frankly, I think the original change (removing the skeleton.dtsi include)
> was misguided and needs to be reverted - the change is imho built upon an
> incorrect assumption that nothing in skeleton.dtsi is required.  That's
> clearly false.

I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
platforms.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-K?nig wrote:
> > On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
> > > Hi Uwe,
> > > 
> > > On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
> > > <u.kleine-koenig@pengutronix.de> wrote:
> > > 
> > > > Would it be nice to add a comment about why this was added? Something to
> > > > prevent a cleanup like "remove empty nodes and invalid memory
> > > > configurations".
> > > 
> > > Do you mean something like this?
> > > 
> > > /* "chosen" and "memory" nodes are mandatory */
> > > chosen {};
> > > memory { device_type = "memory"; reg = <0 0>; };
> > 
> > Not very helpful comment. Something like:
> > 
> > 	/*
> > 	 * The decompressor relies on a pre-existing chosen node to be
> > 	 * available to insert the command line and merge other ATAGS
> > 	 * info.
> > 	 */
> > 
> > Is it difficult to fix the decompressor?
> 
> ... and that comment would be wrong.  Yes, the decompressor relies on it,
> as do some uboot versions.

Good that we talked about this, otherwise I would have removed that once
the decompressor is fixed. So the comment must be

	/*
	 * The decompressor and also some versions of U-Boot rely on a
	 * pre-existing /chosen node to be available to insert the
	 * command line and merge other ATAGS info.
	 * Also for U-Boot there must be a pre-existing /memory node.
	 */
 
> > I didn't understood the breakage regarding the memory node good enough
> > to suggest a comment for that.
> 
> A missing memory node appears to prevent some uboot versions supplying
> any kind of memory layout to the kernel, which then causes the kernel to
> crash very early during boot.
> 
> Again, this is not using appended DTB - this is using a separately loaded
> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
> 
> Frankly, I think the original change (removing the skeleton.dtsi include)
> was misguided and needs to be reverted - the change is imho built upon an
> incorrect assumption that nothing in skeleton.dtsi is required.  That's
> clearly false.

I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
platforms.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Frank Rowand]

On 01/19/17 08:14, Uwe Kleine-König wrote:
> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote:
>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>> Hi Uwe,
>>>>
>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
>>>> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>>>>
>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>> configurations".
>>>>
>>>> Do you mean something like this?
>>>>
>>>> /* "chosen" and "memory" nodes are mandatory */
>>>> chosen {};
>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>
>>> Not very helpful comment. Something like:
>>>
>>> 	/*
>>> 	 * The decompressor relies on a pre-existing chosen node to be
>>> 	 * available to insert the command line and merge other ATAGS
>>> 	 * info.
>>> 	 */
>>>
>>> Is it difficult to fix the decompressor?
>>
>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>> as do some uboot versions.
> 
> Good that we talked about this, otherwise I would have removed that once
> the decompressor is fixed. So the comment must be
> 
> 	/*
> 	 * The decompressor and also some versions of U-Boot rely on a
> 	 * pre-existing /chosen node to be available to insert the
> 	 * command line and merge other ATAGS info.
> 	 * Also for U-Boot there must be a pre-existing /memory node.
> 	 */
>  
>>> I didn't understood the breakage regarding the memory node good enough
>>> to suggest a comment for that.
>>
>> A missing memory node appears to prevent some uboot versions supplying
>> any kind of memory layout to the kernel, which then causes the kernel to
>> crash very early during boot.
>>
>> Again, this is not using appended DTB - this is using a separately loaded
>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>
>> Frankly, I think the original change (removing the skeleton.dtsi include)
>> was misguided and needs to be reverted - the change is imho built upon an
>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>> clearly false.
> 
> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
> platforms.
> 
> Best regards
> Uwe
> 

Adding back the chosen nodes is a bandaid that papers over the actual bug
in the decompressor.

A comment about Fabio's attempt to fix the decompressor noted a possible
issue with the method taken to fix the problem, but then the discussion
of fixing the decompressor was dropped and v3 of the patch to add chosen
into a bunch of .dtsi files was applied by Shawn.

Can someone please help Fabio to create a correct patch to fix the
decompressor?

Note that the ePAPR v1.1 does require at least one memory node in
section 3.1, so the portion of v3 of the patch that adds a memory
node is valid in terms of following the standard, not just a U-boot
requirement.

Thanks,

Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Frank Rowand]

On 01/19/17 08:14, Uwe Kleine-K?nig wrote:
> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-K?nig wrote:
>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>> Hi Uwe,
>>>>
>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>
>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>> configurations".
>>>>
>>>> Do you mean something like this?
>>>>
>>>> /* "chosen" and "memory" nodes are mandatory */
>>>> chosen {};
>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>
>>> Not very helpful comment. Something like:
>>>
>>> 	/*
>>> 	 * The decompressor relies on a pre-existing chosen node to be
>>> 	 * available to insert the command line and merge other ATAGS
>>> 	 * info.
>>> 	 */
>>>
>>> Is it difficult to fix the decompressor?
>>
>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>> as do some uboot versions.
> 
> Good that we talked about this, otherwise I would have removed that once
> the decompressor is fixed. So the comment must be
> 
> 	/*
> 	 * The decompressor and also some versions of U-Boot rely on a
> 	 * pre-existing /chosen node to be available to insert the
> 	 * command line and merge other ATAGS info.
> 	 * Also for U-Boot there must be a pre-existing /memory node.
> 	 */
>  
>>> I didn't understood the breakage regarding the memory node good enough
>>> to suggest a comment for that.
>>
>> A missing memory node appears to prevent some uboot versions supplying
>> any kind of memory layout to the kernel, which then causes the kernel to
>> crash very early during boot.
>>
>> Again, this is not using appended DTB - this is using a separately loaded
>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>
>> Frankly, I think the original change (removing the skeleton.dtsi include)
>> was misguided and needs to be reverted - the change is imho built upon an
>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>> clearly false.
> 
> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
> platforms.
> 
> Best regards
> Uwe
> 

Adding back the chosen nodes is a bandaid that papers over the actual bug
in the decompressor.

A comment about Fabio's attempt to fix the decompressor noted a possible
issue with the method taken to fix the problem, but then the discussion
of fixing the decompressor was dropped and v3 of the patch to add chosen
into a bunch of .dtsi files was applied by Shawn.

Can someone please help Fabio to create a correct patch to fix the
decompressor?

Note that the ePAPR v1.1 does require at least one memory node in
section 3.1, so the portion of v3 of the patch that adds a memory
node is valid in terms of following the standard, not just a U-boot
requirement.

Thanks,

Frank

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Rob Herring]

On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 01/19/17 08:14, Uwe Kleine-König wrote:
>> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote:
>>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>>> Hi Uwe,
>>>>>
>>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
>>>>> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>>>>>
>>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>>> configurations".
>>>>>
>>>>> Do you mean something like this?
>>>>>
>>>>> /* "chosen" and "memory" nodes are mandatory */
>>>>> chosen {};
>>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>>
>>>> Not very helpful comment. Something like:
>>>>
>>>>     /*
>>>>      * The decompressor relies on a pre-existing chosen node to be
>>>>      * available to insert the command line and merge other ATAGS
>>>>      * info.
>>>>      */
>>>>
>>>> Is it difficult to fix the decompressor?
>>>
>>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>>> as do some uboot versions.
>>
>> Good that we talked about this, otherwise I would have removed that once
>> the decompressor is fixed. So the comment must be
>>
>>       /*
>>        * The decompressor and also some versions of U-Boot rely on a
>>        * pre-existing /chosen node to be available to insert the
>>        * command line and merge other ATAGS info.
>>        * Also for U-Boot there must be a pre-existing /memory node.
>>        */
>>
>>>> I didn't understood the breakage regarding the memory node good enough
>>>> to suggest a comment for that.
>>>
>>> A missing memory node appears to prevent some uboot versions supplying
>>> any kind of memory layout to the kernel, which then causes the kernel to
>>> crash very early during boot.
>>>
>>> Again, this is not using appended DTB - this is using a separately loaded
>>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>>
>>> Frankly, I think the original change (removing the skeleton.dtsi include)
>>> was misguided and needs to be reverted - the change is imho built upon an
>>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>>> clearly false.
>>
>> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
>> platforms.
>>
>> Best regards
>> Uwe
>>
>
> Adding back the chosen nodes is a bandaid that papers over the actual bug
> in the decompressor.
>
> A comment about Fabio's attempt to fix the decompressor noted a possible
> issue with the method taken to fix the problem, but then the discussion
> of fixing the decompressor was dropped and v3 of the patch to add chosen
> into a bunch of .dtsi files was applied by Shawn.
>
> Can someone please help Fabio to create a correct patch to fix the
> decompressor?
>
> Note that the ePAPR v1.1 does require at least one memory node in

We have a DT spec now, let's stop pointing people to ePAPR.

> section 3.1, so the portion of v3 of the patch that adds a memory
> node is valid in terms of following the standard, not just a U-boot
> requirement.

I'd have no issue making chosen be required. Any platform with a
serial console should be setting stdout-path anyway. Just add it to
the spec. :) We should also add a dtc check as well if we do that.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Rob Herring]

On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 01/19/17 08:14, Uwe Kleine-K?nig wrote:
>> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-K?nig wrote:
>>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>>> Hi Uwe,
>>>>>
>>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>
>>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>>> configurations".
>>>>>
>>>>> Do you mean something like this?
>>>>>
>>>>> /* "chosen" and "memory" nodes are mandatory */
>>>>> chosen {};
>>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>>
>>>> Not very helpful comment. Something like:
>>>>
>>>>     /*
>>>>      * The decompressor relies on a pre-existing chosen node to be
>>>>      * available to insert the command line and merge other ATAGS
>>>>      * info.
>>>>      */
>>>>
>>>> Is it difficult to fix the decompressor?
>>>
>>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>>> as do some uboot versions.
>>
>> Good that we talked about this, otherwise I would have removed that once
>> the decompressor is fixed. So the comment must be
>>
>>       /*
>>        * The decompressor and also some versions of U-Boot rely on a
>>        * pre-existing /chosen node to be available to insert the
>>        * command line and merge other ATAGS info.
>>        * Also for U-Boot there must be a pre-existing /memory node.
>>        */
>>
>>>> I didn't understood the breakage regarding the memory node good enough
>>>> to suggest a comment for that.
>>>
>>> A missing memory node appears to prevent some uboot versions supplying
>>> any kind of memory layout to the kernel, which then causes the kernel to
>>> crash very early during boot.
>>>
>>> Again, this is not using appended DTB - this is using a separately loaded
>>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>>
>>> Frankly, I think the original change (removing the skeleton.dtsi include)
>>> was misguided and needs to be reverted - the change is imho built upon an
>>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>>> clearly false.
>>
>> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
>> platforms.
>>
>> Best regards
>> Uwe
>>
>
> Adding back the chosen nodes is a bandaid that papers over the actual bug
> in the decompressor.
>
> A comment about Fabio's attempt to fix the decompressor noted a possible
> issue with the method taken to fix the problem, but then the discussion
> of fixing the decompressor was dropped and v3 of the patch to add chosen
> into a bunch of .dtsi files was applied by Shawn.
>
> Can someone please help Fabio to create a correct patch to fix the
> decompressor?
>
> Note that the ePAPR v1.1 does require at least one memory node in

We have a DT spec now, let's stop pointing people to ePAPR.

> section 3.1, so the portion of v3 of the patch that adds a memory
> node is valid in terms of following the standard, not just a U-boot
> requirement.

I'd have no issue making chosen be required. Any platform with a
serial console should be setting stdout-path anyway. Just add it to
the spec. :) We should also add a dtc check as well if we do that.

Rob

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Frank Rowand]

On 01/25/17 14:46, Rob Herring wrote:
> On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 01/19/17 08:14, Uwe Kleine-König wrote:
>>> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>>>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-König wrote:
>>>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>>>> Hi Uwe,
>>>>>>
>>>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-König
>>>>>> <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote:
>>>>>>
>>>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>>>> configurations".
>>>>>>
>>>>>> Do you mean something like this?
>>>>>>
>>>>>> /* "chosen" and "memory" nodes are mandatory */
>>>>>> chosen {};
>>>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>>>
>>>>> Not very helpful comment. Something like:
>>>>>
>>>>>     /*
>>>>>      * The decompressor relies on a pre-existing chosen node to be
>>>>>      * available to insert the command line and merge other ATAGS
>>>>>      * info.
>>>>>      */
>>>>>
>>>>> Is it difficult to fix the decompressor?
>>>>
>>>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>>>> as do some uboot versions.
>>>
>>> Good that we talked about this, otherwise I would have removed that once
>>> the decompressor is fixed. So the comment must be
>>>
>>>       /*
>>>        * The decompressor and also some versions of U-Boot rely on a
>>>        * pre-existing /chosen node to be available to insert the
>>>        * command line and merge other ATAGS info.
>>>        * Also for U-Boot there must be a pre-existing /memory node.
>>>        */
>>>
>>>>> I didn't understood the breakage regarding the memory node good enough
>>>>> to suggest a comment for that.
>>>>
>>>> A missing memory node appears to prevent some uboot versions supplying
>>>> any kind of memory layout to the kernel, which then causes the kernel to
>>>> crash very early during boot.
>>>>
>>>> Again, this is not using appended DTB - this is using a separately loaded
>>>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>>>
>>>> Frankly, I think the original change (removing the skeleton.dtsi include)
>>>> was misguided and needs to be reverted - the change is imho built upon an
>>>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>>>> clearly false.
>>>
>>> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
>>> platforms.
>>>
>>> Best regards
>>> Uwe
>>>
>>
>> Adding back the chosen nodes is a bandaid that papers over the actual bug
>> in the decompressor.
>>
>> A comment about Fabio's attempt to fix the decompressor noted a possible
>> issue with the method taken to fix the problem, but then the discussion
>> of fixing the decompressor was dropped and v3 of the patch to add chosen
>> into a bunch of .dtsi files was applied by Shawn.
>>
>> Can someone please help Fabio to create a correct patch to fix the
>> decompressor?
>>
>> Note that the ePAPR v1.1 does require at least one memory node in
> 
> We have a DT spec now, let's stop pointing people to ePAPR.
> 
>> section 3.1, so the portion of v3 of the patch that adds a memory
>> node is valid in terms of following the standard, not just a U-boot
>> requirement.
> 
> I'd have no issue making chosen be required. Any platform with a
> serial console should be setting stdout-path anyway. Just add it to
> the spec. :) We should also add a dtc check as well if we do that.
> 
> Rob
> 

Sounds good to me.

-Frank
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Frank Rowand]

On 01/25/17 14:46, Rob Herring wrote:
> On Wed, Jan 25, 2017 at 3:13 PM, Frank Rowand <frowand.list@gmail.com> wrote:
>> On 01/19/17 08:14, Uwe Kleine-K?nig wrote:
>>> On Thu, Jan 19, 2017 at 02:56:42PM +0000, Russell King - ARM Linux wrote:
>>>> On Thu, Jan 19, 2017 at 03:46:41PM +0100, Uwe Kleine-K?nig wrote:
>>>>> On Thu, Jan 19, 2017 at 12:35:40PM -0200, Fabio Estevam wrote:
>>>>>> Hi Uwe,
>>>>>>
>>>>>> On Thu, Jan 19, 2017 at 12:13 PM, Uwe Kleine-K?nig
>>>>>> <u.kleine-koenig@pengutronix.de> wrote:
>>>>>>
>>>>>>> Would it be nice to add a comment about why this was added? Something to
>>>>>>> prevent a cleanup like "remove empty nodes and invalid memory
>>>>>>> configurations".
>>>>>>
>>>>>> Do you mean something like this?
>>>>>>
>>>>>> /* "chosen" and "memory" nodes are mandatory */
>>>>>> chosen {};
>>>>>> memory { device_type = "memory"; reg = <0 0>; };
>>>>>
>>>>> Not very helpful comment. Something like:
>>>>>
>>>>>     /*
>>>>>      * The decompressor relies on a pre-existing chosen node to be
>>>>>      * available to insert the command line and merge other ATAGS
>>>>>      * info.
>>>>>      */
>>>>>
>>>>> Is it difficult to fix the decompressor?
>>>>
>>>> ... and that comment would be wrong.  Yes, the decompressor relies on it,
>>>> as do some uboot versions.
>>>
>>> Good that we talked about this, otherwise I would have removed that once
>>> the decompressor is fixed. So the comment must be
>>>
>>>       /*
>>>        * The decompressor and also some versions of U-Boot rely on a
>>>        * pre-existing /chosen node to be available to insert the
>>>        * command line and merge other ATAGS info.
>>>        * Also for U-Boot there must be a pre-existing /memory node.
>>>        */
>>>
>>>>> I didn't understood the breakage regarding the memory node good enough
>>>>> to suggest a comment for that.
>>>>
>>>> A missing memory node appears to prevent some uboot versions supplying
>>>> any kind of memory layout to the kernel, which then causes the kernel to
>>>> crash very early during boot.
>>>>
>>>> Again, this is not using appended DTB - this is using a separately loaded
>>>> DTB in uboot.  uboot fails to update the dtb if these nodes are missing.
>>>>
>>>> Frankly, I think the original change (removing the skeleton.dtsi include)
>>>> was misguided and needs to be reverted - the change is imho built upon an
>>>> incorrect assumption that nothing in skeleton.dtsi is required.  That's
>>>> clearly false.
>>>
>>> I tend to agree as I assume it's not only U-Boot on i.MX but on all ARM
>>> platforms.
>>>
>>> Best regards
>>> Uwe
>>>
>>
>> Adding back the chosen nodes is a bandaid that papers over the actual bug
>> in the decompressor.
>>
>> A comment about Fabio's attempt to fix the decompressor noted a possible
>> issue with the method taken to fix the problem, but then the discussion
>> of fixing the decompressor was dropped and v3 of the patch to add chosen
>> into a bunch of .dtsi files was applied by Shawn.
>>
>> Can someone please help Fabio to create a correct patch to fix the
>> decompressor?
>>
>> Note that the ePAPR v1.1 does require at least one memory node in
> 
> We have a DT spec now, let's stop pointing people to ePAPR.
> 
>> section 3.1, so the portion of v3 of the patch that adds a memory
>> node is valid in terms of following the standard, not just a U-boot
>> requirement.
> 
> I'd have no issue making chosen be required. Any platform with a
> serial console should be setting stdout-path anyway. Just add it to
> the spec. :) We should also add a dtc check as well if we do that.
> 
> Rob
> 

Sounds good to me.

-Frank

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Russell King - ARM Linux]

On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote:
> Adding back the chosen nodes is a bandaid that papers over the actual bug
> in the decompressor.
> 
> A comment about Fabio's attempt to fix the decompressor noted a possible
> issue with the method taken to fix the problem, but then the discussion
> of fixing the decompressor was dropped and v3 of the patch to add chosen
> into a bunch of .dtsi files was applied by Shawn.
> 
> Can someone please help Fabio to create a correct patch to fix the
> decompressor?

I refuse to assist, see below.

The reason this has come up is that this problem has _nothing_ to do
with the decompressor - see recent context on the linux-arm-kernel
list.

The removal of skeleton.dtsi from iMX6 DTS files has caused a
regression in the current -rc kernels when an iMX6 board is booted
with uboot using a _separated_ device tree file.

In that situation, the decompressor has _nothing_ at all to do with
modifying the DT binary file.  The DT binary file ends up being passed
to the kernel from the boot loader unmodified by the decompressor.
Remember that the decompressor's modification of the *appended* DT
file is a band-aid to allow an *ATAG* based system to continue working.
That doesn't apply to iMX6.

The lack of the memory node means that uboot fails to add a memory node
to the DT binary file before passing control to the decompressor, which
in turn causes the kernel to be booted with a DT binary file which says
that the system has no memory.  That causes the kernel to crash in the
very early stages of boot, before the console is up and running.  So
it's a nasty silent crash.

So, while it may be desirable to fix the decompressor, it's completely
pointless as far as whether Fabio's patch gets merged, because this
_regression_ has nothing to do with the decompressor.

We need this regression fixed before 4.10 comes out so systems don't
break when 4.10 comes out.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Russell King - ARM Linux]

On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote:
> Adding back the chosen nodes is a bandaid that papers over the actual bug
> in the decompressor.
> 
> A comment about Fabio's attempt to fix the decompressor noted a possible
> issue with the method taken to fix the problem, but then the discussion
> of fixing the decompressor was dropped and v3 of the patch to add chosen
> into a bunch of .dtsi files was applied by Shawn.
> 
> Can someone please help Fabio to create a correct patch to fix the
> decompressor?

I refuse to assist, see below.

The reason this has come up is that this problem has _nothing_ to do
with the decompressor - see recent context on the linux-arm-kernel
list.

The removal of skeleton.dtsi from iMX6 DTS files has caused a
regression in the current -rc kernels when an iMX6 board is booted
with uboot using a _separated_ device tree file.

In that situation, the decompressor has _nothing_ at all to do with
modifying the DT binary file.  The DT binary file ends up being passed
to the kernel from the boot loader unmodified by the decompressor.
Remember that the decompressor's modification of the *appended* DT
file is a band-aid to allow an *ATAG* based system to continue working.
That doesn't apply to iMX6.

The lack of the memory node means that uboot fails to add a memory node
to the DT binary file before passing control to the decompressor, which
in turn causes the kernel to be booted with a DT binary file which says
that the system has no memory.  That causes the kernel to crash in the
very early stages of boot, before the console is up and running.  So
it's a nasty silent crash.

So, while it may be desirable to fix the decompressor, it's completely
pointless as far as whether Fabio's patch gets merged, because this
_regression_ has nothing to do with the decompressor.

We need this regression fixed before 4.10 comes out so systems don't
break when 4.10 comes out.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
<linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org> wrote:

> We need this regression fixed before 4.10 comes out so systems don't
> break when 4.10 comes out.

It will be fixed in the next rc with the following commit:
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

> We need this regression fixed before 4.10 comes out so systems don't
> break when 4.10 comes out.

It will be fixed in the next rc with the following commit:
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

Hello,

On Mon, Jan 30, 2017 at 06:54:44PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote:
> > Adding back the chosen nodes is a bandaid that papers over the actual bug
> > in the decompressor.
> > 
> > A comment about Fabio's attempt to fix the decompressor noted a possible
> > issue with the method taken to fix the problem, but then the discussion
> > of fixing the decompressor was dropped and v3 of the patch to add chosen
> > into a bunch of .dtsi files was applied by Shawn.
> > 
> > Can someone please help Fabio to create a correct patch to fix the
> > decompressor?
> 
> I refuse to assist, see below.
> 
> The reason this has come up is that this problem has _nothing_ to do
> with the decompressor - see recent context on the linux-arm-kernel
> list.

Several people reported problems with the patch that removes
skeleton.dtsi. There are at least:

 - Russell, who saw U-Boot failing to specify the amount of RAM;
 - somebody (I forgot and I'm lazy so I didn't look up the name) who
   noticed some i.MX6 boards fail in kernelci with barebox because
   barebox assumed /chosen to be present; and
 - somebody else (...) who reported the ATAGS_DTB_COMPAT failure
   (also wrongly called "decompressor failure" in this thread).

I agree that for each single of the above issues it is worth to readd
the /chosen and /memory nodes for now. Still all three are IMHO worth to
be fixed.

barebox is already fixed in v2017.01.0 (commit f49b415b9231 (of: base:
add chosen node if it does not exist when adding initrd),
https://git.pengutronix.de/cgit/barebox/commit/?id=f49b415b9231).

Looking at the ATAGS_DTB_COMPAT problem: The code in question is
(if I understood correctly) merge_fdt_bootargs in
arch/arm/boot/compressed/atags_to_fdt.c:

	setprop_string(fdt, "/chosen", "bootargs", cmdline);

looking further:

	static int node_offset(void *fdt, const char *node_path)
	{       
		int offset = fdt_path_offset(fdt, node_path);
		if (offset == -FDT_ERR_NOTFOUND)
			offset = fdt_add_subnode(fdt, 0, node_path);
		return offset;
	}

	static int setprop_string(void *fdt, const char *node_path,
				  const char *property, const char *string)
	{
		int offset = node_offset(fdt, node_path);
		if (offset < 0)
			return offset;
		return fdt_setprop_string(fdt, offset, property, string);
	}

So it seems "/chosen" not being there should be handled. I don't have a
reproducer for this problem, but it doesn't look that hard to fix with
DEBUG_LL and a system that shows the problem.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Uwe Kleine-König]

Hello,

On Mon, Jan 30, 2017 at 06:54:44PM +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 25, 2017 at 01:13:23PM -0800, Frank Rowand wrote:
> > Adding back the chosen nodes is a bandaid that papers over the actual bug
> > in the decompressor.
> > 
> > A comment about Fabio's attempt to fix the decompressor noted a possible
> > issue with the method taken to fix the problem, but then the discussion
> > of fixing the decompressor was dropped and v3 of the patch to add chosen
> > into a bunch of .dtsi files was applied by Shawn.
> > 
> > Can someone please help Fabio to create a correct patch to fix the
> > decompressor?
> 
> I refuse to assist, see below.
> 
> The reason this has come up is that this problem has _nothing_ to do
> with the decompressor - see recent context on the linux-arm-kernel
> list.

Several people reported problems with the patch that removes
skeleton.dtsi. There are at least:

 - Russell, who saw U-Boot failing to specify the amount of RAM;
 - somebody (I forgot and I'm lazy so I didn't look up the name) who
   noticed some i.MX6 boards fail in kernelci with barebox because
   barebox assumed /chosen to be present; and
 - somebody else (...) who reported the ATAGS_DTB_COMPAT failure
   (also wrongly called "decompressor failure" in this thread).

I agree that for each single of the above issues it is worth to readd
the /chosen and /memory nodes for now. Still all three are IMHO worth to
be fixed.

barebox is already fixed in v2017.01.0 (commit f49b415b9231 (of: base:
add chosen node if it does not exist when adding initrd),
https://git.pengutronix.de/cgit/barebox/commit/?id=f49b415b9231).

Looking at the ATAGS_DTB_COMPAT problem: The code in question is
(if I understood correctly) merge_fdt_bootargs in
arch/arm/boot/compressed/atags_to_fdt.c:

	setprop_string(fdt, "/chosen", "bootargs", cmdline);

looking further:

	static int node_offset(void *fdt, const char *node_path)
	{       
		int offset = fdt_path_offset(fdt, node_path);
		if (offset == -FDT_ERR_NOTFOUND)
			offset = fdt_add_subnode(fdt, 0, node_path);
		return offset;
	}

	static int setprop_string(void *fdt, const char *node_path,
				  const char *property, const char *string)
	{
		int offset = node_offset(fdt, node_path);
		if (offset < 0)
			return offset;
		return fdt_setprop_string(fdt, offset, property, string);
	}

So it seems "/chosen" not being there should be handled. I don't have a
reproducer for this problem, but it doesn't look that hard to fix with
DEBUG_LL and a system that shows the problem.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Stefan Wahren]

Hi,

> On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
> <linux@xxxxxxxxxxxxxxx> wrote:
>
> >/We need this regression fixed before 4.10 comes out so systems don't/
> >/break when 4.10 comes out./
>
> It will be fixed in the next rc with the following commit:
>
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
<https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9>


i want to report that the combination i.MX28 + imx-bootlets (requires
CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also
affected before and fixed by this patch.

Btw are Arnd and Olof informed about this urgent patch?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Stefan Wahren]

Hi,

> On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
> <linux@xxxxxxxxxxxxxxx> wrote:
>
> >/We need this regression fixed before 4.10 comes out so systems don't/
> >/break when 4.10 comes out./
>
> It will be fixed in the next rc with the following commit:
>
https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
<https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9>


i want to report that the combination i.MX28 + imx-bootlets (requires
CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also
affected before and fixed by this patch.

Btw are Arnd and Olof informed about this urgent patch?

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Stefan Wahren]

Am 01.02.2017 um 14:30 schrieb Stefan Wahren:
> Hi,
>
>> On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxx> wrote:
>>
>>> /We need this regression fixed before 4.10 comes out so systems don't/
>>> /break when 4.10 comes out./
>> It will be fixed in the next rc with the following commit:
>>
> https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
> <https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9>
>
>
> i want to report that the combination i.MX28 + imx-bootlets (requires
> CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also
> affected before and fixed by this patch.
>
> Btw are Arnd and Olof informed about this urgent patch?

@Shawn

Okay, 4.10-rc7 is out but this fix isn't applied :-(

Is there a chance for 4.10 or do we have to wait for 4.10.1?

Maybe the subject of the patch is to harmless.

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Stefan Wahren]

Am 01.02.2017 um 14:30 schrieb Stefan Wahren:
> Hi,
>
>> On Mon, Jan 30, 2017 at 4:54 PM, Russell King - ARM Linux
>> <linux@xxxxxxxxxxxxxxx> wrote:
>>
>>> /We need this regression fixed before 4.10 comes out so systems don't/
>>> /break when 4.10 comes out./
>> It will be fixed in the next rc with the following commit:
>>
> https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
> <https://git.kernel.org/cgit/linux/kernel/git/shawnguo/linux.git/commit/?h=imx/fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9>
>
>
> i want to report that the combination i.MX28 + imx-bootlets (requires
> CONFIG_ARM_APPENDED_DTB since bootlets can't handle DTBs) was also
> affected before and fixed by this patch.
>
> Btw are Arnd and Olof informed about this urgent patch?

@Shawn

Okay, 4.10-rc7 is out but this fix isn't applied :-(

Is there a chance for 4.10 or do we have to wait for 4.10.1?

Maybe the subject of the patch is to harmless.

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Hi Stefan,

On Mon, Feb 6, 2017 at 8:49 AM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> wrote:

>> Btw are Arnd and Olof informed about this urgent patch?
>
> @Shawn
>
> Okay, 4.10-rc7 is out but this fix isn't applied :-(
>
> Is there a chance for 4.10 or do we have to wait for 4.10.1?
>
> Maybe the subject of the patch is to harmless.

The patch is in the arm-soc 'fixes' branch:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9

Olof/Arnd,

Will this branch be sent to Linus so that it lands at 4.10 final?

Thanks
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Fabio Estevam]

Hi Stefan,

On Mon, Feb 6, 2017 at 8:49 AM, Stefan Wahren <stefan.wahren@i2se.com> wrote:

>> Btw are Arnd and Olof informed about this urgent patch?
>
> @Shawn
>
> Okay, 4.10-rc7 is out but this fix isn't applied :-(
>
> Is there a chance for 4.10 or do we have to wait for 4.10.1?
>
> Maybe the subject of the patch is to harmless.

The patch is in the arm-soc 'fixes' branch:
https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9

Olof/Arnd,

Will this branch be sent to Linus so that it lands at 4.10 final?

Thanks

Re: [PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Arnd Bergmann]

On Monday, February 6, 2017 9:05:26 AM CET Fabio Estevam wrote:
> > Maybe the subject of the patch is to harmless.
> 
> The patch is in the arm-soc 'fixes' branch:
> https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
> 
> Olof/Arnd,
> 
> Will this branch be sent to Linus so that it lands at 4.10 final?
> 

Yes, that is the plan.

	Arnd

[PATCH v2] ARM: dts: imx: Pass 'chosen' and 'memory' nodes

[Arnd Bergmann]

On Monday, February 6, 2017 9:05:26 AM CET Fabio Estevam wrote:
> > Maybe the subject of the patch is to harmless.
> 
> The patch is in the arm-soc 'fixes' branch:
> https://git.kernel.org/cgit/linux/kernel/git/arm/arm-soc.git/commit/?h=fixes&id=a971c5545c3d45a1e33fda6e57913bb75aaa20c9
> 
> Olof/Arnd,
> 
> Will this branch be sent to Linus so that it lands at 4.10 final?
> 

Yes, that is the plan.

	Arnd