| Message ID | 20171204020513.18708-1-andre.przywara@arm.com |
|---|---|
| Headers | show |
| Series | Fix incorrect usage of the (FIT) DT node unit address | expand |
Hi Andre, On 3 December 2017 at 19:05, Andre Przywara <andre.przywara@arm.com> wrote: > The DT spec[1] demands a unit-address in a node name (name@address) to > match the "reg" property inside that node: > uart0: serial@1c28000 { > reg = <0x01c28000 0x400>; > .... > If there is no reg property in a node, there must not be a unit address > in the node name as well (so no '@' sign at all). > > Newer version of the device tree compiler (dtc) will warn about violations > of this rule: > .... > <stdout>: Warning (unit_address_vs_reg): Node /images/fdt@1 has a unit name, > but no reg property > .... > > To avoid those warnings, but still keep enumerable node names, we replace > the "@" sign with a dash ("-"), which does not have a specical meaning, > but is a valid node name character. So the first fdt file (as referenced > above in the warning message) would be called "fdt-1" instead. > > This affects mostly documenation files and some examples of FIT image > files, but also some code which actually generates FIT images: > - The first four patches fix documentation, example files and comments, > they should not affect actual generated code or files. > In places where having multiple instances of a node is normal (fdt, > hash, signature), I simply replaced the '@' sign with the dash. > Where one would expect only one instance (kernel, initrd), I removed the > bogus '@1' completely, so a "kernel" just goes by just this very name. > - Patch 5/7 fixes the usage in the Allwinner SPL FIT image files, this has > been on the list before. > - Patch 6/7 fixes the usage when the mkimage tool (auto-)generates FIT images. > - The final patch 7/7 fixes the usage for the ARMv8 secure firmware image > handling. I am a bit unsure about this one, as this seems to *look* for > a specific node name, which sounds a bit dodgy to me. I think DT parsers > should never rely on a certain node name, but either use references or look > inside nodes to find a matching one. Also I am not sure who actually > generates those FIT image files this code gets to read. Any input would > be welcome here. > > Please let me know if this makes some sense or not. I thought I read somewhere that there is a dtc option to turn off these warnings? Regards, Simon
Hi Simon, 2017-12-12 13:38 GMT+09:00 Simon Glass <sjg@chromium.org>: > Hi Andre, > > On 3 December 2017 at 19:05, Andre Przywara <andre.przywara@arm.com> wrote: >> The DT spec[1] demands a unit-address in a node name (name@address) to >> match the "reg" property inside that node: >> uart0: serial@1c28000 { >> reg = <0x01c28000 0x400>; >> .... >> If there is no reg property in a node, there must not be a unit address >> in the node name as well (so no '@' sign at all). >> >> Newer version of the device tree compiler (dtc) will warn about violations >> of this rule: >> .... >> <stdout>: Warning (unit_address_vs_reg): Node /images/fdt@1 has a unit name, >> but no reg property >> .... >> >> To avoid those warnings, but still keep enumerable node names, we replace >> the "@" sign with a dash ("-"), which does not have a specical meaning, >> but is a valid node name character. So the first fdt file (as referenced >> above in the warning message) would be called "fdt-1" instead. >> >> This affects mostly documenation files and some examples of FIT image >> files, but also some code which actually generates FIT images: >> - The first four patches fix documentation, example files and comments, >> they should not affect actual generated code or files. >> In places where having multiple instances of a node is normal (fdt, >> hash, signature), I simply replaced the '@' sign with the dash. >> Where one would expect only one instance (kernel, initrd), I removed the >> bogus '@1' completely, so a "kernel" just goes by just this very name. >> - Patch 5/7 fixes the usage in the Allwinner SPL FIT image files, this has >> been on the list before. >> - Patch 6/7 fixes the usage when the mkimage tool (auto-)generates FIT images. >> - The final patch 7/7 fixes the usage for the ARMv8 secure firmware image >> handling. I am a bit unsure about this one, as this seems to *look* for >> a specific node name, which sounds a bit dodgy to me. I think DT parsers >> should never rely on a certain node name, but either use references or look >> inside nodes to find a matching one. Also I am not sure who actually >> generates those FIT image files this code gets to read. Any input would >> be welcome here. >> >> Please let me know if this makes some sense or not. > > I thought I read somewhere that there is a dtc option to turn off > these warnings? > I think -Wno-unit_address_vs_reg is the one, but I prefer to fix the root cause by replacing @ instead of hiding it.
Hi, On 12/12/17 04:50, Masahiro Yamada wrote: > Hi Simon, > > > 2017-12-12 13:38 GMT+09:00 Simon Glass <sjg@chromium.org>: >> Hi Andre, >> >> On 3 December 2017 at 19:05, Andre Przywara <andre.przywara@arm.com> wrote: >>> The DT spec[1] demands a unit-address in a node name (name@address) to >>> match the "reg" property inside that node: >>> uart0: serial@1c28000 { >>> reg = <0x01c28000 0x400>; >>> .... >>> If there is no reg property in a node, there must not be a unit address >>> in the node name as well (so no '@' sign at all). >>> >>> Newer version of the device tree compiler (dtc) will warn about violations >>> of this rule: >>> .... >>> <stdout>: Warning (unit_address_vs_reg): Node /images/fdt@1 has a unit name, >>> but no reg property >>> .... >>> >>> To avoid those warnings, but still keep enumerable node names, we replace >>> the "@" sign with a dash ("-"), which does not have a specical meaning, >>> but is a valid node name character. So the first fdt file (as referenced >>> above in the warning message) would be called "fdt-1" instead. >>> >>> This affects mostly documenation files and some examples of FIT image >>> files, but also some code which actually generates FIT images: >>> - The first four patches fix documentation, example files and comments, >>> they should not affect actual generated code or files. >>> In places where having multiple instances of a node is normal (fdt, >>> hash, signature), I simply replaced the '@' sign with the dash. >>> Where one would expect only one instance (kernel, initrd), I removed the >>> bogus '@1' completely, so a "kernel" just goes by just this very name. >>> - Patch 5/7 fixes the usage in the Allwinner SPL FIT image files, this has >>> been on the list before. >>> - Patch 6/7 fixes the usage when the mkimage tool (auto-)generates FIT images. >>> - The final patch 7/7 fixes the usage for the ARMv8 secure firmware image >>> handling. I am a bit unsure about this one, as this seems to *look* for >>> a specific node name, which sounds a bit dodgy to me. I think DT parsers >>> should never rely on a certain node name, but either use references or look >>> inside nodes to find a matching one. Also I am not sure who actually >>> generates those FIT image files this code gets to read. Any input would >>> be welcome here. >>> >>> Please let me know if this makes some sense or not. >> >> I thought I read somewhere that there is a dtc option to turn off >> these warnings? >> > > I think -Wno-unit_address_vs_reg is the one, > but I prefer to fix the root cause by replacing @ > instead of hiding it. Yes, I pulled this series off purely because Masahiro asked for it before. I am just after patch 5/7 ;-) And I agree with him that we should fix it rather than paper over it - at least for the code parts. And if we do so, we should fix the documentation as well - so here we go. I know it's a pain to review (sorry for that!). I would appreciate if we could push 5/7 through - as without it I see warnings right now and I am sure that it works. 6/7 looks good as well, though I can't say if there are side effects. Normally one would expect that node names are purely descriptive, but 7/7 tells me otherwise. Cheers, Andre.
The DT spec[1] demands a unit-address in a node name (name@address) to match the "reg" property inside that node: uart0: serial@1c28000 { reg = <0x01c28000 0x400>; .... If there is no reg property in a node, there must not be a unit address in the node name as well (so no '@' sign at all). Newer version of the device tree compiler (dtc) will warn about violations of this rule: .... <stdout>: Warning (unit_address_vs_reg): Node /images/fdt@1 has a unit name, but no reg property .... To avoid those warnings, but still keep enumerable node names, we replace the "@" sign with a dash ("-"), which does not have a specical meaning, but is a valid node name character. So the first fdt file (as referenced above in the warning message) would be called "fdt-1" instead. This affects mostly documenation files and some examples of FIT image files, but also some code which actually generates FIT images: - The first four patches fix documentation, example files and comments, they should not affect actual generated code or files. In places where having multiple instances of a node is normal (fdt, hash, signature), I simply replaced the '@' sign with the dash. Where one would expect only one instance (kernel, initrd), I removed the bogus '@1' completely, so a "kernel" just goes by just this very name. - Patch 5/7 fixes the usage in the Allwinner SPL FIT image files, this has been on the list before. - Patch 6/7 fixes the usage when the mkimage tool (auto-)generates FIT images. - The final patch 7/7 fixes the usage for the ARMv8 secure firmware image handling. I am a bit unsure about this one, as this seems to *look* for a specific node name, which sounds a bit dodgy to me. I think DT parsers should never rely on a certain node name, but either use references or look inside nodes to find a matching one. Also I am not sure who actually generates those FIT image files this code gets to read. Any input would be welcome here. Please let me know if this makes some sense or not. Cheers, Andre. [1] https://www.devicetree.org/downloads/devicetree-specification-v0.1-20160524.pdf; chapter 2.2.1 "Node names", page 6 Andre Przywara (7): doc: FIT image: fix incorrect description of DT node unit address doc: FIT image: fix incorrect examples of DT node unit address doc: fix incorrect usage of DT node unit address fix incorrect usage of DT node unit address in comments sunxi: arm64: correct usage of DT node address in FIT generation tools: fix incorrect usage of DT node unit address armv8: secure firmware: fix incorrect unit address in node name .../arm/cpu/armv8/fsl-layerscape/doc/README.falcon | 16 ++-- arch/arm/cpu/armv8/sec_firmware.c | 2 +- board/sunxi/mksunxi_fit_atf.sh | 16 ++-- common/image-fit.c | 16 ++-- common/image-sig.c | 2 +- doc/README.uniphier | 36 ++++---- doc/chromium/chromebook_jerry.its | 16 ++-- doc/chromium/nyan-big.its | 16 ++-- doc/uImage.FIT/beaglebone_vboot.txt | 84 ++++++++--------- doc/uImage.FIT/command_syntax_extensions.txt | 42 ++++----- doc/uImage.FIT/howto.txt | 52 +++++------ doc/uImage.FIT/kernel.its | 28 +++--- doc/uImage.FIT/kernel_fdt.its | 20 ++--- doc/uImage.FIT/multi-with-fpga.its | 28 +++--- doc/uImage.FIT/multi-with-loadables.its | 40 ++++----- doc/uImage.FIT/multi.its | 54 +++++------ doc/uImage.FIT/multi_spl.its | 14 +-- doc/uImage.FIT/overlay-fdt-boot.txt | 78 ++++++++-------- doc/uImage.FIT/sign-configs.its | 18 ++-- doc/uImage.FIT/sign-images.its | 16 ++-- doc/uImage.FIT/signature.txt | 100 ++++++++++----------- doc/uImage.FIT/source_file_format.txt | 26 +++--- doc/uImage.FIT/update3.its | 12 +-- doc/uImage.FIT/update_uboot.its | 4 +- doc/uImage.FIT/x86-fit-boot.txt | 10 +-- include/image.h | 26 +++--- tools/fit_image.c | 24 ++--- tools/image-host.c | 10 +-- 28 files changed, 403 insertions(+), 403 deletions(-)