2020-03-04 07:41:33

by Johan Jonker

[permalink] [raw]
Subject: [PATCH 2/3] ARM: dts: rockchip: add missing @0 to memory nodenames

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 <[email protected]>
---
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(-)

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";
};
--
2.11.0


2020-03-05 21:32:52

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: rockchip: add missing @0 to memory nodenames

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


2020-03-05 22:23:06

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: rockchip: add missing @0 to memory nodenames

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
>
>

2020-03-06 00:00:49

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: rockchip: add missing @0 to memory nodenames

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
> >
> >
>




2020-03-13 09:48:41

by Johan Jonker

[permalink] [raw]
Subject: Re: [PATCH 2/3] ARM: dts: rockchip: add missing @0 to memory nodenames

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
>>>
>>>
>>
>
>
>
>