Re: [PATCH 4/4] ARM: dts: imx8m: imx8mm-mx8menlo: Enable SPL SDP support

From: Peng Fan <peng.fan@oss.nxp.com>
To: Marek Vasut <marex@denx.de>, u-boot@lists.denx.de
Cc: Fabio Estevam <festevam@denx.de>,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>,
	Max Krummenacher <max.krummenacher@toradex.com>,
	Peng Fan <peng.fan@nxp.com>, Stefano Babic <sbabic@denx.de>
Subject: Re: [PATCH 4/4] ARM: dts: imx8m: imx8mm-mx8menlo: Enable SPL SDP support
Date: Wed, 21 Sep 2022 13:30:45 +0800	[thread overview]
Message-ID: <debf4057-db20-e695-1850-a0aa8397661f@oss.nxp.com> (raw)
In-Reply-To: <20220919194118.105820-4-marex@denx.de>

On 9/20/2022 3:41 AM, Marek Vasut wrote:
> Enable DM USB, DM PHY and USB gadget support in imx8mm-mx8menlo SPL
> to let the board continue SDP loading of second stage after the first
> stage was loaded by BootROM SDP implementation. It is not possible to
> jump back into BootROM v1 and let the BootROM implementation continue
> the SDP loading, all this has to be performed by the U-Boot CI HDRC
> controller driver and SDP protocol implementation, both of which fit
> into the SPL just barely.
> 
> With this patch, it is possible to start both U-Boot SPL and U-Boot
> using e.g. uuu on this board as follows:
> 
> $ uuu -brun spl flash.bin
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Fabio Estevam <festevam@denx.de>
> Cc: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Cc: Max Krummenacher <max.krummenacher@toradex.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>   arch/arm/dts/imx8mm-mx8menlo-u-boot.dtsi | 20 ++++++++++++++++++++
>   configs/imx8mm-mx8menlo_defconfig        | 17 ++++++++++++++++-
>   2 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/dts/imx8mm-mx8menlo-u-boot.dtsi b/arch/arm/dts/imx8mm-mx8menlo-u-boot.dtsi
> index 484d493e33c..7f5f8c384e8 100644
> --- a/arch/arm/dts/imx8mm-mx8menlo-u-boot.dtsi
> +++ b/arch/arm/dts/imx8mm-mx8menlo-u-boot.dtsi
> @@ -17,6 +17,26 @@
>   	};
>   };
>   
> +&aips4 {
> +	u-boot,dm-spl;
> +};
> +
>   &i2c4 {
>   	/delete-node/ codec@1a;
>   };
> +
> +&reg_usb_otg1_vbus {
> +	u-boot,dm-spl;
> +};
> +
> +&usbmisc1 {
> +	u-boot,dm-spl;
> +};
> +
> +&usbphynop1 {
> +	u-boot,dm-spl;
> +};
> +
> +&usbotg1 {
> +	u-boot,dm-spl;
> +};
> diff --git a/configs/imx8mm-mx8menlo_defconfig b/configs/imx8mm-mx8menlo_defconfig
> index 929ff382f27..ad6885942d3 100644
> --- a/configs/imx8mm-mx8menlo_defconfig
> +++ b/configs/imx8mm-mx8menlo_defconfig
> @@ -50,6 +50,9 @@ CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x300
>   CONFIG_SPL_I2C=y
>   CONFIG_SPL_POWER=y
> +CONFIG_SPL_USB_HOST=y

Nitpick, would this option required?

Otherwise looks good to me,

Reviewed-by: Peng Fan <peng.fan@nxp.com>

> +CONFIG_SPL_USB_GADGET=y
> +CONFIG_SPL_USB_SDP_SUPPORT=y
>   CONFIG_SPL_WATCHDOG=y
>   CONFIG_SYS_MAXARGS=64
>   CONFIG_SYS_CBSIZE=2048
> @@ -65,6 +68,8 @@ CONFIG_CMD_GPIO=y
>   CONFIG_CMD_I2C=y
>   CONFIG_CMD_MMC=y
>   CONFIG_CMD_USB=y
> +CONFIG_CMD_USB_SDP=y
> +CONFIG_CMD_USB_MASS_STORAGE=y
>   CONFIG_CMD_BOOTCOUNT=y
>   CONFIG_CMD_CACHE=y
>   CONFIG_CMD_UUID=y
> @@ -106,6 +111,8 @@ CONFIG_PHY_MICREL=y
>   CONFIG_PHY_MICREL_KSZ90X1=y
>   CONFIG_FEC_MXC=y
>   CONFIG_MII=y
> +CONFIG_SPL_PHY=y
> +CONFIG_SPL_NOP_PHY=y
>   CONFIG_PINCTRL=y
>   CONFIG_SPL_PINCTRL=y
>   CONFIG_PINCTRL_IMX8M=y
> @@ -125,7 +132,15 @@ CONFIG_SYSRESET_PSCI=y
>   CONFIG_SYSRESET_WATCHDOG=y
>   CONFIG_DM_THERMAL=y
>   CONFIG_USB=y
> -# CONFIG_SPL_DM_USB is not set
>   CONFIG_USB_EHCI_HCD=y
> +CONFIG_MXC_USB_OTG_HACTIVE=y
> +# CONFIG_USB_STORAGE is not set
> +CONFIG_USB_GADGET=y
> +CONFIG_USB_GADGET_MANUFACTURER="Menlo"
> +CONFIG_USB_GADGET_VENDOR_NUM=0x0525
> +CONFIG_USB_GADGET_PRODUCT_NUM=0xa4a5
> +CONFIG_CI_UDC=y
> +CONFIG_SDP_LOADADDR=0x40400000
> +CONFIG_USB_GADGET_DOWNLOAD=y
>   CONFIG_IMX_WATCHDOG=y
>   CONFIG_OF_LIBFDT_OVERLAY=y