[PATCH 2/4] board: ti: am62px: env: include environment for DFU Boot

[Siddharth Vadapalli]

On Wed, Dec 18, 2024 at 10:57:36AM +0100, Mattijs Korpershoek wrote:

Hello Mattijs,

> Hi Siddharth,
> 
> Thank you for the patch.
> 
> On mar., déc. 17, 2024 at 18:46, Siddharth Vadapalli <s-vadapalli at ti.com> wrote:
> 
> > Include the TI K3 DFU environment to support DFU Boot and DFU Flash.
> > Also add "usb" to the list of "boot_targets".
> >
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
> > ---
> >  board/ti/am62px/am62px.env | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/board/ti/am62px/am62px.env b/board/ti/am62px/am62px.env
> > index 7ef54079aa8..e0838196e3a 100644
> > --- a/board/ti/am62px/am62px.env
> > +++ b/board/ti/am62px/am62px.env
> > @@ -1,5 +1,6 @@
> >  #include <env/ti/ti_common.env>
> >  #include <env/ti/mmc.env>
> > +#include <env/ti/k3_dfu.env>
> >
> >  name_kern=Image
> >  console=ttyS2,115200n8
> > @@ -7,7 +8,7 @@ args_all=setenv optargs ${optargs} earlycon=ns16550a,mmio32,0x02800000
> >  	${mtdparts}
> >  run_kern=booti ${loadaddr} ${rd_spec} ${fdtaddr}
> >
> > -boot_targets=mmc1 mmc0 pxe dhcp
> > +boot_targets=mmc1 mmc0 usb pxe dhcp
> >  boot=mmc
> >  mmcdev=1
> >  bootpart=1:2
> > @@ -17,4 +18,4 @@ rd_spec=-
> >  #if CONFIG_BOOTMETH_ANDROID
> >  #include <env/ti/android.env>
> >  adtb_idx=3
> > -#endif
> > \ No newline at end of file
> > +#endif
> 
> This change seems un-related, is it needed?

My editor automatically adds a newline, thereby fixing the warning/error
regarding the absence of a newline. I assume that a newline is expected.
Please let me know if you want me to undo this in the v2 series.

> 
> Also, looking at Martyn's/Sjoerd's series, I see a couple of things
> missing:
> 1. Documentation. now that am62px is compatible with the
>    am62x_r5_usbdfu.config fragment, we need to document this in the board
>    docs. See: commit def64b493748 ("doc: board: Add document for DFU boot on am62x SoCs")

I planned on documenting it once this series got merged. The reason
behind it is that I was unsure if the device-tree patch in this series
will be accepted or will be asked to be enabled via the Linux DT Sync
process. In the latter case, documenting this feature will be incorrect
in the event of a partial merge (i.e. the documentation patch gets
merged but the device-tree patch doesn't).

> 
> 2. Including configs/am62x_a53_usbdfu.config in configs/am62px_evm_a53_defconfig.
>    This is how it's done for am62x, see:
>    commit dfc2dff5a844 ("configs: am62x_evm_*: Enable USB and DFU support")
> 
> Note that If we don't do 2), we cannot use USB gadget from a U-Boot that
> has been booted over DFU:
> 
>     => fastboot usb 0
>     No USB device found
>     USB init failed: -19
>     => usb list
>     USB is stopped. Please issue 'usb start' first.
>     => usb start
>     starting USB...
>     No USB controllers found
>     =>
> 
> For 2, this diff fixes it:
> 
> diff --git a/configs/am62px_evm_a53_defconfig b/configs/am62px_evm_a53_defconfig
> index 9635beb1b27e..81f433c997b5 100644
> --- a/configs/am62px_evm_a53_defconfig
> +++ b/configs/am62px_evm_a53_defconfig
> @@ -183,3 +183,4 @@ CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
>  CONFIG_EFI_SET_TIME=y
>  
>  #include <configs/k3_efi_capsule.config>
> +#include <configs/am62x_a53_usbdfu.config>
> 
> In my opinion, 2) is a valid use case:
> 1. On a blank board, we boot the bootloaders over DFU
> 2. Once U-Boot is started, we start fastboot to flash all images to eMMC.
> 
> Could this be added for v2, please?

Sure, I will include it in the v2 series. Thank you for reviewing this
patch and sharing feedback.

Regards,
Siddharth.