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
________________________________
From: [email protected] 
<[email protected]> on behalf of Quentin Schulz 
<[email protected]>
Sent: Monday, August 8, 2022 4:15:15 PM
To: Belal, Awais; [email protected]
Subject: Re: [OE-core] [PATCH] kernel-fitimage.bbclass: only package unique DTBs

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 <[email protected]>
> ---
>   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
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#169225): 
https://lists.openembedded.org/g/openembedded-core/message/169225
Mute This Topic: https://lists.openembedded.org/mt/92888948/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to