Siddharth Vadapalli s-vadapalli@ti.com writes:

From: Jonathan Humphreys j-humphreys@ti.com

For use-cases such as USB DFU Boot, "spl_dfu_cmd()" will setup the appropriate value for "dfu_alt_info". To facilitate such use-cases and in order to avoid overwriting the value of "dfu_alt_info", invoke the "set_dfu_alt_info()" function only when "dfu_alt_info" is not defined.

Signed-off-by: Jonathan Humphreys j-humphreys@ti.com Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com

---

drivers/dfu/dfu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 756569217bb..ab8abae1d89 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -169,10 +169,13 @@ int dfu_init_env_entities(char *interface, char *devstr) dfu_reinit_needed = false; dfu_alt_info_changed = false;

• str_env = env_get("dfu_alt_info");

#ifdef CONFIG_SET_DFU_ALT_INFO

• set_dfu_alt_info(interface, devstr);

• if (!str_env) {

• set_dfu_alt_info(interface, devstr);
  

• str_env = env_get("dfu_alt_info");
  

• }

#endif

• str_env = env_get("dfu_alt_info"); if (!str_env) { pr_err(""dfu_alt_info" env variable not defined!\n"); return -EINVAL;

-- 2.43.0

Hi Siddharth, thanks. I went ahead and just posted a separate patch to fix this issue. I added a bit more explanation, and wanted it separate so that it can hopefully make the 2025.01 release.

See https://lore.kernel.org/r/20241217204835.3312765-1-j-humphreys@ti.com.

Thanks Jon

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@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@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.

On Wed, Dec 18, 2024 at 12:00:08PM +0100, Mattijs Korpershoek wrote:

On mer., déc. 18, 2024 at 15:44, Siddharth Vadapalli s-vadapalli@ti.com wrote:

...

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

Hello Mattijs,

...

Hi Siddharth,

Thank you for the patch.

[...]

...

...

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.

This is editor dependant. I'm not sure if there is a coding style entry for having a newline present/absent. I'm in favor of keeping the newline addition, however, please mention it in the commit message.

Something between the lines of "while at it, also add a missing newline to the am62p.env file".

Sure, I will mention this in the commit message.

...

...

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).

Hmm, I'm not the one deciding if [PATCH 4/4] will get applied so I can't chime in on that. However, I think it's good practice to send the documentation changes along with the code changes. Otherwise, doc updates might be forgotten/de-prioritized.

I won't block the series if doc does not get send, though.

Maybe for future submissions, you can consider writing this in the cover letter:

"Since i'm unsure that PATCH 4/4 will be accepted, I've decided to send the documentation changes in a future series"

I will include the documentation in the v2 series, while also pointing out that if the device-tree changes don't get in, then the documentation patch should also be dropped.

...

...

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.

Make sure to check that am62px_evm_a53_defconfig does not contain any duplicate entries with the dfu fragment, like:

CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments" CONFIG_USB_GADGET_VENDOR_NUM=0x0451 CONFIG_USB_GADGET_PRODUCT_NUM=0x6165

I will keep this in mind. Thank you.

Regards, Siddharth.

Hi Siddharth,

Thank you for the patch.

On mar., déc. 17, 2024 at 18:46, Siddharth Vadapalli s-vadapalli@ti.com wrote:

Disable configs which are not required for USB DFU functionality, in order to allow reusing this fragment for AM62Px SoC.

Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com

I confirm this is needed, without this, the tispl size is too big and we can't build.

Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

---

configs/am62x_r5_usbdfu.config | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config index 772bb2ab935..f094ed127e6 100644 --- a/configs/am62x_r5_usbdfu.config +++ b/configs/am62x_r5_usbdfu.config @@ -26,3 +26,6 @@ CONFIG_SPL_DFU=y # CONFIG_CMD_MMC is not set # CONFIG_CMD_FAT is not set # CONFIG_MMC_SDHCI is not set +# CONFIG_SPL_MTD is not set +# CONFIG_DMA_CHANNELS is not set

+# CONFIG_TI_K3_NAVSS_UDMA is not set

2.43.0

Hi Siddharth,

Thank you for the patch.

On mar., déc. 17, 2024 at 18:46, Siddharth Vadapalli s-vadapalli@ti.com wrote:

Enable USB0 instance of the USB controller for USB DFU boot functionality which requires the "bootph-all" property in order to have USB enabled at all stages of the USB DFU boot.

Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com

Reviewed-by: Mattijs Korpershoek mkorpershoek@baylibre.com

---

arch/arm/dts/k3-am62p5-sk-u-boot.dtsi | 10 ++++++++++ 1 file changed, 10 insertions(+)

diff --git a/arch/arm/dts/k3-am62p5-sk-u-boot.dtsi b/arch/arm/dts/k3-am62p5-sk-u-boot.dtsi index cf087c6e343..c2091fc1a51 100644 --- a/arch/arm/dts/k3-am62p5-sk-u-boot.dtsi +++ b/arch/arm/dts/k3-am62p5-sk-u-boot.dtsi @@ -16,3 +16,13 @@ &dmsc { bootph-pre-ram; };

+/* Required for USB DFU boot */ +&usbss0 {

• status = "okay";

+};

+/* Required for USB DFU boot */ +&usb0 {

• bootph-all;

+};

2.43.0