Hi Peng,
Thanks for submitting this series.
On Mon, Apr 12, 2021 at 8:43 AM Peng Fan (OSS) <peng....@oss.nxp.com> wrote:
>
> From: Peng Fan <peng....@nxp.com>
>
> Add i.MX8ULP EVK basic support, support SD/I2C/ENET/LPUART
>
> Note: upower API currently not included.
>
> Signed-off-by: Peng Fan <peng....@nxp.com>
> ---
> arch/arm/dts/imx8ulp-emulator-u-boot.dtsi | 32 +
> arch/arm/dts/imx8ulp-emulator.dts | 93 +
These emulator-related files can be dropped as they are only for
internal NXP usage.
> arch/arm/dts/imx8ulp-evk-u-boot.dtsi | 32 +
> arch/arm/dts/imx8ulp-evk.dts | 204 +++
> arch/arm/mach-imx/imx8ulp/Kconfig | 7 +
> board/freescale/imx8ulp_evk/Kconfig | 14 +
> board/freescale/imx8ulp_evk/MAINTAINERS | 6 +
> board/freescale/imx8ulp_evk/Makefile | 7 +
> board/freescale/imx8ulp_evk/ddr_init.c | 207 +++
> board/freescale/imx8ulp_evk/imx8ulp_evk.c | 67 +
> board/freescale/imx8ulp_evk/lpddr4_timing.c | 1696 +++++++++++++++++++
> board/freescale/imx8ulp_evk/spl.c | 146 ++
> configs/imx8ulp_evk_defconfig | 103 ++
> include/configs/imx8ulp_evk.h | 108 ++
> 14 files changed, 2722 insertions(+)
> create mode 100644 arch/arm/dts/imx8ulp-emulator-u-boot.dtsi
> create mode 100644 arch/arm/dts/imx8ulp-emulator.dts
> create mode 100644 arch/arm/dts/imx8ulp-evk-u-boot.dtsi
> create mode 100644 arch/arm/dts/imx8ulp-evk.dts
> create mode 100644 board/freescale/imx8ulp_evk/Kconfig
> create mode 100644 board/freescale/imx8ulp_evk/MAINTAINERS
> create mode 100644 board/freescale/imx8ulp_evk/Makefile
> create mode 100644 board/freescale/imx8ulp_evk/ddr_init.c
> create mode 100644 board/freescale/imx8ulp_evk/imx8ulp_evk.c
> create mode 100644 board/freescale/imx8ulp_evk/lpddr4_timing.c
> create mode 100644 board/freescale/imx8ulp_evk/spl.c
Please add a readme file that explains how the final binary is
generated, the various required firmware files, etc.
I understand that some of the git repo's may not be public at the
moment, but you could put a "TBD" in the git URL for now.
Better to put the read me now, so that when someone wants to test
this, the build instructions are available.
> + /* Check PLL lock status */
> + lock_0 = readl(DENALI_PHY_1564) & 0xffff;
> + lock_1 = (readl(DENALI_PHY_1564) >> 16) & 0xffff;
> + lock_2 = readl(DENALI_PHY_1565) & 0xffff;
> +
> + if ((lock_0 & 0x3) != 0x3 || (lock_1 & 0x3) != 0x3 || (lock_2 & 0x3)
> != 0x3) {
> + printf("De-Skew PLL failed to lock\n");
> + printf("lock_0=0x%x, lock_1=0x%x, lock_2=0x%x\n", lock_0,
> lock_1, lock_2);
> + return -1;
-1 is not an appropriate return value. What about returning -ETIMEDOUT?
Also, shouldn't you wait for some time period until the lock the PLL
gets locked?
> +void enable_bypass_modeenable_bypass_mode(void)
This function is not used anywhere. Please remove it.
> +int board_early_init_f(void)
> +{
> + return 0;
> +}
Better drop this function and its Kconfig option if you don't need it.
> +
> +int board_late_init(void)
> +{
> + return 0;
Better drop this function and its Kconfig option if you don't need it.
> +}
> diff --git a/board/freescale/imx8ulp_evk/lpddr4_timing.c
> b/board/freescale/imx8ulp_evk/lpddr4_timing.c
> new file mode 100644
> index 0000000000..e827f01cd7
>
> +static void xrdc_configure_mda(void)
> +{
> + ulong xrdc_base = 0x292f0000, off;
> + u32 i = 0;
No need to initialize i to 0.
> +
> + /* Set MDA3-5 for PXP, ENET, CAAM to DID 1*/
> + for (i = 3; i <= 5; i++) {
> + off = 0x800 + i * 0x20;
> + writel(0x200000A1, xrdc_base + off);
> + writel(0xA00000A1, xrdc_base + off);
> + }
> +++ b/configs/imx8ulp_evk_defconfig
> @@ -0,0 +1,103 @@
> +CONFIG_ARM=y
> +CONFIG_ARCH_IMX8ULP=y
> +CONFIG_SYS_TEXT_BASE=0x80200000
> +CONFIG_SYS_MALLOC_F_LEN=0x2000
> +CONFIG_DM_GPIO=y
> +CONFIG_TARGET_IMX8ULP_EVK=y
> +CONFIG_SPL_SERIAL_SUPPORT=y
> +CONFIG_NR_DRAM_BANKS=1
> +CONFIG_SPL=y
> +CONFIG_SPL_TEXT_BASE=0x22020000
> +CONFIG_SPL_LOAD_IMX_CONTAINER=y
> +CONFIG_FIT=y
> +CONFIG_FIT_VERBOSE=y
> +CONFIG_BOOTDELAY=0
This is not developer-friendly. Please remove
> +CONFIG_DEFAULT_FDT_FILE="imx8ulp-evk.dtb"
> +CONFIG_SPL_BOARD_INIT=y
> +CONFIG_SPL_SEPARATE_BSS=y
> +#CONFIG_SPL_RAM_SUPPORT=y
> +#CONFIG_SPL_RAM_DEVICE=y
> +#CONFIG_SPL_MMC_SUPPORT=y
This is not the correct way to disable a Kconfig symbol.
Please re-generate this using make savedefconfig.
+CONFIG_SPL_SYS_ICACHE_OFF=y
> +CONFIG_SPL_SYS_DCACHE_OFF=y
Why are the caches turned off?
> +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
> +CONFIG_MISC=y
> +
> +CONFIG_CMD_I2C=y
> +CONFIG_CMD_FUSE=y
> +CONFIG_CMD_GPIO=y
> +CONFIG_CMD_MEMTEST=y
> +CONFIG_CMD_EXT2=y
> +CONFIG_CMD_EXT4=y
> +CONFIG_CMD_EXT4_WRITE=y
> +CONFIG_CMD_FAT=y
> +CONFIG_CMD_CACHE=y
> +CONFIG_CMD_REGULATOR=y
> +CONFIG_BAUDRATE=115200
> +
> +CONFIG_DM_REGULATOR=y
> +CONFIG_DM_REGULATOR_FIXED=y
> +CONFIG_DM_REGULATOR_GPIO=y
> +#define CONFIG_SERIAL_TAG
Do you need this?
> +#define CONFIG_REMAKE_ELF
> +
> +#define CONFIG_BOARD_EARLY_INIT_F
> +#define CONFIG_BOARD_LATE_INIT
This is a Kconfig symbol now.
> +
> +/* ENET Config */
> +#if defined(CONFIG_FEC_MXC)
> +#define CONFIG_ETHPRIME "FEC"
> +#define PHY_ANEG_TIMEOUT 20000
> +
> +#define CONFIG_FEC_XCV_TYPE RMII
> +#define CONFIG_FEC_MXC_PHYADDR 1
> +#define FEC_QUIRK_ENET_MAC
> +
> +#define IMX_FEC_BASE 0x29950000
With DM you don't need these.
