[U-Boot] [PATCH] mips: fix DTC unit warnings

[U-Boot] [PATCH] mips: fix DTC unit warnings

[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 at 0 has a unit name, but no reg property

Signed-off-by: Heiko Schocher <hs@denx.de>
---
This warnings pop up with the DTC compiler:
$ /tmp/dtc/dtc -v
Version: DTC 1.4.1-gbeef80b8

This fixes the compile warnings for:
https://travis-ci.org/hsdenx/u-boot/jobs/123254184

see:
https://travis-ci.org/hsdenx/u-boot/jobs/123281033


 arch/mips/dts/pic32mzda.dtsi | 2 +-
 arch/mips/dts/skeleton.dtsi  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
index 8a554f9..791c364 100644
--- a/arch/mips/dts/pic32mzda.dtsi
+++ b/arch/mips/dts/pic32mzda.dtsi
@@ -27,7 +27,7 @@
 	};
 
 	cpus {
-		cpu at 0 {
+		cpu {
 			compatible = "mips,mips14kc";
 		};
 	};
diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
index 24ee6c3..643996c 100644
--- a/arch/mips/dts/skeleton.dtsi
+++ b/arch/mips/dts/skeleton.dtsi
@@ -16,7 +16,7 @@
 	aliases {
 	};
 
-	memory {
+	memory at 0 {
 		device_type = "memory";
 		reg = <0 0>;
 	};
-- 
2.5.0

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Purna Chandra Mandal]

On 04/15/2016 04:29 PM, Heiko Schocher wrote:

> 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 at 0 has a unit name, but no reg property
>
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> This warnings pop up with the DTC compiler:
> $ /tmp/dtc/dtc -v
> Version: DTC 1.4.1-gbeef80b8
>
> This fixes the compile warnings for:
> https://travis-ci.org/hsdenx/u-boot/jobs/123254184
>
> see:
> https://travis-ci.org/hsdenx/u-boot/jobs/123281033
>
>
>  arch/mips/dts/pic32mzda.dtsi | 2 +-
>  arch/mips/dts/skeleton.dtsi  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
> index 8a554f9..791c364 100644
> --- a/arch/mips/dts/pic32mzda.dtsi
> +++ b/arch/mips/dts/pic32mzda.dtsi
> @@ -27,7 +27,7 @@
>  	};
>  
>  	cpus {
> -		cpu at 0 {
> +		cpu {
>  			compatible = "mips,mips14kc";
>  		};
>  	};
> diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
> index 24ee6c3..643996c 100644
> --- a/arch/mips/dts/skeleton.dtsi
> +++ b/arch/mips/dts/skeleton.dtsi
> @@ -16,7 +16,7 @@
>  	aliases {
>  	};
>  
> -	memory {
> +	memory at 0 {
>  		device_type = "memory";
>  		reg = <0 0>;
>  	};

Reviewed-by: Purna Chandra Mandal <purna.mandal@microchip.com>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Daniel Schwierzeck]

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 at 0 has a unit name, but no reg property
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---

Acked-by: Daniel Schwierzeck <daniel.schwierzeck@gmail.com>

> This warnings pop up with the DTC compiler:
> $ /tmp/dtc/dtc -v
> Version: DTC 1.4.1-gbeef80b8
> 
> This fixes the compile warnings for:
> https://travis-ci.org/hsdenx/u-boot/jobs/123254184
> 
> see:
> https://travis-ci.org/hsdenx/u-boot/jobs/123281033
> 
> 
>  arch/mips/dts/pic32mzda.dtsi | 2 +-
>  arch/mips/dts/skeleton.dtsi  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
> index 8a554f9..791c364 100644
> --- a/arch/mips/dts/pic32mzda.dtsi
> +++ b/arch/mips/dts/pic32mzda.dtsi
> @@ -27,7 +27,7 @@
>  	};
>  
>  	cpus {
> -		cpu at 0 {
> +		cpu {
>  			compatible = "mips,mips14kc";
>  		};
>  	};
> diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
> index 24ee6c3..643996c 100644
> --- a/arch/mips/dts/skeleton.dtsi
> +++ b/arch/mips/dts/skeleton.dtsi
> @@ -16,7 +16,7 @@
>  	aliases {
>  	};
>  
> -	memory {
> +	memory at 0 {
>  		device_type = "memory";
>  		reg = <0 0>;
>  	};
> 

-- 
- Daniel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160415/6be5aea5/attachment.sig>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Andreas Färber]

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 at 0 has a unit name, but no reg property
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> ---
> This warnings pop up with the DTC compiler:
> $ /tmp/dtc/dtc -v
> Version: DTC 1.4.1-gbeef80b8
> 
> This fixes the compile warnings for:
> https://travis-ci.org/hsdenx/u-boot/jobs/123254184
> 
> see:
> https://travis-ci.org/hsdenx/u-boot/jobs/123281033
> 
> 
>  arch/mips/dts/pic32mzda.dtsi | 2 +-
>  arch/mips/dts/skeleton.dtsi  | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
> index 8a554f9..791c364 100644
> --- a/arch/mips/dts/pic32mzda.dtsi
> +++ b/arch/mips/dts/pic32mzda.dtsi
> @@ -27,7 +27,7 @@
>  	};
>  
>  	cpus {
> -		cpu at 0 {
> +		cpu {
>  			compatible = "mips,mips14kc";
>  		};
>  	};
> diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
> index 24ee6c3..643996c 100644
> --- a/arch/mips/dts/skeleton.dtsi
> +++ b/arch/mips/dts/skeleton.dtsi
> @@ -16,7 +16,7 @@
>  	aliases {
>  	};
>  
> -	memory {
> +	memory at 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 at 0.

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

Regards,
Andreas

>  		device_type = "memory";
>  		reg = <0 0>;
>  	};
> 


-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Tom Rini]

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 at 0 has a unit name, but no reg property
> > 
> > Signed-off-by: Heiko Schocher <hs@denx.de>
> > ---
> > This warnings pop up with the DTC compiler:
> > $ /tmp/dtc/dtc -v
> > Version: DTC 1.4.1-gbeef80b8
> > 
> > This fixes the compile warnings for:
> > https://travis-ci.org/hsdenx/u-boot/jobs/123254184
> > 
> > see:
> > https://travis-ci.org/hsdenx/u-boot/jobs/123281033
> > 
> > 
> >  arch/mips/dts/pic32mzda.dtsi | 2 +-
> >  arch/mips/dts/skeleton.dtsi  | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
> > index 8a554f9..791c364 100644
> > --- a/arch/mips/dts/pic32mzda.dtsi
> > +++ b/arch/mips/dts/pic32mzda.dtsi
> > @@ -27,7 +27,7 @@
> >  	};
> >  
> >  	cpus {
> > -		cpu at 0 {
> > +		cpu {
> >  			compatible = "mips,mips14kc";
> >  		};
> >  	};
> > diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
> > index 24ee6c3..643996c 100644
> > --- a/arch/mips/dts/skeleton.dtsi
> > +++ b/arch/mips/dts/skeleton.dtsi
> > @@ -16,7 +16,7 @@
> >  	aliases {
> >  	};
> >  
> > -	memory {
> > +	memory at 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 at 0.
> 
> If that is untrue, please someone object on the Linux mailing lists.

Uh, what?  From dtc:

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

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160415/3c746d5b/attachment.sig>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[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 at 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 at 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 at 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 at 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 at 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 at 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.

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Tom Rini]

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 at 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 at 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 at 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 at 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 at 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 at NNNN.

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

True.  And devicetree-spec is very quiet and devicetree is very very
loud, and somewhere along the line for me at least vger kicked me off,
sigh.  Fixing that at least.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160415/1f6114ff/attachment.sig>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Stephen Warren]

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 at 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 at 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 at 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 at 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 at 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 at 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 at nnn 
rather than plain /memory actual work...)

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Tom Rini]

On Fri, Apr 15, 2016 at 11:37:40AM -0600, Stephen Warren wrote:
> 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 at 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 at 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 at 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 at 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 at 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 at NNNN.
> 
> Nodes without a unit address are far more common currently, on ARM at least:

Brain fart, you are correct.  I git grepped both memory and cpu and then
got 'em backwards in my reply.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160415/b67551b7/attachment.sig>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Heiko Stübner]

Am Freitag, 15. April 2016, 12:30:58 schrieb Tom Rini:
> 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 at 0 has a unit name, but no reg property
> > > 
> > > Signed-off-by: Heiko Schocher <hs@denx.de>
> > > ---
> > > This warnings pop up with the DTC compiler:
> > > $ /tmp/dtc/dtc -v
> > > Version: DTC 1.4.1-gbeef80b8
> > > 
> > > This fixes the compile warnings for:
> > > https://travis-ci.org/hsdenx/u-boot/jobs/123254184
> > > 
> > > see:
> > > https://travis-ci.org/hsdenx/u-boot/jobs/123281033
> > > 
> > >  arch/mips/dts/pic32mzda.dtsi | 2 +-
> > >  arch/mips/dts/skeleton.dtsi  | 2 +-
> > >  2 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
> > > index 8a554f9..791c364 100644
> > > --- a/arch/mips/dts/pic32mzda.dtsi
> > > +++ b/arch/mips/dts/pic32mzda.dtsi
> > > @@ -27,7 +27,7 @@
> > > 
> > >  	};
> > >  	
> > >  	cpus {
> > > 
> > > -		cpu at 0 {
> > > +		cpu {
> > > 
> > >  			compatible = "mips,mips14kc";
> > >  		
> > >  		};
> > >  	
> > >  	};
> > > 
> > > diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
> > > index 24ee6c3..643996c 100644
> > > --- a/arch/mips/dts/skeleton.dtsi
> > > +++ b/arch/mips/dts/skeleton.dtsi
> > > @@ -16,7 +16,7 @@
> > > 
> > >  	aliases {
> > >  	};
> > > 
> > > -	memory {
> > > +	memory at 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 at 0.
> > 
> > If that is untrue, please someone object on the Linux mailing lists.
> 
> Uh, what?  From dtc:
> 
> 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.

and that is supposed to get a special case for memory. On the kernel variant 
of dtc the change got deactivated again and will only trigger the warnings 
with a special command currently.

Also in another thread [0] on the arm-kernel-list Rob Herring explicitly 
stated:

>> This has come up before and been beaten to death. Bottom line is plain
>> "memory" is allowed, and I plan to add that exception to dtc."

And as stated there as well, memory at 0 is not correct in all cases, as it would 
need to be memory at baseaddress, so for example on Rockchip
- memory at 0 for the rk3288
- memory at 60000000 for rk3188 and other A9s

So it's definitly not suitable for a general node in that skeleton,dtsi


Heiko

[0] http://www.spinics.net/lists/arm-kernel/msg494038.html

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Heiko Schocher]

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 at 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 at 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 at 0" -> "cpu0"?

>>>> diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
>
>>>> -    memory {
>>>> +    memory at 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 at 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 at 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 at 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 at 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
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Purna Chandra Mandal]

On 04/15/2016 08:53 PM, 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 at 0 has a unit name, but no reg property
>>
>> Signed-off-by: Heiko Schocher <hs@denx.de>
>> ---
>> This warnings pop up with the DTC compiler:
>> $ /tmp/dtc/dtc -v
>> Version: DTC 1.4.1-gbeef80b8
>>
>> This fixes the compile warnings for:
>> https://travis-ci.org/hsdenx/u-boot/jobs/123254184
>>
>> see:
>> https://travis-ci.org/hsdenx/u-boot/jobs/123281033
>>
>>
>>  arch/mips/dts/pic32mzda.dtsi | 2 +-
>>  arch/mips/dts/skeleton.dtsi  | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/mips/dts/pic32mzda.dtsi b/arch/mips/dts/pic32mzda.dtsi
>> index 8a554f9..791c364 100644
>> --- a/arch/mips/dts/pic32mzda.dtsi
>> +++ b/arch/mips/dts/pic32mzda.dtsi
>> @@ -27,7 +27,7 @@
>>  	};
>>  
>>  	cpus {
>> -		cpu at 0 {
>> +		cpu {
>>  			compatible = "mips,mips14kc";
>>  		};
>>  	};
>> diff --git a/arch/mips/dts/skeleton.dtsi b/arch/mips/dts/skeleton.dtsi
>> index 24ee6c3..643996c 100644
>> --- a/arch/mips/dts/skeleton.dtsi
>> +++ b/arch/mips/dts/skeleton.dtsi
>> @@ -16,7 +16,7 @@
>>  	aliases {
>>  	};
>>  
>> -	memory {
>> +	memory at 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 at 0.
>
> If that is untrue, please someone object on the Linux mailing lists.

Interestingly MIPS arch does not update size on /memory node during bootm.
Either we can fix that in arch/mips/ or all boards have to implement board
specific ft_board_setup() to address this. Shortly you'll see a patch.

In PIC32 boards(may be in some other boards as well) embedded dts is used
for U-Boot and for Linux full dts is downloaded from external media. And
external version has updated memory reg=<> property so problem due to
'/memory at 0' fix is not seen.

>
> Regards,
> Andreas
>
>>  		device_type = "memory";
>>  		reg = <0 0>;
>>  	};

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Tom Rini]

On Mon, Apr 18, 2016 at 07:38:36AM +0200, Heiko Schocher wrote:
> 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 at 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 at 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 ...

Yes, we need to, but this isn't strictly a U-Boot problem, so this is by
and large the wrong place to start with these patches.

> 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 at 0" -> "cpu0"?

No, in this case MIPS needs to update their skeleton to match the normal
CPU binding.

[snip]
> >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 ...

Well, sorry, but we're not going to address these warnings any time
soon, by and large.  The vast majority of these are in DT files that we
inherit, almost entirely, from the kernel.  The ones that we don't (ie
most of x86) we should only update to fix this warning once everyone
else agrees on the correct new style.  Or if they already have, we need
to do it that way.

You may wish to have travis-ci stuff pin down to an older DTC version
for a while, to cut down on noise.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160418/7e219a42/attachment.sig>

[U-Boot] [PATCH] mips: fix DTC unit warnings

[Tom Rini]

On Fri, Apr 15, 2016 at 12:59:36PM +0200, Heiko Schocher wrote:

> 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 at 0 has a unit name, but no reg property
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>

This just isn't right as-is (that cpu one is wrong, MIPS cannot make up
a different binding from the right of the arches).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160418/f0eecc68/attachment.sig>