[U-Boot] [PATCH] ARM: SPL: FIT: fix DTC warnings on FIT generation

Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/board/sunxi/mksunxi_fit_atf.sh b/board/sunxi/mksunxi_fit_atf.sh index b1d6e0e16a..36abe9efed 100755 --- a/board/sunxi/mksunxi_fit_atf.sh +++ b/board/sunxi/mksunxi_fit_atf.sh @@ -21,7 +21,7 @@ cat << __HEADER_EOF #address-cells = <1>;
images { - uboot@1 { + uboot { description = "U-Boot (64-bit)"; data = /incbin/("u-boot-nodtb.bin"); type = "standalone"; @@ -29,7 +29,7 @@ cat << __HEADER_EOF compression = "none"; load = <0x4a000000>; }; - atf@1 { + atf { description = "ARM Trusted Firmware"; data = /incbin/("$BL31"); type = "firmware"; @@ -44,7 +44,7 @@ cnt=1 for dtname in $* do cat << __FDT_IMAGE_EOF - fdt@$cnt { + fdt_$cnt { description = "$(basename $dtname .dtb)"; data = /incbin/("$dtname"); type = "flat_dt"; @@ -57,7 +57,7 @@ done cat << __CONF_HEADER_EOF }; configurations { - default = "config@1"; + default = "config_1";
__CONF_HEADER_EOF
@@ -65,11 +65,11 @@ cnt=1 for dtname in $* do cat << __CONF_SECTION_EOF - config@$cnt { + config_$cnt { description = "$(basename $dtname .dtb)"; - firmware = "uboot@1"; - loadables = "atf@1"; - fdt = "fdt@$cnt"; + firmware = "uboot"; + loadables = "atf"; + fdt = "fdt_$cnt"; }; __CONF_SECTION_EOF cnt=$((cnt+1))

Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
Regards, Simon

On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
thanks!

On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.

On Tue, Oct 17, 2017 at 2:43 AM, Tom Rini trini@konsulko.com wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Yes, this can be true if the node changes are more common in between.
thanks!

On Tuesday 17 October 2017 02:43 AM, Tom Rini wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Kernel suppress these warning by default[1] and enables these warnings with W= compiler option. May be this should be included in u-boot as well?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
Thanks and regards, Lokesh

On Mon, Oct 23, 2017 at 11:08:57AM +0530, Lokesh Vutla wrote:
On Tuesday 17 October 2017 02:43 AM, Tom Rini wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Kernel suppress these warning by default[1] and enables these warnings with W= compiler option. May be this should be included in u-boot as well?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
We have slightly different logic for that today in scripts/Makefile.extrawarn, perhaps we need to re-sync?

On Tuesday 24 October 2017 02:04 AM, Tom Rini wrote:
On Mon, Oct 23, 2017 at 11:08:57AM +0530, Lokesh Vutla wrote:
On Tuesday 17 October 2017 02:43 AM, Tom Rini wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Kernel suppress these warning by default[1] and enables these warnings with W= compiler option. May be this should be included in u-boot as well?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
We have slightly different logic for that today in scripts/Makefile.extrawarn, perhaps we need to re-sync?
ahh you are right. I do not see these warning with the latest U-Boot. But I still agree with the $patch as these warnings are meant to be fixed at some point.
Thanks and regards, Lokesh

Hi Andre,
2017-10-24 11:03 GMT+09:00 Lokesh Vutla lokeshvutla@ti.com:
On Tuesday 24 October 2017 02:04 AM, Tom Rini wrote:
On Mon, Oct 23, 2017 at 11:08:57AM +0530, Lokesh Vutla wrote:
On Tuesday 17 October 2017 02:43 AM, Tom Rini wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote: > Newer versions of the device tree compiler (rightfully) complain about > mismatches between attributed node names (name@<addr>) and a missing > reg property in that node. > Adjust the FIT build script for 64-bit Allwinner boards to remove the > bogus addresses from the node names and avoid the warnings. > > Signed-off-by: Andre Przywara andre.przywara@arm.com > --- > board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Kernel suppress these warning by default[1] and enables these warnings with W= compiler option. May be this should be included in u-boot as well?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
We have slightly different logic for that today in scripts/Makefile.extrawarn, perhaps we need to re-sync?
ahh you are right. I do not see these warning with the latest U-Boot. But I still agree with the $patch as these warnings are meant to be fixed at some point.
Please use "-" instead of "@".
Update all the FIT examples. You may need to update some C files.
This is the conclusion of Device Tree community. See the following commit of Linux.
commit b21569cf1de925e0a42c9964bd7f520cb4a4d875 Author: Viresh Kumar viresh.kumar@linaro.org Date: Thu Jun 22 09:15:11 2017 +0530
PM / OPP: Use - instead of @ for DT entries
Compiling the DT file with W=1, DTC warns like follows:
Warning (unit_address_vs_reg): Node /opp_table0/opp@1000000000 has a unit name, but no reg property
Fix this by replacing '@' with '-' as the OPP nodes will never have a "reg" property.
Reported-by: Krzysztof Kozlowski krzk@kernel.org Reported-by: Masahiro Yamada yamada.masahiro@socionext.com Suggested-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Viresh Kumar viresh.kumar@linaro.org Acked-by: Rob Herring robh@kernel.org Reviewed-by: Stephen Boyd sboyd@codeaurora.org Signed-off-by: Rafael J. Wysocki rafael.j.wysocki@intel.com

On 24/10/17 03:31, Masahiro Yamada wrote:
Hi,
2017-10-24 11:03 GMT+09:00 Lokesh Vutla lokeshvutla@ti.com:
On Tuesday 24 October 2017 02:04 AM, Tom Rini wrote:
On Mon, Oct 23, 2017 at 11:08:57AM +0530, Lokesh Vutla wrote:
On Tuesday 17 October 2017 02:43 AM, Tom Rini wrote:
On Tue, Oct 17, 2017 at 02:29:14AM +0530, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote: > Hi Andre, > > On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote: >> Newer versions of the device tree compiler (rightfully) complain about >> mismatches between attributed node names (name@<addr>) and a missing >> reg property in that node. >> Adjust the FIT build script for 64-bit Allwinner boards to remove the >> bogus addresses from the node names and avoid the warnings. >> >> Signed-off-by: Andre Przywara andre.przywara@arm.com >> --- >> board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- >> 1 file changed, 8 insertions(+), 8 deletions(-) > > It looks like we have this problem all over the place. The > documentation in doc/uImage now seems to have this problem too. > > I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
Anything that DTC is warning about in a dts that we get from the kernel, should be fixed in the kernel. The kernel dtc is what we're using, and is/will/can also complain about it.
Kernel suppress these warning by default[1] and enables these warnings with W= compiler option. May be this should be included in u-boot as well?
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scri...
We have slightly different logic for that today in scripts/Makefile.extrawarn, perhaps we need to re-sync?
ahh you are right. I do not see these warning with the latest U-Boot. But I still agree with the $patch as these warnings are meant to be fixed at some point.
Please use "-" instead of "@".
Update all the FIT examples. You may need to update some C files.
Yeah! "find ./ -type f | xargs -1 sed -i -e s/@/-/g" didn't give the expected results, though :-D But I bit the bullet and fixed every @1 sucker I could find, just need some more time to make proper patches.
Cheers, Andre.
This is the conclusion of Device Tree community. See the following commit of Linux.
commit b21569cf1de925e0a42c9964bd7f520cb4a4d875 Author: Viresh Kumar viresh.kumar@linaro.org Date: Thu Jun 22 09:15:11 2017 +0530
PM / OPP: Use - instead of @ for DT entries Compiling the DT file with W=1, DTC warns like follows: Warning (unit_address_vs_reg): Node /opp_table0/opp@1000000000 has a unit name, but no reg property Fix this by replacing '@' with '-' as the OPP nodes will never have a "reg" property. Reported-by: Krzysztof Kozlowski <krzk@kernel.org> Reported-by: Masahiro Yamada <yamada.masahiro@socionext.com> Suggested-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Acked-by: Rob Herring <robh@kernel.org> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Hi,
sorry Simon for dropping the ball earlier. I will try to answer both Jagan's and your concern below.
On 16/10/17 21:59, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
This is not the kernel .dts, but the FIT image description (using the DTS format), which is purely private to U-Boot. I erroneously used addresses (fdt@1) to enumerate different FDTs. I don't think this is right, since those FDTs don't have anything which would resemble an address. Instead the SPL just chooses one of them, and the script generates as many as we give it (from defconfig).
So I don't see much sense in introducing a "reg" property. Multiple instances of an UART are alive at the same time, so an address property makes sense. But we just need *one* FDT and some unique name used to match configuration entries to the appropriate image. Actually those identifiers could be totally random as well, or we actually use something derived from the filename. But for simplicity I'd just go with the underscore notation, unless someone convinces me otherwise.
Cheers, Andre.

Hi Andre,
On 16 October 2017 at 23:30, André Przywara andre.przywara@arm.com wrote:
Hi,
sorry Simon for dropping the ball earlier. I will try to answer both Jagan's and your concern below.
On 16/10/17 21:59, Jagan Teki wrote:
On Mon, Oct 9, 2017 at 10:15 AM, Simon Glass sjg@chromium.org wrote:
Hi Andre,
On 4 October 2017 at 17:24, Andre Przywara andre.przywara@arm.com wrote:
Newer versions of the device tree compiler (rightfully) complain about mismatches between attributed node names (name@<addr>) and a missing reg property in that node. Adjust the FIT build script for 64-bit Allwinner boards to remove the bogus addresses from the node names and avoid the warnings.
Signed-off-by: Andre Przywara andre.przywara@arm.com
board/sunxi/mksunxi_fit_atf.sh | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
It looks like we have this problem all over the place. The documentation in doc/uImage now seems to have this problem too.
I wonder if instead we should add reg / #address-cells / #size-cells properties?
If the update on dts, might be an another-overhead to maintain u-boot dts wrt Linux dts sync.
This is not the kernel .dts, but the FIT image description (using the DTS format), which is purely private to U-Boot. I erroneously used addresses (fdt@1) to enumerate different FDTs. I don't think this is right, since those FDTs don't have anything which would resemble an address. Instead the SPL just chooses one of them, and the script generates as many as we give it (from defconfig).
So I don't see much sense in introducing a "reg" property. Multiple instances of an UART are alive at the same time, so an address property makes sense. But we just need *one* FDT and some unique name used to match configuration entries to the appropriate image. Actually those identifiers could be totally random as well, or we actually use something derived from the filename. But for simplicity I'd just go with the underscore notation, unless someone convinces me otherwise.
OTOH this really just a feature of the DT format. Adding a 'reg' property can be justified on that basis. Or perhaps we should have an option for dtc to support 'degenerate' .dts files?
The underscore is normally used for phandles.
Regards, Simon
participants (7)
-
Andre Przywara
-
André Przywara
-
Jagan Teki
-
Lokesh Vutla
-
Masahiro Yamada
-
Simon Glass
-
Tom Rini