On Mon, Jul 08, 2024 at 09:04:43AM -0500, Ryan Eatmon wrote:
>
>
> On 7/8/2024 7:36 AM, Siddharth Vadapalli wrote:
[...]
> > diff --git a/meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf
> > b/meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf
> > index 2af3317e..5ba8c27e 100644
> > --- a/meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf
> > +++ b/meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf
> > @@ -9,3 +9,4 @@ SYSFW_CONFIG = "evm"
> > SYSFW_SUFFIX = "hs-fs"
> > UBOOT_MACHINE = "am62ax_evm_r5_defconfig"
> > +UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
>
> This will not work with the new BSP system we have in meta-ti. Since we are
> trying to have support for multiple kernel/uboot/graphics combinations we
> need to be aware of which of those other combinations that are present in
> the BSP file will or wont work with the setting we are trying to do.
>
> In this case, I see am62x_r5_usbdfu.config in ti-u-boot-2024.04 and in
> upstream, but I do not see it in ti-u-boot-2023.04. So the correct setting
> here would be:
>
> UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
> UBOOT_CONFIG_FRAGMENTS:bsp-ti-6_1 = ""
So I have to set "bsp-ti-6_1" to the empty string to indicate that the
CONFIG_FRAGMENT is not applicable to it. I wasn't aware of this. Thank you
for reviewing the patch and pointing this out.
>
>
> > diff --git a/meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf
> > b/meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf
> > index 36915381..9b68e626 100644
> > --- a/meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf
> > +++ b/meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf
> > @@ -10,3 +10,4 @@ SYSFW_CONFIG = "evm"
> > SYSFW_SUFFIX = "hs-fs"
> > UBOOT_MACHINE = "am62px_evm_r5_defconfig"
> > +UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
> For this one, there are already UBOOT_CONFIG_FRAGMENTS later in the file
> that you are not taking into account by just setting the variable. This
> file is going to be a little more difficult to do correctly.
>
> Since we already have a setting with an override, I think the best plan
> would be to rework the existing logic while adding your changes. Add some
> intermediate variables to construct the final value. Something like this:
>
> BASE_FRAGMENTS = "am62x_r5_usbdfu.config"
> BASE_FRAGMENTS:ti-bsp-6_1 = ""
>
> # UBOOT_CONFIG_FRAGMENTS holds the list of u-boot config fragments which has
> to be build
> # along with the base defconfig mentioned in UBOOT_MACHINE. Refer
> u-boot-mergeconfig.inc
> # under meta-ti-bsp/recipes-bsp/u-boot/ for more details.
> # For AM62P tisdk-display-cluster image, splash screen is handled by SBL.
> # Hence, disable the A53 based splash screen using the
> am62x_evm_prune_splashscreen.config fragment present in ti-u-boot tree
> PRUNE_SPLASHSCREEN_FRAGMENT =
> "${@oe.utils.conditional('DISPLAY_CLUSTER_ENABLE', '1',
> 'am62x_evm_prune_splashscreen.config', '', d)}"
> PRUNE_SPLASHSCREEN_FRAGMENT :bsp-ti-6_1 =
> "${@oe.utils.conditional('DISPLAY_CLUSTER_ENABLE', '1',
> 'am62px_evm_prune_splashscreen.config', '', d)}"
>
> UBOOT_CONFIG_FRAGMENTS = " $(BASE_FRAGMENTS) $(PRUNE_SPLASHSCREEN_FRAGMENT)"
Since USB DFU is independent of Splash Screen, I agree that the above
approach you have suggested is the right way to implement this. I will
implement your suggestion in the v4 patch.
>
>
> > diff --git a/meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf
> > b/meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf
> > index 548369ca..85c48334 100644
> > --- a/meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf
> > +++ b/meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf
> > @@ -10,3 +10,4 @@ SYSFW_CONFIG = "evm"
> > SYSFW_SUFFIX = "hs-fs"
> > UBOOT_MACHINE = "am62x_evm_r5_defconfig"
> > +UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
>
> Same as the first comment.
I will fix this.
>
> > diff --git a/meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf
> > b/meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf
> > index 52b69a72..24f96858 100644
> > --- a/meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf
> > +++ b/meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf
> > @@ -10,3 +10,4 @@ SYSFW_CONFIG = "evm"
> > SYSFW_SUFFIX = "hs-fs"
> > UBOOT_MACHINE = "am62x_lpsk_r5_defconfig"
> > +UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
>
> Same as the first comment.
Will fix this too.
>
> > diff --git a/meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf
> > b/meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf
> > index 55bc530b..b6045d89 100644
> > --- a/meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf
> > +++ b/meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf
> > @@ -13,3 +13,4 @@ UBOOT_MACHINE = "am62xsip_evm_r5_defconfig"
> > UBOOT_MACHINE:bsp-ti-6_1 = "am62x_evm_r5_defconfig"
> > UBOOT_CONFIG_FRAGMENTS:bsp-ti-6_1 = "am62xsip_sk_r5.config"
> > +UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
>
> For this one there is already a setting for bsp-ti-6_1 so this is
> technically ok. But in general we should try and have the variables in
> "correct" order. The setting of the default, followed by the overrides of
> the variable. So:
>
> UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config"
> UBOOT_CONFIG_FRAGMENTS:bsp-ti-6_1 = "am62xsip_sk_r5.config"
I understand. I will fix the ordering. Thank you for reviewing the patch
and providing your valuable feedback.
Regards,
Siddharth.
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#17835):
https://lists.yoctoproject.org/g/meta-ti/message/17835
Mute This Topic: https://lists.yoctoproject.org/mt/107101381/21656
Group Owner: [email protected]
Unsubscribe: https://lists.yoctoproject.org/g/meta-ti/unsub
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
