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

Teresa Remmet t.remmet at phytec.de
Wed Jan 13 12:31:28 CET 2021


Hello Heiko,

Am Montag, den 11.01.2021, 13:11 +0100 schrieb Heiko Schocher:
> Hello Teresa,
> 
> Am 11.01.21 um 08:30 schrieb Teresa Remmet:
> > Add initial support PHYTEC phyCORE-i.MX8MP SOM.
> > 
> >     Supported features:
> >      - 2GB LPDDR4 RAM
> >      - eMMC
> >      - external SD
> >      - debug UART2
> >      - watchdog
> > 
> > Signed-off-by: Teresa Remmet <t.remmet at phytec.de>
> > ---
> > Changes in v2:
> > - remove not needed spl board init
> > - remove ifdef from board_fit_config_name_match
> > - disabled not needed i2c busses from defconfig
> > - not use size macro for PHYS_SDRAM
> > - remove makros from phycore_imx8mp.h that are defined
> >   elsewhere
> > 
> >  arch/arm/dts/Makefile                        |    1 +
> >  arch/arm/dts/phycore-imx8mp-u-boot.dtsi      |  110 ++
> >  arch/arm/dts/phycore-imx8mp.dts              |  224 ++++
> 
> Could you please keep the dts file in sync with your patchset
> on
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2021-January/629273.html
> in particular the seperation between SoM and carrier board.

ok, I will update the device tree.

> 
> >  arch/arm/mach-imx/imx8m/Kconfig              |    7 +
> >  board/phytec/phycore_imx8mp/Kconfig          |   12 +
> >  board/phytec/phycore_imx8mp/MAINTAINERS      |    9 +
> >  board/phytec/phycore_imx8mp/Makefile         |   11 +
> >  board/phytec/phycore_imx8mp/lpddr4_timing.c  | 1849
> > ++++++++++++++++++++++++++
> >  board/phytec/phycore_imx8mp/phycore-imx8mp.c |   39 +
> >  board/phytec/phycore_imx8mp/spl.c            |  129 ++
> >  configs/phycore-imx8mp_defconfig             |   95 ++
> >  include/configs/phycore_imx8mp.h             |  107 ++
> >  12 files changed, 2593 insertions(+)
> >  create mode 100644 arch/arm/dts/phycore-imx8mp-u-boot.dtsi
> >  create mode 100644 arch/arm/dts/phycore-imx8mp.dts
> >  create mode 100644 board/phytec/phycore_imx8mp/Kconfig
> >  create mode 100644 board/phytec/phycore_imx8mp/MAINTAINERS
> >  create mode 100644 board/phytec/phycore_imx8mp/Makefile
> >  create mode 100644 board/phytec/phycore_imx8mp/lpddr4_timing.c
> >  create mode 100644 board/phytec/phycore_imx8mp/phycore-imx8mp.c
> >  create mode 100644 board/phytec/phycore_imx8mp/spl.c
> >  create mode 100644 configs/phycore-imx8mp_defconfig
> >  create mode 100644 include/configs/phycore_imx8mp.h
> > 
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index fd47e408f826..5c4130272023 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -779,6 +779,7 @@ dtb-$(CONFIG_ARCH_IMX8M) += \
> >  	imx8mm-beacon-kit.dtb \
> >  	imx8mq-phanbell.dtb \
> >  	imx8mp-evk.dtb \
> > +	phycore-imx8mp.dtb \
> 
> Please keep list sorted ...

it seems not sorted at all. After taking the device tree from kernel
the name will change anyway. It fits better then :).

> 
> >  	imx8mq-pico-pi.dtb
> >  
> >  dtb-$(CONFIG_ARCH_IMXRT) += imxrt1050-evk.dtb \
> > diff --git a/arch/arm/dts/phycore-imx8mp-u-boot.dtsi
> > b/arch/arm/dts/phycore-imx8mp-u-boot.dtsi
> > new file mode 100644
> > index 000000000000..1b504618031c
> > --- /dev/null
> > +++ b/arch/arm/dts/phycore-imx8mp-u-boot.dtsi
> > @@ -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>
> > + */
> > +
> > +/ {
> > +	wdt-reboot {
> > +		compatible = "wdt-reboot";
> > +		wdt = <&wdog1>;
> > +		u-boot,dm-spl;
> > +	};
> > +};
> > +
> > +&{/soc at 0} {
> > +	u-boot,dm-pre-reloc;
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&clk {
> > +	u-boot,dm-spl;
> > +	u-boot,dm-pre-reloc;
> > +};
> > +
> > +&osc_32k {
> > +	u-boot,dm-spl;
> > +	u-boot,dm-pre-reloc;
> > +};
> > +
> > +&osc_24m {
> > +	u-boot,dm-spl;
> > +	u-boot,dm-pre-reloc;
> > +};
> > +
> > +&aips1 {
> > +	u-boot,dm-spl;
> > +	u-boot,dm-pre-reloc;
> > +};
> > +
> > +&aips2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&aips3 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&iomuxc {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&reg_usdhc2_vmmc {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&pinctrl_uart2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&pinctrl_usdhc2_gpio {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&pinctrl_usdhc2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&pinctrl_usdhc3 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&gpio1 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&gpio2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&gpio3 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&gpio4 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&gpio5 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&uart2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&i2c1 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&usdhc2 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&usdhc3 {
> > +	u-boot,dm-spl;
> > +};
> > +
> > +&wdog1 {
> > +	u-boot,dm-spl;
> > +};
> 
> Here are a lot of similarities to "arch/arm/dts/imx8mp-evk-u-
> boot.dtsi"
> 
> May we can collect them in a common file?

Things like clk and ioctrl will be needed for all boards, so yes
probably.
Could this be done in a second step? 

> > diff --git a/arch/arm/dts/phycore-imx8mp.dts
> > b/arch/arm/dts/phycore-imx8mp.dts
> > new file mode 100644
> > index 000000000000..7f97d193950a
> > --- /dev/null
> > +++ b/arch/arm/dts/phycore-imx8mp.dts
> > @@ -0,0 +1,224 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet at phytec.de>
> > + */
> > +
> > +/dts-v1/;
> 
> As commented above, please keep this in sync with linux, thanks.
> 
> [...]
> 
> > diff --git a/board/phytec/phycore_imx8mp/lpddr4_timing.c
> > b/board/phytec/phycore_imx8mp/lpddr4_timing.c
> > new file mode 100644
> > index 000000000000..e59dd74377cb
> > --- /dev/null
> > +++ b/board/phytec/phycore_imx8mp/lpddr4_timing.c
> > @@ -0,0 +1,1849 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright 2020 Phytec Messtechnik GmbH
> > + *
> > + * Generated code from MX8M_DDR_tool
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <asm/arch/ddr.h>
> > +
> > +static struct dram_cfg_param ddr_ddrc_cfg[] = {
> > +	/** Initialize DDRC registers **/
> > +	{ 0x3d400304, 0x1 },
> > +	{ 0x3d400030, 0x1 },
> > +	{ 0x3d400000, 0xa1080020 },
> 
> What a long file with a lot of magic values ... but this applies
> to all imx8m* boards ... :-(
> 
> [...]
> 
> > diff --git a/board/phytec/phycore_imx8mp/phycore-imx8mp.c
> > b/board/phytec/phycore_imx8mp/phycore-imx8mp.c
> > new file mode 100644
> > index 000000000000..6cb2ba5fc21c
> > --- /dev/null
> > +++ b/board/phytec/phycore_imx8mp/phycore-imx8mp.c
> > @@ -0,0 +1,39 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2020 PHYTEC Messtechnik GmbH
> > + * Author: Teresa Remmet <t.remmet at phytec.de>
> > + */
> > +
> > +#include <common.h>
> > +#include <asm/arch/sys_proto.h>
> > +#include <asm/io.h>
> > +#include <asm/mach-imx/boot_mode.h>
> > +#include <env.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +int board_init(void)
> > +{
> > +	return 0;
> > +}
> > +
> > +int board_mmc_get_env_dev(int devno)
> > +{
> > +	return devno;
> > +}
> > +
> > +int board_late_init(void)
> > +{
> > +	switch (get_boot_device()) {
> > +	case SD2_BOOT:
> > +		env_set_ulong("mmcdev", 1);
> > +		break;
> > +	case MMC3_BOOT:
> > +		env_set_ulong("mmcdev", 2);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> Cool, that we are now at a stage, where board code is very small.
> 
> Without checking code ... may it is possible to define
> weak board_init() and board_mmc_get_env_dev() functions,
> so not every board code must implement them?

The board_init() will probably not be empty after ethernet support is
added.

For board_mmc_get_env_dev() there is already a weak implementation in
arch/arm/mach-imx/mmc_env.c. There was also a patch that returned devno
if DM_MMC is enabled. But this has been reverted again. I have not
found the reason. The default does not fit for me.

[...]

> 
> > --- /dev/null
> > +++ b/include/configs/phycore_imx8mp.h
> > @@ -0,0 +1,107 @@
> > +/* 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
> 
> May there is a chance to get i2c with DM support enabled in SPL?
> 
> Or are hw resources to small?

As I understand it the pmic PCA9450 driver is non DM so i2c support
needs to be also.
But hw resources are a topic. Adding i2c DM support requires also CCL
support in SPL and eats up 20kB more of space. At least when I played
around with it.

> 
> > +
> > +#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			0x40000000
> > +#define PHYS_SDRAM_SIZE			0x80000000
> > +
> > +/* UART */
> > +#define CONFIG_MXC_UART_BASE		UART2_BASE_ADDR
> > +
> > +/* Monitor Command Prompt */
> > +#define CONFIG_SYS_CBSIZE		SZ_2K
> > +#define CONFIG_SYS_MAXARGS		64
> > +#define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
> > +
> > +/* 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 */
> > 
> 
> beside of the nitpicks:
> 
> Reviewed-by: Heiko Schocher <hs at denx.de>

Thanks for the review.

Teresa

> 
> bye,
> Heiko



More information about the U-Boot mailing list