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