[U-Boot] [PATCH v3 2/2] board: Add support for iMX8QXP AI_ML board

[Manivannan Sadhasivam]

Hi Fabio,

On Thu, Jul 18, 2019 at 09:43:54PM -0300, Fabio Estevam wrote:
> Hi Manivannan,
> 
> On Fri, Jul 12, 2019 at 3:12 PM Manivannan Sadhasivam
> <manivannan.sadhasivam at linaro.org> wrote:
> 
> > +#if IS_ENABLED(CONFIG_DM_GPIO)
> > +static void board_gpio_init(void)
> > +{
> > +       /* TODO */
> > +}
> > +#else
> > +static inline void board_gpio_init(void) {}
> > +#endif
> > +
> 
> Is this function really needed? It is always empty.
> 

True but I intend to add some pre configured GPIOs in future, so
I'll keep it.

> > +int board_late_init(void)
> > +{
> > +#ifdef CONFIG_ENV_VARS_UBOOT_RUNTIME_CONFIG
> > +       env_set("board_name", "AI_ML");
> > +       env_set("board_rev", "v1.0");
> > +#endif
> 
> It seems you don't make any use of board_name and board_rev.
> 
> Are they really needed?
> 

Right, not used now. will remove it.

> > +#include <config_distro_bootcmd.h>
> > +/* Initial environment variables */
> > +#define CONFIG_EXTRA_ENV_SETTINGS                              \
> > +       "console=ttyLP2 earlycon\0" \
> > +       "fdt_addr_r=0x83000000\0" \
> > +       "kernel_addr_r=0x81000000\0" \
> > +       "ramdisk_addr_r=0x94400000\0" \
> > +       "scriptaddr=0x89000000\0" \
> > +       "fdtfile=imx8qxp-ai_ml.dtb\0"   \
> > +       "fdt_high=0xffffffffffffffff\0"         \
> > +       "image=Image\0" \
> > +       "initrd_addr=0x83800000\0" \
> > +       "initrd_high=0xffffffffffffffff\0" \
> > +       "mmcargs=setenv bootargs console=${console},${baudrate} " \
> > +               "root="CONFIG_MMCROOT" rootwait rw" \
> > +       "mmcdev=" __stringify(CONFIG_SYS_MMC_ENV_DEV) "\0" \
> > +       "mmcpart=" __stringify(CONFIG_SYS_MMC_IMG_LOAD_PART) "\0" \
> 
> This is not needed after you switch to distro config.
> This root= information is distro specific and should not be part of
> the standard config.
> 
> Please look at include/configs/wandboard.h for example for a much more
> compact example.
> 

Ack.

> > +       "netargs=setenv bootargs console=${console},${baudrate} " \
> > +               "root=/dev/nfs ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp" \
> > +               "\0" \
> > +       "nfsboot=run netargs; dhcp ${loadaddr} ${image}; tftp ${fdt_addr} " \
> > +               "imx8qxp-ai_ml/${fdt_file}; booti ${loadaddr} - ${fdt_addr}\0" \
> > +       "script=boot.scr\0" \
> 
> Not used with distro config.
> 

Ack.

> > +/* Serial */
> > +#define CONFIG_BAUDRATE                        115200
> 
> This is not needed.

Ack.

Thanks,
Mani