Hi Manivannan, On Thu, Jul 11, 2019 at 3:07 PM Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org> wrote: > > This commit adds initial board support for iMXQXP AI_ML board from
iMXQXP --> i.MX8QXP > Einfochips. This board is one of the 96Boards Consumer Edition and AI boards > of the 96Boards family based on i.MXQXP SoC from NXP/Freescale. i.MXQXP -> i.MX8QXP > --- /dev/null > +++ b/board/einfochips/imx8qxp_ai_ml/README > @@ -0,0 +1,49 @@ > +U-Boot for the Einfochips i.MX8QXP AI_ML board > + > +Quick Start > +=========== > + > +- Build the ARM Trusted firmware binary The first instruction here is to build the ATF... > +- Get scfw_tcm.bin and ahab-container.img > +- Build U-Boot > +- Flash the binary into the SD card > +- Boot > + > +Get and Build the ARM Trusted firmware > +====================================== > + > +$ git clone https://source.codeaurora.org/external/imx/imx-atf and later it is described how to get the ATF. Looks like the order needs to be inverted. > +void detail_board_ddr_info(void) > +{ > + puts("\nDDR "); Is this function really useful as its only purpose is to print "DDR" ? > +} > + > +/* > + * Board specific reset that is system reset. > + */ You could use single line comment style instead. > +#ifdef CONFIG_SPL_LOAD_FIT > +int board_fit_config_name_match(const char *name) > +{ > + /* Just empty function now - can't decide what to choose */ > + debug("%s: %s\n", __func__, name); It seems you don't need this function then. > +CONFIG_ARM=y > +CONFIG_SPL_SYS_ICACHE_OFF=y > +CONFIG_SPL_SYS_DCACHE_OFF=y Why do you turn off the caches? > +CONFIG_PHY_GIGE=y > +CONFIG_FEC_MXC_SHARE_MDIO=y > +CONFIG_FEC_MXC_MDIO_BASE=0x5B040000 Just wondering why CONFIG_FEC_MXC_MDIO_BASE is set in a board config file? Shouldn't this base address be retrieved from device tree? > +/* Flat Device Tree Definitions */ > +#define CONFIG_OF_BOARD_SETUP > + > +#define CONFIG_FSL_ESDHC > +#define CONFIG_FSL_USDHC > +#define CONFIG_SYS_FSL_ESDHC_ADDR 0 > +#define USDHC1_BASE_ADDR 0x5B010000 > +#define USDHC2_BASE_ADDR 0x5B020000 These base addresses should not be needed as they can be retrieved from device tree. > +#define CONFIG_ENV_OVERWRITE > + > +#define CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG > + > +/* Initial environment variables */ > +#define CONFIG_EXTRA_ENV_SETTINGS \ > + "script=boot.scr\0" \ > + "image=Image\0" \ > + "panel=NULL\0" \ If you don't need the variable 'panel', then there is no need to define it. > +#define CONFIG_BOOTCOMMAND \ > + "mmc dev ${mmcdev}; if mmc rescan; then " \ > + "if run loadbootscript; then " \ > + "run bootscript; " \ > + "else " \ > + "if run loadimage; then " \ > + "run mmcboot; " \ > + "else run netboot; " \ > + "fi; " \ > + "fi; " \ > + "else booti ${loadaddr} - ${fdt_addr}; fi" You may consider to use distro_boot for this community board. It makes easier for Linux distros to support it. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot