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.graute@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).