| Message ID | 20171004232439.20817-1-andre.przywara@arm.com |
|---|---|
| State | Rejected |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot] ARM: SPL: FIT: fix DTC warnings on FIT generation | expand |
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!
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
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/scripts/Makefile.lib#n272 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/scripts/Makefile.lib#n272 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/scripts/Makefile.lib#n272 > > 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/scripts/Makefile.lib#n272 >> >> 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/scripts/Makefile.lib#n272 >>> >>> 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> > > >
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))
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(-)