[PATCH] board: phytec: imx8mp: Add PHYTEC phyCORE-i.MX8MP support

Fri Jan 8 14:46:48 CET 2021

> > > +}
> > > +
> > > +#if 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;
> > > +}
> > 
> > This is also a dummy copied form evk board, just ask if it is not
> > better
> > to remove it at all and introduce it when it is needed.

The CONFIG_SPL_LOAD_FIT seems to be required by the spl_imx_romapi
which we make use of.
So I rather remove the ifdef here, as it is always needed.

Teresa

> 
> Yes, I will remove for now.
> 
> > > +#endif
> > > +
> > > +#define UART_PAD_CTRL   (PAD_CTL_DSE6 | PAD_CTL_FSEL1)
> > > +#define WDOG_PAD_CTRL   (PAD_CTL_DSE6 | PAD_CTL_ODE |
> > > PAD_CTL_PUE
> > > > PAD_CTL_PE)
> > > +
> > > +static iomux_v3_cfg_t const uart_pads[] = {
> > > +	MX8MP_PAD_UART2_RXD__UART2_DCE_RX |
> > > MUX_PAD_CTRL(UART_PAD_CTRL),
> > > +	MX8MP_PAD_UART2_TXD__UART2_DCE_TX |
> > > MUX_PAD_CTRL(UART_PAD_CTRL),
> > > +};
> > > +
> > > +static iomux_v3_cfg_t const wdog_pads[] = {
> > > +	MX8MP_PAD_GPIO1_IO02__WDOG1_WDOG_B  |
> > > MUX_PAD_CTRL(WDOG_PAD_CTRL),
> > > +};
> > > +
> > > +int board_early_init_f(void)
> > > +{
> > > +	struct wdog_regs *wdog = (struct wdog_regs *)WDOG1_BASE_ADDR;
> > > +
> > > +	imx_iomux_v3_setup_multiple_pads(wdog_pads,
> > > ARRAY_SIZE(wdog_pads));
> > > +
> > > +	set_wdog_reset(wdog);
> > > +
> > > +	imx_iomux_v3_setup_multiple_pads(uart_pads,
> > > ARRAY_SIZE(uart_pads));
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +void board_init_f(ulong dummy)
> > > +{
> > > +	int ret;
> > > +
> > > +	arch_cpu_init();
> > > +
> > > +	init_uart_clk(1);
> > > +
> > > +	board_early_init_f();
> > > +
> > > +	ret = spl_early_init();
> > > +	if (ret) {
> > > +		debug("spl_early_init() failed: %d\n", ret);
> > > +		hang();
> > > +	}
> > > +
> > > +	preloader_console_init();
> > > +
> > > +	enable_tzc380();
> > > +
> > > +	setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &i2c_pad_info1);
> > > +
> > > +	power_init_board();
> > > +
> > > +	/* DDR initialization */
> > > +	spl_dram_init();
> > > +}
> > > diff --git a/configs/phycore-imx8mp_defconfig b/configs/phycore-
> > > imx8mp_defconfig
> > > new file mode 100644
> > > index 000000000000..03bd1c0d511a
> > > --- /dev/null
> > > +++ b/configs/phycore-imx8mp_defconfig
> > > @@ -0,0 +1,98 @@
> > > +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=0x10000
> > > +CONFIG_ENV_SIZE=0x10000
> > > +CONFIG_ENV_OFFSET=0x3C0000
> > > +CONFIG_SYS_I2C_MXC_I2C1=y
> > > +CONFIG_SYS_I2C_MXC_I2C2=y
> > > +CONFIG_SYS_I2C_MXC_I2C3=y
> > 
> > Do you have 3 I2C bus or just one as reported in DTS ?
> 
> we use one on the SoM so I will remove the other ones from defconfig.
> 
> > > +CONFIG_DM_GPIO=y
> > > +CONFIG_SPL_TEXT_BASE=0x920000
> > > +CONFIG_TARGET_PHYCORE_IMX8MP=y
> > > +CONFIG_SPL_MMC_SUPPORT=y
> > > +CONFIG_SPL_SERIAL_SUPPORT=y
> > > +CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
> > > +CONFIG_SPL=y
> > > +CONFIG_SPL_IMX_ROMAPI_LOADADDR=0x48000000
> > > +CONFIG_DEFAULT_DEVICE_TREE="phycore-imx8mp"
> > > +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_OF_SYSTEM_SETUP=y
> > > +CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=arch/arm/mach-
> > > imx/imx8m/imximage-8mp-lpddr4.cfg"
> > > +CONFIG_DEFAULT_FDT_FILE="oftree"
> > > +CONFIG_BOARD_LATE_INIT=y
> > > +CONFIG_SPL_BOARD_INIT=y
> > > +CONFIG_SPL_BOOTROM_SUPPORT=y
> > > +CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> > > +CONFIG_SPL_SEPARATE_BSS=y
> > > +CONFIG_SPL_I2C_SUPPORT=y
> > > +CONFIG_SPL_POWER_SUPPORT=y
> > > +CONFIG_SPL_WATCHDOG_SUPPORT=y
> > > +CONFIG_HUSH_PARSER=y
> > > +CONFIG_SYS_PROMPT="u-boot=> "
> > > +# CONFIG_CMD_EXPORTENV is not set
> > > +# CONFIG_CMD_IMPORTENV is not set
> > > +# CONFIG_CMD_CRC32 is not set
> > > +CONFIG_CMD_CLK=y
> > > +CONFIG_CMD_EEPROM=y
> > > +CONFIG_CMD_FUSE=y
> > > +CONFIG_CMD_GPIO=y
> > > +CONFIG_CMD_I2C=y
> > > +CONFIG_CMD_MMC=y
> > > +CONFIG_CMD_DHCP=y
> > > +CONFIG_CMD_MII=y
> > > +CONFIG_CMD_PING=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_ENV_OVERWRITE=y
> > > +CONFIG_ENV_IS_IN_MMC=y
> > > +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > > +CONFIG_SYS_MMC_ENV_DEV=2
> > > +CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG=y
> > > +CONFIG_SPL_DM=y
> > > +CONFIG_CLK_COMPOSITE_CCF=y
> > > +CONFIG_CLK_IMX8MP=y
> > > +CONFIG_MXC_GPIO=y
> > > +CONFIG_DM_I2C=y
> > > +CONFIG_SYS_I2C_MXC=y
> > > +CONFIG_MISC=y
> > > +CONFIG_I2C_EEPROM=y
> > > +CONFIG_SYS_I2C_EEPROM_ADDR=0x51
> > > +CONFIG_SYS_EEPROM_SIZE=4096
> > > +CONFIG_SYS_EEPROM_PAGE_WRITE_BITS=5
> > > +CONFIG_SYS_EEPROM_PAGE_WRITE_DELAY_MS=5
> > > +CONFIG_SYS_I2C_EEPROM_ADDR_LEN=2
> > > +CONFIG_SYS_I2C_EEPROM_ADDR_OVERFLOW=0x0
> > > +CONFIG_DM_MMC=y
> > > +CONFIG_SUPPORT_EMMC_BOOT=y
> > > +CONFIG_MMC_IO_VOLTAGE=y
> > > +CONFIG_MMC_UHS_SUPPORT=y
> > > +CONFIG_MMC_HS400_ES_SUPPORT=y
> > > +CONFIG_MMC_HS400_SUPPORT=y
> > > +CONFIG_FSL_ESDHC_IMX=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_SYSRESET=y
> > > +CONFIG_SPL_SYSRESET=y
> > > +CONFIG_SYSRESET_PSCI=y
> > > +CONFIG_SYSRESET_WATCHDOG=y
> > > +CONFIG_DM_THERMAL=y
> > > +CONFIG_IMX_WATCHDOG=y
> > > diff --git a/include/configs/phycore_imx8mp.h
> > > b/include/configs/phycore_imx8mp.h
> > > new file mode 100644
> > > index 000000000000..c63d0e0380e4
> > > --- /dev/null
> > > +++ b/include/configs/phycore_imx8mp.h
> > > @@ -0,0 +1,110 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-or-later
> > > + *
> > > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > > + * Author: Teresa Remmet <t.remmet at phytec.de>
> > > + */
> > > +
> > > +#ifndef __PHYCORE_IMX8MP_H
> > > +#define __PHYCORE_IMX8MP_H
> > > +
> > > +#include <linux/sizes.h>
> > > +#include <asm/arch/imx-regs.h>
> > > +
> > > +#define CONFIG_SYS_BOOTM_LEN		SZ_64M
> > > +
> > > +#define CONFIG_SPL_MAX_SIZE		(152 * SZ_1K)
> > > +#define CONFIG_SYS_MONITOR_LEN		SZ_512K
> > > +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
> > > +#define CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR	0x300
> > > +#define CONFIG_SYS_UBOOT_BASE \
> > > +		(QSPI0_AMBA_BASE +
> > > CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512)
> > > +
> > > +#ifdef CONFIG_SPL_BUILD
> > > +#define CONFIG_SPL_LDSCRIPT		"arch/arm/cpu/armv8/u-
> > > boot-spl.lds"
> > > +#define CONFIG_SPL_STACK		0x960000
> > > +#define CONFIG_SPL_BSS_START_ADDR	0x98FC00
> > > +#define CONFIG_SPL_BSS_MAX_SIZE		SZ_1K
> > > +#define CONFIG_SYS_SPL_MALLOC_START	0x42200000
> > > +#define CONFIG_SYS_SPL_MALLOC_SIZE	SZ_512K
> > > +
> > > +#define CONFIG_SPL_ABORT_ON_RAW_IMAGE
> > > +
> > > +#define CONFIG_POWER
> > > +#define CONFIG_POWER_I2C
> > > +#define CONFIG_POWER_PCA9450
> > > +
> > > +#undef CONFIG_DM_I2C
> > > +#define CONFIG_SYS_I2C
> > > +
> > > +#endif
> > > +
> > > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > > +	"image=Image\0" \
> > > +	"console=ttymxc1,115200\0" \
> > > +	"fdt_addr=0x48000000\0" \
> > > +	"fdt_file=" CONFIG_DEFAULT_FDT_FILE "\0" \
> > > +	"mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
> > > +	"mmcpart=" __stringify(CONFIG_SYS_MMC_IMG_LOAD_PART) "\0" \
> > > +	"mmcroot=2\0" \
> > > +	"mmcautodetect=yes\0" \
> > > +	"mmcargs=setenv bootargs console=${console} " \
> > > +		"root=/dev/mmcblk${mmcdev}p${mmcroot} rootwait rw\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 run loadfdt; then " \
> > > +			"booti ${loadaddr} - ${fdt_addr}; " \
> > > +		"else " \
> > > +			"echo WARN: Cannot load the DT; " \
> > > +		"fi;\0 " \
> > > +
> > > +#define CONFIG_BOOTCOMMAND \
> > > +	"mmc dev ${mmcdev}; if mmc rescan; then " \
> > > +		"if run loadimage; then " \
> > > +			"run mmcboot; " \
> > > +		"else run netboot; " \
> > > +		"fi; " \
> > > +	"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	SZ_512K
> > > +#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_MMCROOT			"/dev/mmcblk2p2"  /*
> > > USDHC3 */
> > > +
> > > +/* Size of malloc() pool */
> > > +#define CONFIG_SYS_MALLOC_LEN		SZ_32M
> > > +#define CONFIG_SYS_SDRAM_BASE		0x40000000
> > > +
> > > +#define PHYS_SDRAM			SZ_1G
> > 
> > Even if value is the same, it sounds to me weird to set a start
> > address
> > with a macro used for Size.
> 
> True, I will change this back.
> 
> > > +#define PHYS_SDRAM_SIZE			0x80000000
> > > +
> > > +/* UART */
> > > +#define CONFIG_MXC_UART_BASE		UART2_BASE_ADDR
> > > +
> > > +/* Monitor Command Prompt */
> > > +#define CONFIG_SYS_PROMPT_HUSH_PS2	"> "
> > 
> > But this is the value defined in common/cli_hush.c , so you do not
> > need
> > to set it. (I know, copy&paste from other boards...it is also not
> > needed
> > in other boards' header).
> 
> thanks for pointing this out, as I find it quite difficult to find a
> reference for what actually state of the art is.
> 
> > > +#define CONFIG_SYS_CBSIZE		SZ_2K
> > > +#define CONFIG_SYS_MAXARGS		64
> > > +#define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
> > > +#define CONFIG_SYS_PBSIZE		(CONFIG_SYS_CBSIZE + \
> > > +					sizeof(CONFIG_SYS_PROMPT) + 16)
> > This macro is already in fallback, you do not need to define
> > CONFIG_SYS_PBSIZE
> 
> will be removed also.
> 
> Thanks for the review.
> 
> Teresa
> > > +
> > > +/* USDHC */
> > > +#define CONFIG_FSL_USDHC
> > > +#define CONFIG_SYS_FSL_USDHC_NUM	2
> > > +#define CONFIG_SYS_FSL_ESDHC_ADDR       0
> > > +#define CONFIG_SYS_MMC_IMG_LOAD_PART	1
> > > +
> > > +/* I2C */
> > > +#define CONFIG_SYS_I2C_SPEED		100000
> > > +
> > > +#endif /* __PHYCORE_IMX8MP_H */
> > > 
> > 
> > Best regards,
> > Stefano Babic
> > 