[2/3] ARM: dts: rockchip: add missing @0 to memory nodenames
diff mbox series

Message ID 20200304074051.8742-2-jbx6244@gmail.com
State New
Headers show
Series
  • [1/3] ARM: dts: rockchip: add missing model properties
Related show

Commit Message

Johan Jonker March 4, 2020, 7:40 a.m. UTC
A test with the command below gives for example this error:

arch/arm/boot/dts/rk3288-tinker.dt.yaml: /: memory:
False schema does not allow
{'device_type': ['memory'], 'reg': [[0, 0, 0, 2147483648]]}

The memory nodes all have a reg property that requires '@' in
the nodename. Fix this error by adding the missing '@0' to
the involved memory nodenames.

make ARCH=arm dtbs_check
DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
schemas/root-node.yaml

Signed-off-by: Johan Jonker <jbx6244@gmail.com>
---
 arch/arm/boot/dts/rk3288-phycore-som.dtsi | 2 +-
 arch/arm/boot/dts/rk3288-tinker.dtsi      | 2 +-
 arch/arm/boot/dts/rk3288-veyron.dtsi      | 2 +-
 arch/arm/boot/dts/rk3288-vyasa.dts        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Comments

Heiko Stübner March 5, 2020, 9:31 p.m. UTC | #1
Hi Johan,

Am Mittwoch, 4. März 2020, 08:40:50 CET schrieb Johan Jonker:
> A test with the command below gives for example this error:
> 
> arch/arm/boot/dts/rk3288-tinker.dt.yaml: /: memory:
> False schema does not allow
> {'device_type': ['memory'], 'reg': [[0, 0, 0, 2147483648]]}
> 
> The memory nodes all have a reg property that requires '@' in
> the nodename. Fix this error by adding the missing '@0' to
> the involved memory nodenames.
> 
> make ARCH=arm dtbs_check
> DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
> schemas/root-node.yaml

changes to memory nodes you sadly cannot do in such an automated fashion.
If you read the comment in rk3288-veyron.dtsi you'll see that a previous
similar iteration broke all of those machines as their coreboot doesn't
copy with memory@0 and would insert another memory node without @0

In the past iteration the consensus then was that memory without @0
is also ok (as it isn't changeable anyway).

As I don't really want to repeat that, I'd like actual hardware tests
before touching memory nodes.

Heiko
Johan Jonker March 5, 2020, 10:21 p.m. UTC | #2
Hi Heiko,

Goal was to reduce the error output of existing code a little bit,
so that we can use it for the review of new patches.
Some questions:
As I don't have the hardware, where else is coreboot used?
Is this a rk3288-veyron.dtsi problem only?
ie. Is it a option to produce a patch serie v2 without veyron?
Can someone help testing?

Johan

On 3/5/20 10:31 PM, Heiko Stuebner wrote:
> Hi Johan,
>
> Am Mittwoch, 4. März 2020, 08:40:50 CET schrieb Johan Jonker:
>> A test with the command below gives for example this error:
>>
>> arch/arm/boot/dts/rk3288-tinker.dt.yaml: /: memory:
>> False schema does not allow
>> {'device_type': ['memory'], 'reg': [[0, 0, 0, 2147483648]]}
>>
>> The memory nodes all have a reg property that requires '@' in
>> the nodename. Fix this error by adding the missing '@0' to
>> the involved memory nodenames.
>>
>> make ARCH=arm dtbs_check
>> DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
>> schemas/root-node.yaml
>
> changes to memory nodes you sadly cannot do in such an automated fashion.
> If you read the comment in rk3288-veyron.dtsi you'll see that a previous
> similar iteration broke all of those machines as their coreboot doesn't
> copy with memory@0 and would insert another memory node without @0
>
> In the past iteration the consensus then was that memory without @0
> is also ok (as it isn't changeable anyway).
>

> As I don't really want to repeat that, I'd like actual hardware tests
> before touching memory nodes.

Any suggestion/feedback rapport welcome.

>
> Heiko
>
>
Heiko Stübner March 5, 2020, 11:58 p.m. UTC | #3
Hi Johan, Rob,

Am Donnerstag, 5. März 2020, 23:21:52 CET schrieb Johan Jonker:
> Goal was to reduce the error output of existing code a little bit,
> so that we can use it for the review of new patches.
> Some questions:
> As I don't have the hardware, where else is coreboot used?
> Is this a rk3288-veyron.dtsi problem only?
> ie. Is it a option to produce a patch serie v2 without veyron?
> Can someone help testing?

I believe that is more question for @Rob :

In the past we said that it would be ok to have "memory" nodes without
address, so "memory {}" instead of "memory@0 {}", simply because
bootloaders mess up sometimes.

Question now would be how to make the yaml bindings happy.

Thanks
Heiko


> 
> Johan
> 
> On 3/5/20 10:31 PM, Heiko Stuebner wrote:
> > Hi Johan,
> >
> > Am Mittwoch, 4. März 2020, 08:40:50 CET schrieb Johan Jonker:
> >> A test with the command below gives for example this error:
> >>
> >> arch/arm/boot/dts/rk3288-tinker.dt.yaml: /: memory:
> >> False schema does not allow
> >> {'device_type': ['memory'], 'reg': [[0, 0, 0, 2147483648]]}
> >>
> >> The memory nodes all have a reg property that requires '@' in
> >> the nodename. Fix this error by adding the missing '@0' to
> >> the involved memory nodenames.
> >>
> >> make ARCH=arm dtbs_check
> >> DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
> >> schemas/root-node.yaml
> >
> > changes to memory nodes you sadly cannot do in such an automated fashion.
> > If you read the comment in rk3288-veyron.dtsi you'll see that a previous
> > similar iteration broke all of those machines as their coreboot doesn't
> > copy with memory@0 and would insert another memory node without @0
> >
> > In the past iteration the consensus then was that memory without @0
> > is also ok (as it isn't changeable anyway).
> >
> 
> > As I don't really want to repeat that, I'd like actual hardware tests
> > before touching memory nodes.
> 
> Any suggestion/feedback rapport welcome.
> 
> >
> > Heiko
> >
> >
>
Johan Jonker March 13, 2020, 9:47 a.m. UTC | #4
Hi Heiko, Rob,

From https://coreboot.org/status/board-status.html

The only supported boards listed are:

Veyron Rockchip RK3288 boards
Veyron Mickey Rockchip RK3288 board
Veyron Rialto Rockchip RK3288 board
Gru Rockchip RK3399 reference board

Fixed with:
ARM: dts: rockchip: Remove @0 from the veyron memory node
https://patchwork.kernel.org/patch/10688081/

The problem is rk3288-veyron only I think.
Else fix coreboot to comply with DT rules, not the other way around.
Will make v2.

Can robh give advice here?

Thanks

On 3/6/20 12:58 AM, Heiko Stuebner wrote:
> Hi Johan, Rob,
> 
> Am Donnerstag, 5. März 2020, 23:21:52 CET schrieb Johan Jonker:
>> Goal was to reduce the error output of existing code a little bit,
>> so that we can use it for the review of new patches.
>> Some questions:
>> As I don't have the hardware, where else is coreboot used?
>> Is this a rk3288-veyron.dtsi problem only?
>> ie. Is it a option to produce a patch serie v2 without veyron?
>> Can someone help testing?
> 
> I believe that is more question for @Rob :
> 
> In the past we said that it would be ok to have "memory" nodes without
> address, so "memory {}" instead of "memory@0 {}", simply because
> bootloaders mess up sometimes.
> 
> Question now would be how to make the yaml bindings happy.
> 
> Thanks
> Heiko
> 
> 
>>
>> Johan
>>
>> On 3/5/20 10:31 PM, Heiko Stuebner wrote:
>>> Hi Johan,
>>>
>>> Am Mittwoch, 4. März 2020, 08:40:50 CET schrieb Johan Jonker:
>>>> A test with the command below gives for example this error:
>>>>
>>>> arch/arm/boot/dts/rk3288-tinker.dt.yaml: /: memory:
>>>> False schema does not allow
>>>> {'device_type': ['memory'], 'reg': [[0, 0, 0, 2147483648]]}
>>>>
>>>> The memory nodes all have a reg property that requires '@' in
>>>> the nodename. Fix this error by adding the missing '@0' to
>>>> the involved memory nodenames.
>>>>
>>>> make ARCH=arm dtbs_check
>>>> DT_SCHEMA_FILES=~/.local/lib/python3.5/site-packages/dtschema/
>>>> schemas/root-node.yaml
>>>
>>> changes to memory nodes you sadly cannot do in such an automated fashion.
>>> If you read the comment in rk3288-veyron.dtsi you'll see that a previous
>>> similar iteration broke all of those machines as their coreboot doesn't
>>> copy with memory@0 and would insert another memory node without @0
>>>
>>> In the past iteration the consensus then was that memory without @0
>>> is also ok (as it isn't changeable anyway).
>>>
>>
>>> As I don't really want to repeat that, I'd like actual hardware tests
>>> before touching memory nodes.
>>
>> Any suggestion/feedback rapport welcome.
>>
>>>
>>> Heiko
>>>
>>>
>>
> 
> 
> 
>

Patch
diff mbox series

diff --git a/arch/arm/boot/dts/rk3288-phycore-som.dtsi b/arch/arm/boot/dts/rk3288-phycore-som.dtsi
index 77a47b9b7..9e76166c3 100644
--- a/arch/arm/boot/dts/rk3288-phycore-som.dtsi
+++ b/arch/arm/boot/dts/rk3288-phycore-som.dtsi
@@ -16,7 +16,7 @@ 
 	 * Set the minimum memory size here and
 	 * let the bootloader set the real size.
 	 */
-	memory {
+	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x8000000>;
 	};
diff --git a/arch/arm/boot/dts/rk3288-tinker.dtsi b/arch/arm/boot/dts/rk3288-tinker.dtsi
index 312582c1b..77ae303b0 100644
--- a/arch/arm/boot/dts/rk3288-tinker.dtsi
+++ b/arch/arm/boot/dts/rk3288-tinker.dtsi
@@ -12,7 +12,7 @@ 
 		stdout-path = "serial2:115200n8";
 	};
 
-	memory {
+	memory@0 {
 		reg = <0x0 0x0 0x0 0x80000000>;
 		device_type = "memory";
 	};
diff --git a/arch/arm/boot/dts/rk3288-veyron.dtsi b/arch/arm/boot/dts/rk3288-veyron.dtsi
index 54a6838d7..c089ce008 100644
--- a/arch/arm/boot/dts/rk3288-veyron.dtsi
+++ b/arch/arm/boot/dts/rk3288-veyron.dtsi
@@ -18,7 +18,7 @@ 
 	 * The default coreboot on veyron devices ignores memory@0 nodes
 	 * and would instead create another memory node.
 	 */
-	memory {
+	memory@0 {
 		device_type = "memory";
 		reg = <0x0 0x0 0x0 0x80000000>;
 	};
diff --git a/arch/arm/boot/dts/rk3288-vyasa.dts b/arch/arm/boot/dts/rk3288-vyasa.dts
index ba06e9f97..889b95e95 100644
--- a/arch/arm/boot/dts/rk3288-vyasa.dts
+++ b/arch/arm/boot/dts/rk3288-vyasa.dts
@@ -14,7 +14,7 @@ 
 		stdout-path = &uart2;
 	};
 
-	memory {
+	memory@0 {
 		reg = <0x0 0x0 0x0 0x80000000>;
 		device_type = "memory";
 	};