[U-Boot] [PATCH V2 12/23] imx: mx8m: add soc related settings and files

Peng Fan van.freenix at gmail.com
Thu Dec 7 09:06:23 UTC 2017


Hi Stefano,
On Thu, Dec 07, 2017 at 09:39:23AM +0100, Stefano Babic wrote:
>Hi Peng,
>
>On 04/12/2017 05:31, Peng Fan wrote:
>> Add SoC level initialization code
>>  - arch_cpu_init
>>  - mmu table
>>  - detect cpu revision
>>  - reset cpu and wdog settings
>>  - M4 boot
>>  - timer init
>>  - wdog settings
>>  - lowlevel init to save/restore registers
>>  - a few dummy header file to avoid build failure
>>  - ft_system_setup and ft_add_optee_node
>>  - mmc env related
>> 
>> Signed-off-by: Peng Fan <peng.fan at nxp.com>
>> ---
>>  arch/arm/include/asm/arch-mx8m/crm_regs.h  |  10 +
>>  arch/arm/include/asm/arch-mx8m/gpio.h      |  12 +
>>  arch/arm/include/asm/arch-mx8m/sys_proto.h |  18 +
>>  arch/arm/mach-imx/Makefile                 |   1 +
>>  arch/arm/mach-imx/mx8m/Makefile            |   3 +-
>>  arch/arm/mach-imx/mx8m/lowlevel_init.S     |  63 ++++
>>  arch/arm/mach-imx/mx8m/soc.c               | 539 +++++++++++++++++++++++++++++
>>  7 files changed, 645 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm/include/asm/arch-mx8m/crm_regs.h
>>  create mode 100644 arch/arm/include/asm/arch-mx8m/gpio.h
>>  create mode 100644 arch/arm/include/asm/arch-mx8m/sys_proto.h
>>  create mode 100644 arch/arm/mach-imx/mx8m/lowlevel_init.S
>>  create mode 100644 arch/arm/mach-imx/mx8m/soc.c
>> 
>> diff --git a/arch/arm/include/asm/arch-mx8m/crm_regs.h b/arch/arm/include/asm/arch-mx8m/crm_regs.h
>> new file mode 100644
>> index 0000000000..6582318983
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-mx8m/crm_regs.h
>> @@ -0,0 +1,10 @@
>> +/*
>> + * Copyright 2017 NXP
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#ifndef _ASM_ARCH_MX8M_CRM_REGS_H
>> +#define _ASM_ARCH_MX8M_CRM_REGS_H
>> +/* Dummy header, some imx-common code needs this file */
>> +#endif
>> diff --git a/arch/arm/include/asm/arch-mx8m/gpio.h b/arch/arm/include/asm/arch-mx8m/gpio.h
>> new file mode 100644
>> index 0000000000..b666d37700
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-mx8m/gpio.h
>> @@ -0,0 +1,12 @@
>> +/*
>> + * Copyright 2017 NXP
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#ifndef __ASM_ARCH_MX8M_GPIO_H
>> +#define __ASM_ARCH_MX8M_GPIO_H
>> +
>> +#include <asm/mach-imx/gpio.h>
>> +
>> +#endif
>> diff --git a/arch/arm/include/asm/arch-mx8m/sys_proto.h b/arch/arm/include/asm/arch-mx8m/sys_proto.h
>> new file mode 100644
>> index 0000000000..8bf9ac6697
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-mx8m/sys_proto.h
>> @@ -0,0 +1,18 @@
>> +/*
>> + * Copyright (C) 2017 NXP
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#ifndef __ARCH_MX8M_SYS_PROTO_H
>> +#define __ARCH_MX8M_SYS_PROTO_H
>> +
>> +#include <asm/mach-imx/sys_proto.h>
>> +
>> +void set_wdog_reset(struct wdog_regs *wdog);
>> +void enable_tzc380(void);
>> +void restore_boot_params(void);
>> +extern unsigned long rom_pointer[];
>> +enum boot_device get_boot_device(void);
>> +bool is_usb_boot(void);
>> +#endif
>> diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
>> index c807174363..27c154b8b3 100644
>> --- a/arch/arm/mach-imx/Makefile
>> +++ b/arch/arm/mach-imx/Makefile
>> @@ -127,4 +127,5 @@ obj-$(CONFIG_MX5) += mx5/
>>  obj-$(CONFIG_MX6) += mx6/
>>  obj-$(CONFIG_MX7) += mx7/
>>  obj-$(CONFIG_ARCH_MX7ULP) += mx7ulp/
>> +obj-$(CONFIG_MX8M) += mx8m/
>>  
>> diff --git a/arch/arm/mach-imx/mx8m/Makefile b/arch/arm/mach-imx/mx8m/Makefile
>> index 05f38842f0..b1c5d74aab 100644
>> --- a/arch/arm/mach-imx/mx8m/Makefile
>> +++ b/arch/arm/mach-imx/mx8m/Makefile
>> @@ -4,4 +4,5 @@
>>  # SPDX-License-Identifier:	GPL-2.0+
>>  #
>>  
>> -obj-y += clock.o clock_slice.o
>> +obj-y += lowlevel_init.o
>> +obj-y += clock.o clock_slice.o soc.o
>> diff --git a/arch/arm/mach-imx/mx8m/lowlevel_init.S b/arch/arm/mach-imx/mx8m/lowlevel_init.S
>> new file mode 100644
>> index 0000000000..d388f3ba95
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/mx8m/lowlevel_init.S
>> @@ -0,0 +1,63 @@
>> +/*
>> + * Copyright 2017 NXP
>> + *
>> + * SPDX-License-Identifier:	GPL-2.0+
>> + */
>> +
>> +#include <config.h>
>> +
>> +.align 8
>> +.global rom_pointer
>> +rom_pointer:
>> +	.space 256
>> +
>> +/*
>> + * Routine: save_boot_params (called after reset from start.S)
>> + */
>> +
>> +.global save_boot_params
>> +save_boot_params:
>
>save_boot_params is a weak function, but there is a comment for it about
>that stack is not available at this point, any you are using the stack.

No stack usage. This is to save ROM context.

>
>And what about this with exception_entry / exception_exit ? Your looks a
>rewrite of functions in arch/arm/cpu/armv8/exceptions.S

I could not reuse exception_entry/exit. The SP is ROM SP, I would
not like to push data to ROM stack from U-Boot.

>
>> +	/* The firmware provided ATAG/FDT address can be found in r2/x0 */
>> +	adr	x0, rom_pointer
>> +	stp	x1, x2, [x0], #16
>> +	stp	x3, x4, [x0], #16
>> +	stp	x5, x6, [x0], #16
>> +	stp	x7, x8, [x0], #16
>> +	stp	x9, x10, [x0], #16
>> +	stp	x11, x12, [x0], #16
>> +	stp	x13, x14, [x0], #16
>> +	stp	x15, x16, [x0], #16
>> +	stp	x17, x18, [x0], #16
>> +	stp	x19, x20, [x0], #16
>> +	stp	x21, x22, [x0], #16
>> +	stp	x23, x24, [x0], #16
>> +	stp	x25, x26, [x0], #16
>> +	stp	x27, x28, [x0], #16
>> +	stp	x29, x30, [x0], #16
>> +	mov	x30, sp
>> +	str	x30, [x0], #8
>> +
>> +	/* Returns */
>> +	b	save_boot_params_ret
>> +
>> +.global restore_boot_params
>> +restore_boot_params:
>> +	adr	x0, rom_pointer
>> +	ldp	x1, x2, [x0], #16
>> +	ldp	x3, x4, [x0], #16
>> +	ldp	x5, x6, [x0], #16
>> +	ldp	x7, x8, [x0], #16
>> +	ldp	x9, x10, [x0], #16
>> +	ldp	x11, x12, [x0], #16
>> +	ldp	x13, x14, [x0], #16
>> +	ldp	x15, x16, [x0], #16
>> +	ldp	x17, x18, [x0], #16
>> +	ldp	x19, x20, [x0], #16
>> +	ldp	x21, x22, [x0], #16
>> +	ldp	x23, x24, [x0], #16
>> +	ldp	x25, x26, [x0], #16
>> +	ldp	x27, x28, [x0], #16
>> +	ldp	x29, x30, [x0], #16
>> +	ldr	x0, [x0]
>> +	mov	sp, x0
>> +	ret
>> diff --git a/arch/arm/mach-imx/mx8m/soc.c b/arch/arm/mach-imx/mx8m/soc.c
>> new file mode 100644
>> index 0000000000..740e84caa4
>> --- /dev/null
>> +++ b/arch/arm/mach-imx/mx8m/soc.c
>> @@ -0,0 +1,539 @@
>> +/*
>> + * Copyright 2017 NXP
>> + *
>> + * Peng Fan <peng.fan at nxp.com>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <asm/arch/imx-regs.h>
>> +#include <asm/io.h>
>> +#include <asm/arch/clock.h>
>> +#include <asm/arch/sys_proto.h>
>> +#include <asm/mach-imx/hab.h>
>> +#include <asm/mach-imx/boot_mode.h>
>> +#include <asm/mach-imx/syscounter.h>
>> +#include <asm/armv8/mmu.h>
>> +#include <errno.h>
>> +#include <fdt_support.h>
>> +#include <fsl_wdog.h>
>> +#include <imx_sip.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#if defined(CONFIG_SECURE_BOOT)
>> +struct imx_sec_config_fuse_t const imx_sec_config_fuse = {
>> +	.bank = 1,
>> +	.word = 3,
>> +};
>> +#endif
>> +
>> +/*
>> + * OCOTP_TESTER3[9:8] (see Fusemap Description Table offset 0x440)
>> + * defines a 2-bit SPEED_GRADING
>> + */
>> +#define OCOTP_TESTER3_SPEED_SHIFT	8
>> +#define OCOTP_TESTER3_SPEED_800MHZ	0
>> +#define OCOTP_TESTER3_SPEED_1GHZ	1
>> +#define OCOTP_TESTER3_SPEED_1300MHZ	2
>> +#define OCOTP_TESTER3_SPEED_1500MHZ	3
>> +
>> +u32 get_cpu_speed_grade_hz(void)
>> +{
>> +	struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
>> +	struct fuse_bank *bank = &ocotp->bank[1];
>> +	struct fuse_bank1_regs *fuse =
>> +		(struct fuse_bank1_regs *)bank->fuse_regs;
>> +	u32 val;
>> +
>> +	val = readl(&fuse->tester3);
>> +	val >>= OCOTP_TESTER3_SPEED_SHIFT;
>> +	val &= 0x3;
>> +
>> +	switch (val) {
>> +	case OCOTP_TESTER3_SPEED_800MHZ:
>> +		return 800000000;
>> +	case OCOTP_TESTER3_SPEED_1GHZ:
>> +		return 1000000000;
>> +	case OCOTP_TESTER3_SPEED_1300MHZ:
>> +		return 1300000000;
>> +	case OCOTP_TESTER3_SPEED_1500MHZ:
>> +		return 1500000000;
>> +	}
>> +	return 0;
>> +}
>> +
>> +/*
>> + * OCOTP_TESTER3[7:6] (see Fusemap Description Table offset 0x440)
>> + * defines a 2-bit SPEED_GRADING
>> + */
>> +#define OCOTP_TESTER3_TEMP_SHIFT	6
>> +
>> +/* CPU Temperature Grades */
>> +#define TEMP_COMMERCIAL         0
>> +#define TEMP_EXTCOMMERCIAL      1
>> +#define TEMP_INDUSTRIAL         2
>> +#define TEMP_AUTOMOTIVE         3
>> +
>> +u32 get_cpu_temp_grade(int *minc, int *maxc)
>> +{
>> +	struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
>> +	struct fuse_bank *bank = &ocotp->bank[1];
>> +	struct fuse_bank1_regs *fuse =
>> +		(struct fuse_bank1_regs *)bank->fuse_regs;
>> +	u32 val;
>> +
>> +	val = readl(&fuse->tester3);
>> +	val >>= OCOTP_TESTER3_TEMP_SHIFT;
>> +	val &= 0x3;
>> +
>> +	if (minc && maxc) {
>> +		if (val == TEMP_AUTOMOTIVE) {
>> +			*minc = -40;
>> +			*maxc = 125;
>> +		} else if (val == TEMP_INDUSTRIAL) {
>> +			*minc = -40;
>> +			*maxc = 105;
>> +		} else if (val == TEMP_EXTCOMMERCIAL) {
>> +			*minc = -20;
>> +			*maxc = 105;
>> +		} else {
>> +			*minc = 0;
>> +			*maxc = 95;
>> +		}
>> +	}
>> +	return val;
>> +}
>
>Even if they appear cross-architecture: get_cpu_speed_grade_hz() and
>get_cpu_temp_grade() are very similar to the function in mx6/soc.c and
>(*sigh*) in mx7/soc.c. This becomes the *third* duplication of the same
>code. Can we factorize this and put the function in a file under
>mach-imx, shared between mx6/mx7/mx8m ?
>
>The expectation is mac-imx/mxX contains just specific code for that
>architecture, while common code can flow into mach-imx.

The mx8m could share with mx7. But mx6 use different fuse banks.
I'll try.

>
>> +
>> +int timer_init(void)
>> +{
>> +#ifdef CONFIG_SPL_BUILD
>> +	struct sctr_regs *sctr = (struct sctr_regs *)SYSCNT_CTRL_BASE_ADDR;
>> +	unsigned long freq = readl(&sctr->cntfid0);
>> +
>> +	/* Update with accurate clock frequency */
>> +	asm volatile("msr cntfrq_el0, %0" : : "r" (freq) : "memory");
>> +
>> +	clrsetbits_le32(&sctr->cntcr, SC_CNTCR_FREQ0 | SC_CNTCR_FREQ1,
>> +			SC_CNTCR_FREQ0 | SC_CNTCR_ENABLE | SC_CNTCR_HDBG);
>> +#endif
>> +
>> +	gd->arch.tbl = 0;
>> +	gd->arch.tbu = 0;
>> +
>> +	return 0;
>> +}
>> > +void enable_tzc380(void)
>> +{
>> +	struct iomuxc_gpr_base_regs *gpr =
>> +		(struct iomuxc_gpr_base_regs *)IOMUXC_GPR_BASE_ADDR;
>> +
>> +	/* Enable TZASC and lock setting */
>> +	setbits_le32(&gpr->gpr[10], GPR_TZASC_EN);
>> +	setbits_le32(&gpr->gpr[10], GPR_TZASC_EN_LOCK);
>> +}
>> +
>> +void set_wdog_reset(struct wdog_regs *wdog)
>> +{
>> +	/*
>> +	 * Output WDOG_B signal to reset external pmic or POR_B decided by
>> +	 * the board desgin.
>
>s/desgin/design/

Fix in V3.

>
>> Without external reset, the peripherals/DDR/
>> +	 * PMIC are not reset, that may cause system working abnormal.
>> +	 * WDZST bit is write-once only bit. Align this bit in kernel,
>> +	 * otherwise kernel code will have no chance to set this bit.
>> +	 */
>> +	setbits_le16(&wdog->wcr, WDOG_WDT_MASK | WDOG_WDZST_MASK);
>> +}
>> +
>> +static struct mm_region imx8m_mem_map[] = {
>> +	{
>> +		.virt = 0x0UL,
>> +		.phys = 0x0UL,
>> +		.size = 0x100000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>> +			 PTE_BLOCK_OUTER_SHARE
>> +	}, {
>> +		.virt = 0x100000UL,
>> +		.phys = 0x100000UL,
>> +		.size = 0x8000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
>> +			 PTE_BLOCK_NON_SHARE |
>> +			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
>> +	}, {
>> +		.virt = 0x7C0000UL,
>> +		.phys = 0x7C0000UL,
>> +		.size = 0x80000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
>> +			 PTE_BLOCK_NON_SHARE |
>> +			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
>> +	}, {
>> +		.virt = 0x900000UL,
>> +		.phys = 0x900000UL,
>> +		.size = 0x200000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>> +			 PTE_BLOCK_OUTER_SHARE
>> +	}, {
>> +		.virt = 0xB00000UL,
>> +		.phys = 0xB00000UL,
>> +		.size = 0x3f500000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
>> +			 PTE_BLOCK_NON_SHARE |
>> +			 PTE_BLOCK_PXN | PTE_BLOCK_UXN
>> +	}, {
>> +		.virt = 0x40000000UL,
>> +		.phys = 0x40000000UL,
>> +		.size = 0xC0000000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>> +			 PTE_BLOCK_OUTER_SHARE
>> +	}, {
>> +		.virt = 0x100000000UL,
>> +		.phys = 0x100000000UL,
>> +		.size = 0x040000000UL,
>> +		.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>> +			 PTE_BLOCK_OUTER_SHARE
>> +	}, {
>> +		/* List terminator */
>> +		0,
>> +	}
>> +};
>> +
>
>If you can, taking into account that currently documentation is scarce
>or missing, can you add comments to each entry to explain each region ?
>Thanks !

Will add in V3.

>
>> +struct mm_region *mem_map = imx8m_mem_map;
>> +
>> +u32 get_cpu_rev(void)
>> +{
>> +	struct anamix_pll *ana_pll = (struct anamix_pll *)ANA_PLL_BASE_ADDR;
>
>This require a small explanation. My point: I would like that registers
>with the same meaning could have the same name, even if defined in
>different files according to the i.MX variants.
>
>This looks a base address and it is ANATOP_BASE_ADDR for other
>architectures. Any reason for changing it ? Can we at least try to use
>the same nomenclature across i.MX processors ?

I just use the name(ANA_PLL) from DOC.
It is ok for me use ANATOP_BASE_ADDR. 

>
>> +	u32 reg = readl(&ana_pll->digprog);
>> +	u32 type = (reg >> 16) & 0xff;
>> +	u32 rom_version;
>> +
>> +	reg &= 0xff;
>> +
>> +	if (reg == 0x10) {
>> +		/*
>> +		 * For B0 chip, the DIGPROG is not updated, still TO1.0.
>> +		 * we have to check ROM version further
>> +		 */
>> +		rom_version = readl((void __iomem *)0x800);
>
>Has a name this address ? Something like SCU_BASE_ADDR (just copied the
>name, no idea if it is correct ) ?
>
>I prefer to remove magic addresses like 0x800, 0x83c and use define for
>them.

ok. Fix in V3.

>
>> +		if (rom_version != 0x10) {
>> +			rom_version = readl((void __iomem *)0x83c);
>> +			if (rom_version >= 0x20)
>> +				reg = 0x20;
>> +		}
>> +	}
>> +
>> +	return (type << 12) | reg;
>> +}
>> +
>> +static void imx_set_wdog_powerdown(bool enable)
>> +{
>> +	struct wdog_regs *wdog1 = (struct wdog_regs *)WDOG1_BASE_ADDR;
>> +	struct wdog_regs *wdog2 = (struct wdog_regs *)WDOG2_BASE_ADDR;
>> +	struct wdog_regs *wdog3 = (struct wdog_regs *)WDOG3_BASE_ADDR;
>> +
>> +	/* Write to the PDE (Power Down Enable) bit */
>> +	writew(enable, &wdog1->wmcr);
>> +	writew(enable, &wdog2->wmcr);
>> +	writew(enable, &wdog3->wmcr);
>> +}
>> +
>> +int arch_cpu_init(void)
>> +{
>> +	/*
>> +	 * Init timer at very early state, because sscg pll setting
>> +	 * will use it
>> +	 */
>> +	timer_init();
>> +
>> +	if (IS_ENABLED(CONFIG_SPL_BUILD)) {
>> +		clock_init();
>> +		imx_set_wdog_powerdown(false);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +#if defined(CONFIG_FEC_MXC)
>> +void imx_get_mac_from_fuse(int dev_id, unsigned char *mac)
>> +{
>> +	struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
>> +	struct fuse_bank *bank = &ocotp->bank[9];
>> +	struct fuse_bank9_regs *fuse =
>> +		(struct fuse_bank9_regs *)bank->fuse_regs;
>> +	u32 value = readl(&fuse->mac_addr1);
>> +
>> +	mac[0] = (value >> 8);
>> +	mac[1] = value;
>> +
>> +	value = readl(&fuse->mac_addr0);
>> +	mac[2] = value >> 24;
>> +	mac[3] = value >> 16;
>> +	mac[4] = value >> 8;
>> +	mac[5] = value;
>> +}
>
>This is the last copy of imx_get_mac_from_fuse() that we introduce in
>U-Boot. At least the 3rd copy, and it is time to factorize them. All
>functions are doing the same !

ok. I'll try to factorize them, but mx6 use different fuse banks.

>
>> +#endif
>> +
>> +#ifdef CONFIG_IMX_BOOTAUX
>> +int arch_auxiliary_core_up(u32 core_id, ulong boot_private_data)
>> +{
>> +	u32 stack, pc;
>> +
>> +	if (!boot_private_data)
>> +		return -EINVAL;
>> +
>> +	stack = *(u32 *)boot_private_data;
>> +	pc = *(u32 *)(boot_private_data + 4);
>> +
>> +	/* Set the stack and pc to M4 bootROM */
>> +	writel(stack, M4_BOOTROM_BASE_ADDR);
>> +	writel(pc, M4_BOOTROM_BASE_ADDR + 4);
>> +
>> +	/* Enable M4 */
>> +	call_imx_sip(IMX_SIP_SRC, IMX_SIP_SRC_M4_START, 0, 0);
>> +
>> +	return 0;
>> +}
>
>Quite the same, apart sip call, and this can be factorized, too.

Ok.

>
>> +
>> +int arch_auxiliary_core_check_up(u32 core_id)
>> +{
>> +	return call_imx_sip(IMX_SIP_SRC, IMX_SIP_SRC_M4_STARTED, 0, 0);
>> +}
>> +#endif
>> +
>> +enum boot_device get_boot_device(void)
>> +{
>> +	struct bootrom_sw_info **p =
>> +		is_soc_rev(CHIP_REV_1_0) ?
>> +		(struct bootrom_sw_info **)ROM_SW_INFO_ADDR_A0 :
>> +		(struct bootrom_sw_info **)ROM_SW_INFO_ADDR;
>> +
>> +	enum boot_device boot_dev = SD1_BOOT;
>> +	u8 boot_type = (*p)->boot_dev_type;
>> +	u8 boot_instance = (*p)->boot_dev_instance;
>> +
>> +	switch (boot_type) {
>> +	case BOOT_TYPE_SD:
>> +		boot_dev = boot_instance + SD1_BOOT;
>> +		break;
>> +	case BOOT_TYPE_MMC:
>> +		boot_dev = boot_instance + MMC1_BOOT;
>> +		break;
>> +	case BOOT_TYPE_NAND:
>> +		boot_dev = NAND_BOOT;
>> +		break;
>> +	case BOOT_TYPE_QSPI:
>> +		boot_dev = QSPI_BOOT;
>> +		break;
>> +	case BOOT_TYPE_WEIM:
>> +		boot_dev = WEIM_NOR_BOOT;
>> +		break;
>> +	case BOOT_TYPE_SPINOR:
>> +		boot_dev = SPI_NOR_BOOT;
>> +		break;
>> +	case BOOT_TYPE_USB:
>> +		boot_dev = USB_BOOT;
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +
>
>I suggest to add BOOT_TYPE_USB for all i.MXes and factorized this
>function, too. This is really identical to the one in
>arch/arm/mach-imx/mx7/soc.c

Ok.

>
>> +	return boot_dev;
>> +}
>> +
>> +bool is_usb_boot(void)
>> +{
>> +	return get_boot_device() == USB_BOOT;
>> +}
>> +
>> +#ifdef CONFIG_ENV_IS_IN_MMC
>> +__weak int board_mmc_get_env_dev(int devno)
>> +{
>> +	return CONFIG_SYS_MMC_ENV_DEV;
>> +}
>> +
>> +int mmc_get_env_dev(void)
>> +{
>> +	struct bootrom_sw_info **p =
>> +		is_soc_rev(CHIP_REV_1_0) ?
>> +		(struct bootrom_sw_info **)ROM_SW_INFO_ADDR_A0 :
>> +		(struct bootrom_sw_info **)ROM_SW_INFO_ADDR;
>> +	int devno = (*p)->boot_dev_instance;
>> +	u8 boot_type = (*p)->boot_dev_type;
>> +
>> +	/* If not boot from sd/mmc, use default value */
>> +	if ((boot_type != BOOT_TYPE_SD) && (boot_type != BOOT_TYPE_MMC))
>> +		return CONFIG_SYS_MMC_ENV_DEV;
>> +
>> +	return board_mmc_get_env_dev(devno);
>> +}
>
>Quite the same for  mmc_get_env_dev()
>
>I sumarize: as you explained, there are much more similarities with imx6
>/ imx7 in MX8M. This means we can share the same code between arch, at
>least between i.MX7 and i.MX8M (but, ideally, as much as we can).

ok.

>
>> +#endif
>> +
>> +#ifdef CONFIG_OF_SYSTEM_SETUP
>> +static int ft_add_optee_node(void *fdt, bd_t *bd)
>> +{
>> +	const char *path, *subpath;
>> +	int offs;
>> +
>> +	/*
>> +	 * No TEE space allocated indicating no TEE running, so no
>> +	 * need to add optee node in dts
>> +	 */
>> +	if (!rom_pointer[1])
>> +		return 0;
>> +
>> +	offs = fdt_increase_size(fdt, 512);
>> +	if (offs) {
>> +		printf("No Space for dtb\n");
>> +		return offs;
>> +	}
>> +
>> +	path = "/firmware";
>> +	offs = fdt_path_offset(fdt, path);
>> +	if (offs < 0) {
>> +		path = "/";
>> +		offs = fdt_path_offset(fdt, path);
>> +
>> +		if (offs < 0) {
>> +			printf("Could not find root node.\n");
>> +			return offs;
>> +		}
>> +
>> +		subpath = "firmware";
>> +		offs = fdt_add_subnode(fdt, offs, subpath);
>> +		if (offs < 0) {
>> +			printf("Could not create %s node.\n", subpath);
>> +			return offs;
>> +		}
>> +	}
>> +
>> +	subpath = "optee";
>> +	offs = fdt_add_subnode(fdt, offs, subpath);
>> +	if (offs < 0) {
>> +		printf("Could not create %s node.\n", subpath);
>> +		return offs;
>> +	}
>> +
>> +	fdt_setprop_string(fdt, offs, "compatible", "linaro,optee-tz");
>> +	fdt_setprop_string(fdt, offs, "method", "smc");
>> +
>> +	return 0;
>> +}
>
>This has nothing to do with the specific SOC and should be moved.

For MX6/7, no need this code. Now only MX8M needs this code, so
could this be moved later when more needs it?

>
>> +
>> +int ft_system_setup(void *blob, bd_t *bd)
>> +{
>> +	static const char * const status = "disabled";
>> +	static const char * const usb_dwc3_path = "/usb at 38100000/dwc3";
>> +	static const char * const speed = "high-speed";
>> +	int i = 0;
>> +	int rc;
>> +	int nodeoff;
>> +
>> +	if (get_boot_device() == USB_BOOT) {
>> +		static const char * const nodes_path[] = {
>> +			"/dcss at 32e00000",
>> +			"/hdmi at 32c00000",
>> +			"/hdmi_cec at 32c33800",
>> +			"/hdmi_drm at 32c00000",
>> +			"/display-subsystem",
>> +			"/sound-hdmi"
>> +		};
>> +
>
>This should be moved, too, but why do we increase blob size if boot
>device is USB ?

This is MX8M specific. The following code will modify dtb
,to avoid NOSPACE error, we increase blob size.


Thanks,
Peng.
-- 


More information about the U-Boot mailing list