On Mon, Aug 31, 2020 at 04:54:40PM +0200, Oliver Graute wrote: > Add i.MX8QM DMSSE20 a1 board support [snip] > diff --git a/board/advantech/imx8qm_dmsse20_a1/MAINTAINERS > b/board/advantech/imx8qm_dmsse20_a1/MAINTAINERS > new file mode 100644 > index 0000000000..c1e8048bbb > --- /dev/null > +++ b/board/advantech/imx8qm_dmsse20_a1/MAINTAINERS > @@ -0,0 +1,6 @@ > +i.MX8QM ROM DMSSE20 a1 BOARD > +M: Oliver Graute <oliver.gra...@kococonnector.com> > +S: Maintained > +F: board/advantech/imx8qm_dmsse20_a1/ > +F: include/configs/imx8qm_dmsse20.h > +F: configs/imx8qm_dmsse20a1_defconfig
You should also list the dts files. [snip] > diff --git a/board/advantech/imx8qm_dmsse20_a1/README > b/board/advantech/imx8qm_dmsse20_a1/README > new file mode 100644 > index 0000000000..2105e9c425 > --- /dev/null > +++ b/board/advantech/imx8qm_dmsse20_a1/README This should be rST and in doc/board/ [snip] > diff --git a/include/configs/imx8qm_dmsse20.h > b/include/configs/imx8qm_dmsse20.h > new file mode 100644 > index 0000000000..e1e0bc6e38 > --- /dev/null > +++ b/include/configs/imx8qm_dmsse20.h > @@ -0,0 +1,219 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2017-2019 NXP > + */ > + > +#ifndef __IMX8QM_DMSSE20_H > +#define __IMX8QM_DMSSE20_H > + > +#include <linux/sizes.h> > +#include <asm/arch/imx-regs.h> Do we really need these includes here? > +#define CONFIG_REMAKE_ELF > + > +#ifdef CONFIG_SPL_BUILD > +#define CONFIG_SPL_MAX_SIZE (124 * 1024) > +#define CONFIG_SPL_BSS_START_ADDR 0x00128000 > +#define CONFIG_SPL_BSS_MAX_SIZE 0x1000 /* 4 KB */ > +#endif These guards are unnecessary. > +#define CONFIG_NR_DRAM_BANKS 4 > + > +/* #define CONFIG_ARCH_MISC_INIT */ Don't include commented out defines please. > + > +/* #define CONFIG_CMD_READ */ > + > +/* 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 You cannot enable/disable commands in the header file, that's an error to checkpatch as they've all been in Kconfig for some time. CONFIG_BOOTM_xxx has been for a bit as well. Please audit the whole file for things which are already in Kconfig, thanks. > +#define CONFIG_MFG_ENV_SETTINGS \ > + "mfgtool_args=setenv bootargs console=${console},${baudrate} " \ > + "initrd_addr=0x83100000\0" \ > + "initrd_high=0xffffffffffffffff\0" \ It's not a good idea, but not fatal to disable initrd relocation. > + "emmc_dev=0\0" \ > + "sd_dev=1\0" \ > + > +/* Initial environment variables */ > +#define CONFIG_EXTRA_ENV_SETTINGS \ > + CONFIG_MFG_ENV_SETTINGS \ > + M4_BOOT_ENV \ > + "script=boot.scr\0" \ > + "image=Image\0" \ > + "panel=NULL\0" \ > + "console=ttyLP0\0" \ > + "earlycon=lpuart32,0x5a060000\0" \ > + "fdt_addr=0x83000000\0" \ > + "fdt_high=0xffffffffffffffff\0" \ > + It is however fatal to disable fdt relocation. It's far too easy for real life examples to generated non-8-byte-aligned addresses for the dtb and the Linux blows up. Please also see include/configs/ti_armv7_common.h and the big block comment about where to place and how much space to have between the kernel, initrd and device tree. While that's for a 32bit platform, it's all applicable to aarch64 (perhaps with the caveat that dtb can be up to 2MiB? off the top of my head). -- Tom
signature.asc
Description: PGP signature