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

Heiko Schocher hs at denx.de
Wed Jan 13 12:56:53 CET 2021


Hello Teresa,

Am 13.01.21 um 12:31 schrieb Teresa Remmet:
> 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 :).

Fine!

>>>  	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? 

>From my side, yes.

>>> 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.
> 
> [...]

Ah, did not looked into code. Ok, thanks for the clarification.

>>> --- /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.

Ok, may fixed than in future.

>>> +
>>> +#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

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de


More information about the U-Boot mailing list