Hi Siddharth,
On 09/07/24 09:36, Siddharth Vadapalli wrote:
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.
Agreed with Ryan. This approach looks much cleaner. I would encourage you
to build test the case where both am62x_evm_prune_splashscreen.config &
am62x_r5_usbdfu.config
are passed to u-boot just to avoid last moment hiccups during final code
freeze stage :)
Thanks,
Chirag
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 (#17836):
https://lists.yoctoproject.org/g/meta-ti/message/17836
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]]
-=-=-=-=-=-=-=-=-=-=-=-
