From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Fri, 15 Apr 2016 11:37:40 -0600 Subject: [U-Boot] [PATCH] mips: fix DTC unit warnings In-Reply-To: <20160415171147.GY13577@bill-the-cat> References: <1460717976-24683-1-git-send-email-hs@denx.de> <5711078A.5010803@suse.de> <20160415163058.GX13577@bill-the-cat> <57111D48.8000706@wwwdotorg.org> <20160415171147.GY13577@bill-the-cat> Message-ID: <571126E4.1020401@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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...)