On Mon, Jan 09, 2023 at 02:59:55PM -0300, Fabio Estevam wrote:

> Add the board support for the i.MX8MM Cloos PHG board.
>     
> This board uses a imx8mm-tqma8mqml SoM from TQ-Group.
> 
> imx8mm-phg.dts and imx8mm-tqma8mqml.dtsi are taken
> directly from Linux 6.2-rc3.

This is going to need to be rebased on top of current master due to
CONFIG -> CFG or Kconfig migration.  On top of that:

[snip]
>  create mode 100644 board/cloos/imx8mm_phg/README

This needs to be doc/board/cloos/imx8mm_phg.rst (and related index, etc
changes).

[snip]
> +#ifndef CONFIG_SPL_BUILD
> +#define BOOT_TARGET_DEVICES(func) \
> +     func(MMC, mmc, 1) \
> +     func(MMC, mmc, 2) \
> +     func(DHCP, dhcp, na)
> +
> +#include <config_distro_bootcmd.h>
> +#endif

No, you cannot guard stuff with CONFIG_SPL_BUILD as that way leads to
inconsistencies. There should be no size impact in SPL from having this.

> +/* Initial environment variables */
> +#define CONFIG_EXTRA_ENV_SETTINGS            \
> +     BOOTENV \
> +     "scriptaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> +     "kernel_addr_r=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> +     "image=Image\0" \
> +     "console=ttymxc1,115200\0" \
> +     "fdt_addr_r=0x43000000\0"                       \
> +     "boot_fit=no\0" \
> +     "fdtfile=imx8mm-phg.dtb\0" \
> +     "initrd_addr=0x43800000\0"              \
> +     "bootm_size=0x10000000\0" \
> +     "mmcdev=0\0" \
> +     "mmcargs=setenv bootargs console=ttymxc1,115200 
> root=/dev/mmcblk${mmcdev}p${mmcpart} rw rootwait quiet\0" \
> +     "bootcmd=env exists mmcpart || setenv mmcpart 1; run mmcargs; \
> +     mmc dev ${mmcdev}; \
> +     load mmc ${mmcdev}:${mmcpart} ${loadaddr} boot/Image; \
> +     load mmc ${mmcdev}:${mmcpart} ${fdt_addr_r} boot/${fdtfile}; \
> +     booti ${loadaddr} - ${fdt_addr_r}\0"

And all of this should be in a text based environment instead.  The
macros from distro boot have to stay in the header, for now, but the
rest does not.

> +#define CONFIG_SYS_SDRAM_BASE                0x40000000
> +#define PHYS_SDRAM                   0x40000000
> +#define PHYS_SDRAM_SIZE              0x80000000 /* 2GB DDR */

Please make sure we need to define PHYS_SDRAM* at all.  Thanks!

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to