[v2] ARM: tegra: Restore DT ABI on Tegra124 Chromebooks

Message ID 20190211110919.10388-1-thierry.reding@gmail.com
State New
Headers show
Series
  • [v2] ARM: tegra: Restore DT ABI on Tegra124 Chromebooks
Related show

Commit Message

Thierry Reding Feb. 11, 2019, 11:09 a.m.
From: Thierry Reding <treding@nvidia.com>

Commit 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings
for /memory") inadventently broke device tree ABI by adding a unit-
address to the "/memory" node because the device tree compiler flagged
the missing unit-address as a warning.

Tegra124 Chromebooks (a.k.a. Nyan) use a bootloader that relies on the
full name of the memory node in device tree being exactly "/memory". It
can be argued whether this was a good decision or not, and some other
bootloaders (such as U-Boot) do accept a unit-address in the name of the
node, but the device tree is an ABI and we can't break existing setups
just because the device tree compiler considers it bad practice to omit
the unit-address nowadays.

This partially reverts the offending commit and restores device tree ABI
compatibility.

Fixes: 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings for /memory")
Reported-by: Tristan Bastian <tristan-c.bastian@gmx.de>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- remove memory@80000000 node pulled in from tegra124.dtsi
- add device_type = "memory" property

 arch/arm/boot/dts/tegra124-nyan.dtsi | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Tristan Bastian Feb. 11, 2019, 8:44 p.m. | #1
Thierry Reding – Mon, 11. February 2019 12:09
> From: Thierry Reding <treding@nvidia.com>
> 
> Commit 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings
> for /memory") inadventently broke device tree ABI by adding a unit-
> address to the "/memory" node because the device tree compiler flagged
> the missing unit-address as a warning.
> 
> Tegra124 Chromebooks (a.k.a. Nyan) use a bootloader that relies on the
> full name of the memory node in device tree being exactly "/memory". It
> can be argued whether this was a good decision or not, and some other
> bootloaders (such as U-Boot) do accept a unit-address in the name of the
> node, but the device tree is an ABI and we can't break existing setups
> just because the device tree compiler considers it bad practice to omit
> the unit-address nowadays.
> 
> This partially reverts the offending commit and restores device tree ABI
> compatibility.
> 
> Fixes: 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings for
> /memory")
> Reported-by: Tristan Bastian <tristan-c.bastian@gmx.de>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - remove memory@80000000 node pulled in from tegra124.dtsi
> - add device_type = "memory" property
> 
> arch/arm/boot/dts/tegra124-nyan.dtsi | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi
> b/arch/arm/boot/dts/tegra124-nyan.dtsi
> index d5f11d6d987e..bc85b6a166c7 100644
> --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> @@ -13,10 +13,25 @@
> stdout-path = "serial0:115200n8";
> };
> 
> - memory@80000000 {
> + /*
> + * Note that recent version of the device tree compiler (starting with
> + * version 1.4.2) warn about this node containing a reg property, but
> + * missing a unit-address. However, the bootloader on these Chromebook
> + * devices relies on the full name of this node to be exactly /memory.
> + * Adding the unit-address causes the bootloader to create a /memory
> + * node and write the memory bank configuration to that node, which in
> + * turn leads the kernel to believe that the device has 2 GiB of
> + * memory instead of the amount detected by the bootloader.
> + *
> + * The name of this node is effectively ABI and must not be changed.
> + */
> + memory {
> + device_type = "memory";
> reg = <0x0 0x80000000 0x0 0x80000000>;
> };
> 
> + /delete-node/ memory@80000000;
> +
> host1x@50000000 {
> hdmi@54280000 {
> status = "okay";
> -- 
> 2.19.1

Thanks Thierry

Tristan

Tested-by: Tristan Bastian <tristan-c.bastian@gmx.de>
Thierry Reding Feb. 18, 2019, 3:13 p.m. | #2
On Mon, Feb 11, 2019 at 08:44:52PM +0000, Tristan Bastian wrote:
> Thierry Reding – Mon, 11. February 2019 12:09
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Commit 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings
> > for /memory") inadventently broke device tree ABI by adding a unit-
> > address to the "/memory" node because the device tree compiler flagged
> > the missing unit-address as a warning.
> > 
> > Tegra124 Chromebooks (a.k.a. Nyan) use a bootloader that relies on the
> > full name of the memory node in device tree being exactly "/memory". It
> > can be argued whether this was a good decision or not, and some other
> > bootloaders (such as U-Boot) do accept a unit-address in the name of the
> > node, but the device tree is an ABI and we can't break existing setups
> > just because the device tree compiler considers it bad practice to omit
> > the unit-address nowadays.
> > 
> > This partially reverts the offending commit and restores device tree ABI
> > compatibility.
> > 
> > Fixes: 482997699ef0 ("ARM: tegra: Fix unit_address_vs_reg DTC warnings for
> > /memory")
> > Reported-by: Tristan Bastian <tristan-c.bastian@gmx.de>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v2:
> > - remove memory@80000000 node pulled in from tegra124.dtsi
> > - add device_type = "memory" property
> > 
> > arch/arm/boot/dts/tegra124-nyan.dtsi | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > index d5f11d6d987e..bc85b6a166c7 100644
> > --- a/arch/arm/boot/dts/tegra124-nyan.dtsi
> > +++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
> > @@ -13,10 +13,25 @@
> > stdout-path = "serial0:115200n8";
> > };
> > 
> > - memory@80000000 {
> > + /*
> > + * Note that recent version of the device tree compiler (starting with
> > + * version 1.4.2) warn about this node containing a reg property, but
> > + * missing a unit-address. However, the bootloader on these Chromebook
> > + * devices relies on the full name of this node to be exactly /memory.
> > + * Adding the unit-address causes the bootloader to create a /memory
> > + * node and write the memory bank configuration to that node, which in
> > + * turn leads the kernel to believe that the device has 2 GiB of
> > + * memory instead of the amount detected by the bootloader.
> > + *
> > + * The name of this node is effectively ABI and must not be changed.
> > + */
> > + memory {
> > + device_type = "memory";
> > reg = <0x0 0x80000000 0x0 0x80000000>;
> > };
> > 
> > + /delete-node/ memory@80000000;
> > +
> > host1x@50000000 {
> > hdmi@54280000 {
> > status = "okay";
> > -- 
> > 2.19.1
> 
> Thanks Thierry
> 
> Tristan
> 
> Tested-by: Tristan Bastian <tristan-c.bastian@gmx.de>

Hi ARM SoC maintainers,

I had almost forgotten about this. Ideally I'd like this to go into v5.0
since it fixes a regression introduced in v4.19, so we'd keep the number
of versions that it needs to get backported to fairly low.

The patchwork link is here:

	http://patchwork.ozlabs.org/patch/1039743/

I don't have any other fixes for v5.0, so I don't think sending a pull
request would help you very much, but I can prepare one if you prefer.

Thanks,
Thierry

Patch

diff --git a/arch/arm/boot/dts/tegra124-nyan.dtsi b/arch/arm/boot/dts/tegra124-nyan.dtsi
index d5f11d6d987e..bc85b6a166c7 100644
--- a/arch/arm/boot/dts/tegra124-nyan.dtsi
+++ b/arch/arm/boot/dts/tegra124-nyan.dtsi
@@ -13,10 +13,25 @@ 
 		stdout-path = "serial0:115200n8";
 	};
 
-	memory@80000000 {
+	/*
+	 * Note that recent version of the device tree compiler (starting with
+	 * version 1.4.2) warn about this node containing a reg property, but
+	 * missing a unit-address. However, the bootloader on these Chromebook
+	 * devices relies on the full name of this node to be exactly /memory.
+	 * Adding the unit-address causes the bootloader to create a /memory
+	 * node and write the memory bank configuration to that node, which in
+	 * turn leads the kernel to believe that the device has 2 GiB of
+	 * memory instead of the amount detected by the bootloader.
+	 *
+	 * The name of this node is effectively ABI and must not be changed.
+	 */
+	memory {
+		device_type = "memory";
 		reg = <0x0 0x80000000 0x0 0x80000000>;
 	};
 
+	/delete-node/ memory@80000000;
+
 	host1x@50000000 {
 		hdmi@54280000 {
 			status = "okay";