[PATCH v4 0/7] Add DFU and usb boot for TI am62x SK and beagleplay

[PATCH v4 0/7] Add DFU and usb boot for TI am62x SK and beagleplay

[Sjoerd Simons]

This series adds DFU support for TI AM62 SK board and new since this
version also for beagleplay.

Since the last revision, apart from beagleplay support the main changes
are:
* Documentation was added on how to use this functionality on both boards
* the R5 configuration is now done via a config fragment rather
  then a full configuraiton
* A new patch was added for dwc3 to work in device mode even if dr_mode is
  configured as otg. Avoiding the need for an explicit switch to
  periphal in the u-boot dts

I will also submit the dts changes to linux so that those can be dropped
again in the near future (hopefully)

Changes in v4:
- Add config dependency on SYSCON
- Move defines and constants outside out of function scope
- Don't force usb0 into peripheral mode
- Move R5 dfu config to a config fragment rather then a full defconfig
- Don't enable XHCI for the R5 SPL, unneeded

Change in v3:
- Add dfu via environment rather then config headers
- Enable usb nodes in all boot phases
- Run savedefconfig to adjust to more recent u-boot

Changes in v2:
- Switch dwc3 glue to a seperate driver rather then in dwc-generic
- Minimize config changes to just DFU configuration
- Only enable usb port 0 DFU in SPL
- Create a seperate defconfig for R5

Sjoerd Simons (7):
  usb: dwc3: Add dwc3 glue driver for am62
  usb: dwc3: Switch to device mode on gadget start
  board: ti: am62x: am62x: include env for DFU
  arm: dts: k3-am625-sk: Enable usb port in u-boot
  configs: am62x_evm_*: Enable USB and DFU support
  beagleplay: Add DFU support
  doc: board: Add document for DFU boot on am62x SoCs

 arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |   8 ++
 arch/arm/dts/k3-am625-sk-u-boot.dtsi         |   8 ++
 board/beagle/beagleplay/beagleplay.env       |   1 +
 board/ti/am62x/am62x.env                     |   1 +
 configs/am62x_beagleplay_a53_defconfig       |  30 +++++
 configs/am62x_evm_a53_defconfig              |  30 +++++
 configs/am62x_r5_usbdfu.config               |  28 +++++
 doc/board/beagle/am62x_beagleplay.rst        |  12 ++
 doc/board/ti/am62x_sk.rst                    |  37 ++++++
 drivers/usb/dwc3/Kconfig                     |  14 +++
 drivers/usb/dwc3/Makefile                    |   1 +
 drivers/usb/dwc3/core.c                      |  10 +-
 drivers/usb/dwc3/core.h                      |   1 +
 drivers/usb/dwc3/dwc3-am62.c                 | 125 +++++++++++++++++++
 drivers/usb/dwc3/gadget.c                    |   6 +
 15 files changed, 307 insertions(+), 5 deletions(-)
 create mode 100644 configs/am62x_r5_usbdfu.config
 create mode 100644 drivers/usb/dwc3/dwc3-am62.c

-- 
2.43.0

[PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62

[Sjoerd Simons]

Add glue code for TI AM62 to the dwc3 driver; Most code adopted from
TI vendor u-boot code.

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- Add config dependency on SYSCON
- Move defines and constants outside out of function scope

Changes in v2:
- Switch dwc3 glue to a seperate driver rather then in dwc-generic

 drivers/usb/dwc3/Kconfig     |  14 ++++
 drivers/usb/dwc3/Makefile    |   1 +
 drivers/usb/dwc3/dwc3-am62.c | 125 +++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+)
 create mode 100644 drivers/usb/dwc3/dwc3-am62.c

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index c0c8c16fd9c..0100723a68b 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -37,6 +37,20 @@ config SPL_USB_DWC3_GENERIC
 	  Select this for Xilinx ZynqMP and similar Platforms.
 	  This wrapper supports Host and Peripheral operation modes.
 
+config SPL_USB_DWC3_AM62
+	bool "TI AM62 USB wrapper"
+	depends on SPL_DM_USB && SPL_USB_DWC3_GENERIC && SPL_SYSCON
+	help
+	  Select this for TI AM62 Platforms.
+	  This wrapper supports Host and Peripheral operation modes.
+
+config USB_DWC3_AM62
+	bool "TI AM62 USB wrapper"
+	depends on DM_USB && USB_DWC3_GENERIC && SYSCON
+	help
+	  Select this for TI AM62 Platforms.
+	  This wrapper supports Host and Peripheral operation modes.
+
 config USB_DWC3_MESON_G12A
 	bool "Amlogic Meson G12A USB wrapper"
 	depends on DM_USB && USB_DWC3 && ARCH_MESON
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 97b4f7191ca..a46b6824ab7 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -6,6 +6,7 @@ dwc3-y					:= core.o
 
 obj-$(CONFIG_USB_DWC3_GADGET)		+= gadget.o ep0.o
 
+obj-$(CONFIG_$(SPL_)USB_DWC3_AM62)	+= dwc3-am62.o
 obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
 obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
 obj-$(CONFIG_USB_DWC3_MESON_GXL)	+= dwc3-meson-gxl.o
diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
new file mode 100644
index 00000000000..99519602eb2
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-am62.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * TI AM62 specific glue layer for DWC3
+ */
+
+#include <dm.h>
+#include <dm/device_compat.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/io.h>
+
+#include "dwc3-generic.h"
+
+#define USBSS_MODE_CONTROL		0x1c
+#define USBSS_PHY_CONFIG		0x8
+#define USBSS_PHY_VBUS_SEL_MASK		GENMASK(2, 1)
+#define USBSS_PHY_VBUS_SEL_SHIFT	1
+#define USBSS_MODE_VALID	BIT(0)
+#define PHY_PLL_REFCLK_MASK	GENMASK(3, 0)
+static const int dwc3_ti_am62_rate_table[] = {	/* in KHZ */
+	9600,
+	10000,
+	12000,
+	19200,
+	20000,
+	24000,
+	25000,
+	26000,
+	38400,
+	40000,
+	58000,
+	50000,
+	52000,
+};
+
+static void dwc3_ti_am62_glue_configure(struct udevice *dev, int index,
+					enum usb_dr_mode mode)
+{
+	struct clk usb2_refclk;
+	int rate_code, i, ret;
+	unsigned long rate;
+	u32 reg;
+	void *usbss;
+	bool vbus_divider;
+	struct regmap *syscon;
+	struct ofnode_phandle_args args;
+
+	usbss = dev_remap_addr_index(dev, 0);
+	if (IS_ERR(usbss)) {
+		dev_err(dev, "can't map IOMEM resource\n");
+		return;
+	}
+
+	ret = clk_get_by_name(dev, "ref", &usb2_refclk);
+	if (ret) {
+		dev_err(dev, "can't get usb2_refclk\n");
+		return;
+	}
+
+	/* Calculate the rate code */
+	rate = clk_get_rate(&usb2_refclk);
+	rate /= 1000;	/* To KHz */
+	for (i = 0; i < ARRAY_SIZE(dwc3_ti_am62_rate_table); i++) {
+		if (dwc3_ti_am62_rate_table[i] == rate)
+			break;
+	}
+
+	if (i == ARRAY_SIZE(dwc3_ti_am62_rate_table)) {
+		dev_err(dev, "unsupported usb2_refclk rate: %lu KHz\n", rate);
+		return;
+	}
+
+	rate_code = i;
+
+	/* Read the syscon property */
+	syscon = syscon_regmap_lookup_by_phandle(dev, "ti,syscon-phy-pll-refclk");
+	if (IS_ERR(syscon)) {
+		dev_err(dev, "unable to get ti,syscon-phy-pll-refclk regmap\n");
+		return;
+	}
+
+	ret = ofnode_parse_phandle_with_args(dev_ofnode(dev), "ti,syscon-phy-pll-refclk", NULL, 1,
+					     0, &args);
+	if (ret)
+		return;
+
+	/* Program PHY PLL refclk by reading syscon property */
+	ret = regmap_update_bits(syscon, args.args[0], PHY_PLL_REFCLK_MASK, rate_code);
+	if (ret) {
+		dev_err(dev, "failed to set phy pll reference clock rate\n");
+		return;
+	}
+
+	/* VBUS divider select */
+	reg = readl(usbss + USBSS_PHY_CONFIG);
+	vbus_divider = dev_read_bool(dev, "ti,vbus-divider");
+	if (vbus_divider)
+		reg |= 1 << USBSS_PHY_VBUS_SEL_SHIFT;
+
+	writel(reg, usbss + USBSS_PHY_CONFIG);
+
+	/* Set mode valid */
+	reg = readl(usbss + USBSS_MODE_CONTROL);
+	reg |= USBSS_MODE_VALID;
+	writel(reg, usbss + USBSS_MODE_CONTROL);
+}
+
+struct dwc3_glue_ops ti_am62_ops = {
+	.glue_configure = dwc3_ti_am62_glue_configure,
+};
+
+static const struct udevice_id dwc3_am62_match[] = {
+	{ .compatible = "ti,am62-usb", .data = (ulong)&ti_am62_ops },
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(dwc3_am62_wrapper) = {
+	.name	= "dwc3-am62",
+	.id	= UCLASS_SIMPLE_BUS,
+	.of_match = dwc3_am62_match,
+	.bind = dwc3_glue_bind,
+	.probe = dwc3_glue_probe,
+	.remove = dwc3_glue_remove,
+	.plat_auto	= sizeof(struct dwc3_glue_data),
+};
-- 
2.43.0

Re: [PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Add glue code for TI AM62 to the dwc3 driver; Most code adopted from
> TI vendor u-boot code.
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
>
> ---
>
> Changes in v4:
> - Add config dependency on SYSCON
> - Move defines and constants outside out of function scope
>
> Changes in v2:
> - Switch dwc3 glue to a seperate driver rather then in dwc-generic
>
>  drivers/usb/dwc3/Kconfig     |  14 ++++
>  drivers/usb/dwc3/Makefile    |   1 +
>  drivers/usb/dwc3/dwc3-am62.c | 125 +++++++++++++++++++++++++++++++++++
>  3 files changed, 140 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-am62.c
>
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index c0c8c16fd9c..0100723a68b 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -37,6 +37,20 @@ config SPL_USB_DWC3_GENERIC
>  	  Select this for Xilinx ZynqMP and similar Platforms.
>  	  This wrapper supports Host and Peripheral operation modes.
>  
> +config SPL_USB_DWC3_AM62
> +	bool "TI AM62 USB wrapper"
> +	depends on SPL_DM_USB && SPL_USB_DWC3_GENERIC && SPL_SYSCON
> +	help
> +	  Select this for TI AM62 Platforms.
> +	  This wrapper supports Host and Peripheral operation modes.
> +
> +config USB_DWC3_AM62
> +	bool "TI AM62 USB wrapper"
> +	depends on DM_USB && USB_DWC3_GENERIC && SYSCON
> +	help
> +	  Select this for TI AM62 Platforms.
> +	  This wrapper supports Host and Peripheral operation modes.
> +
>  config USB_DWC3_MESON_G12A
>  	bool "Amlogic Meson G12A USB wrapper"
>  	depends on DM_USB && USB_DWC3 && ARCH_MESON
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index 97b4f7191ca..a46b6824ab7 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -6,6 +6,7 @@ dwc3-y					:= core.o
>  
>  obj-$(CONFIG_USB_DWC3_GADGET)		+= gadget.o ep0.o
>  
> +obj-$(CONFIG_$(SPL_)USB_DWC3_AM62)	+= dwc3-am62.o
>  obj-$(CONFIG_USB_DWC3_OMAP)		+= dwc3-omap.o
>  obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
>  obj-$(CONFIG_USB_DWC3_MESON_GXL)	+= dwc3-meson-gxl.o
> diff --git a/drivers/usb/dwc3/dwc3-am62.c b/drivers/usb/dwc3/dwc3-am62.c
> new file mode 100644
> index 00000000000..99519602eb2
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-am62.c
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI AM62 specific glue layer for DWC3
> + */
> +
> +#include <dm.h>
> +#include <dm/device_compat.h>
> +#include <regmap.h>
> +#include <syscon.h>
> +#include <asm/io.h>
> +
> +#include "dwc3-generic.h"
> +
> +#define USBSS_MODE_CONTROL		0x1c
> +#define USBSS_PHY_CONFIG		0x8
> +#define USBSS_PHY_VBUS_SEL_MASK		GENMASK(2, 1)
> +#define USBSS_PHY_VBUS_SEL_SHIFT	1
> +#define USBSS_MODE_VALID	BIT(0)
> +#define PHY_PLL_REFCLK_MASK	GENMASK(3, 0)
> +static const int dwc3_ti_am62_rate_table[] = {	/* in KHZ */
> +	9600,
> +	10000,
> +	12000,
> +	19200,
> +	20000,
> +	24000,
> +	25000,
> +	26000,
> +	38400,
> +	40000,
> +	58000,
> +	50000,
> +	52000,
> +};
> +
> +static void dwc3_ti_am62_glue_configure(struct udevice *dev, int index,
> +					enum usb_dr_mode mode)
> +{
> +	struct clk usb2_refclk;
> +	int rate_code, i, ret;
> +	unsigned long rate;
> +	u32 reg;
> +	void *usbss;
> +	bool vbus_divider;
> +	struct regmap *syscon;
> +	struct ofnode_phandle_args args;
> +
> +	usbss = dev_remap_addr_index(dev, 0);
> +	if (IS_ERR(usbss)) {
> +		dev_err(dev, "can't map IOMEM resource\n");
> +		return;
> +	}
> +
> +	ret = clk_get_by_name(dev, "ref", &usb2_refclk);
> +	if (ret) {
> +		dev_err(dev, "can't get usb2_refclk\n");
> +		return;
> +	}
> +
> +	/* Calculate the rate code */
> +	rate = clk_get_rate(&usb2_refclk);
> +	rate /= 1000;	/* To KHz */
> +	for (i = 0; i < ARRAY_SIZE(dwc3_ti_am62_rate_table); i++) {
> +		if (dwc3_ti_am62_rate_table[i] == rate)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(dwc3_ti_am62_rate_table)) {
> +		dev_err(dev, "unsupported usb2_refclk rate: %lu KHz\n", rate);
> +		return;
> +	}
> +
> +	rate_code = i;
> +
> +	/* Read the syscon property */
> +	syscon = syscon_regmap_lookup_by_phandle(dev, "ti,syscon-phy-pll-refclk");
> +	if (IS_ERR(syscon)) {
> +		dev_err(dev, "unable to get ti,syscon-phy-pll-refclk regmap\n");
> +		return;
> +	}
> +
> +	ret = ofnode_parse_phandle_with_args(dev_ofnode(dev), "ti,syscon-phy-pll-refclk", NULL, 1,
> +					     0, &args);
> +	if (ret)
> +		return;
> +
> +	/* Program PHY PLL refclk by reading syscon property */
> +	ret = regmap_update_bits(syscon, args.args[0], PHY_PLL_REFCLK_MASK, rate_code);
> +	if (ret) {

The doc of ofnode_parse_phandle_with_args() states that:

 * Caller is responsible to call of_node_put() on the returned out_args->np
 * pointer.

Should we call of_node_put(args->np); before returning here?

Should the cleanup be done in case of success as well?

With that fixed:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> +		dev_err(dev, "failed to set phy pll reference clock rate\n");
> +		return;
> +	}
> +
> +	/* VBUS divider select */
> +	reg = readl(usbss + USBSS_PHY_CONFIG);
> +	vbus_divider = dev_read_bool(dev, "ti,vbus-divider");
> +	if (vbus_divider)
> +		reg |= 1 << USBSS_PHY_VBUS_SEL_SHIFT;
> +
> +	writel(reg, usbss + USBSS_PHY_CONFIG);
> +
> +	/* Set mode valid */
> +	reg = readl(usbss + USBSS_MODE_CONTROL);
> +	reg |= USBSS_MODE_VALID;
> +	writel(reg, usbss + USBSS_MODE_CONTROL);
> +}
> +
> +struct dwc3_glue_ops ti_am62_ops = {
> +	.glue_configure = dwc3_ti_am62_glue_configure,
> +};
> +
> +static const struct udevice_id dwc3_am62_match[] = {
> +	{ .compatible = "ti,am62-usb", .data = (ulong)&ti_am62_ops },
> +	{ /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(dwc3_am62_wrapper) = {
> +	.name	= "dwc3-am62",
> +	.id	= UCLASS_SIMPLE_BUS,
> +	.of_match = dwc3_am62_match,
> +	.bind = dwc3_glue_bind,
> +	.probe = dwc3_glue_probe,
> +	.remove = dwc3_glue_remove,
> +	.plat_auto	= sizeof(struct dwc3_glue_data),
> +};
> -- 
> 2.43.0

Re: [PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62

[Martyn Welch]

On Tue, 2024-01-16 at 11:22 +0100, Mattijs Korpershoek wrote:
> > +	/* Program PHY PLL refclk by reading syscon property */
> > +	ret = regmap_update_bits(syscon, args.args[0],
> > PHY_PLL_REFCLK_MASK, rate_code);
> > +	if (ret) {
> 
> The doc of ofnode_parse_phandle_with_args() states that:
> 
>  * Caller is responsible to call of_node_put() on the returned
> out_args->np
>  * pointer.
> 
> Should we call of_node_put(args->np); before returning here?
> 

It doesn't seem that this is done in U-Boot as the definition of
of_node_put() here is:

   /* Dummy functions to mirror Linux. These are not used in U-Boot */
   #define of_node_get(x) (x)
   static inline void of_node_put(const struct device_node *np) { }

Martyn

> Should the cleanup be done in case of success as well?
> 
> With that fixed:
> 
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
> 

Re: [PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62

[Mattijs Korpershoek]

Hi Martyn,

On mer., mai 01, 2024 at 14:56, Martyn Welch <martyn.welch@collabora.com> wrote:

> On Tue, 2024-01-16 at 11:22 +0100, Mattijs Korpershoek wrote:
>> > +	/* Program PHY PLL refclk by reading syscon property */
>> > +	ret = regmap_update_bits(syscon, args.args[0],
>> > PHY_PLL_REFCLK_MASK, rate_code);
>> > +	if (ret) {
>> 
>> The doc of ofnode_parse_phandle_with_args() states that:
>> 
>>  * Caller is responsible to call of_node_put() on the returned
>> out_args->np
>>  * pointer.
>> 
>> Should we call of_node_put(args->np); before returning here?
>> 
>
> It doesn't seem that this is done in U-Boot as the definition of
> of_node_put() here is:
>
>    /* Dummy functions to mirror Linux. These are not used in U-Boot */
>    #define of_node_get(x) (x)
>    static inline void of_node_put(const struct device_node *np) { }

Indeed, thank you for looking into this.

In that case,

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>
> Martyn
>
>> Should the cleanup be done in case of success as well?
>> 
>> With that fixed:
>> 
>> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
>> 

Re: [PATCH v4 1/7] usb: dwc3: Add dwc3 glue driver for am62

[Sverdlin, Alexander]

Hi Sjoerd!

Thank you for your efforts on the topic!

On Fri, 2024-01-12 at 09:52 +0100, Sjoerd Simons wrote:
> Add glue code for TI AM62 to the dwc3 driver; Most code adopted from
> TI vendor u-boot code.
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Works for me on AM6232:

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> 
> Changes in v4:
> - Add config dependency on SYSCON
> - Move defines and constants outside out of function scope
> 
> Changes in v2:
> - Switch dwc3 glue to a seperate driver rather then in dwc-generic
> 
>  drivers/usb/dwc3/Kconfig     |  14 ++++
>  drivers/usb/dwc3/Makefile    |   1 +
>  drivers/usb/dwc3/dwc3-am62.c | 125 +++++++++++++++++++++++++++++++++++
>  3 files changed, 140 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-am62.c

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

[PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Sjoerd Simons]

When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
However in this mode the gadget functionality doesn't work without
further configuration. To resolve that on gadget start switch to _DEVICE
mode globally and go back to _OTG on stop again.

For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
conflict with the same function exposed by xhci-dwc3

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- New patch

 drivers/usb/dwc3/core.c   | 10 +++++-----
 drivers/usb/dwc3/core.h   |  1 +
 drivers/usb/dwc3/gadget.c |  6 ++++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4b4fcd8a22e..d22d4c4bb6a 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -42,7 +42,7 @@
 static LIST_HEAD(dwc3_list);
 /* -------------------------------------------------------------------------- */
 
-static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
+void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode)
 {
 	u32 reg;
 
@@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 
 	switch (dwc->dr_mode) {
 	case USB_DR_MODE_PERIPHERAL:
-		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 		ret = dwc3_gadget_init(dwc);
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize gadget\n");
@@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		}
 		break;
 	case USB_DR_MODE_HOST:
-		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
+		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
@@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
 		}
 		break;
 	case USB_DR_MODE_OTG:
-		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
 		ret = dwc3_host_init(dwc);
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
@@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 	 * switch back to peripheral mode
 	 * This enables the phy to enter idle and then, if enabled, suspend.
 	 */
-	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+	dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 	dwc3_gadget_run(dwc);
 }
 
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 4162a682298..1e7eda89a34 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
 void dwc3_of_parse(struct dwc3 *dwc);
 int dwc3_init(struct dwc3 *dwc);
 void dwc3_remove(struct dwc3 *dwc);
+void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode);
 
 static inline int dwc3_host_init(struct dwc3 *dwc)
 { return 0; }
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 406d36ceafe..69d9fe40e2f 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
 
 	dwc->gadget_driver	= driver;
 
+	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
+		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
+
 	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
 	reg &= ~(DWC3_DCFG_SPEED_MASK);
 
@@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
 	__dwc3_gadget_ep_disable(dwc->eps[0]);
 	__dwc3_gadget_ep_disable(dwc->eps[1]);
 
+	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
+		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
+
 	dwc->gadget_driver	= NULL;
 
 	spin_unlock_irqrestore(&dwc->lock, flags);
-- 
2.43.0

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Roger Quadros]

On 12/01/2024 10:52, Sjoerd Simons wrote:
> When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
> However in this mode the gadget functionality doesn't work without
> further configuration. To resolve that on gadget start switch to _DEVICE
> mode globally and go back to _OTG on stop again.
> 
> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
> conflict with the same function exposed by xhci-dwc3

Aren't they both doing the same thing? I'd rather define them at one place
i.e. dwc3/core.c.

The driver model design for dwc3 looks really broken in u-boot.

"snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by dwc3/core.c.
This is probably because at the time USB host was the more needed use
case at u-boot.

Ideally dwc3/core.c should claim "snps,dwc3" device and instantiate
the respective Host/Device/OTG device based on the provided otg mode.

For Host/Device mode it is straight forward.
dwc3/core.c does dwc3_set_mode() depending on the mode and:
for host mode -> register xhci-usb driver.
for device mode -> register UDC device driver.

For dual-role mode, the solution is not very clear as I don't think
there is a role switch framework present.

To begin with, we could probably restrict it to just device mode
as most platforms were forcing it to device mode in any case as they
usually have a 2nd USB port that is host only.

A better solution would be to introduce a role switch mechanism 
like it exists in Linux so user can choose what mode he wants to
operate. That could wait till someone really wants such a feature.

> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  drivers/usb/dwc3/core.c   | 10 +++++-----
>  drivers/usb/dwc3/core.h   |  1 +
>  drivers/usb/dwc3/gadget.c |  6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4b4fcd8a22e..d22d4c4bb6a 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -42,7 +42,7 @@
>  static LIST_HEAD(dwc3_list);
>  /* -------------------------------------------------------------------------- */
>  
> -static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode)
>  {
>  	u32 reg;
>  
> @@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  
>  	switch (dwc->dr_mode) {
>  	case USB_DR_MODE_PERIPHERAL:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize gadget\n");
> @@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_HOST:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_OTG:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>  	 * switch back to peripheral mode
>  	 * This enables the phy to enter idle and then, if enabled, suspend.
>  	 */
> -	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +	dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  	dwc3_gadget_run(dwc);
>  }
>  
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4162a682298..1e7eda89a34 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>  void dwc3_of_parse(struct dwc3 *dwc);
>  int dwc3_init(struct dwc3 *dwc);
>  void dwc3_remove(struct dwc3 *dwc);
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode);
>  
>  static inline int dwc3_host_init(struct dwc3 *dwc)
>  { return 0; }
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 406d36ceafe..69d9fe40e2f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>  
>  	dwc->gadget_driver	= driver;
>  
> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +
>  	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>  	reg &= ~(DWC3_DCFG_SPEED_MASK);
>  
> @@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>  	__dwc3_gadget_ep_disable(dwc->eps[0]);
>  	__dwc3_gadget_ep_disable(dwc->eps[1]);
>  
> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +
>  	dwc->gadget_driver	= NULL;
>  
>  	spin_unlock_irqrestore(&dwc->lock, flags);

-- 
cheers,
-roger

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Sjoerd Simons]

On Fri, 2024-01-12 at 12:39 +0200, Roger Quadros wrote:
> 
> 
> On 12/01/2024 10:52, Sjoerd Simons wrote:
> > When dr_mode is "otg" the dwc3 is initially configured in _OTG
> > mode;
> > However in this mode the gadget functionality doesn't work without
> > further configuration. To resolve that on gadget start switch to
> > _DEVICE
> > mode globally and go back to _OTG on stop again.
> > 
> > For this the dwc3_set_mode is renamed to dwc3_core_set_mode to
> > avoid a
> > conflict with the same function exposed by xhci-dwc3
> 
> Aren't they both doing the same thing? I'd rather define them at one
> place
> i.e. dwc3/core.c.

They twiddle the same registers afaict indeed; but the way to get there
is rather different. So having just one for both really needs a bigger
amount of rework.

> The driver model design for dwc3 looks really broken in u-boot.
> 
> "snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by
> dwc3/core.c.
> This is probably because at the time USB host was the more needed use
> case at u-boot.
> 
> Ideally dwc3/core.c should claim "snps,dwc3" device and instantiate
> the respective Host/Device/OTG device based on the provided otg mode.
> 
> For Host/Device mode it is straight forwa
> dwc3/core.c does dwc3_set_mode() depending on the mode and:
> for host mode -> register xhci-usb driver.
> for device mode -> register UDC device driver.
> 
> For dual-role mode, the solution is not very clear as I don't think
> there is a role switch framework present
> 
> To begin with, we could probably restrict it to just device mode
> as most platforms were forcing it to device mode in any case as they
> usually have a 2nd USB port that is host only.

Right we don't have role switching; And if we had we'd have to add
support of the Type-C controller on the SK boards which determines the
roles for this.

This one more minimal patch was modelled after the discussion last year
around otg mode[0]. And similar to how the cdns3 driver handles it.
Essentially letting host/gadget mode be determined by the usage rather
then the cables plugged, which is not pretty but works.

While I agree with you this could all be a lot nicer of u-boot did
"proper" OTG/role-switching; I unfortunately don't have the bandwidth
to introduce all of that and it seems a shame to block DFU support on
it. For reference my previous series just set the peripheral role in
the dts which reflects your suggestion of forcing device mode. I'd be
wary to do so in the driver because i would hope the OTG handling in
dwc3 is there for a reason :) 


0: https://lists.denx.de/pipermail/u-boot/2023-August/527709.html


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Roger Quadros]

On 12/01/2024 13:06, Sjoerd Simons wrote:
> On Fri, 2024-01-12 at 12:39 +0200, Roger Quadros wrote:
>>
>>
>> On 12/01/2024 10:52, Sjoerd Simons wrote:
>>> When dr_mode is "otg" the dwc3 is initially configured in _OTG
>>> mode;
>>> However in this mode the gadget functionality doesn't work without
>>> further configuration. To resolve that on gadget start switch to
>>> _DEVICE
>>> mode globally and go back to _OTG on stop again.
>>>
>>> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to
>>> avoid a
>>> conflict with the same function exposed by xhci-dwc3
>>
>> Aren't they both doing the same thing? I'd rather define them at one
>> place
>> i.e. dwc3/core.c.
> 
> They twiddle the same registers afaict indeed; but the way to get there
> is rather different. So having just one for both really needs a bigger
> amount of rework.
> 
>> The driver model design for dwc3 looks really broken in u-boot.
>>
>> "snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by
>> dwc3/core.c.
>> This is probably because at the time USB host was the more needed use
>> case at u-boot.
>>
>> Ideally dwc3/core.c should claim "snps,dwc3" device and instantiate
>> the respective Host/Device/OTG device based on the provided otg mode.
>>
>> For Host/Device mode it is straight forwa
>> dwc3/core.c does dwc3_set_mode() depending on the mode and:
>> for host mode -> register xhci-usb driver.
>> for device mode -> register UDC device driver.
>>
>> For dual-role mode, the solution is not very clear as I don't think
>> there is a role switch framework present
>>
>> To begin with, we could probably restrict it to just device mode
>> as most platforms were forcing it to device mode in any case as they
>> usually have a 2nd USB port that is host only.
> 
> Right we don't have role switching; And if we had we'd have to add
> support of the Type-C controller on the SK boards which determines the
> roles for this.
> 
> This one more minimal patch was modelled after the discussion last year
> around otg mode[0]. And similar to how the cdns3 driver handles it.
> Essentially letting host/gadget mode be determined by the usage rather
> then the cables plugged, which is not pretty but works.
> 
> While I agree with you this could all be a lot nicer of u-boot did
> "proper" OTG/role-switching; I unfortunately don't have the bandwidth
> to introduce all of that and it seems a shame to block DFU support on
> it. For reference my previous series just set the peripheral role in
> the dts which reflects your suggestion of forcing device mode. I'd be
> wary to do so in the driver because i would hope the OTG handling in
> dwc3 is there for a reason :) 
> 
> 
> 0: https://lists.denx.de/pipermail/u-boot/2023-August/527709.html
> 
> 

In your current series, how does it work?
What happens if user starts both host and device controllers?

e.g.
> usb start
> dfu 0

-- 
cheers,
-roger

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Sjoerd Simons]

On Fri, 2024-01-12 at 14:56 +0200, Roger Quadros wrote:
> 
> 
> On 12/01/2024 13:06, Sjoerd Simons wrote:
> > On Fri, 2024-01-12 at 12:39 +0200, Roger Quadros wrote:
> > > 
> > > 
> > > On 12/01/2024 10:52, Sjoerd Simons wrote:
> > > > When dr_mode is "otg" the dwc3 is initially configured in _OTG
> > > > mode;
> > > > However in this mode the gadget functionality doesn't work
> > > > without
> > > > further configuration. To resolve that on gadget start switch
> > > > to
> > > > _DEVICE
> > > > mode globally and go back to _OTG on stop again.
> > > > 
> > > > For this the dwc3_set_mode is renamed to dwc3_core_set_mode to
> > > > avoid a
> > > > conflict with the same function exposed by xhci-dwc3
> > > 
> > > Aren't they both doing the same thing? I'd rather define them at
> > > one
> > > place
> > > i.e. dwc3/core.c.
> > 
> > They twiddle the same registers afaict indeed; but the way to get
> > there
> > is rather different. So having just one for both really needs a
> > bigger
> > amount of rework.
> > 
> > > The driver model design for dwc3 looks really broken in u-boot.
> > > 
> > > "snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by
> > > dwc3/core.c.
> > > This is probably because at the time USB host was the more needed
> > > use
> > > case at u-boot.
> > > 
> > > Ideally dwc3/core.c should claim "snps,dwc3" device and
> > > instantiate
> > > the respective Host/Device/OTG device based on the provided otg
> > > mode.
> > > 
> > > For Host/Device mode it is straight forwa
> > > dwc3/core.c does dwc3_set_mode() depending on the mode and:
> > > for host mode -> register xhci-usb driver.
> > > for device mode -> register UDC device driver.
> > > 
> > > For dual-role mode, the solution is not very clear as I don't
> > > think
> > > there is a role switch framework present
> > > 
> > > To begin with, we could probably restrict it to just device mode
> > > as most platforms were forcing it to device mode in any case as
> > > they
> > > usually have a 2nd USB port that is host only.
> > 
> > Right we don't have role switching; And if we had we'd have to add
> > support of the Type-C controller on the SK boards which determines
> > the
> > roles for this.
> > 
> > This one more minimal patch was modelled after the discussion last
> > year
> > around otg mode[0]. And similar to how the cdns3 driver handles it.
> > Essentially letting host/gadget mode be determined by the usage
> > rather
> > then the cables plugged, which is not pretty but works.
> > 
> > While I agree with you this could all be a lot nicer of u-boot did
> > "proper" OTG/role-switching; I unfortunately don't have the
> > bandwidth
> > to introduce all of that and it seems a shame to block DFU support
> > on
> > it. For reference my previous series just set the peripheral role
> > in
> > the dts which reflects your suggestion of forcing device mode. I'd
> > be
> > wary to do so in the driver because i would hope the OTG handling
> > in
> > dwc3 is there for a reason :) 
> > 
> > 
> > 0: https://lists.denx.de/pipermail/u-boot/2023-August/527709.html
> > 
> > 
> 
> In your current series, how does it work?
> What happens if user starts both host and device controllers?

Testing host functionality on the same port as used for DFU (the only
one that's otg) doesn't seem to work at all, not sure why that is. host
on usb1 (usb-a port, host only) works fine ofcourse.

practically however once the user has used a gadget it's likely they'd
have to reset the host side (or at least do a rescan) though as likely
they'll have also switched around the cable :).

> 
> e.g.
> > usb start
> > dfu 0

That seems to work fine, modulo usb0 not seeing attached devices, but
that's not a regression introduced by this patch.



-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Roger Quadros]

On 12/01/2024 16:18, Sjoerd Simons wrote:
> On Fri, 2024-01-12 at 14:56 +0200, Roger Quadros wrote:
>>
>>
>> On 12/01/2024 13:06, Sjoerd Simons wrote:
>>> On Fri, 2024-01-12 at 12:39 +0200, Roger Quadros wrote:
>>>>
>>>>
>>>> On 12/01/2024 10:52, Sjoerd Simons wrote:
>>>>> When dr_mode is "otg" the dwc3 is initially configured in _OTG
>>>>> mode;
>>>>> However in this mode the gadget functionality doesn't work
>>>>> without
>>>>> further configuration. To resolve that on gadget start switch
>>>>> to
>>>>> _DEVICE
>>>>> mode globally and go back to _OTG on stop again.
>>>>>
>>>>> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to
>>>>> avoid a
>>>>> conflict with the same function exposed by xhci-dwc3
>>>>
>>>> Aren't they both doing the same thing? I'd rather define them at
>>>> one
>>>> place
>>>> i.e. dwc3/core.c.
>>>
>>> They twiddle the same registers afaict indeed; but the way to get
>>> there
>>> is rather different. So having just one for both really needs a
>>> bigger
>>> amount of rework.
>>>
>>>> The driver model design for dwc3 looks really broken in u-boot.
>>>>
>>>> "snps,dwc3" is claimed by xhci-dwc3.c instead of being claimed by
>>>> dwc3/core.c.
>>>> This is probably because at the time USB host was the more needed
>>>> use
>>>> case at u-boot.
>>>>
>>>> Ideally dwc3/core.c should claim "snps,dwc3" device and
>>>> instantiate
>>>> the respective Host/Device/OTG device based on the provided otg
>>>> mode.
>>>>
>>>> For Host/Device mode it is straight forwa
>>>> dwc3/core.c does dwc3_set_mode() depending on the mode and:
>>>> for host mode -> register xhci-usb driver.
>>>> for device mode -> register UDC device driver.
>>>>
>>>> For dual-role mode, the solution is not very clear as I don't
>>>> think
>>>> there is a role switch framework present
>>>>
>>>> To begin with, we could probably restrict it to just device mode
>>>> as most platforms were forcing it to device mode in any case as
>>>> they
>>>> usually have a 2nd USB port that is host only.
>>>
>>> Right we don't have role switching; And if we had we'd have to add
>>> support of the Type-C controller on the SK boards which determines
>>> the
>>> roles for this.
>>>
>>> This one more minimal patch was modelled after the discussion last
>>> year
>>> around otg mode[0]. And similar to how the cdns3 driver handles it.
>>> Essentially letting host/gadget mode be determined by the usage
>>> rather
>>> then the cables plugged, which is not pretty but works.
>>>
>>> While I agree with you this could all be a lot nicer of u-boot did
>>> "proper" OTG/role-switching; I unfortunately don't have the
>>> bandwidth
>>> to introduce all of that and it seems a shame to block DFU support
>>> on
>>> it. For reference my previous series just set the peripheral role
>>> in
>>> the dts which reflects your suggestion of forcing device mode. I'd
>>> be
>>> wary to do so in the driver because i would hope the OTG handling
>>> in
>>> dwc3 is there for a reason :) 
>>>
>>>
>>> 0: https://lists.denx.de/pipermail/u-boot/2023-August/527709.html
>>>
>>>
>>
>> In your current series, how does it work?
>> What happens if user starts both host and device controllers?
> 
> Testing host functionality on the same port as used for DFU (the only
> one that's otg) doesn't seem to work at all, not sure why that is. host
> on usb1 (usb-a port, host only) works fine ofcourse.
> 
> practically however once the user has used a gadget it's likely they'd
> have to reset the host side (or at least do a rescan) though as likely
> they'll have also switched around the cable :).
> 
>>
>> e.g.
>>> usb start
>>> dfu 0
> 
> That seems to work fine, modulo usb0 not seeing attached devices, but
> that's not a regression introduced by this patch.

am62-sk USB0 was never tested in host mode at u-boot. Let me try out and see
if I can get it to work.

-- 
cheers,
-roger

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Caleb Connolly]

Hi Sjoerd,

On 12/01/2024 08:52, Sjoerd Simons wrote:
> When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
> However in this mode the gadget functionality doesn't work without
> further configuration. To resolve that on gadget start switch to _DEVICE
> mode globally and go back to _OTG on stop again.
> 
> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
> conflict with the same function exposed by xhci-dwc3

I think exporting dwc3_core_set_mode() is probably sensible here. But
I'm not so sure on calling it from dwc3_gadget_start(), that's making
assumptions about board specific implementation details - some boards
might require additional configuration to switch into gadget mode.

What about calling dwc3_core_set_mode() from within your board-specific
board_usb_init() function?

Obviously as you said the ideal solution here is that we can correctly
traverse the type-c connector model from within U-Boot, but that's a
whole other kettle of fish :P

On Qualcomm platforms I'm currently handling this by overriding the
dr_mode property in a <board>-u-boot.dtsi file, if you aren't using the
same DT for U-Boot and Linux then maybe this would be acceptable for you
too?

Kind regards,
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  drivers/usb/dwc3/core.c   | 10 +++++-----
>  drivers/usb/dwc3/core.h   |  1 +
>  drivers/usb/dwc3/gadget.c |  6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4b4fcd8a22e..d22d4c4bb6a 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -42,7 +42,7 @@
>  static LIST_HEAD(dwc3_list);
>  /* -------------------------------------------------------------------------- */
>  
> -static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode)
>  {
>  	u32 reg;
>  
> @@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  
>  	switch (dwc->dr_mode) {
>  	case USB_DR_MODE_PERIPHERAL:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize gadget\n");
> @@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_HOST:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_OTG:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>  	 * switch back to peripheral mode
>  	 * This enables the phy to enter idle and then, if enabled, suspend.
>  	 */
> -	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +	dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  	dwc3_gadget_run(dwc);
>  }
>  
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4162a682298..1e7eda89a34 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>  void dwc3_of_parse(struct dwc3 *dwc);
>  int dwc3_init(struct dwc3 *dwc);
>  void dwc3_remove(struct dwc3 *dwc);
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode);
>  
>  static inline int dwc3_host_init(struct dwc3 *dwc)
>  { return 0; }
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 406d36ceafe..69d9fe40e2f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>  
>  	dwc->gadget_driver	= driver;
>  
> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +
>  	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>  	reg &= ~(DWC3_DCFG_SPEED_MASK);
>  
> @@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>  	__dwc3_gadget_ep_disable(dwc->eps[0]);
>  	__dwc3_gadget_ep_disable(dwc->eps[1]);
>  
> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +
>  	dwc->gadget_driver	= NULL;
>  
>  	spin_unlock_irqrestore(&dwc->lock, flags);

-- 
// Caleb (they/them)

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Mattijs Korpershoek]

Hi Sjoerd, Caleb

On ven., janv. 12, 2024 at 12:55, Caleb Connolly <caleb.connolly@linaro.org> wrote:

> Hi Sjoerd,
>
> On 12/01/2024 08:52, Sjoerd Simons wrote:
>> When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
>> However in this mode the gadget functionality doesn't work without
>> further configuration. To resolve that on gadget start switch to _DEVICE
>> mode globally and go back to _OTG on stop again.
>> 
>> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
>> conflict with the same function exposed by xhci-dwc3
>
> I think exporting dwc3_core_set_mode() is probably sensible here. But
> I'm not so sure on calling it from dwc3_gadget_start(), that's making
> assumptions about board specific implementation details - some boards
> might require additional configuration to switch into gadget mode.

Indeed. As an example, take the Khadas VIM3 which has a glue driver to
do the mode switching.

>
> What about calling dwc3_core_set_mode() from within your board-specific
> board_usb_init() function?

With DM_USB_GADGET, board_usb_init/cleanup() are no longer used.

For VIM3, I solved this with:
https://lore.kernel.org/all/20221024-meson-dm-usb-v1-1-2ab077a503b9@baylibre.com/

Maybe that can help?

>
> Obviously as you said the ideal solution here is that we can correctly
> traverse the type-c connector model from within U-Boot, but that's a
> whole other kettle of fish :P
>
> On Qualcomm platforms I'm currently handling this by overriding the
> dr_mode property in a <board>-u-boot.dtsi file, if you aren't using the
> same DT for U-Boot and Linux then maybe this would be acceptable for you
> too?

I would prefer a <board>-u-boot.dtsi change as well.

>
> Kind regards,
>> 
>> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
>> 
>> ---
>> 
>> Changes in v4:
>> - New patch
>> 
>>  drivers/usb/dwc3/core.c   | 10 +++++-----
>>  drivers/usb/dwc3/core.h   |  1 +
>>  drivers/usb/dwc3/gadget.c |  6 ++++++
>>  3 files changed, 12 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 4b4fcd8a22e..d22d4c4bb6a 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -42,7 +42,7 @@
>>  static LIST_HEAD(dwc3_list);
>>  /* -------------------------------------------------------------------------- */
>>  
>> -static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
>> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode)
>>  {
>>  	u32 reg;
>>  
>> @@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>  
>>  	switch (dwc->dr_mode) {
>>  	case USB_DR_MODE_PERIPHERAL:
>> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>>  		ret = dwc3_gadget_init(dwc);
>>  		if (ret) {
>>  			dev_err(dwc->dev, "failed to initialize gadget\n");
>> @@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>  		}
>>  		break;
>>  	case USB_DR_MODE_HOST:
>> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>>  		ret = dwc3_host_init(dwc);
>>  		if (ret) {
>>  			dev_err(dwc->dev, "failed to initialize host\n");
>> @@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>>  		}
>>  		break;
>>  	case USB_DR_MODE_OTG:
>> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>>  		ret = dwc3_host_init(dwc);
>>  		if (ret) {
>>  			dev_err(dwc->dev, "failed to initialize host\n");
>> @@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>>  	 * switch back to peripheral mode
>>  	 * This enables the phy to enter idle and then, if enabled, suspend.
>>  	 */
>> -	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> +	dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>>  	dwc3_gadget_run(dwc);
>>  }
>>  
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index 4162a682298..1e7eda89a34 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>>  void dwc3_of_parse(struct dwc3 *dwc);
>>  int dwc3_init(struct dwc3 *dwc);
>>  void dwc3_remove(struct dwc3 *dwc);
>> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode);
>>  
>>  static inline int dwc3_host_init(struct dwc3 *dwc)
>>  { return 0; }
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 406d36ceafe..69d9fe40e2f 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>>  
>>  	dwc->gadget_driver	= driver;
>>  
>> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
>> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> +
>>  	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>>  	reg &= ~(DWC3_DCFG_SPEED_MASK);
>>  
>> @@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>>  	__dwc3_gadget_ep_disable(dwc->eps[0]);
>>  	__dwc3_gadget_ep_disable(dwc->eps[1]);
>>  
>> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
>> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>> +
>>  	dwc->gadget_driver	= NULL;
>>  
>>  	spin_unlock_irqrestore(&dwc->lock, flags);
>
> -- 
> // Caleb (they/them)

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
> However in this mode the gadget functionality doesn't work without
> further configuration. To resolve that on gadget start switch to _DEVICE
> mode globally and go back to _OTG on stop again.
>
> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
> conflict with the same function exposed by xhci-dwc3
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

nitpick below.

>
> ---
>
> Changes in v4:
> - New patch
>
>  drivers/usb/dwc3/core.c   | 10 +++++-----
>  drivers/usb/dwc3/core.h   |  1 +
>  drivers/usb/dwc3/gadget.c |  6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4b4fcd8a22e..d22d4c4bb6a 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -42,7 +42,7 @@
>  static LIST_HEAD(dwc3_list);
>  /* -------------------------------------------------------------------------- */
>  
> -static void dwc3_set_mode(struct dwc3 *dwc, u32 mode)
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode)
>  {
>  	u32 reg;
>  
> @@ -736,7 +736,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  
>  	switch (dwc->dr_mode) {
>  	case USB_DR_MODE_PERIPHERAL:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  		ret = dwc3_gadget_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize gadget\n");
> @@ -744,7 +744,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_HOST:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -752,7 +752,7 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
>  		}
>  		break;
>  	case USB_DR_MODE_OTG:
> -		dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> @@ -810,7 +810,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>  	 * switch back to peripheral mode
>  	 * This enables the phy to enter idle and then, if enabled, suspend.
>  	 */
> -	dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +	dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>  	dwc3_gadget_run(dwc);
>  }
>  
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 4162a682298..1e7eda89a34 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1057,6 +1057,7 @@ int dwc3_gadget_resize_tx_fifos(struct dwc3 *dwc);
>  void dwc3_of_parse(struct dwc3 *dwc);
>  int dwc3_init(struct dwc3 *dwc);
>  void dwc3_remove(struct dwc3 *dwc);
> +void dwc3_core_set_mode(struct dwc3 *dwc, u32 mode);
>  
>  static inline int dwc3_host_init(struct dwc3 *dwc)
>  { return 0; }
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 406d36ceafe..69d9fe40e2f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1468,6 +1468,9 @@ static int dwc3_gadget_start(struct usb_gadget *g,
>  
>  	dwc->gadget_driver	= driver;
>  

Doesn't this bit deserves a comment ? When looking at the code it's not
obvious that we do this because PRTCAP_OTG is non-functional without any
additional configuration.

Maybe something in the lines of:

   /**
    * WORKAROUND: in OTG mode, gadget functionality is non-functional.
    * Switch to gadget mode only to enable gadget mode
    */

?

> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> +
>  	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
>  	reg &= ~(DWC3_DCFG_SPEED_MASK);
>  
> @@ -1559,6 +1562,9 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>  	__dwc3_gadget_ep_disable(dwc->eps[0]);
>  	__dwc3_gadget_ep_disable(dwc->eps[1]);
>  
> +	if (dwc->dr_mode == DWC3_GCTL_PRTCAP_OTG)
> +		dwc3_core_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> +
>  	dwc->gadget_driver	= NULL;
>  
>  	spin_unlock_irqrestore(&dwc->lock, flags);
> -- 
> 2.43.0

Re: [PATCH v4 2/7] usb: dwc3: Switch to device mode on gadget start

[Sverdlin, Alexander]

Hi Sjoerd!

On Fri, 2024-01-12 at 09:52 +0100, Sjoerd Simons wrote:
> When dr_mode is "otg" the dwc3 is initially configured in _OTG mode;
> However in this mode the gadget functionality doesn't work without
> further configuration. To resolve that on gadget start switch to _DEVICE
> mode globally and go back to _OTG on stop again.
> 
> For this the dwc3_set_mode is renamed to dwc3_core_set_mode to avoid a
> conflict with the same function exposed by xhci-dwc3
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Works for me with both dr_mode "otg" and "peripheral" on AM6232:

Tested-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> 
> Changes in v4:
> - New patch
> 
>  drivers/usb/dwc3/core.c   | 10 +++++-----
>  drivers/usb/dwc3/core.h   |  1 +
>  drivers/usb/dwc3/gadget.c |  6 ++++++
>  3 files changed, 12 insertions(+), 5 deletions(-)

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

[PATCH v4 3/7] board: ti: am62x: am62x: include env for DFU

[Sjoerd Simons]

Include standard TI K3 dfu environment

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

(no changes since v3)

Changes in v3:
- Add dfu via environment rather then config headers

Changes in v2:
- Minimize config changes to just DFU configuration

 board/ti/am62x/am62x.env | 1 +
 1 file changed, 1 insertion(+)

diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
index e53a55c38fb..0651b9cd7cb 100644
--- a/board/ti/am62x/am62x.env
+++ b/board/ti/am62x/am62x.env
@@ -1,6 +1,7 @@
 #include <env/ti/ti_common.env>
 #include <env/ti/default_findfdt.env>
 #include <env/ti/mmc.env>
+#include <env/ti/k3_dfu.env>
 
 name_kern=Image
 console=ttyS2,115200n8
-- 
2.43.0

Re: [PATCH v4 3/7] board: ti: am62x: am62x: include env for DFU

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Include standard TI K3 dfu environment
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Add dfu via environment rather then config headers
>
> Changes in v2:
> - Minimize config changes to just DFU configuration
>
>  board/ti/am62x/am62x.env | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
> index e53a55c38fb..0651b9cd7cb 100644
> --- a/board/ti/am62x/am62x.env
> +++ b/board/ti/am62x/am62x.env
> @@ -1,6 +1,7 @@
>  #include <env/ti/ti_common.env>
>  #include <env/ti/default_findfdt.env>
>  #include <env/ti/mmc.env>
> +#include <env/ti/k3_dfu.env>
>  
>  name_kern=Image
>  console=ttyS2,115200n8
> -- 
> 2.43.0

Re: [PATCH v4 3/7] board: ti: am62x: am62x: include env for DFU

[Sverdlin, Alexander]

Hi Sjoerd!

On Fri, 2024-01-12 at 09:52 +0100, Sjoerd Simons wrote:
> Include standard TI K3 dfu environment
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Add dfu via environment rather then config headers
> 
> Changes in v2:
> - Minimize config changes to just DFU configuration
> 
>  board/ti/am62x/am62x.env | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/board/ti/am62x/am62x.env b/board/ti/am62x/am62x.env
> index e53a55c38fb..0651b9cd7cb 100644
> --- a/board/ti/am62x/am62x.env
> +++ b/board/ti/am62x/am62x.env
> @@ -1,6 +1,7 @@
>  #include <env/ti/ti_common.env>
>  #include <env/ti/default_findfdt.env>
>  #include <env/ti/mmc.env>
> +#include <env/ti/k3_dfu.env>
>  
>  name_kern=Image
>  console=ttyS2,115200n8

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

[PATCH v4 4/7] arm: dts: k3-am625-sk: Enable usb port in u-boot

[Sjoerd Simons]

Enable usb0 in all boot phases for use with DFU

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- Don't force usb0 into peripheral mode

Changes in v3:
- Enable usb nodes in all boot phases

Changes in v2:
- Only enable usb port 0 DFU in SPL

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

diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
index fa778b0ff4c..67c9fa2bbc3 100644
--- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
@@ -46,3 +46,11 @@
 &cpsw_port2 {
 	status = "disabled";
 };
+
+&usbss0 {
+	bootph-all;
+};
+
+&usb0 {
+	bootph-all;
+};
-- 
2.43.0

Re: [PATCH v4 4/7] arm: dts: k3-am625-sk: Enable usb port in u-boot

[Mattijs Korpershoek]

Hi Sjoerd

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Enable usb0 in all boot phases for use with DFU
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
>
> ---
>
> Changes in v4:
> - Don't force usb0 into peripheral mode

I know that dr_mode = "peripheral" has been removed as requested by
Roger here:

https://lore.kernel.org/all/054aa15f-05b9-4645-ab00-fac1be46dad7@kernel.org/

However, the reason for this was the following series:
https://lore.kernel.org/all/20230706-handle-otg-as-periph-v3-0-27e24fa17345@baylibre.com/

I've spoken to Julien (off-list) about the above series and he confirmed
that there are no plans to keep working this.

Therefore, I'd prefer to re-instate dr_mode = "peripheral" here to make
this series simpler and focus on enabling DFU.

Would that be possible ?

>
> Changes in v3:
> - Enable usb nodes in all boot phases
>
> Changes in v2:
> - Only enable usb port 0 DFU in SPL
>
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index fa778b0ff4c..67c9fa2bbc3 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -46,3 +46,11 @@
>  &cpsw_port2 {
>  	status = "disabled";
>  };
> +
> +&usbss0 {
> +	bootph-all;
> +};
> +
> +&usb0 {
> +	bootph-all;
> +};
> -- 
> 2.43.0

Re: [PATCH v4 4/7] arm: dts: k3-am625-sk: Enable usb port in u-boot

[Sjoerd Simons]

On Tue, 2024-01-16 at 12:17 +0100, Mattijs Korpershoek wrote:
> Hi Sjoerd
> 
> Thank you for the patch.
> 
> On ven., janv. 12, 2024 at 09:52, Sjoerd Simons
> <sjoerd@collabora.com> wrote:
> 
> > Enable usb0 in all boot phases for use with DFU
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> > 
> > ---
> > 
> > Changes in v4:
> > - Don't force usb0 into peripheral mode
> 
> I know that dr_mode = "peripheral" has been removed as requested by
> Roger here:
> 
> https://lore.kernel.org/all/054aa15f-05b9-4645-ab00-fac1be46dad7@kernel.org/
> 
> However, the reason for this was the following series:
> https://lore.kernel.org/all/20230706-handle-otg-as-periph-v3-0-27e24fa17345@baylibre.com/
> 
> I've spoken to Julien (off-list) about the above series and he
> confirmed
> that there are no plans to keep working this.
> 
> Therefore, I'd prefer to re-instate dr_mode = "peripheral" here to
> make
> this series simpler and focus on enabling DFU.
> 
> Would that be possible ?

If others agree sure; I really don't care either way (I wouldn't use
the port as host in any case), I only addressed it because i got a
complaint about it :)

Roger?

> 
> > 
> > Changes in v3:
> > - Enable usb nodes in all boot phases
> > 
> > Changes in v2:
> > - Only enable usb port 0 DFU in SPL
> > 
> >  arch/arm/dts/k3-am625-sk-u-boot.dtsi | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> > b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> > index fa778b0ff4c..67c9fa2bbc3 100644
> > --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> > +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> > @@ -46,3 +46,11 @@
> >  &cpsw_port2 {
> >  	status = "disabled";
> >  };
> > +
> > +&usbss0 {
> > +	bootph-all;
> > +};
> > +
> > +&usb0 {
> > +	bootph-all;
> > +};
> > -- 
> > 2.43.0

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 4/7] arm: dts: k3-am625-sk: Enable usb port in u-boot

[Sverdlin, Alexander]

Hi Sjoerd!

On Fri, 2024-01-12 at 09:52 +0100, Sjoerd Simons wrote:
> Enable usb0 in all boot phases for use with DFU
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

> ---
> 
> Changes in v4:
> - Don't force usb0 into peripheral mode
> 
> Changes in v3:
> - Enable usb nodes in all boot phases
> 
> Changes in v2:
> - Only enable usb port 0 DFU in SPL
> 
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm/dts/k3-am625-sk-u-boot.dtsi b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> index fa778b0ff4c..67c9fa2bbc3 100644
> --- a/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-sk-u-boot.dtsi
> @@ -46,3 +46,11 @@
>  &cpsw_port2 {
>         status = "disabled";
>  };
> +
> +&usbss0 {
> +       bootph-all;
> +};
> +
> +&usb0 {
> +       bootph-all;
> +};

-- 
Alexander Sverdlin
Siemens AG
www.siemens.com

[PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Sjoerd Simons]

Enable USB host as well as USB gadget and DFU support for a53; For the
r5 due to the smaller available size create a config fragment for DFU
supports which disables support for persistent storage to free up space
for USB support

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- Move R5 dfu config to a config fragment rather then a full defconfig
- Don't enable XHCI for the R5 SPL, unneeded

Changes in v3:
- Run savedefconfig to adjust to more recent u-boot

Changes in v2:
- Create a seperate defconfig for R5

 configs/am62x_evm_a53_defconfig | 30 ++++++++++++++++++++++++++++++
 configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 configs/am62x_r5_usbdfu.config

diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
index aa96c1b3125..f335eb11e63 100644
--- a/configs/am62x_evm_a53_defconfig
+++ b/configs/am62x_evm_a53_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_ARCH_K3=y
+CONFIG_SYS_MALLOC_LEN=0x2000000
 CONFIG_SYS_MALLOC_F_LEN=0x8000
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
@@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
+CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
 CONFIG_SPL_DM_MAILBOX=y
 CONFIG_SPL_DM_SPI_FLASH=y
 CONFIG_SPL_POWER_DOMAIN=y
+CONFIG_SPL_RAM_SUPPORT=y
+CONFIG_SPL_RAM_DEVICE=y
 # CONFIG_SPL_SPI_FLASH_TINY is not set
 CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
 CONFIG_SPL_SPI_LOAD=y
 CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
+CONFIG_SPL_USB_GADGET=y
+CONFIG_SPL_DFU=y
 CONFIG_SPL_YMODEM_SUPPORT=y
+CONFIG_CMD_DFU=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_USB=y
 CONFIG_OF_CONTROL=y
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_MULTI_DTB_FIT=y
@@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
 CONFIG_SPL_DM_SEQ_ALIAS=y
 CONFIG_REGMAP=y
 CONFIG_SPL_REGMAP=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_CLK=y
 CONFIG_SPL_CLK=y
 CONFIG_CLK_TI_SCI=y
+CONFIG_DFU_MMC=y
+CONFIG_DFU_RAM=y
+CONFIG_DFU_SF=y
+CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
+CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
 CONFIG_DMA_CHANNELS=y
 CONFIG_TI_K3_NAVSS_UDMA=y
 CONFIG_TI_SCI_PROTOCOL=y
@@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
 CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_TI_SCI=y
+CONFIG_USB=y
+CONFIG_DM_USB_GADGET=y
+CONFIG_SPL_DM_USB_GADGET=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_AM62=y
+CONFIG_USB_DWC3_AM62=y
+CONFIG_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
+CONFIG_USB_GADGET_VENDOR_NUM=0x0451
+CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
+CONFIG_USB_GADGET_DOWNLOAD=y
 CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
new file mode 100644
index 00000000000..772bb2ab935
--- /dev/null
+++ b/configs/am62x_r5_usbdfu.config
@@ -0,0 +1,28 @@
+CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
+CONFIG_MISC=y
+CONFIG_USB=y
+CONFIG_DM_USB_GADGET=y
+CONFIG_SPL_DM_USB_GADGET=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_AM62=y
+CONFIG_USB_GADGET=y
+CONFIG_SPL_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
+CONFIG_USB_GADGET_VENDOR_NUM=0x0451
+CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
+CONFIG_USB_GADGET_DOWNLOAD=y
+CONFIG_SPL_DFU=y
+# CONFIG_SPL_MMC is not set
+# CONFIG_SPL_FS_FAT is not set
+# CONFIG_SPL_LIBDISK_SUPPORT is not set
+# CONFIG_SPL_SPI is not set
+# CONFIG_SPL_SYS_MALLOC is not set
+# CONFIG_CMD_GPT is not set
+# CONFIG_CMD_MMC is not set
+# CONFIG_CMD_FAT is not set
+# CONFIG_MMC_SDHCI is not set
-- 
2.43.0

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Roger Quadros]

Hi Sjoerd,

On 12/01/2024 10:52, Sjoerd Simons wrote:
> Enable USB host as well as USB gadget and DFU support for a53; For the
> r5 due to the smaller available size create a config fragment for DFU
> supports which disables support for persistent storage to free up space
> for USB support
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - Move R5 dfu config to a config fragment rather then a full defconfig
> - Don't enable XHCI for the R5 SPL, unneeded
> 
> Changes in v3:
> - Run savedefconfig to adjust to more recent u-boot
> 
> Changes in v2:
> - Create a seperate defconfig for R5
> 
>  configs/am62x_evm_a53_defconfig | 30 ++++++++++++++++++++++++++++++
>  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 configs/am62x_r5_usbdfu.config
> 
> diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
> index aa96c1b3125..f335eb11e63 100644
> --- a/configs/am62x_evm_a53_defconfig
> +++ b/configs/am62x_evm_a53_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  # CONFIG_SPL_SPI_FLASH_TINY is not set
>  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_SPL_DFU=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_MULTI_DTB_FIT=y
> @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
>  CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_REGMAP=y
>  CONFIG_SPL_REGMAP=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_DFU_SF=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
> new file mode 100644
> index 00000000000..772bb2ab935
> --- /dev/null
> +++ b/configs/am62x_r5_usbdfu.config
> @@ -0,0 +1,28 @@
> +CONFIG_SPL_ENV_SUPPORT=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_MISC=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y

Why do we need USB DFU support at R5 SPL stage?
What is the use case?

> +# CONFIG_SPL_MMC is not set
> +# CONFIG_SPL_FS_FAT is not set
> +# CONFIG_SPL_LIBDISK_SUPPORT is not set
> +# CONFIG_SPL_SPI is not set
> +# CONFIG_SPL_SYS_MALLOC is not set
> +# CONFIG_CMD_GPT is not set
> +# CONFIG_CMD_MMC is not set
> +# CONFIG_CMD_FAT is not set
> +# CONFIG_MMC_SDHCI is not set

-- 
cheers,
-roger

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Sjoerd Simons]

On Fri, 2024-01-12 at 11:58 +0200, Roger Quadros wrote:
> Hi Sjoerd,
> 
> On 12/01/2024 10:52, Sjoerd Simons wrote:
> > Enable USB host as well as USB gadget and DFU support for a53; For
> > the
> > r5 due to the smaller available size create a config fragment for
> > DFU
> > supports which disables support for persistent storage to free up
> > space
> > for USB support
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> > 
> > ---
> > 
> > Changes in v4:
> > - Move R5 dfu config to a config fragment rather then a full
> > defconfig
> > - Don't enable XHCI for the R5 SPL, unneeded
> > 
> > Changes in v3:
> > - Run savedefconfig to adjust to more recent u-boot
> > 
> > Changes in v2:
> > - Create a seperate defconfig for R5
> > 
> >  configs/am62x_evm_a53_defconfig | 30
> > ++++++++++++++++++++++++++++++
> >  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> >  create mode 100644 configs/am62x_r5_usbdfu.config
> > 
> > diff --git a/configs/am62x_evm_a53_defconfig
> > b/configs/am62x_evm_a53_defconfig
> > index aa96c1b3125..f335eb11e63 100644
> > --- a/configs/am62x_evm_a53_defconfig
> > +++ b/configs/am62x_evm_a53_defconfig
> > @@ -1,5 +1,6 @@
> >  CONFIG_ARM=y
> >  CONFIG_ARCH_K3=y
> > +CONFIG_SYS_MALLOC_LEN=0x2000000
> >  CONFIG_SYS_MALLOC_F_LEN=0x8000
> >  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> >  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> >  CONFIG_SPL_STACK_R=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> > +CONFIG_SPL_ENV_SUPPORT=y
> >  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
> >  CONFIG_SPL_DM_MAILBOX=y
> >  CONFIG_SPL_DM_SPI_FLASH=y
> >  CONFIG_SPL_POWER_DOMAIN=y
> > +CONFIG_SPL_RAM_SUPPORT=y
> > +CONFIG_SPL_RAM_DEVICE=y
> >  # CONFIG_SPL_SPI_FLASH_TINY is not set
> >  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
> >  CONFIG_SPL_SPI_LOAD=y
> >  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> > +CONFIG_SPL_USB_GADGET=y
> > +CONFIG_SPL_DFU=y
> >  CONFIG_SPL_YMODEM_SUPPORT=y
> > +CONFIG_CMD_DFU=y
> >  CONFIG_CMD_MMC=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_OF_CONTROL=y
> >  CONFIG_SPL_OF_CONTROL=y
> >  CONFIG_MULTI_DTB_FIT=y
> > @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
> >  CONFIG_SPL_DM_SEQ_ALIAS=y
> >  CONFIG_REGMAP=y
> >  CONFIG_SPL_REGMAP=y
> > +CONFIG_SYSCON=y
> > +CONFIG_SPL_SYSCON=y
> >  CONFIG_SPL_OF_TRANSLATE=y
> >  CONFIG_CLK=y
> >  CONFIG_SPL_CLK=y
> >  CONFIG_CLK_TI_SCI=y
> > +CONFIG_DFU_MMC=y
> > +CONFIG_DFU_RAM=y
> > +CONFIG_DFU_SF=y
> > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> > +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
> >  CONFIG_DMA_CHANNELS=y
> >  CONFIG_TI_K3_NAVSS_UDMA=y
> >  CONFIG_TI_SCI_PROTOCOL=y
> > @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
> >  CONFIG_SYSRESET=y
> >  CONFIG_SPL_SYSRESET=y
> >  CONFIG_SYSRESET_TI_SCI=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB_GADGET=y
> > +CONFIG_SPL_DM_USB_GADGET=y
> > +CONFIG_USB_XHCI_HCD=y
> > +CONFIG_USB_XHCI_DWC3=y
> > +CONFIG_USB_DWC3=y
> > +CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_AM62=y
> > +CONFIG_USB_DWC3_AM62=y
> > +CONFIG_USB_GADGET=y
> > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> > +CONFIG_USB_GADGET_DOWNLOAD=y
> >  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> > diff --git a/configs/am62x_r5_usbdfu.config
> > b/configs/am62x_r5_usbdfu.config
> > new file mode 100644
> > index 00000000000..772bb2ab935
> > --- /dev/null
> > +++ b/configs/am62x_r5_usbdfu.config
> > @@ -0,0 +1,28 @@
> > +CONFIG_SPL_ENV_SUPPORT=y
> > +CONFIG_SYSCON=y
> > +CONFIG_SPL_SYSCON=y
> > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> > +CONFIG_MISC=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB_GADGET=y
> > +CONFIG_SPL_DM_USB_GADGET=y
> > +CONFIG_USB_DWC3=y
> > +CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_AM62=y
> > +CONFIG_USB_GADGET=y
> > +CONFIG_SPL_USB_GADGET=y
> > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> > +CONFIG_USB_GADGET_DOWNLOAD=y
> > +CONFIG_SPL_DFU=y
> 
> Why do we need USB DFU support at R5 SPL stage?
> What is the use case?

So we can load the A53 SPL over DFU ? There is no way to get a full DFU
bootchain without the R5 also taking part 


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Roger Quadros]

On 12/01/2024 12:44, Sjoerd Simons wrote:
> On Fri, 2024-01-12 at 11:58 +0200, Roger Quadros wrote:
>> Hi Sjoerd,
>>
>> On 12/01/2024 10:52, Sjoerd Simons wrote:
>>> Enable USB host as well as USB gadget and DFU support for a53; For
>>> the
>>> r5 due to the smaller available size create a config fragment for
>>> DFU
>>> supports which disables support for persistent storage to free up
>>> space
>>> for USB support
>>>
>>> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - Move R5 dfu config to a config fragment rather then a full
>>> defconfig
>>> - Don't enable XHCI for the R5 SPL, unneeded
>>>
>>> Changes in v3:
>>> - Run savedefconfig to adjust to more recent u-boot
>>>
>>> Changes in v2:
>>> - Create a seperate defconfig for R5
>>>
>>>  configs/am62x_evm_a53_defconfig | 30
>>> ++++++++++++++++++++++++++++++
>>>  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
>>>  2 files changed, 58 insertions(+)
>>>  create mode 100644 configs/am62x_r5_usbdfu.config
>>>
>>> diff --git a/configs/am62x_evm_a53_defconfig
>>> b/configs/am62x_evm_a53_defconfig
>>> index aa96c1b3125..f335eb11e63 100644
>>> --- a/configs/am62x_evm_a53_defconfig
>>> +++ b/configs/am62x_evm_a53_defconfig
>>> @@ -1,5 +1,6 @@
>>>  CONFIG_ARM=y
>>>  CONFIG_ARCH_K3=y
>>> +CONFIG_SYS_MALLOC_LEN=0x2000000
>>>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>>>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>>>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
>>> @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>>>  CONFIG_SPL_STACK_R=y
>>>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>>>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
>>> +CONFIG_SPL_ENV_SUPPORT=y
>>>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>>>  CONFIG_SPL_DM_MAILBOX=y
>>>  CONFIG_SPL_DM_SPI_FLASH=y
>>>  CONFIG_SPL_POWER_DOMAIN=y
>>> +CONFIG_SPL_RAM_SUPPORT=y
>>> +CONFIG_SPL_RAM_DEVICE=y
>>>  # CONFIG_SPL_SPI_FLASH_TINY is not set
>>>  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
>>>  CONFIG_SPL_SPI_LOAD=y
>>>  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
>>> +CONFIG_SPL_USB_GADGET=y
>>> +CONFIG_SPL_DFU=y
>>>  CONFIG_SPL_YMODEM_SUPPORT=y
>>> +CONFIG_CMD_DFU=y
>>>  CONFIG_CMD_MMC=y
>>> +CONFIG_CMD_USB=y
>>>  CONFIG_OF_CONTROL=y
>>>  CONFIG_SPL_OF_CONTROL=y
>>>  CONFIG_MULTI_DTB_FIT=y
>>> @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
>>>  CONFIG_SPL_DM_SEQ_ALIAS=y
>>>  CONFIG_REGMAP=y
>>>  CONFIG_SPL_REGMAP=y
>>> +CONFIG_SYSCON=y
>>> +CONFIG_SPL_SYSCON=y
>>>  CONFIG_SPL_OF_TRANSLATE=y
>>>  CONFIG_CLK=y
>>>  CONFIG_SPL_CLK=y
>>>  CONFIG_CLK_TI_SCI=y
>>> +CONFIG_DFU_MMC=y
>>> +CONFIG_DFU_RAM=y
>>> +CONFIG_DFU_SF=y
>>> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
>>> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>>>  CONFIG_DMA_CHANNELS=y
>>>  CONFIG_TI_K3_NAVSS_UDMA=y
>>>  CONFIG_TI_SCI_PROTOCOL=y
>>> @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
>>>  CONFIG_SYSRESET=y
>>>  CONFIG_SPL_SYSRESET=y
>>>  CONFIG_SYSRESET_TI_SCI=y
>>> +CONFIG_USB=y
>>> +CONFIG_DM_USB_GADGET=y
>>> +CONFIG_SPL_DM_USB_GADGET=y
>>> +CONFIG_USB_XHCI_HCD=y
>>> +CONFIG_USB_XHCI_DWC3=y
>>> +CONFIG_USB_DWC3=y
>>> +CONFIG_USB_DWC3_GENERIC=y
>>> +CONFIG_SPL_USB_DWC3_GENERIC=y
>>> +CONFIG_SPL_USB_DWC3_AM62=y
>>> +CONFIG_USB_DWC3_AM62=y
>>> +CONFIG_USB_GADGET=y
>>> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
>>> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
>>> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
>>> +CONFIG_USB_GADGET_DOWNLOAD=y
>>>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
>>> diff --git a/configs/am62x_r5_usbdfu.config
>>> b/configs/am62x_r5_usbdfu.config
>>> new file mode 100644
>>> index 00000000000..772bb2ab935
>>> --- /dev/null
>>> +++ b/configs/am62x_r5_usbdfu.config
>>> @@ -0,0 +1,28 @@
>>> +CONFIG_SPL_ENV_SUPPORT=y
>>> +CONFIG_SYSCON=y
>>> +CONFIG_SPL_SYSCON=y
>>> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
>>> +CONFIG_MISC=y
>>> +CONFIG_USB=y
>>> +CONFIG_DM_USB_GADGET=y
>>> +CONFIG_SPL_DM_USB_GADGET=y
>>> +CONFIG_USB_DWC3=y
>>> +CONFIG_USB_DWC3_GENERIC=y
>>> +CONFIG_SPL_USB_DWC3_GENERIC=y
>>> +CONFIG_SPL_USB_DWC3_AM62=y
>>> +CONFIG_USB_GADGET=y
>>> +CONFIG_SPL_USB_GADGET=y
>>> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
>>> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
>>> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
>>> +CONFIG_USB_GADGET_DOWNLOAD=y
>>> +CONFIG_SPL_DFU=y
>>
>> Why do we need USB DFU support at R5 SPL stage?
>> What is the use case?
> 
> So we can load the A53 SPL over DFU ? There is no way to get a full DFU
> bootchain without the R5 also taking part 
> 
> 
Got it. I should have read the documentation in patch 7 before asking. ;)

-- 
cheers,
-roger

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Nishanth Menon]

On 09:52-20240112, Sjoerd Simons wrote:
> Enable USB host as well as USB gadget and DFU support for a53; For the
> r5 due to the smaller available size create a config fragment for DFU
> supports which disables support for persistent storage to free up space
> for USB support
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - Move R5 dfu config to a config fragment rather then a full defconfig
> - Don't enable XHCI for the R5 SPL, unneeded
> 
> Changes in v3:
> - Run savedefconfig to adjust to more recent u-boot
> 
> Changes in v2:
> - Create a seperate defconfig for R5
> 
>  configs/am62x_evm_a53_defconfig | 30 ++++++++++++++++++++++++++++++
>  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 configs/am62x_r5_usbdfu.config
> 
> diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
> index aa96c1b3125..f335eb11e63 100644
> --- a/configs/am62x_evm_a53_defconfig
> +++ b/configs/am62x_evm_a53_defconfig

Should we make the a53 also a defconfig fragment allowing reuse across
multiple boards?

> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  # CONFIG_SPL_SPI_FLASH_TINY is not set
>  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_SPL_DFU=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_MULTI_DTB_FIT=y
> @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
>  CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_REGMAP=y
>  CONFIG_SPL_REGMAP=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_DFU_SF=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
> new file mode 100644
> index 00000000000..772bb2ab935
> --- /dev/null
> +++ b/configs/am62x_r5_usbdfu.config
> @@ -0,0 +1,28 @@
> +CONFIG_SPL_ENV_SUPPORT=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_MISC=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y
> +# CONFIG_SPL_MMC is not set
> +# CONFIG_SPL_FS_FAT is not set
> +# CONFIG_SPL_LIBDISK_SUPPORT is not set
> +# CONFIG_SPL_SPI is not set
> +# CONFIG_SPL_SYS_MALLOC is not set
> +# CONFIG_CMD_GPT is not set
> +# CONFIG_CMD_MMC is not set
> +# CONFIG_CMD_FAT is not set
> +# CONFIG_MMC_SDHCI is not set
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Sjoerd Simons]

On Fri, 2024-01-12 at 06:37 -0600, Nishanth Menon wrote:
> On 09:52-20240112, Sjoerd Simons wrote:
> > Enable USB host as well as USB gadget and DFU support for a53; For
> > the
> > r5 due to the smaller available size create a config fragment for
> > DFU
> > supports which disables support for persistent storage to free up
> > space
> > for USB support
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> > 
> > ---
> > 
> > Changes in v4:
> > - Move R5 dfu config to a config fragment rather then a full
> > defconfig
> > - Don't enable XHCI for the R5 SPL, unneeded
> > 
> > Changes in v3:
> > - Run savedefconfig to adjust to more recent u-boot
> > 
> > Changes in v2:
> > - Create a seperate defconfig for R5
> > 
> >  configs/am62x_evm_a53_defconfig | 30
> > ++++++++++++++++++++++++++++++
> >  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> >  create mode 100644 configs/am62x_r5_usbdfu.config
> > 
> > diff --git a/configs/am62x_evm_a53_defconfig
> > b/configs/am62x_evm_a53_defconfig
> > index aa96c1b3125..f335eb11e63 100644
> > --- a/configs/am62x_evm_a53_defconfig
> > +++ b/configs/am62x_evm_a53_defconfig
> 
> Should we make the a53 also a defconfig fragment allowing reuse
> across
> multiple boards?

Pros and cons. having all the various options (USB boot, ETH boot, misc
other) as fragments would allow easier re-use but make it more
complicated for the end-user to get a "full-featured" u-boot. 

My personal preference, as i'm coming with a distribution background,
is to always enable as much as possible in a default build to allow
maximum flexibility for end-users; Those that need
space/performance/whatever optimisation can always do custom builds.

It would be great if we could have a level of indirection here where by
default a set of fragments get included such that it's invisible to the
end-user that the underlying config is actually quite modular. But I
don't think that's currently possible, hence preferring a fat defconfig
such that the user/distro can just do `make am62x_evm_a53_defconfig`
(and similar for beagleplay)

Regards,
  Sjoerd

> > @@ -1,5 +1,6 @@
> >  CONFIG_ARM=y
> >  CONFIG_ARCH_K3=y
> > +CONFIG_SYS_MALLOC_LEN=0x2000000
> >  CONFIG_SYS_MALLOC_F_LEN=0x8000
> >  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> >  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> >  CONFIG_SPL_STACK_R=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> > +CONFIG_SPL_ENV_SUPPORT=y
> >  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
> >  CONFIG_SPL_DM_MAILBOX=y
> >  CONFIG_SPL_DM_SPI_FLASH=y
> >  CONFIG_SPL_POWER_DOMAIN=y
> > +CONFIG_SPL_RAM_SUPPORT=y
> > +CONFIG_SPL_RAM_DEVICE=y
> >  # CONFIG_SPL_SPI_FLASH_TINY is not set
> >  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
> >  CONFIG_SPL_SPI_LOAD=y
> >  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> > +CONFIG_SPL_USB_GADGET=y
> > +CONFIG_SPL_DFU=y
> >  CONFIG_SPL_YMODEM_SUPPORT=y
> > +CONFIG_CMD_DFU=y
> >  CONFIG_CMD_MMC=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_OF_CONTROL=y
> >  CONFIG_SPL_OF_CONTROL=y
> >  CONFIG_MULTI_DTB_FIT=y
> > @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
> >  CONFIG_SPL_DM_SEQ_ALIAS=y
> >  CONFIG_REGMAP=y
> >  CONFIG_SPL_REGMAP=y
> > +CONFIG_SYSCON=y
> > +CONFIG_SPL_SYSCON=y
> >  CONFIG_SPL_OF_TRANSLATE=y
> >  CONFIG_CLK=y
> >  CONFIG_SPL_CLK=y
> >  CONFIG_CLK_TI_SCI=y
> > +CONFIG_DFU_MMC=y
> > +CONFIG_DFU_RAM=y
> > +CONFIG_DFU_SF=y
> > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> > +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
> >  CONFIG_DMA_CHANNELS=y
> >  CONFIG_TI_K3_NAVSS_UDMA=y
> >  CONFIG_TI_SCI_PROTOCOL=y
> > @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
> >  CONFIG_SYSRESET=y
> >  CONFIG_SPL_SYSRESET=y
> >  CONFIG_SYSRESET_TI_SCI=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB_GADGET=y
> > +CONFIG_SPL_DM_USB_GADGET=y
> > +CONFIG_USB_XHCI_HCD=y
> > +CONFIG_USB_XHCI_DWC3=y
> > +CONFIG_USB_DWC3=y
> > +CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_AM62=y
> > +CONFIG_USB_DWC3_AM62=y
> > +CONFIG_USB_GADGET=y
> > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> > +CONFIG_USB_GADGET_DOWNLOAD=y
> >  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> > diff --git a/configs/am62x_r5_usbdfu.config
> > b/configs/am62x_r5_usbdfu.config
> > new file mode 100644
> > index 00000000000..772bb2ab935
> > --- /dev/null
> > +++ b/configs/am62x_r5_usbdfu.config
> > @@ -0,0 +1,28 @@
> > +CONFIG_SPL_ENV_SUPPORT=y
> > +CONFIG_SYSCON=y
> > +CONFIG_SPL_SYSCON=y
> > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> > +CONFIG_MISC=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB_GADGET=y
> > +CONFIG_SPL_DM_USB_GADGET=y
> > +CONFIG_USB_DWC3=y
> > +CONFIG_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_GENERIC=y
> > +CONFIG_SPL_USB_DWC3_AM62=y
> > +CONFIG_USB_GADGET=y
> > +CONFIG_SPL_USB_GADGET=y
> > +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> > +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> > +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> > +CONFIG_USB_GADGET_DOWNLOAD=y
> > +CONFIG_SPL_DFU=y
> > +# CONFIG_SPL_MMC is not set
> > +# CONFIG_SPL_FS_FAT is not set
> > +# CONFIG_SPL_LIBDISK_SUPPORT is not set
> > +# CONFIG_SPL_SPI is not set
> > +# CONFIG_SPL_SYS_MALLOC is not set
> > +# CONFIG_CMD_GPT is not set
> > +# CONFIG_CMD_MMC is not set
> > +# CONFIG_CMD_FAT is not set
> > +# CONFIG_MMC_SDHCI is not set
> > -- 
> > 2.43.0
> > 
> 

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Nishanth Menon]

On 14:09-20240112, Sjoerd Simons wrote:
[...]

> > > diff --git a/configs/am62x_evm_a53_defconfig
> > > b/configs/am62x_evm_a53_defconfig
> > > index aa96c1b3125..f335eb11e63 100644
> > > --- a/configs/am62x_evm_a53_defconfig
> > > +++ b/configs/am62x_evm_a53_defconfig
> > 
> > Should we make the a53 also a defconfig fragment allowing reuse
> > across
> > multiple boards?
> 
> Pros and cons. having all the various options (USB boot, ETH boot, misc
> other) as fragments would allow easier re-use but make it more
> complicated for the end-user to get a "full-featured" u-boot. 
> 
> My personal preference, as i'm coming with a distribution background,
> is to always enable as much as possible in a default build to allow
> maximum flexibility for end-users; Those that need
> space/performance/whatever optimisation can always do custom builds.
> 
> It would be great if we could have a level of indirection here where by
> default a set of fragments get included such that it's invisible to the
> end-user that the underlying config is actually quite modular. But I
> don't think that's currently possible, hence preferring a fat defconfig
> such that the user/distro can just do `make am62x_evm_a53_defconfig`
> (and similar for beagleplay)
> 

I understand, but I am looking to reduce duplication between boards
and making it easier for new board devs to be able to pick the feature
group they need - almost all am62x users who desire dfu on a53 will
need the same options enabled. This also keeps the default u-boot foot
print lower to allow for emmc boot0 partition limitations if any (I
removed that dependency on beagle by using uda, but other platforms
still keep u-boot in emmc boot0 and depending on the part and TEE
binary size, the available space can be constraining)

I am starting to wonder if
https://lore.kernel.org/u-boot/20231101170519.39627-1-afd@ti.com/
will help us here.


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Sjoerd Simons]

On Fri, 2024-01-12 at 07:19 -0600, Nishanth Menon wrote:
> On 14:09-20240112, Sjoerd Simons wrote:
> [...]
> 
> > > > diff --git a/configs/am62x_evm_a53_defconfig
> > > > b/configs/am62x_evm_a53_defconfig
> > > > index aa96c1b3125..f335eb11e63 100644
> > > > --- a/configs/am62x_evm_a53_defconfig
> > > > +++ b/configs/am62x_evm_a53_defconfig
> > > 
> > > Should we make the a53 also a defconfig fragment allowing reuse
> > > across
> > > multiple boards?
> > 
> > Pros and cons. having all the various options (USB boot, ETH boot,
> > misc
> > other) as fragments would allow easier re-use but make it more
> > complicated for the end-user to get a "full-featured" u-boot. 
> > 
> > My personal preference, as i'm coming with a distribution
> > background,
> > is to always enable as much as possible in a default build to allow
> > maximum flexibility for end-users; Those that need
> > space/performance/whatever optimisation can always do custom
> > builds.
> > 
> > It would be great if we could have a level of indirection here
> > where by
> > default a set of fragments get included such that it's invisible to
> > the
> > end-user that the underlying config is actually quite modular. But
> > I
> > don't think that's currently possible, hence preferring a fat
> > defconfig
> > such that the user/distro can just do `make
> > am62x_evm_a53_defconfig`
> > (and similar for beagleplay)
> > 
> 
> I understand, but I am looking to reduce duplication between boards
> and making it easier for new board devs to be able to pick the
> feature
> group they need - almost all am62x users who desire dfu on a53 will
> need the same options enabled. This also keeps the default u-boot
> foot
> print lower to allow for emmc boot0 partition limitations if any (I
> removed that dependency on beagle by using uda, but other platforms
> still keep u-boot in emmc boot0 and depending on the part and TEE
> binary size, the available space can be constraining)

Fwiw just testing; for the SK board the difference in size for
tispl.bin and u-boot.img is about 200 kilobytes with this extra config.
 
The SK board i have has about 32 megabytes for the hardware boot
partition so unlikely an issue there; For the beagleplay it seems to be
only 4M, but that's currently already on UDA indeed so shouldn't matter
there.

Mind i agree with you in general; and i was looking at moving the full
u-boot into the hw boot partition on beagleplay already so the OS i
flash doesn't have to be device specific.

> 
> I am starting to wonder if
> https://lore.kernel.org/u-boot/20231101170519.39627-1-afd@ti.com/
> will help us here.

Yes absolutely that would be perfect. Can i suggest we land it as is
for now ("fat monolithic config") and once those patches land i'm happy
to split things up? 


-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Nishanth Menon]

On 16:06-20240112, Sjoerd Simons wrote:
[...]
> > 
> > I am starting to wonder if
> > https://lore.kernel.org/u-boot/20231101170519.39627-1-afd@ti.com/
> > will help us here.
> 
> Yes absolutely that would be perfect. Can i suggest we land it as is
> for now ("fat monolithic config") and once those patches land i'm happy
> to split things up? 
> 

https://libera.irclog.whitequark.org/u-boot/2024-01-12#35590530 I think
Tom agrees in principle (and Simon already acked), so we could go down
this road and refactor - I am always a bit squeamish about redoing work
if the baseline changes or has a reasonable chance of change.. Just
doing it once and cleanly is my preference.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Sjoerd Simons]

On Fri, 2024-01-12 at 09:40 -0600, Nishanth Menon wrote:
> On 16:06-20240112, Sjoerd Simons wrote:
> [...]
> > > 
> > > I am starting to wonder if
> > > https://lore.kernel.org/u-boot/20231101170519.39627-1-afd@ti.com/
> > > will help us here.
> > 
> > Yes absolutely that would be perfect. Can i suggest we land it as
> > is
> > for now ("fat monolithic config") and once those patches land i'm
> > happy
> > to split things up? 
> > 
> 
> https://libera.irclog.whitequark.org/u-boot/2024-01-12#35590530 I
> think
> Tom agrees in principle (and Simon already acked), so we could go
> down
> this road and refactor - I am always a bit squeamish about redoing
> work
> if the baseline changes or has a reasonable chance of change.. Just
> doing it once and cleanly is my preference.

Well lets see if this will land soonish then, that would be 
amazing :) 

And yes, no doubt, doing it right the first time makes sense. On the
other hand I'm wary about adding more and more blockers for this set. 

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On Fri, Jan 12, 2024 at 04:58:50PM +0100, Sjoerd Simons wrote:
> On Fri, 2024-01-12 at 09:40 -0600, Nishanth Menon wrote:
> > On 16:06-20240112, Sjoerd Simons wrote:
> > [...]
> > > > 
> > > > I am starting to wonder if
> > > > https://lore.kernel.org/u-boot/20231101170519.39627-1-afd@ti.com/
> > > > will help us here.
> > > 
> > > Yes absolutely that would be perfect. Can i suggest we land it as
> > > is
> > > for now ("fat monolithic config") and once those patches land i'm
> > > happy
> > > to split things up? 
> > > 
> > 
> > https://libera.irclog.whitequark.org/u-boot/2024-01-12#35590530 I
> > think
> > Tom agrees in principle (and Simon already acked), so we could go
> > down
> > this road and refactor - I am always a bit squeamish about redoing
> > work
> > if the baseline changes or has a reasonable chance of change.. Just
> > doing it once and cleanly is my preference.
> 
> Well lets see if this will land soonish then, that would be 
> amazing :) 
> 
> And yes, no doubt, doing it right the first time makes sense. On the
> other hand I'm wary about adding more and more blockers for this set. 

I've merged Andrew's patch for allowing defconfigs to use fragments this
morning.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v4 5/7] configs: am62x_evm_*: Enable USB and DFU support

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Enable USB host as well as USB gadget and DFU support for a53; For the
> r5 due to the smaller available size create a config fragment for DFU
> supports which disables support for persistent storage to free up space
> for USB support
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

I know this will be reworked in v5 (to use defconfig fragments) as
discussed in [1] but i'd still like to give some feedback on my testing.

Applying this series seems to break eMMC booting for me on AM62x SK EVM:

U-Boot SPL 2024.04-rc1-00106-g7c3798e43744 (Feb 08 2024 - 10:41:17 +0100)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.1.8--v09.01.08 (Kool Koala)')
SPL initial stack usage: 13368 bytes
Trying to boot from MMC1
alloc space exhausted
spl_load_image_fat: error reading image tispl.bin, err - -22
SPL: failed to boot from all boot devices
### ERROR ### Please RESET the board ###

It seems that the size of tispl.bin gets too big because of this commit.

Before series:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
796     tispl.bin_unsigned

After series:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
876     tispl.bin_unsigned

Per my understanding, this change increases the size of the a53 U-Boot
SPL, which is why we can no longer boot from eMMC.

Here is are other depending project refs:
- optee_os: 439c5ecbb68b ("core: arm: fix integer overflow in generic_timer_{handler,start}()")
- ti-linux-firmware: 0350c2edecab ("ti-dm: Update firmware for J722S device")
- trustef-firmware-a: 17bef2248d45 ("Merge "feat(fvp): delegate FFH RAS handling to SP" into integration")

Reverting just this commit makes it boot to eMMC for me again.

I also tried this downstream change[2] to increase the alloc space in
the SPL. That did not fix the booting problem.

To fix it, I had to remove:
CONFIG_SPL_ENV_SUPPORT=y
CONFIG_SPL_DFU=y

Which bring down the tispl to:
~/work/ti/src/bootloaders-upstream/out/ $ du -s tispl.bin_unsigned
820     tispl.bin_unsigned

Of course, removing both config options is not an acceptable solution since
it defeats the purpose of this patch.

Did you test booting from eMMC when developping this series, or did you
just tested booting from DFU ?

[1] https://lore.kernel.org/all/b587fcaebed8b61285a901b468e3ccbddf7d06f2.camel@collabora.com/
[2] https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/?h=ti-u-boot-2023.04&id=28c1b1ec8ba89b7c7b2cd43483054a432552008f

>
> ---
>
> Changes in v4:
> - Move R5 dfu config to a config fragment rather then a full defconfig
> - Don't enable XHCI for the R5 SPL, unneeded
>
> Changes in v3:
> - Run savedefconfig to adjust to more recent u-boot
>
> Changes in v2:
> - Create a seperate defconfig for R5
>
>  configs/am62x_evm_a53_defconfig | 30 ++++++++++++++++++++++++++++++
>  configs/am62x_r5_usbdfu.config  | 28 ++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
>  create mode 100644 configs/am62x_r5_usbdfu.config
>
> diff --git a/configs/am62x_evm_a53_defconfig b/configs/am62x_evm_a53_defconfig
> index aa96c1b3125..f335eb11e63 100644
> --- a/configs/am62x_evm_a53_defconfig
> +++ b/configs/am62x_evm_a53_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
>  CONFIG_SPL_LIBGENERIC_SUPPORT=y
> @@ -41,16 +42,23 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  # CONFIG_SPL_SPI_FLASH_TINY is not set
>  CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
>  CONFIG_SPL_SPI_LOAD=y
>  CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_SPL_DFU=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_MULTI_DTB_FIT=y
> @@ -61,10 +69,17 @@ CONFIG_SPL_DM=y
>  CONFIG_SPL_DM_SEQ_ALIAS=y
>  CONFIG_REGMAP=y
>  CONFIG_SPL_REGMAP=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
>  CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_DFU_SF=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -103,4 +118,19 @@ CONFIG_CADENCE_QSPI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
> diff --git a/configs/am62x_r5_usbdfu.config b/configs/am62x_r5_usbdfu.config
> new file mode 100644
> index 00000000000..772bb2ab935
> --- /dev/null
> +++ b/configs/am62x_r5_usbdfu.config
> @@ -0,0 +1,28 @@
> +CONFIG_SPL_ENV_SUPPORT=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_MISC=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y
> +# CONFIG_SPL_MMC is not set
> +# CONFIG_SPL_FS_FAT is not set
> +# CONFIG_SPL_LIBDISK_SUPPORT is not set
> +# CONFIG_SPL_SPI is not set
> +# CONFIG_SPL_SYS_MALLOC is not set
> +# CONFIG_CMD_GPT is not set
> +# CONFIG_CMD_MMC is not set
> +# CONFIG_CMD_FAT is not set
> +# CONFIG_MMC_SDHCI is not set
> -- 
> 2.43.0

[PATCH v4 6/7] beagleplay: Add DFU support

[Sjoerd Simons]

DFU mode on a beagleplay can be used via the Type-C connector by holding
the USR switch while powering on.

Configuration is only added for the A53 u-boot parts, for R5 the
am62x_r5_usbdfu.config fragment should be used.

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- New patch

 arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |  8 ++++++
 board/beagle/beagleplay/beagleplay.env       |  1 +
 configs/am62x_beagleplay_a53_defconfig       | 30 ++++++++++++++++++++
 3 files changed, 39 insertions(+)

diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
index a723caa5805..0b1e5e70fe2 100644
--- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
+++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
@@ -61,6 +61,14 @@
 	>;
 };
 
+&usbss0 {
+	bootph-all;
+};
+
+&usb0 {
+	bootph-all;
+};
+
 #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
 
 #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env
index 4f0a94a8113..85c94856017 100644
--- a/board/beagle/beagleplay/beagleplay.env
+++ b/board/beagle/beagleplay/beagleplay.env
@@ -1,6 +1,7 @@
 #include <env/ti/ti_common.env>
 #include <env/ti/default_findfdt.env>
 #include <env/ti/mmc.env>
+#include <env/ti/k3_dfu.env>
 
 name_kern=Image
 console=ttyS2,115200n8
diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig
index 0be20045a97..dfe04b71810 100644
--- a/configs/am62x_beagleplay_a53_defconfig
+++ b/configs/am62x_beagleplay_a53_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_ARCH_K3=y
+CONFIG_SYS_MALLOC_LEN=0x2000000
 CONFIG_SYS_MALLOC_F_LEN=0x8000
 CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
@@ -43,15 +44,20 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
+CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
 CONFIG_SPL_I2C=y
 CONFIG_SPL_DM_MAILBOX=y
 CONFIG_SPL_POWER_DOMAIN=y
+CONFIG_SPL_RAM_SUPPORT=y
+CONFIG_SPL_RAM_DEVICE=y
 CONFIG_SPL_YMODEM_SUPPORT=y
+CONFIG_CMD_DFU=y
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_GPIO_READ=y
 CONFIG_CMD_I2C=y
 CONFIG_CMD_MMC=y
+CONFIG_CMD_USB=y
 CONFIG_CMD_PMIC=y
 CONFIG_CMD_REGULATOR=y
 CONFIG_OF_CONTROL=y
@@ -68,6 +74,10 @@ CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_CLK=y
 CONFIG_SPL_CLK=y
 CONFIG_CLK_TI_SCI=y
+CONFIG_DFU_MMC=y
+CONFIG_DFU_RAM=y
+CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
+CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
 CONFIG_DMA_CHANNELS=y
 CONFIG_TI_K3_NAVSS_UDMA=y
 CONFIG_TI_SCI_PROTOCOL=y
@@ -113,6 +123,26 @@ CONFIG_SOC_TI=y
 CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
 CONFIG_SYSRESET_TI_SCI=y
+CONFIG_USB=y
+CONFIG_DM_USB_GADGET=y
+CONFIG_SPL_DM_USB_GADGET=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_GENERIC=y
+CONFIG_SPL_USB_DWC3_AM62=y
+CONFIG_USB_DWC3_AM62=y
+CONFIG_USB_GADGET=y
+CONFIG_SPL_USB_GADGET=y
+CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
+CONFIG_USB_GADGET_VENDOR_NUM=0x0451
+CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
+CONFIG_USB_GADGET_DOWNLOAD=y
+CONFIG_SPL_DFU=y
 CONFIG_EXT4_WRITE=y
 CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
 CONFIG_LZO=y
+CONFIG_SYSCON=y
+CONFIG_SPL_SYSCON=y
+
-- 
2.43.0

Re: [PATCH v4 6/7] beagleplay: Add DFU support

[Roger Quadros]

On 12/01/2024 10:52, Sjoerd Simons wrote:
> DFU mode on a beagleplay can be used via the Type-C connector by holding
> the USR switch while powering on.
> 
> Configuration is only added for the A53 u-boot parts, for R5 the
> am62x_r5_usbdfu.config fragment should be used.
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |  8 ++++++
>  board/beagle/beagleplay/beagleplay.env       |  1 +
>  configs/am62x_beagleplay_a53_defconfig       | 30 ++++++++++++++++++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index a723caa5805..0b1e5e70fe2 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -61,6 +61,14 @@
>  	>;
>  };
>  
> +&usbss0 {
> +	bootph-all;
> +};
> +
> +&usb0 {
> +	bootph-all;
> +};
> +
>  #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
>  
>  #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
> diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env
> index 4f0a94a8113..85c94856017 100644
> --- a/board/beagle/beagleplay/beagleplay.env
> +++ b/board/beagle/beagleplay/beagleplay.env
> @@ -1,6 +1,7 @@
>  #include <env/ti/ti_common.env>
>  #include <env/ti/default_findfdt.env>
>  #include <env/ti/mmc.env>
> +#include <env/ti/k3_dfu.env>
>  
>  name_kern=Image
>  console=ttyS2,115200n8
> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig
> index 0be20045a97..dfe04b71810 100644
> --- a/configs/am62x_beagleplay_a53_defconfig
> +++ b/configs/am62x_beagleplay_a53_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_GPIO=y
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> @@ -43,15 +44,20 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_I2C=y
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_GPIO_READ=y
>  CONFIG_CMD_I2C=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
>  CONFIG_OF_CONTROL=y
> @@ -68,6 +74,10 @@ CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -113,6 +123,26 @@ CONFIG_SOC_TI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y

XHCI features are really not required for DFU
but now I see that we depend on XHCI driver to get
probed even for device mode. lol.

> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y
>  CONFIG_EXT4_WRITE=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
>  CONFIG_LZO=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +

-- 
cheers,
-roger

Re: [PATCH v4 6/7] beagleplay: Add DFU support

[Sjoerd Simons]

On Fri, 2024-01-12 at 12:41 +0200, Roger Quadros wrote:
> 
> 
> On 12/01/2024 10:52, Sjoerd Simons wrote:
> > DFU mode on a beagleplay can be used via the Type-C connector by
> > holding
> > the USR switch while powering on.
> > 
> > Configuration is only added for the A53 u-boot parts, for R5 the
> > am62x_r5_usbdfu.config fragment should be used.
> > 
> > Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> > 
> > ---
> > 
> > Changes in v4:
> > - New patch
> > 
> >  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |  8 ++++++
> >  board/beagle/beagleplay/beagleplay.env       |  1 +
> >  configs/am62x_beagleplay_a53_defconfig       | 30
> > ++++++++++++++++++++
> >  3 files changed, 39 insertions(+)
> > 
> > diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > index a723caa5805..0b1e5e70fe2 100644
> > --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> > @@ -61,6 +61,14 @@
> >  	>;
> >  };
> >  
> > +&usbss0 {
> > +	bootph-all;
> > +};
> > +
> > +&usb0 {
> > +	bootph-all;
> > +};
> > +
> >  #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
> >  
> >  #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
> > diff --git a/board/beagle/beagleplay/beagleplay.env
> > b/board/beagle/beagleplay/beagleplay.env
> > index 4f0a94a8113..85c94856017 100644
> > --- a/board/beagle/beagleplay/beagleplay.env
> > +++ b/board/beagle/beagleplay/beagleplay.env
> > @@ -1,6 +1,7 @@
> >  #include <env/ti/ti_common.env>
> >  #include <env/ti/default_findfdt.env>
> >  #include <env/ti/mmc.env>
> > +#include <env/ti/k3_dfu.env>
> >  
> >  name_kern=Image
> >  console=ttyS2,115200n8
> > diff --git a/configs/am62x_beagleplay_a53_defconfig
> > b/configs/am62x_beagleplay_a53_defconfig
> > index 0be20045a97..dfe04b71810 100644
> > --- a/configs/am62x_beagleplay_a53_defconfig
> > +++ b/configs/am62x_beagleplay_a53_defconfig
> > @@ -1,5 +1,6 @@
> >  CONFIG_ARM=y
> >  CONFIG_ARCH_K3=y
> > +CONFIG_SYS_MALLOC_LEN=0x2000000
> >  CONFIG_SYS_MALLOC_F_LEN=0x8000
> >  CONFIG_SPL_GPIO=y
> >  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > @@ -43,15 +44,20 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> >  CONFIG_SPL_STACK_R=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> >  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> > +CONFIG_SPL_ENV_SUPPORT=y
> >  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
> >  CONFIG_SPL_I2C=y
> >  CONFIG_SPL_DM_MAILBOX=y
> >  CONFIG_SPL_POWER_DOMAIN=y
> > +CONFIG_SPL_RAM_SUPPORT=y
> > +CONFIG_SPL_RAM_DEVICE=y
> >  CONFIG_SPL_YMODEM_SUPPORT=y
> > +CONFIG_CMD_DFU=y
> >  CONFIG_CMD_GPIO=y
> >  CONFIG_CMD_GPIO_READ=y
> >  CONFIG_CMD_I2C=y
> >  CONFIG_CMD_MMC=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_CMD_PMIC=y
> >  CONFIG_CMD_REGULATOR=y
> >  CONFIG_OF_CONTROL=y
> > @@ -68,6 +74,10 @@ CONFIG_SPL_OF_TRANSLATE=y
> >  CONFIG_CLK=y
> >  CONFIG_SPL_CLK=y
> >  CONFIG_CLK_TI_SCI=y
> > +CONFIG_DFU_MMC=y
> > +CONFIG_DFU_RAM=y
> > +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> > +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
> >  CONFIG_DMA_CHANNELS=y
> >  CONFIG_TI_K3_NAVSS_UDMA=y
> >  CONFIG_TI_SCI_PROTOCOL=y
> > @@ -113,6 +123,26 @@ CONFIG_SOC_TI=y
> >  CONFIG_SYSRESET=y
> >  CONFIG_SPL_SYSRESET=y
> >  CONFIG_SYSRESET_TI_SCI=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB_GADGET=y
> > +CONFIG_SPL_DM_USB_GADGET=y
> > +CONFIG_USB_XHCI_HCD=y
> > +CONFIG_USB_XHCI_DWC3=y
> 
> XHCI features are really not required for DFU
> but now I see that we depend on XHCI driver to get
> probed even for device mode. lol.

Right yeah i should have made it more clear in the commit message; This
also adds USB host support in the full u-boot in principle (i admit i
didn't actually test that on beagleplay). Which we can do now as the
usb glue is there.

Fwiw in the R5 SPL i've disable XHCI support now based on your comments
last time as USB host support there is not needed ofcourse



-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 6/7] beagleplay: Add DFU support

[Nishanth Menon]

On 09:52-20240112, Sjoerd Simons wrote:
> DFU mode on a beagleplay can be used via the Type-C connector by holding
> the USR switch while powering on.
> 
> Configuration is only added for the A53 u-boot parts, for R5 the
> am62x_r5_usbdfu.config fragment should be used.
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |  8 ++++++
>  board/beagle/beagleplay/beagleplay.env       |  1 +
>  configs/am62x_beagleplay_a53_defconfig       | 30 ++++++++++++++++++++
>  3 files changed, 39 insertions(+)
> 
> diff --git a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> index a723caa5805..0b1e5e70fe2 100644
> --- a/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> +++ b/arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi
> @@ -61,6 +61,14 @@
>  	>;
>  };
>  
> +&usbss0 {
> +	bootph-all;
> +};
> +
> +&usb0 {
> +	bootph-all;
> +};
> +
>  #ifdef CONFIG_TARGET_AM625_A53_BEAGLEPLAY
>  
>  #define SPL_NODTB "spl/u-boot-spl-nodtb.bin"
> diff --git a/board/beagle/beagleplay/beagleplay.env b/board/beagle/beagleplay/beagleplay.env
> index 4f0a94a8113..85c94856017 100644
> --- a/board/beagle/beagleplay/beagleplay.env
> +++ b/board/beagle/beagleplay/beagleplay.env
> @@ -1,6 +1,7 @@
>  #include <env/ti/ti_common.env>
>  #include <env/ti/default_findfdt.env>
>  #include <env/ti/mmc.env>
> +#include <env/ti/k3_dfu.env>
>  
>  name_kern=Image
>  console=ttyS2,115200n8
> diff --git a/configs/am62x_beagleplay_a53_defconfig b/configs/am62x_beagleplay_a53_defconfig
> index 0be20045a97..dfe04b71810 100644
> --- a/configs/am62x_beagleplay_a53_defconfig
> +++ b/configs/am62x_beagleplay_a53_defconfig
> @@ -1,5 +1,6 @@
>  CONFIG_ARM=y
>  CONFIG_ARCH_K3=y
> +CONFIG_SYS_MALLOC_LEN=0x2000000
>  CONFIG_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL_GPIO=y
>  CONFIG_SPL_LIBCOMMON_SUPPORT=y
> @@ -43,15 +44,20 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>  CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
> +CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_LOAD_PAYLOAD_NAME="u-boot.img"
>  CONFIG_SPL_I2C=y
>  CONFIG_SPL_DM_MAILBOX=y
>  CONFIG_SPL_POWER_DOMAIN=y
> +CONFIG_SPL_RAM_SUPPORT=y
> +CONFIG_SPL_RAM_DEVICE=y
>  CONFIG_SPL_YMODEM_SUPPORT=y
> +CONFIG_CMD_DFU=y
Couldn't these go to config fragments?
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_GPIO_READ=y
>  CONFIG_CMD_I2C=y
>  CONFIG_CMD_MMC=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_PMIC=y
>  CONFIG_CMD_REGULATOR=y
>  CONFIG_OF_CONTROL=y
> @@ -68,6 +74,10 @@ CONFIG_SPL_OF_TRANSLATE=y
>  CONFIG_CLK=y
>  CONFIG_SPL_CLK=y
>  CONFIG_CLK_TI_SCI=y
> +CONFIG_DFU_MMC=y
> +CONFIG_DFU_RAM=y
> +CONFIG_SYS_DFU_DATA_BUF_SIZE=0x5000
> +CONFIG_SYS_DFU_MAX_FILE_SIZE=0x800000
>  CONFIG_DMA_CHANNELS=y
>  CONFIG_TI_K3_NAVSS_UDMA=y
>  CONFIG_TI_SCI_PROTOCOL=y
> @@ -113,6 +123,26 @@ CONFIG_SOC_TI=y
>  CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_SYSRESET_TI_SCI=y
> +CONFIG_USB=y
> +CONFIG_DM_USB_GADGET=y
> +CONFIG_SPL_DM_USB_GADGET=y
> +CONFIG_USB_XHCI_HCD=y
> +CONFIG_USB_XHCI_DWC3=y
> +CONFIG_USB_DWC3=y
> +CONFIG_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_GENERIC=y
> +CONFIG_SPL_USB_DWC3_AM62=y
> +CONFIG_USB_DWC3_AM62=y
> +CONFIG_USB_GADGET=y
> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0451
> +CONFIG_USB_GADGET_PRODUCT_NUM=0x6165
> +CONFIG_USB_GADGET_DOWNLOAD=y
> +CONFIG_SPL_DFU=y
>  CONFIG_EXT4_WRITE=y
>  CONFIG_FS_FAT_MAX_CLUSTSIZE=16384
>  CONFIG_LZO=y
> +CONFIG_SYSCON=y
> +CONFIG_SPL_SYSCON=y
> +
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

[PATCH v4 7/7] doc: board: Add document for DFU boot on am62x SoCs

[Sjoerd Simons]

Both AM62 SK and beagleplay support DFU boot in a similar way now;
Document how to actually run DFU boot for both boards

Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

---

Changes in v4:
- New patch

 doc/board/beagle/am62x_beagleplay.rst | 12 +++++++++
 doc/board/ti/am62x_sk.rst             | 37 +++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/doc/board/beagle/am62x_beagleplay.rst b/doc/board/beagle/am62x_beagleplay.rst
index 7784e62b0b7..4c8b0845845 100644
--- a/doc/board/beagle/am62x_beagleplay.rst
+++ b/doc/board/beagle/am62x_beagleplay.rst
@@ -270,6 +270,18 @@ for details.
 To switch to SD card boot mode, hold the USR button while powering on
 with Type-C power supply, then release when power LED lights up.
 
+DFU based boot
+--------------
+
+To boot the board over DFU, ensure there is no SD card inserted with a
+bootloader. Hold the USR switch while plugging into the Type C to boot into DFU
+mode. After power-on the build artifacts needs to be uploaded one by one with a
+tool like dfu-util.
+
+.. include::  ../ti/am62x_sk.rst
+    :start-after: .. am62x_evm_rst_include_start_dfu_boot
+    :end-before: .. am62x_evm_rst_include_end_dfu_boot
+
 Debugging U-Boot
 ----------------
 
diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
index b12dc85f06b..904a54cd5ff 100644
--- a/doc/board/ti/am62x_sk.rst
+++ b/doc/board/ti/am62x_sk.rst
@@ -105,6 +105,20 @@ Set the variables corresponding to this platform:
 
 * 3.1 R5:
 
+.. include::  ../ti/k3.rst
+    :start-after: .. k3_rst_include_start_build_steps_spl_r5
+    :end-before: .. k3_rst_include_end_build_steps_spl_r5
+
+* 3.1.1 Alternatively build R5 for DFU boot:
+
+As the SPL size can get to big when building with support for booting both from
+local storage *and* DFU an extra config fragment should be used to enable DFU
+support (and disable storage support)
+
+.. prompt:: bash $
+
+  export UBOOT_CFG_CORTEXR=${UBOOT_CFG_CORTEXR} am62x_r5_usbdfu.config
+
 .. include::  ../ti/k3.rst
     :start-after: .. k3_rst_include_start_build_steps_spl_r5
     :end-before: .. k3_rst_include_end_build_steps_spl_r5
@@ -251,6 +265,29 @@ https://www.ti.com/lit/pdf/spruiv7 under the `Boot Mode Pins` section.
 
 For SW2 and SW1, the switch state in the "ON" position = 1.
 
+DFU based boot
+--------------
+
+To boot the board over DFU, set the switches to DFU mode and connect to the
+USB Type C DRD Port on the board. After power-on the build artifacts needs to be
+uploaded one by one with a tool like dfu-util.
+
+.. am62x_evm_rst_include_start_dfu_boot
+
+The initial ROM will have a DFU alt named `bootloader` for the initial R5 spl
+upload. The next stages as exposed by u-boot have target alts matching the name
+of the artifacts, for these a USB reset has to be done after each upload.
+
+When using dfu-util the following commands can be used to boot to a u-boot shell:
+
+.. prompt:: bash $
+
+  dfu-util -a bootloader -D tiboot3.bin
+  dfu-util -R -a tispl -D tispl.bin
+  dfu-util -R -a u-boot.img -D u-boot.img
+
+.. am62x_evm_rst_include_end_dfu_boot
+
 Debugging U-Boot
 ----------------
 
-- 
2.43.0

Re: [PATCH v4 7/7] doc: board: Add document for DFU boot on am62x SoCs

[Nishanth Menon]

On 09:52-20240112, Sjoerd Simons wrote:
> Both AM62 SK and beagleplay support DFU boot in a similar way now;
> Document how to actually run DFU boot for both boards
> 
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>
> 
> ---
> 
> Changes in v4:
> - New patch
> 
>  doc/board/beagle/am62x_beagleplay.rst | 12 +++++++++
>  doc/board/ti/am62x_sk.rst             | 37 +++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/doc/board/beagle/am62x_beagleplay.rst b/doc/board/beagle/am62x_beagleplay.rst
> index 7784e62b0b7..4c8b0845845 100644
> --- a/doc/board/beagle/am62x_beagleplay.rst
> +++ b/doc/board/beagle/am62x_beagleplay.rst
> @@ -270,6 +270,18 @@ for details.
>  To switch to SD card boot mode, hold the USR button while powering on
>  with Type-C power supply, then release when power LED lights up.
>  
> +DFU based boot
> +--------------
> +
> +To boot the board over DFU, ensure there is no SD card inserted with a
> +bootloader. Hold the USR switch while plugging into the Type C to boot into DFU
> +mode. After power-on the build artifacts needs to be uploaded one by one with a
> +tool like dfu-util.

Don't we also need a wiped out emmc unfortunately?

> +
> +.. include::  ../ti/am62x_sk.rst
> +    :start-after: .. am62x_evm_rst_include_start_dfu_boot
> +    :end-before: .. am62x_evm_rst_include_end_dfu_boot
> +
>  Debugging U-Boot
>  ----------------
>  
> diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
> index b12dc85f06b..904a54cd5ff 100644
> --- a/doc/board/ti/am62x_sk.rst
> +++ b/doc/board/ti/am62x_sk.rst
> @@ -105,6 +105,20 @@ Set the variables corresponding to this platform:
>  
>  * 3.1 R5:
>  
> +.. include::  ../ti/k3.rst
> +    :start-after: .. k3_rst_include_start_build_steps_spl_r5
> +    :end-before: .. k3_rst_include_end_build_steps_spl_r5
> +
> +* 3.1.1 Alternatively build R5 for DFU boot:
> +
> +As the SPL size can get to big when building with support for booting both from
> +local storage *and* DFU an extra config fragment should be used to enable DFU
> +support (and disable storage support)
> +
> +.. prompt:: bash $
> +
> +  export UBOOT_CFG_CORTEXR=${UBOOT_CFG_CORTEXR} am62x_r5_usbdfu.config
> +
>  .. include::  ../ti/k3.rst
>      :start-after: .. k3_rst_include_start_build_steps_spl_r5
>      :end-before: .. k3_rst_include_end_build_steps_spl_r5
> @@ -251,6 +265,29 @@ https://www.ti.com/lit/pdf/spruiv7 under the `Boot Mode Pins` section.
>  
>  For SW2 and SW1, the switch state in the "ON" position = 1.
>  
> +DFU based boot
> +--------------
> +
> +To boot the board over DFU, set the switches to DFU mode and connect to the
> +USB Type C DRD Port on the board. After power-on the build artifacts needs to be
> +uploaded one by one with a tool like dfu-util.
> +
> +.. am62x_evm_rst_include_start_dfu_boot
> +
> +The initial ROM will have a DFU alt named `bootloader` for the initial R5 spl
> +upload. The next stages as exposed by u-boot have target alts matching the name
> +of the artifacts, for these a USB reset has to be done after each upload.
> +
> +When using dfu-util the following commands can be used to boot to a u-boot shell:
> +
> +.. prompt:: bash $
> +
> +  dfu-util -a bootloader -D tiboot3.bin
> +  dfu-util -R -a tispl -D tispl.bin
> +  dfu-util -R -a u-boot.img -D u-boot.img
> +
> +.. am62x_evm_rst_include_end_dfu_boot
> +
>  Debugging U-Boot
>  ----------------
>  
> -- 
> 2.43.0
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

Re: [PATCH v4 7/7] doc: board: Add document for DFU boot on am62x SoCs

[Sjoerd Simons]

On Fri, 2024-01-12 at 06:36 -0600, Nishanth Menon wrote:
> On 09:52-20240112, Sjoerd Simons wrote:
> > 
> > +DFU based boot
> > +--------------
> > +
> > +To boot the board over DFU, ensure there is no SD card inserted
> > with a
> > +bootloader. Hold the USR switch while plugging into the Type C to
> > boot into DFU
> > +mode. After power-on the build artifacts needs to be uploaded one
> > by one with a
> > +tool like dfu-util.
> 
> Don't we also need a wiped out emmc unfortunately?

Nope just not having an SD card without boot artifacts is enough (or to
keep things simple no SD card). the USR switch changes the bootmode to
have SD as primary, DFU as fallback, so what's on the eMMC is luckily
not relevant.

So i've been happily testing with the factory eMMC image untouched to
allow me to compare against the factory u-boot behaviour 

> 
> > +
> > +.. include::  ../ti/am62x_sk.rst
> > +    :start-after: .. am62x_evm_rst_include_start_dfu_boot
> > +    :end-before: .. am62x_evm_rst_include_end_dfu_boot
> > +
> >  Debugging U-Boot
> >  ----------------
> >  
> > diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
> > index b12dc85f06b..904a54cd5ff 100644
> > --- a/doc/board/ti/am62x_sk.rst
> > +++ b/doc/board/ti/am62x_sk.rst
> > @@ -105,6 +105,20 @@ Set the variables corresponding to this
> > platform:
> >  
> >  * 3.1 R5:
> >  
> > +.. include::  ../ti/k3.rst
> > +    :start-after: .. k3_rst_include_start_build_steps_spl_r5
> > +    :end-before: .. k3_rst_include_end_build_steps_spl_r5
> > +
> > +* 3.1.1 Alternatively build R5 for DFU boot:
> > +
> > +As the SPL size can get to big when building with support for
> > booting both from
> > +local storage *and* DFU an extra config fragment should be used to
> > enable DFU
> > +support (and disable storage support)
> > +
> > +.. prompt:: bash $
> > +
> > +  export UBOOT_CFG_CORTEXR=${UBOOT_CFG_CORTEXR}
> > am62x_r5_usbdfu.config
> > +
> >  .. include::  ../ti/k3.rst
> >      :start-after: .. k3_rst_include_start_build_steps_spl_r5
> >      :end-before: .. k3_rst_include_end_build_steps_spl_r5
> > @@ -251,6 +265,29 @@ https://www.ti.com/lit/pdf/spruiv7 under the
> > `Boot Mode Pins` section.
> >  
> >  For SW2 and SW1, the switch state in the "ON" position = 1.
> >  
> > +DFU based boot
> > +--------------
> > +
> > +To boot the board over DFU, set the switches to DFU mode and
> > connect to the
> > +USB Type C DRD Port on the board. After power-on the build
> > artifacts needs to be
> > +uploaded one by one with a tool like dfu-util.
> > +
> > +.. am62x_evm_rst_include_start_dfu_boot
> > +
> > +The initial ROM will have a DFU alt named `bootloader` for the
> > initial R5 spl
> > +upload. The next stages as exposed by u-boot have target alts
> > matching the name
> > +of the artifacts, for these a USB reset has to be done after each
> > upload.
> > +
> > +When using dfu-util the following commands can be used to boot to
> > a u-boot shell:
> > +
> > +.. prompt:: bash $
> > +
> > +  dfu-util -a bootloader -D tiboot3.bin
> > +  dfu-util -R -a tispl -D tispl.bin
> > +  dfu-util -R -a u-boot.img -D u-boot.img
> > +
> > +.. am62x_evm_rst_include_end_dfu_boot
> > +
> >  Debugging U-Boot
> >  ----------------
> >  
> > -- 
> > 2.43.0
> > 
> 

-- 
Sjoerd Simons
Collabora Ltd.

Re: [PATCH v4 7/7] doc: board: Add document for DFU boot on am62x SoCs

[Mattijs Korpershoek]

Hi Sjoerd, Nishanth,

On Fri, Jan 12, 2024 at 13:58, Sjoerd Simons <sjoerd@collabora.com> wrote:

> On Fri, 2024-01-12 at 06:36 -0600, Nishanth Menon wrote:
>> On 09:52-20240112, Sjoerd Simons wrote:
>> > 
>> > +DFU based boot
>> > +--------------
>> > +
>> > +To boot the board over DFU, ensure there is no SD card inserted
>> > with a
>> > +bootloader. Hold the USR switch while plugging into the Type C to
>> > boot into DFU
>> > +mode. After power-on the build artifacts needs to be uploaded one
>> > by one with a
>> > +tool like dfu-util.
>> 
>> Don't we also need a wiped out emmc unfortunately?
>
> Nope just not having an SD card without boot artifacts is enough (or to
> keep things simple no SD card). the USR switch changes the bootmode to
> have SD as primary, DFU as fallback, so what's on the eMMC is luckily
> not relevant.
>
> So i've been happily testing with the factory eMMC image untouched to
> allow me to compare against the factory u-boot behaviour 

I've been using DFU for quite some time on Beagle Play and I observe
the same behaviour as Sjoerd. What is on eMMC is fortunetly not relevant
to have DFU mode functional.

>
>> 
>> > +
>> > +.. include::  ../ti/am62x_sk.rst
>> > +    :start-after: .. am62x_evm_rst_include_start_dfu_boot
>> > +    :end-before: .. am62x_evm_rst_include_end_dfu_boot
>> > +
>> >  Debugging U-Boot
>> >  ----------------
>> >  
>> > diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
>> > index b12dc85f06b..904a54cd5ff 100644
>> > --- a/doc/board/ti/am62x_sk.rst
>> > +++ b/doc/board/ti/am62x_sk.rst
>> > @@ -105,6 +105,20 @@ Set the variables corresponding to this
>> > platform:
>> >  
>> >  * 3.1 R5:
>> >  
>> > +.. include::  ../ti/k3.rst
>> > +    :start-after: .. k3_rst_include_start_build_steps_spl_r5
>> > +    :end-before: .. k3_rst_include_end_build_steps_spl_r5
>> > +
>> > +* 3.1.1 Alternatively build R5 for DFU boot:
>> > +
>> > +As the SPL size can get to big when building with support for
>> > booting both from
>> > +local storage *and* DFU an extra config fragment should be used to
>> > enable DFU
>> > +support (and disable storage support)
>> > +
>> > +.. prompt:: bash $
>> > +
>> > +  export UBOOT_CFG_CORTEXR=${UBOOT_CFG_CORTEXR}
>> > am62x_r5_usbdfu.config
>> > +
>> >  .. include::  ../ti/k3.rst
>> >      :start-after: .. k3_rst_include_start_build_steps_spl_r5
>> >      :end-before: .. k3_rst_include_end_build_steps_spl_r5
>> > @@ -251,6 +265,29 @@ https://www.ti.com/lit/pdf/spruiv7 under the
>> > `Boot Mode Pins` section.
>> >  
>> >  For SW2 and SW1, the switch state in the "ON" position = 1.
>> >  
>> > +DFU based boot
>> > +--------------
>> > +
>> > +To boot the board over DFU, set the switches to DFU mode and
>> > connect to the
>> > +USB Type C DRD Port on the board. After power-on the build
>> > artifacts needs to be
>> > +uploaded one by one with a tool like dfu-util.
>> > +
>> > +.. am62x_evm_rst_include_start_dfu_boot
>> > +
>> > +The initial ROM will have a DFU alt named `bootloader` for the
>> > initial R5 spl
>> > +upload. The next stages as exposed by u-boot have target alts
>> > matching the name
>> > +of the artifacts, for these a USB reset has to be done after each
>> > upload.
>> > +
>> > +When using dfu-util the following commands can be used to boot to
>> > a u-boot shell:
>> > +
>> > +.. prompt:: bash $
>> > +
>> > +  dfu-util -a bootloader -D tiboot3.bin
>> > +  dfu-util -R -a tispl -D tispl.bin
>> > +  dfu-util -R -a u-boot.img -D u-boot.img
>> > +
>> > +.. am62x_evm_rst_include_end_dfu_boot
>> > +
>> >  Debugging U-Boot
>> >  ----------------
>> >  
>> > -- 
>> > 2.43.0
>> > 
>> 
>
> -- 
> Sjoerd Simons
> Collabora Ltd.

Re: [PATCH v4 7/7] doc: board: Add document for DFU boot on am62x SoCs

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the patch.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> Both AM62 SK and beagleplay support DFU boot in a similar way now;
> Document how to actually run DFU boot for both boards
>
> Signed-off-by: Sjoerd Simons <sjoerd@collabora.com>

>
> ---
>
> Changes in v4:
> - New patch
>
>  doc/board/beagle/am62x_beagleplay.rst | 12 +++++++++
>  doc/board/ti/am62x_sk.rst             | 37 +++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
>
> diff --git a/doc/board/beagle/am62x_beagleplay.rst b/doc/board/beagle/am62x_beagleplay.rst
> index 7784e62b0b7..4c8b0845845 100644
> --- a/doc/board/beagle/am62x_beagleplay.rst
> +++ b/doc/board/beagle/am62x_beagleplay.rst
> @@ -270,6 +270,18 @@ for details.
>  To switch to SD card boot mode, hold the USR button while powering on
>  with Type-C power supply, then release when power LED lights up.
>  
> +DFU based boot
> +--------------
> +
> +To boot the board over DFU, ensure there is no SD card inserted with a
> +bootloader. Hold the USR switch while plugging into the Type C to boot into DFU
> +mode. After power-on the build artifacts needs to be uploaded one by one with a
> +tool like dfu-util.
> +
> +.. include::  ../ti/am62x_sk.rst
> +    :start-after: .. am62x_evm_rst_include_start_dfu_boot
> +    :end-before: .. am62x_evm_rst_include_end_dfu_boot
> +
>  Debugging U-Boot
>  ----------------
>  
> diff --git a/doc/board/ti/am62x_sk.rst b/doc/board/ti/am62x_sk.rst
> index b12dc85f06b..904a54cd5ff 100644
> --- a/doc/board/ti/am62x_sk.rst
> +++ b/doc/board/ti/am62x_sk.rst
> @@ -105,6 +105,20 @@ Set the variables corresponding to this platform:
>  
>  * 3.1 R5:
>  
> +.. include::  ../ti/k3.rst
> +    :start-after: .. k3_rst_include_start_build_steps_spl_r5
> +    :end-before: .. k3_rst_include_end_build_steps_spl_r5
> +
> +* 3.1.1 Alternatively build R5 for DFU boot:
> +
> +As the SPL size can get to big when building with support for booting both from

s/to/too

> +local storage *and* DFU an extra config fragment should be used to enable DFU
> +support (and disable storage support)
> +
> +.. prompt:: bash $
> +
> +  export UBOOT_CFG_CORTEXR=${UBOOT_CFG_CORTEXR} am62x_r5_usbdfu.config
> +
>  .. include::  ../ti/k3.rst
>      :start-after: .. k3_rst_include_start_build_steps_spl_r5
>      :end-before: .. k3_rst_include_end_build_steps_spl_r5
> @@ -251,6 +265,29 @@ https://www.ti.com/lit/pdf/spruiv7 under the `Boot Mode Pins` section.
>  
>  For SW2 and SW1, the switch state in the "ON" position = 1.
>  
> +DFU based boot
> +--------------
> +
> +To boot the board over DFU, set the switches to DFU mode and connect to the
> +USB Type C DRD Port on the board. After power-on the build artifacts needs to be

s/Type/type
s/Port/port

> +uploaded one by one with a tool like dfu-util.
> +
> +.. am62x_evm_rst_include_start_dfu_boot
> +
> +The initial ROM will have a DFU alt named `bootloader` for the initial R5 spl
> +upload. The next stages as exposed by u-boot have target alts matching the name

s/u-boot/U-Boot

> +of the artifacts, for these a USB reset has to be done after each upload.
> +
> +When using dfu-util the following commands can be used to boot to a u-boot shell:

s/u-boot/U-Boot

With all above typos addressed:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>


> +
> +.. prompt:: bash $
> +
> +  dfu-util -a bootloader -D tiboot3.bin
> +  dfu-util -R -a tispl -D tispl.bin
> +  dfu-util -R -a u-boot.img -D u-boot.img
> +
> +.. am62x_evm_rst_include_end_dfu_boot
> +
>  Debugging U-Boot
>  ----------------
>  
> -- 
> 2.43.0

Re: [PATCH v4 0/7] Add DFU and usb boot for TI am62x SK and beagleplay

[Mattijs Korpershoek]

Hi Sjoerd,

Thank you for the series.

On ven., janv. 12, 2024 at 09:52, Sjoerd Simons <sjoerd@collabora.com> wrote:

> This series adds DFU support for TI AM62 SK board and new since this
> version also for beagleplay.
>
> Since the last revision, apart from beagleplay support the main changes
> are:
> * Documentation was added on how to use this functionality on both boards
> * the R5 configuration is now done via a config fragment rather
>   then a full configuraiton
> * A new patch was added for dwc3 to work in device mode even if dr_mode is
>   configured as otg. Avoiding the need for an explicit switch to
>   periphal in the u-boot dts

I've tested on Beagle Play that I could load/recover the board via
snagboot:

U-Boot SPL 2023.04 (Sep 27 2023 - 11:19:48 +0200)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
SPL initial stack usage: 13392 bytes
Trying to boot from DFU
##########################################DOWNLOAD ... OK
Ctrl+C to exit ...
##########################################DOWNLOAD ... OK
Ctrl+C to exit ...
Loading Environment from nowhere... OK
init_env from device 10 not supported!
Starting ATF on ARM64 core...

NOTICE:  BL3'.9(release):d7a7135d32a8
NOTICE:  BL31: Built : 10:53:52, Jan 16 2024

U-Boot SPL 2024.01-14350-g416e52ada83a (Jan 16 2024 - 10:54:58 +0100)
SYSFW ABI: 3.1 (firmware rev 0x0009 '9.0.5--v09.00.05 (Kool Koala)')
SPL initial stack usage: 1888 bytes
Trying to boot from DFU


U-Boot 2024.01-14350-g416e52ada83a (Jan 16 2024 - 10:54:58 +0100)

SoC:   AM62X SR1.0 GP
Model: BeagleBoard.org BeaglePlay
DRAM:  2 GiB
Core:  104 devices, 29 uclasses, devicetree: separate
MMC:   mmc@fa10000: 0, mmc@fa00000: 1, mmc@fa20000: 2
Loading Environment from nowhere... OK
In:    serial@2800000
Out:   serial@2800000
Err:   serial@2800000
Net:   No ethernet found.

Press SPACE to abort autoboot in 2 seconds


I've also tested (from the DFU'd U-Boot) that I could scan for usb keys:
=> usb start
[..]
1 USB Device(s) found

So for the series:
Tested-by: Mattijs Korpershoek <mkorpershoek@baylibre.com> # on beagle play

>
> I will also submit the dts changes to linux so that those can be dropped
> again in the near future (hopefully)
>
> Changes in v4:
> - Add config dependency on SYSCON
> - Move defines and constants outside out of function scope
> - Don't force usb0 into peripheral mode
> - Move R5 dfu config to a config fragment rather then a full defconfig
> - Don't enable XHCI for the R5 SPL, unneeded
>
> Change in v3:
> - Add dfu via environment rather then config headers
> - Enable usb nodes in all boot phases
> - Run savedefconfig to adjust to more recent u-boot
>
> Changes in v2:
> - Switch dwc3 glue to a seperate driver rather then in dwc-generic
> - Minimize config changes to just DFU configuration
> - Only enable usb port 0 DFU in SPL
> - Create a seperate defconfig for R5
>
> Sjoerd Simons (7):
>   usb: dwc3: Add dwc3 glue driver for am62
>   usb: dwc3: Switch to device mode on gadget start
>   board: ti: am62x: am62x: include env for DFU
>   arm: dts: k3-am625-sk: Enable usb port in u-boot
>   configs: am62x_evm_*: Enable USB and DFU support
>   beagleplay: Add DFU support
>   doc: board: Add document for DFU boot on am62x SoCs
>
>  arch/arm/dts/k3-am625-beagleplay-u-boot.dtsi |   8 ++
>  arch/arm/dts/k3-am625-sk-u-boot.dtsi         |   8 ++
>  board/beagle/beagleplay/beagleplay.env       |   1 +
>  board/ti/am62x/am62x.env                     |   1 +
>  configs/am62x_beagleplay_a53_defconfig       |  30 +++++
>  configs/am62x_evm_a53_defconfig              |  30 +++++
>  configs/am62x_r5_usbdfu.config               |  28 +++++
>  doc/board/beagle/am62x_beagleplay.rst        |  12 ++
>  doc/board/ti/am62x_sk.rst                    |  37 ++++++
>  drivers/usb/dwc3/Kconfig                     |  14 +++
>  drivers/usb/dwc3/Makefile                    |   1 +
>  drivers/usb/dwc3/core.c                      |  10 +-
>  drivers/usb/dwc3/core.h                      |   1 +
>  drivers/usb/dwc3/dwc3-am62.c                 | 125 +++++++++++++++++++
>  drivers/usb/dwc3/gadget.c                    |   6 +
>  15 files changed, 307 insertions(+), 5 deletions(-)
>  create mode 100644 configs/am62x_r5_usbdfu.config
>  create mode 100644 drivers/usb/dwc3/dwc3-am62.c
>
> -- 
> 2.43.0