On 04/15/2016 10:30 AM, Tom Rini wrote:

On Fri, Apr 15, 2016 at 05:23:54PM +0200, Andreas Färber wrote:

...

Am 15.04.2016 um 12:59 schrieb Heiko Schocher:

...

Fix following warnings for all mips based boards: mips: + pic32mzdask +Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name +Warning (unit_address_vs_reg): Node /cpus/cpu@0 has a unit name, but no reg property

Note that I am quite out-of-the-loop on these warning. I wrote the dtc patch that triggers them years ago, but it's only recently been applied due to Rob's efforts. I'm at most tangentially aware of the discussions surrounding applying it now.

...

...

diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi

...

...

cpus {

• cpu@0 {
  

• cpu {
  compatible = "mips,mips14kc";
  

Surely the correct fix is to add a reg property? (Of course, this depends on the binding definition; for ARM my assertion would certainly be true). If not, what does MIPS do about SMP? Even if you write, say, 4 nodes with name "cpu" they'll all become the same single node in the DTB.

...

...

diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi

...

...

• memory {

• memory@0 {

I have just been told on linux-rockchip mailing list that such a change should not be done as /memory is being special-cased in dtc warnings for the benefit of U-Boot. Supposedly U-Boot cannot handle updating memory size on /memory@0.

If that is untrue, please someone object on the Linux mailing lists.

Uh, what? From dtc:

I vaguely recall seeing discussion that /memory *would* be special-cased, but as you point out obviously isn't yet. I doubt it's anything to do with U-Boot itself, but rather the more general problem that if /memory@NNNN changes name based on what RAM is present, it's not possible for any bootloader to update it in a sane way (what node name do you search for to edit), or any OS to read it in a sane way (what node name do you search for to find out where memory is). As such, a special case is logically required.

commit c9d9121683b35281239305e15adddfff2b462cf9 Author: Stephen Warren swarren@nvidia.com Date: Fri Feb 19 15:59:29 2016 +1100

 Warn on node name unit-address presence/absence mismatch

 ePAPR 1.1 section 2.2.1.1 "Node Name Requirements" specifies that any
 node that has a reg property must include a unit address in its name
 with value matching the first entry in its reg property. Conversely, if
 a node does not have a reg property, the node name must not include a
 unit address. Also allow ranges property as it is deemed valid, but ePAPR
 is not clear about it.

 Implement a check for this. The code doesn't validate the format of the
 unit address; ePAPR implies this may vary from (containing bus) binding
 to binding, so doing so would be much more complex.

 Signed-off-by: Stephen Warren <swarren@nvidia.com>
 [robh: also allow non-empty ranges]
 Signed-off-by: Rob Herring <robh@kernel.org>
 [moved new test in check_table]
 Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

That's where that warning comes from.

But some real quick grepping makes it seem like the cpu@0 error is also a sign of a bigger problem in the dts file

But I would suggest that whomever is making special case warnings for U-Boot bring that up on the U-Boot mailing list or otherwise ask? We're not a black box...

Equally though, anyone working on DT in U-Boot should be hanging out on the device tree mailing list, and getting actively involved in discussions, rather than hiding here.

On 04/15/2016 11:11 AM, Tom Rini wrote:

On Fri, Apr 15, 2016 at 10:56:40AM -0600, Stephen Warren wrote:

...

On 04/15/2016 10:30 AM, Tom Rini wrote:

...

On Fri, Apr 15, 2016 at 05:23:54PM +0200, Andreas Färber wrote:

...

Am 15.04.2016 um 12:59 schrieb Heiko Schocher:

...

Fix following warnings for all mips based boards: mips: + pic32mzdask +Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name +Warning (unit_address_vs_reg): Node /cpus/cpu@0 has a unit name, but no reg property

Note that I am quite out-of-the-loop on these warning. I wrote the dtc patch that triggers them years ago, but it's only recently been applied due to Rob's efforts. I'm at most tangentially aware of the discussions surrounding applying it now.

...

...

...

diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi

...

...

...

cpus {

• cpu@0 {
  

• cpu {
  compatible = "mips,mips14kc";
  

Surely the correct fix is to add a reg property? (Of course, this depends on the binding definition; for ARM my assertion would certainly be true). If not, what does MIPS do about SMP? Even if you write, say, 4 nodes with name "cpu" they'll all become the same single node in the DTB.

So the likely answer here is that the dtsi is wrong and needs to be fixed rather than just dropping @0.

...

...

...

...

diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi

...

...

...

• memory {

• memory@0 {

I have just been told on linux-rockchip mailing list that such a change should not be done as /memory is being special-cased in dtc warnings for the benefit of U-Boot. Supposedly U-Boot cannot handle updating memory size on /memory@0.

If that is untrue, please someone object on the Linux mailing lists.

Uh, what? From dtc:

I vaguely recall seeing discussion that /memory *would* be special-cased, but as you point out obviously isn't yet. I doubt it's anything to do with U-Boot itself, but rather the more general problem that if /memory@NNNN changes name based on what RAM is present, it's not possible for any bootloader to update it in a sane way (what node name do you search for to edit), or any OS to read it in a sane way (what node name do you search for to find out where memory is). As such, a special case is logically required.

Right, makes sense. But it'll also have to handle that today (nearly) everyone is /memory@NNNN.

Nodes without a unit address are far more common currently, on ARM at least:

u-boot$ grep -HrnI 'memory@' arch/arm/dts|wc -l 3 u-boot$ grep -HrnI 'memory {' arch/arm/dts|wc -l 86

kernel.git$ grep -HrnI 'memory {' arch/arm/boot/dts|wc -l 528 kernel.git$ grep -HrnI memory@ arch/arm/boot/dts|wc -l 27 kernel.git$ cat arch/arm/boot/dts/skeleton.dtsi ... memory { device_type = "memory"; reg = <0 0>; };

(That last one is the base DT file that is typically included in all board files, and so should in theory set the correct example).

U-Boot's /memory updating code doesn't handle a unit address; see common/fdt_support.c fdt_fixup_memory_banks().

Linux's /memory parsing code doesn't handle a unit address, except for one PPC32 special case; see drivers/of/fdt.c early_init_dt_scan_memory().

(Which makes me wonder how any of the systems which do have /memory@nnn rather than plain /memory actual work...)

Hello Stephen,

Am 15.04.2016 um 18:56 schrieb Stephen Warren:

On 04/15/2016 10:30 AM, Tom Rini wrote:

...

On Fri, Apr 15, 2016 at 05:23:54PM +0200, Andreas Färber wrote:

...

Am 15.04.2016 um 12:59 schrieb Heiko Schocher:

...

Fix following warnings for all mips based boards: mips: + pic32mzdask +Warning (unit_address_vs_reg): Node /memory has a reg or ranges property, but no unit name +Warning (unit_address_vs_reg): Node /cpus/cpu@0 has a unit name, but no reg property

Note that I am quite out-of-the-loop on these warning. I wrote the dtc patch that triggers them years ago, but it's only recently been applied due to Rob's efforts. I'm at most tangentially aware of the discussions surrounding applying it now.

...

...

...

diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi

...

...

...

  cpus {

•    cpu@0 {
  

•    cpu {
          compatible = "mips,mips14kc";
  

Surely the correct fix is to add a reg property? (Of course, this depends on the binding definition; for ARM my assertion would certainly be true). If not, what does MIPS do about SMP? Even if you write, say, 4 nodes with name "cpu" they'll all become the same single node in the DTB.

Yes, everywhere I removed the @address I hope to get a response with the correct reg value. It is not possible for me to look through all the RMs ... nor test the new DTS on real boards ... and nobody has volunteered to fix the warnings, so I posted this "fixup" patch ...

So, lets discuss the correct fix ...

Hmm... is there a valid reg property for cpu ? Or can we add a dummy one?

Or may we do a "cpu@0" -> "cpu0"?

...

...

...

diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi

...

...

...

• memory {

• memory@0 {

I have just been told on linux-rockchip mailing list that such a change should not be done as /memory is being special-cased in dtc warnings for the benefit of U-Boot. Supposedly U-Boot cannot handle updating memory size on /memory@0.

If that is untrue, please someone object on the Linux mailing lists.

Uh, what? From dtc:

I vaguely recall seeing discussion that /memory *would* be special-cased, but as you point out obviously isn't yet. I doubt it's anything to do with U-Boot itself, but rather the more general problem that if /memory@NNNN changes name based on what RAM is present, it's not possible for any bootloader to update it in a sane way (what node name do you search for to edit), or any OS to read it in a sane way (what node name do you search for to find out where memory is). As such, a special case is logically required.

Hmm.. dummy question ... why is:

memory: memory@address {

not possible?

...

commit c9d9121683b35281239305e15adddfff2b462cf9 Author: Stephen Warren swarren@nvidia.com Date: Fri Feb 19 15:59:29 2016 +1100

 Warn on node name unit-address presence/absence mismatch

 ePAPR 1.1 section 2.2.1.1 "Node Name Requirements" specifies that any
 node that has a reg property must include a unit address in its name
 with value matching the first entry in its reg property. Conversely, if
 a node does not have a reg property, the node name must not include a
 unit address. Also allow ranges property as it is deemed valid, but ePAPR
 is not clear about it.

 Implement a check for this. The code doesn't validate the format of the
 unit address; ePAPR implies this may vary from (containing bus) binding
 to binding, so doing so would be much more complex.

 Signed-off-by: Stephen Warren <swarren@nvidia.com>
 [robh: also allow non-empty ranges]
 Signed-off-by: Rob Herring <robh@kernel.org>
 [moved new test in check_table]
 Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

That's where that warning comes from.

But some real quick grepping makes it seem like the cpu@0 error is also a sign of a bigger problem in the dts file

But I would suggest that whomever is making special case warnings for U-Boot bring that up on the U-Boot mailing list or otherwise ask? We're not a black box...

Equally though, anyone working on DT in U-Boot should be hanging out on the device tree mailing list, and getting actively involved in discussions, rather than hiding here.

Hmm... hidding is maybe the wrong definition ... instead I would call it "stack overflow" ... I try to follow device tree mailing list too, but also U-Boot, lkml, linux-mtd, ... and beside of this, I have also to work

so, yes I miss some(a lot currently) threads ... so, sorry for poping this thread up, but I want to get rid of this warnings ...

bye, Heiko