| Message ID | 20220808103227.104898-1-awais_belal@mentor.com |
|---|---|
| State | Accepted, archived |
| Commit | 98acfea1e82a90c920bdd636033f930ac034b318 |
| Headers | show |
| Series | kernel-fitimage.bbclass: only package unique DTBs | expand |
Hi Awais, On 8/8/22 12:32, Awais Belal wrote: > The KERNEL_DEVICETREE and related variables could potentially have a device > tree listed multiple times and this works okay for most scenarios. However, > when we create FIT entries for these we get duplicate nodes and uboot-mkimage > fails with > > fit-image-initramfs-image.its:219.58-229.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-ecspi-slave.dtb: Duplicate node name > fit-image-initramfs-image.its:307.50-317.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-ndm.dtb: Duplicate node name > fit-image-initramfs-image.its:362.54-372.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-rm67199.dtb: Duplicate node name > fit-image-initramfs-image.its:417.56-427.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-usdhc1-m2.dtb: Duplicate node name > fit-image-initramfs-image.its:648.59-658.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-ecspi-slave.dtb: Duplicate node name > fit-image-initramfs-image.its:744.51-754.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-ndm.dtb: Duplicate node name > fit-image-initramfs-image.its:804.55-814.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-rm67199.dtb: Duplicate node name > fit-image-initramfs-image.its:864.57-874.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-usdhc1-m2.dtb: Duplicate node name > ERROR: Input tree has errors, aborting (use -f to force output) > uboot-mkimage: Can't open arch/arm64/boot/fitImage.tmp: No such file or directory > > We fix this by tracking the DTBs we're compiling in the FIT and only picking > up unique ones. > > Signed-off-by: Awais Belal <awais_belal@mentor.com> > --- > meta/classes/kernel-fitimage.bbclass | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/meta/classes/kernel-fitimage.bbclass b/meta/classes/kernel-fitimage.bbclass > index 753164551c..2a07a57d5d 100644 > --- a/meta/classes/kernel-fitimage.bbclass > +++ b/meta/classes/kernel-fitimage.bbclass > @@ -529,6 +529,14 @@ fitimage_assemble() { > fi > > DTB=$(echo "$DTB" | tr '/' '_') > + > + # Skip DTB if we've picked it up previously > + case ${DTBS} in > + *${DTB}*) I think this might be a bit too much. e.g. I could have a DTS/DTB named this-is-a.dtb.dtb and another one this-is-a.dtb and this would say it is a duplicate whereas it isn't (poor naming choice, though.. yes :) ). I believe that: for DTB in $(echo "${DTBS}" | tr ' ' '\n' | sort -u); do would work and avoid unnecessary loops. > + continue > + ;; > + esac > + > DTBS="$DTBS $DTB" > fitimage_emit_section_dtb $1 $DTB $DTB_PATH > done > @@ -538,6 +546,14 @@ fitimage_assemble() { > dtbcount=1 > for DTB in $(find "${EXTERNAL_KERNEL_DEVICETREE}" \( -name '*.dtb' -o -name '*.dtbo' \) -printf '%P\n' | sort); do I believe using sort -u here should be enough for this second diff? Cheers, Quentin
Hi Quentin, Thanks for the feedback. > I think this might be a bit too much. e.g. I could have a DTS/DTB named > this-is-a.dtb.dtb and another one this-is-a.dtb and this would say it is > a duplicate whereas it isn't (poor naming choice, though.. yes :) ). > I believe that: > for DTB in $(echo "${DTBS}" | tr ' ' '\n' | sort -u); do > would work and avoid unnecessary loops. this would definitely avoid unnecessary loops however I don't think sorting would be the right thing here as the first dt in the list gets to become the default config which is something assumed for the runtime and sorting would break that. If we want to cover up the corner case you described above would something such fit the bill echo $DTBS | tr ' ' '\n' | grep -xq $DTB && continue > I believe using sort -u here should be enough for this second diff? sort -u wouldn't help here if the same name dt is present in KERNEL_DEVICETREE and the EXTERNEL_KERNEL_DEVICETREE as we'll run into the same problem of having the same dt being packaged twice. I'd rather use the same technique here if you agree. BR, Awais
Hi Awais, On 8/11/22 05:23, Belal, Awais wrote: > Hi Quentin, > > > Thanks for the feedback. > > >> I think this might be a bit too much. e.g. I could have a DTS/DTB named >> this-is-a.dtb.dtb and another one this-is-a.dtb and this would say it is >> a duplicate whereas it isn't (poor naming choice, though.. yes :) ). > >> I believe that: >> for DTB in $(echo "${DTBS}" | tr ' ' '\n' | sort -u); do > >> would work and avoid unnecessary loops. > > > this would definitely avoid unnecessary loops however I don't think sorting would be the right thing here as the first dt in the list gets to become the default config which is something assumed for the runtime and sorting would break that. If we want to cover up the corner case you described above would something such fit the bill > I completely forgot about this. I think this is a not so great behavior :/ But nothing to discuss in this patch :) > > echo $DTBS | tr ' ' '\n' | grep -xq $DTB && continue > Don't forget to quote the shell variables to make shellcheck happy :) > >> I believe using sort -u here should be enough for this second diff? > > > sort -u wouldn't help here if the same name dt is present in KERNEL_DEVICETREE and the EXTERNEL_KERNEL_DEVICETREE as we'll run into the same problem of having the same dt being packaged twice. I'd rather use the same technique here if you agree. > Sounds like a plan :) Cheers, Quentin
diff --git a/meta/classes/kernel-fitimage.bbclass b/meta/classes/kernel-fitimage.bbclass index 753164551c..2a07a57d5d 100644 --- a/meta/classes/kernel-fitimage.bbclass +++ b/meta/classes/kernel-fitimage.bbclass @@ -529,6 +529,14 @@ fitimage_assemble() { fi DTB=$(echo "$DTB" | tr '/' '_') + + # Skip DTB if we've picked it up previously + case ${DTBS} in + *${DTB}*) + continue + ;; + esac + DTBS="$DTBS $DTB" fitimage_emit_section_dtb $1 $DTB $DTB_PATH done @@ -538,6 +546,14 @@ fitimage_assemble() { dtbcount=1 for DTB in $(find "${EXTERNAL_KERNEL_DEVICETREE}" \( -name '*.dtb' -o -name '*.dtbo' \) -printf '%P\n' | sort); do DTB=$(echo "$DTB" | tr '/' '_') + + # Skip DTB if we've picked it up previously + case ${DTBS} in + *${DTB}*) + continue + ;; + esac + DTBS="$DTBS $DTB" fitimage_emit_section_dtb $1 $DTB "${EXTERNAL_KERNEL_DEVICETREE}/$DTB" done
The KERNEL_DEVICETREE and related variables could potentially have a device tree listed multiple times and this works okay for most scenarios. However, when we create FIT entries for these we get duplicate nodes and uboot-mkimage fails with fit-image-initramfs-image.its:219.58-229.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-ecspi-slave.dtb: Duplicate node name fit-image-initramfs-image.its:307.50-317.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-ndm.dtb: Duplicate node name fit-image-initramfs-image.its:362.54-372.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-rm67199.dtb: Duplicate node name fit-image-initramfs-image.its:417.56-427.19: ERROR (duplicate_node_names): /images/fdt-freescale_imx8mp-evk-usdhc1-m2.dtb: Duplicate node name fit-image-initramfs-image.its:648.59-658.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-ecspi-slave.dtb: Duplicate node name fit-image-initramfs-image.its:744.51-754.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-ndm.dtb: Duplicate node name fit-image-initramfs-image.its:804.55-814.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-rm67199.dtb: Duplicate node name fit-image-initramfs-image.its:864.57-874.19: ERROR (duplicate_node_names): /configurations/conf-freescale_imx8mp-evk-usdhc1-m2.dtb: Duplicate node name ERROR: Input tree has errors, aborting (use -f to force output) uboot-mkimage: Can't open arch/arm64/boot/fitImage.tmp: No such file or directory We fix this by tracking the DTBs we're compiling in the FIT and only picking up unique ones. Signed-off-by: Awais Belal <awais_belal@mentor.com> --- meta/classes/kernel-fitimage.bbclass | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)