[U-Boot] [PATCH V2 51/51] imx: add i.MX8MN DDR4 board support
Peng Fan
peng.fan at nxp.com
Wed Jul 10 10:05:43 UTC 2019
> Subject: Re: [U-Boot] [PATCH V2 51/51] imx: add i.MX8MN DDR4 board
> support
>
[...]
> > +
> > MX8MM_IOMUXC_NAND_CE1_B_USDHC3_STROBE 0x190
>
> Those changes looks identical. Why do you need to change those lines?
> (Wrong formatting) ?
Wrongly added. Drop in V3.
>
> > >;
> > +};
[...]
>
> > diff --git a/arch/arm/dts/imx8mn-ddr4-evk.dts
> > b/arch/arm/dts/imx8mn-ddr4-evk.dts new file mode 100644
> > index 0000000000..9b2c1727a8
> > --- /dev/null
> > +++ b/arch/arm/dts/imx8mn-ddr4-evk.dts
>
> DTS files shall be added in a separate commit, to avoid one very large
> one when you add the board.
Fix in V3.
>
> > @@ -0,0 +1,221 @@
> > + { 0x3d400498, 0x03ff0000 },
> > + { 0x3d40049c, 0x01000e00 },
> > + { 0x3d4004a0, 0x03ff0000 },
> > +};
>
> Isn't there auto training code for i.MX8 as it is for i.MX6Q?
>
> For example:
> https://gitlab.denx.de/u-boot/u-boot/blob/master/arch/arm/mach-imx/mx6
> /ddr.c
> https://gitlab.denx.de/u-boot/u-boot/blob/master/arch/arm/mach-imx/ddr
> mc-vf610-calibration.c
The DDR FW will do auto training. The DDR controller is
complicated and different from i.MX6.
>
>
> > +
> > +/* PHY Initialize Configuration */
> > +struct dram_cfg_param ddr_ddrphy_cfg[] = {
> > +};
[....]
> > +
>
> The list of binary data is quite impressive for i.MX8
There are many many many registers in the DDRC in i.MX8M needs
to configured.
>
> > +struct dram_fsp_msg ddr_dram_fsp_msg[] = {
[..]
> > +{
> > + struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> > +
> > + imx_iomux_v3_setup_multiple_pads(wdog_pads,
>
> Shouldn't those pins be configured via DTS/DM? As fair as I see this is
> the imx8mn_evk.c file (which seems not to be the SPL).
DM WDOG is not enabled here. Just use reset_cpu to do external reset
>
> > ARRAY_SIZE(wdog_pads)); +
> > + set_wdog_reset(wdog);
> > +
> > + imx_iomux_v3_setup_multiple_pads(uart_pads,
> > ARRAY_SIZE(uart_pads)); +
> > + init_uart_clk(1);
>
> This also seems like not DM/DTS code.
We not enable DM UART.
>
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_BOARD_POSTCLK_INIT
> > +int board_postclk_init(void)
> > +{
> > + /* TODO */
> > + return 0;
> > +}
> > +#endif
> > +
> > +int dram_init(void)
> > +{
> > + /* rom_pointer[1] contains the size of TEE occupies */
> > + if (rom_pointer[1])
> > + gd->ram_size = PHYS_SDRAM_SIZE - rom_pointer[1];
> > + else
> > + gd->ram_size = PHYS_SDRAM_SIZE;
> > +
> > + return 0;
> > +}
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, bd_t *bd)
> > +{
> > + return 0;
> > +}
> > +#endif
> > +
> > +int board_init(void)
> > +{
> > + return 0;
> > +}
> > +
> > +int board_mmc_get_env_dev(int devno)
> > +{
> > + return devno - 1;
> > +}
>
> Shouldn't the number of mmc device be get via alias defined in DTS (or
> assigned automatically)?
>
> What is the purpose of this function?
mmc_get_env_dev will call this function. devno is the controller index
inside SoC, however not every controller is enabled in a board.
>
> > +
> > +int board_late_init(void)
> > +{
> > +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> > + env_set("board_name", "DDR4 EVK");
> > + env_set("board_rev", "iMX8MN");
> > +#endif
> > + return 0;
> > +}
> > diff --git a/board/freescale/imx8mn_evk/spl.c
> > b/board/freescale/imx8mn_evk/spl.c new file mode 100644
> > index 0000000000..aa5f37fde0
> > --- /dev/null
> > +++ b/board/freescale/imx8mn_evk/spl.c
> > @@ -0,0 +1,90 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2019 NXP
> > + */
> > +
> > +#include <common.h>
> > +#include <spl.h>
> > +#include <asm/io.h>
> > +#include <errno.h>
> > +#include <asm/arch/imx8mn_pins.h>
> > +#include <asm/arch/sys_proto.h>
> > +#include <asm/arch/clock.h>
> > +#include <asm/gpio.h>
> > +#include <asm/arch/ddr.h>
> > +#include <asm/mach-imx/boot_mode.h>
> > +#include <asm/mach-imx/gpio.h>
> > +#include <asm/mach-imx/iomux-v3.h>
> > +#include <asm/mach-imx/mxc_i2c.h>
> > +
> > +#include <dm/uclass.h>
> > +#include <dm/device.h>
> > +#include <dm/uclass-internal.h>
> > +#include <dm/device-internal.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +int spl_board_boot_device(enum boot_device boot_dev_spl)
> > +{
> > + return BOOT_DEVICE_BOOTROM;
> > +}
> > +
> > +void spl_dram_init(void)
> > +{
> > + ddr_init(&dram_timing);
> > +}
> > +
> > +void spl_board_init(void)
> > +{
> > + struct udevice *dev;
> > +
> > + puts("Normal Boot\n");
> > +
> > + uclass_find_first_device(UCLASS_CLK, &dev);
> > +
> > + for (; dev; uclass_find_next_device(&dev)) {
> > + if (device_probe(dev))
> > + continue;
>
> Is this correct that in the SPL you call probe on all UCLASS_CLK
> devices registered for i.MX8?
Any issue you forseen?
>
> > + }
> > +}
> > +
> > +#ifdef CONFIG_SPL_LOAD_FIT
> > +int board_fit_config_name_match(const char *name)
> > +{
> > + /* Just empty function now - can't decide what to choose */
> > + debug("%s: %s\n", __func__, name);
> > +
> > + return 0;
> > +}
> > +#endif
> > +
> > +void board_init_f(ulong dummy)
> > +{
> > + int ret;
> > +
> > + arch_cpu_init();
> > +
> > + init_uart_clk(1);
> > +
> > + board_early_init_f();
> > +
> > + timer_init();
> > +
> > + preloader_console_init();
> > +
> > + /* Clear the BSS. */
> > + memset(__bss_start, 0, __bss_end - __bss_start);
> > +
> > + ret = spl_init();
> > + if (ret) {
> > + debug("spl_init() failed: %d\n", ret);
> > + hang();
> > + }
> > +
> > + enable_tzc380();
> > +
> > + /* DDR initialization */
> > + spl_dram_init();
> > +
> > + board_init_r(NULL, 0);
> > +}
> > diff --git a/configs/imx8mn_ddr4_evk_defconfig
> > b/configs/imx8mn_ddr4_evk_defconfig new file mode 100644
> > index 0000000000..01e10fc427
> > --- /dev/null
> > +++ b/configs/imx8mn_ddr4_evk_defconfig
> > @@ -0,0 +1,53 @@
> > +CONFIG_ARM=y
> > +CONFIG_ARCH_IMX8M=y
> > +CONFIG_SYS_TEXT_BASE=0x40200000
> > +CONFIG_SPL_GPIO_SUPPORT=y
> > +CONFIG_SPL_LIBCOMMON_SUPPORT=y
> > +CONFIG_SPL_LIBGENERIC_SUPPORT=y
> > +CONFIG_SYS_MALLOC_F_LEN=0x6000
> > +CONFIG_TARGET_IMX8MN_EVK=y
> > +CONFIG_SPL_SERIAL_SUPPORT=y
> > +CONFIG_SPL=y
> > +CONFIG_IMX_ROMAPI_LOADADDR=0x48000000
> > +CONFIG_FIT=y
> > +CONFIG_FIT_EXTERNAL_OFFSET=0x3000
> > +CONFIG_SPL_LOAD_FIT=y
> > +CONFIG_SPL_FIT_GENERATOR="arch/arm/mach-imx/mkimage_fit_atf.sh"
> >
> +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-imx/imx8m
> /imximage-8mn-ddr4.cfg"
> > +CONFIG_DEFAULT_FDT_FILE="fsl-imx8mn-ddr4-evk.dtb"
> > +CONFIG_SPL_TEXT_BASE=0x912000
> > +CONFIG_SPL_BOARD_INIT=y
> > +CONFIG_SPL_BOOTROM_SUPPORT=y
> > +CONFIG_SPL_SEPARATE_BSS=y
> > +CONFIG_SPL_I2C_SUPPORT=y
> > +CONFIG_HUSH_PARSER=y
> > +CONFIG_CMD_CLK=y
> > +CONFIG_CMD_GPIO=y
> > +CONFIG_CMD_I2C=y
> > +CONFIG_CMD_CACHE=y
> > +CONFIG_CMD_REGULATOR=y
> > +CONFIG_CMD_EXT2=y
> > +CONFIG_CMD_EXT4=y
> > +CONFIG_CMD_EXT4_WRITE=y
> > +CONFIG_CMD_FAT=y
> > +CONFIG_OF_CONTROL=y
> > +CONFIG_SPL_OF_CONTROL=y
> > +CONFIG_DEFAULT_DEVICE_TREE="imx8mm-evk"
> > +CONFIG_SPL_DM=y
> > +CONFIG_SPL_CLK=y
> > +CONFIG_CLK_IMX8MN=y
> > +CONFIG_DM_GPIO=y
> > +CONFIG_MXC_GPIO=y
> > +CONFIG_DM_I2C=y
> > +CONFIG_SYS_I2C_MXC=y
> > +CONFIG_DM_MMC=y
> > +CONFIG_PHYLIB=y
> > +CONFIG_DM_ETH=y
> > +CONFIG_PINCTRL=y
> > +CONFIG_SPL_PINCTRL=y
> > +CONFIG_PINCTRL_IMX8M=y
> > +CONFIG_DM_REGULATOR=y
> > +CONFIG_DM_REGULATOR_FIXED=y
> > +CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_MXC_UART=y
> > +CONFIG_DM_THERMAL=y
>
> You could also from the beginning enable CONFIG_WDT and
> CONFIG_SYSRESET
> (as i.MX6 already uses it).
I'll try.
>
> > diff --git a/include/configs/imx8mn_evk.h
> > b/include/configs/imx8mn_evk.h new file mode 100644
> > index 0000000000..6ec8a2e362
> > --- /dev/null
> > +++ b/include/configs/imx8mn_evk.h
> > @@ -0,0 +1,198 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Copyright 2018 NXP
> > + */
> > +
> > +#ifndef __IMX8MN_EVK_H
> > +#define __IMX8MN_EVK_H
> > +
> > +#include <linux/sizes.h>
> > +#include <asm/arch/imx-regs.h>
> > +
> > +#ifdef CONFIG_SECURE_BOOT
> > +#define CONFIG_CSF_SIZE 0x2000 /* 8K region */
> > +#endif
> > +
> > +#define CONFIG_SPL_MAX_SIZE (148 * 1024)
> > +#define CONFIG_SYS_MONITOR_LEN (512 * 1024)
> > +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> > +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR 0x300
> > +#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1
> > +#define CONFIG_SYS_UBOOT_BASE \
> > + (QSPI0_AMBA_BASE +
> CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR *
> > 512) +
> > +#ifdef CONFIG_SPL_BUILD
> > +/*#define CONFIG_ENABLE_DDR_TRAINING_DEBUG*/
> > +#define CONFIG_SPL_WATCHDOG_SUPPORT
> > +#define CONFIG_SPL_DRIVERS_MISC_SUPPORT
> > +#define CONFIG_SPL_POWER_SUPPORT
> > +#define
> > CONFIG_SPL_LDSCRIPT "arch/arm/cpu/armv8/u-boot-spl.lds"
> > +#define CONFIG_SPL_STACK 0x95fff0 +#define
> > CONFIG_SPL_BSS_START_ADDR 0x00950000 +#define
> > CONFIG_SPL_BSS_MAX_SIZE 0x2000 /* 8 KB */
>
> Please fix globally -> replace 0x2000 -> SZ_8K from
> include/linux/sizes.h
Fix in V3.
>
> > +#define CONFIG_SYS_SPL_MALLOC_START 0x42200000 +#define
> > CONFIG_SYS_SPL_MALLOC_SIZE 0x80000 /* 64 KB */ +#define
> > CONFIG_SYS_ICACHE_OFF +#define CONFIG_SYS_DCACHE_OFF
> > +
> > +/* malloc f used before GD_FLG_FULL_MALLOC_INIT set */
> > +#define CONFIG_MALLOC_F_ADDR 0x00940000
> > +
> > +/* For RAW image gives a error info not panic */
> > +#define CONFIG_SPL_ABORT_ON_RAW_IMAGE
> > +
> > +#undef CONFIG_DM_MMC
> > +#undef CONFIG_DM_PMIC
> > +#undef CONFIG_DM_PMIC_PFUZE100
> > +
> > +#define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> > +
> > +#endif
> > +
> > +#define CONFIG_REMAKE_ELF
> > +
> > +#define CONFIG_BOARD_EARLY_INIT_F
> > +#define CONFIG_BOARD_POSTCLK_INIT
> > +#define CONFIG_BOARD_LATE_INIT
> > +
> > +/* Flat Device Tree Definitions */
> > +#define CONFIG_OF_BOARD_SETUP
> > +
> > +#undef CONFIG_CMD_EXPORTENV
> > +#undef CONFIG_CMD_IMPORTENV
> > +#undef CONFIG_CMD_IMLS
> > +
> > +#undef CONFIG_CMD_CRC32
> > +#undef CONFIG_BOOTM_NETBSD
> > +
> > +/* Initial environment variables */
> > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > + "script=boot.scr\0" \
> > + "image=Image\0" \
> > + "console=ttymxc1,115200
> > earlycon=ec_imx6q,0x30890000,115200\0" \
> > + "fdt_addr=0x43000000\0" \
> > + "fdt_high=0xffffffffffffffff\0" \
> > + "boot_fdt=try\0" \
> > + "fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \
> > + "initrd_addr=0x43800000\0" \
> > + "initrd_high=0xffffffffffffffff\0" \
> > + "mmcdev="__stringify(CONFIG_SYS_MMC_ENV_DEV)"\0" \
> > + "mmcpart=" __stringify(CONFIG_SYS_MMC_IMG_LOAD_PART) "\0" \
> > + "mmcroot=" CONFIG_MMCROOT " rootwait rw\0" \
> > + "mmcautodetect=yes\0" \
> > + "mmcargs=setenv bootargs console=${console}
> > root=${mmcroot}\0 " \
> > + "loadbootscript=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr}
> > ${script};\0" \
> > + "bootscript=echo Running bootscript from mmc ...; " \
> > + "source\0" \
> > + "loadimage=fatload mmc ${mmcdev}:${mmcpart} ${loadaddr}
> > ${image}\0" \
> > + "loadfdt=fatload mmc ${mmcdev}:${mmcpart} ${fdt_addr}
> > ${fdt_file}\0" \
> > + "mmcboot=echo Booting from mmc ...; " \
> > + "run mmcargs; " \
> > + "if test ${boot_fdt} = yes || test ${boot_fdt} =
> > try; then " \
> > + "if run loadfdt; then " \
> > + "booti ${loadaddr} - ${fdt_addr}; " \
>
> Please correct me if I'm wrong, but why do you use 'booti' (and
> presumably boot Image?).
>
> Please use fitImage (in think that it is mature enough to be used for
> new ports - other imx SoCs use it already for some time);
>
> It can be called as: bootm ${loadaddr}#conf@${fdt_conf}
No. We not switch to FIT when uboot booting kernel.
>
>
> > + "else " \
> > + "echo WARN: Cannot load the DT; " \
> > + "fi; " \
> > + "else " \
> > + "echo wait for boot; " \
> > + "fi;\0" \
> > + "netargs=setenv bootargs console=${console} " \
> > + "root=/dev/nfs " \
> > + "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
> > + "netboot=echo Booting from net ...; " \
> > + "run netargs; " \
> > + "if test ${ip_dyn} = yes; then " \
> > + "setenv get_cmd dhcp; " \
> > + "else " \
> > + "setenv get_cmd tftp; " \
> > + "fi; " \
> > + "${get_cmd} ${loadaddr} ${image}; " \
> > + "if test ${boot_fdt} = yes || test ${boot_fdt} =
> > try; then " \
> > + "if ${get_cmd} ${fdt_addr} ${fdt_file}; then
> > " \
> > + "booti ${loadaddr} - ${fdt_addr}; " \
> > + "else " \
> > + "echo WARN: Cannot load the DT; " \
> > + "fi; " \
> > + "else " \
> > + "booti; " \
> > + "fi;\0"
> > +
> > +#define CONFIG_BOOTCOMMAND \
> > + "mmc dev ${mmcdev}; if mmc rescan; then " \
> > + "if run loadbootscript; then " \
> > + "run bootscript; " \
> > + "else " \
> > + "if run loadimage; then " \
> > + "run mmcboot; " \
> > + "else run netboot; " \
> > + "fi; " \
> > + "fi; " \
> > + "else booti ${loadaddr} - ${fdt_addr}; fi"
> > +
> > +/* Link Definitions */
> > +#define CONFIG_LOADADDR 0x40480000
> > +
> > +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
> > +
> > +#define CONFIG_SYS_INIT_RAM_ADDR 0x40000000
> > +#define CONFIG_SYS_INIT_RAM_SIZE 0x80000
> > +#define CONFIG_SYS_INIT_SP_OFFSET \
> > + (CONFIG_SYS_INIT_RAM_SIZE - GENERATED_GBL_DATA_SIZE)
> > +#define CONFIG_SYS_INIT_SP_ADDR \
> > + (CONFIG_SYS_INIT_RAM_ADDR + CONFIG_SYS_INIT_SP_OFFSET)
> > +
> > +#define CONFIG_ENV_OVERWRITE
> > +#if defined(CONFIG_ENV_IS_IN_MMC)
> > +#define CONFIG_ENV_OFFSET (64 * SZ_64K)
> > +#endif
> > +#define CONFIG_ENV_SIZE 0x1000
> > +#define CONFIG_SYS_MMC_ENV_DEV 0 /* USDHC2 */
> > +#define CONFIG_MMCROOT "/dev/mmcblk1p2" /*
> > USDHC2 */ +
> > +/* Size of malloc() pool */
> > +#define CONFIG_SYS_MALLOC_LEN SZ_32M
> > +
> > +#define CONFIG_SYS_SDRAM_BASE 0x40000000
> > +#define PHYS_SDRAM 0x40000000
> > +#define PHYS_SDRAM_SIZE 0x80000000 /* 2GB DDR
> > */ +
> > +#define CONFIG_SYS_MEMTEST_START PHYS_SDRAM
> > +#define CONFIG_SYS_MEMTEST_END
> (CONFIG_SYS_MEMTEST_START +
> > (PHYS_SDRAM_SIZE >> 1)) +
> > +#define CONFIG_BAUDRATE 115200
> > +
> > +#define CONFIG_MXC_UART_BASE UART2_BASE_ADDR
> > +
> > +/* Monitor Command Prompt */
> > +#undef CONFIG_SYS_PROMPT
> > +#define CONFIG_SYS_PROMPT "u-boot=> "
> > +#define CONFIG_SYS_PROMPT_HUSH_PS2 "> "
> > +#define CONFIG_SYS_CBSIZE 2048
> > +#define CONFIG_SYS_MAXARGS 64
> > +#define CONFIG_SYS_BARGSIZE CONFIG_SYS_CBSIZE
> > +#define CONFIG_SYS_PBSIZE (CONFIG_SYS_CBSIZE + \
> > + sizeof(CONFIG_SYS_PROMPT) +
> > 16) +
> > +#define CONFIG_IMX_BOOTAUX
>
> Has the i.MX8 the use case for IMX_BOOTAUX - if not please remove it.
i.MX8M use bootaux to kick M4
>
> > +
> > +/* USDHC */
> > +#define CONFIG_CMD_MMC
> > +#define CONFIG_FSL_ESDHC
> > +#define CONFIG_FSL_USDHC
> > +
> > +#define CONFIG_SYS_FSL_USDHC_NUM 2
> > +#define CONFIG_SYS_FSL_ESDHC_ADDR 0
>
> Some of the above are in Kconfig.
>
> > +
> > +#define CONFIG_SUPPORT_EMMC_BOOT /* eMMC specific */
> > +#define CONFIG_SYS_MMC_IMG_LOAD_PART 1
> > +
> > +#define CONFIG_CMD_FUSE
> > +
> > +#define CONFIG_SYS_I2C_MXC_I2C1 /* enable I2C bus 1 */
> > +#define CONFIG_SYS_I2C_MXC_I2C2 /* enable I2C bus 2 */
> > +#define CONFIG_SYS_I2C_MXC_I2C3 /* enable I2C bus 3 */
>
> > +#define CONFIG_SYS_I2C_SPEED 100000
>
> Those #defines (I2C related) are already supported in Kconfig. Please
> use them.
>
>
> Please review above defines and move to Kconfig.
Sure.
Thanks,
Peng.
>
> > +
> > +#define CONFIG_OF_SYSTEM_SETUP
> > +#endif
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma at denx.de
More information about the U-Boot
mailing list