| Message ID | 20190211110919.10388-1-thierry.reding@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | [v2] ARM: tegra: Restore DT ABI on Tegra124 Chromebooks | expand |
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>
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
On Mon, Feb 18, 2019 at 4:14 PM Thierry Reding <thierry.reding@gmail.com> wrote: > > 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> > > 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. Applied to the fixes branch, thanks! Arnd
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";