| Message ID | 20240708123653.2200919-1-s-vadapalli@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Ryan Eatmon |
| Headers | show |
| Series | [meta-ti,master/scarthgap,v3] conf: machine: am62*-evm-k3r5.conf: apply fragment for enabling USB DFU | expand |
On 7/8/2024 7:36 AM, Siddharth Vadapalli wrote: > The config fragment "am62x_r5_usbdfu.config" is applicable to AM62x, > AM62x-SIP, AM62x-LP-EVM, AM62Ax and AM62Px devices and enables USB DFU boot > support at the R5 stage of boot. > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> > --- > > Hello, > > This patch is based on commit > ff9bf7b9 CI/CD Auto-Merger: cicd.scarthgap.202407071800 > of scarthgap-next branch of meta-ti tree. > > v2: > https://lore.kernel.org/r/20240708060054.1985156-1-s-vadapalli@ti.com/ > Changes since v2: > - Based on feedback from Chirag Shilwant <c-shilwant@ti.com> at: > https://lore.kernel.org/r/32db66e5-730c-42ec-9145-72abdf35b6d1@ti.com/ > the subject-prefix has been updated to master/scarthgap and the config > fragment for USB DFU is being applied to AM62x SIP as well. > > v1: > https://lore.kernel.org/r/20240708052457.1974547-1-s-vadapalli@ti.com/ > Changes since v1: > - As pointed out by Vignesh Raghavendra <vigneshr@ti.com> at: > https://lore.kernel.org/r/829dc5b5-e87c-4679-9e2d-7417c744a9c9@ti.com/ > the A53 defconfigs already include the "am62x_a53_usbdfu.config" config > fragment. Hence, they are not required in the am62*-evm.conf files and > have been dropped in this patch. > - Commit message has been updated accordingly. > > Regards, > Siddharth. > > meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf | 1 + > meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf | 1 + > meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf | 1 + > meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf | 1 + > meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf | 1 + > 5 files changed, 5 insertions(+) > > 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 = "" > 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)" > 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. > 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. > 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"
On 7/8/2024 9:04 AM, Ryan Eatmon via lists.yoctoproject.org wrote: > > Also, is this something that beagleplay should also have in it's config, or will that board not support this? The logic for supporting this will be a little more complicated because of the beagle uboot settings if so.
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.
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.
On Tue, Jul 09, 2024 at 09:55:41AM +0530, Chirag Shilwant wrote: > Hi Siddharth, > On 09/07/24 09:36, Siddharth Vadapalli wrote: [...] > > 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 :) > I have built and tested the combination: am62px_evm_r5_defconfig + am62x_r5_usbdfu.config + am62x_evm_prune_splashscreen.config I did not face any build errors and have also validated USB DFU functionatlity. Logs: https://gist.github.com/Siddharth-Vadapalli-at-TI/38331822c5f8ed0887225ecab51f3395 Regards, Siddharth.
Hi Siddharth, On 09/07/24 11:01, Siddharth Vadapalli wrote: > On Tue, Jul 09, 2024 at 09:55:41AM +0530, Chirag Shilwant wrote: >> Hi Siddharth, >> On 09/07/24 09:36, Siddharth Vadapalli wrote: > [...] > >>> 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 :) >> > I have built and tested the combination: > am62px_evm_r5_defconfig + am62x_r5_usbdfu.config + > am62x_evm_prune_splashscreen.config > > I did not face any build errors and have also validated USB DFU > functionatlity. Logs: > https://gist.github.com/Siddharth-Vadapalli-at-TI/38331822c5f8ed0887225ecab51f3395 Thanks Siddharth for confirmation. With Ryan's comments getting addressed in v4, you have my Acked-by: Chirag Shilwant <c-shilwant@ti.com> > > Regards, > Siddharth.
On Mon, Jul 08, 2024 at 09:04:43AM -0500, Ryan Eatmon wrote: [...] > > 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)" I failed to notice it in my earlier response. The above file that you are referring to is different from the one that this patch modifies. This patch is updating: meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf while the file that you are referring to above is: meta-ti-bsp/conf/machine/am62pxx-evm.conf So UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config" UBOOT_CONFIG_FRAGMENTS:bsp-ti-6_1 = "" is applicable to this as well, similar to other files. I will implement the above change uniformly across all files in the v4 patch. [...] Regards, Siddharth.
On 7/9/2024 2:58 AM, Siddharth Vadapalli wrote: > On Mon, Jul 08, 2024 at 09:04:43AM -0500, Ryan Eatmon wrote: > > [...] > >>> 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)" > > I failed to notice it in my earlier response. The above file that you > are referring to is different from the one that this patch modifies. > This patch is updating: > meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf > while the file that you are referring to above is: > meta-ti-bsp/conf/machine/am62pxx-evm.conf > > So > UBOOT_CONFIG_FRAGMENTS = "am62x_r5_usbdfu.config" > UBOOT_CONFIG_FRAGMENTS:bsp-ti-6_1 = "" > is applicable to this as well, similar to other files. > I will implement the above change uniformly across all files in the v4 > patch. Good point. I looked in the wrong file when I was looking in the files. > [...] > > Regards, > Siddharth.
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" 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" 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" 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" 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"
The config fragment "am62x_r5_usbdfu.config" is applicable to AM62x, AM62x-SIP, AM62x-LP-EVM, AM62Ax and AM62Px devices and enables USB DFU boot support at the R5 stage of boot. Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com> --- Hello, This patch is based on commit ff9bf7b9 CI/CD Auto-Merger: cicd.scarthgap.202407071800 of scarthgap-next branch of meta-ti tree. v2: https://lore.kernel.org/r/20240708060054.1985156-1-s-vadapalli@ti.com/ Changes since v2: - Based on feedback from Chirag Shilwant <c-shilwant@ti.com> at: https://lore.kernel.org/r/32db66e5-730c-42ec-9145-72abdf35b6d1@ti.com/ the subject-prefix has been updated to master/scarthgap and the config fragment for USB DFU is being applied to AM62x SIP as well. v1: https://lore.kernel.org/r/20240708052457.1974547-1-s-vadapalli@ti.com/ Changes since v1: - As pointed out by Vignesh Raghavendra <vigneshr@ti.com> at: https://lore.kernel.org/r/829dc5b5-e87c-4679-9e2d-7417c744a9c9@ti.com/ the A53 defconfigs already include the "am62x_a53_usbdfu.config" config fragment. Hence, they are not required in the am62*-evm.conf files and have been dropped in this patch. - Commit message has been updated accordingly. Regards, Siddharth. meta-ti-bsp/conf/machine/am62axx-evm-k3r5.conf | 1 + meta-ti-bsp/conf/machine/am62pxx-evm-k3r5.conf | 1 + meta-ti-bsp/conf/machine/am62xx-evm-k3r5.conf | 1 + meta-ti-bsp/conf/machine/am62xx-lp-evm-k3r5.conf | 1 + meta-ti-bsp/conf/machine/am62xxsip-evm-k3r5.conf | 1 + 5 files changed, 5 insertions(+)