From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Date: Thu, 28 Mar 2019 15:20:28 +0000
Subject: [U-Boot] [PATCH v2 13/22] colibri_vf: add distroboot support
In-Reply-To: <CAByghJYF2dRgU0SBoDTNUvo9RtCY7ofOuiqYT-UnezVwxEtozA@mail.gmail.com>
References: <20190325162512.8702-1-marcel@ziswiler.com>
 <20190325162512.8702-14-marcel@ziswiler.com>
 <CAByghJYF2dRgU0SBoDTNUvo9RtCY7ofOuiqYT-UnezVwxEtozA@mail.gmail.com>
Message-ID: <9ce1f5e08de604f34983ab536b6e4699893a3ca4.camel@toradex.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Igor

On Thu, 2019-03-28 at 15:01 +0000, Igor Opaniuk wrote:
> Hi Marcel,
> 
> Reviewed-by: Igor Opaniuk <igor.opaniuk@toradex.com>
> 
> Please check some minor comments below (could be addressed in a
> separate patchset):
> 
> On Mon, Mar 25, 2019 at 6:35 PM Marcel Ziswiler <marcel@ziswiler.com>
> wrote:
> > From: Stefan Agner <stefan.agner@toradex.com>
> > 
> > Add support for distro boot. This is especially helpful for
> > external
> > devices. There is a global boot command which scans a predefined
> > list of boot targets:
> >   run distro_bootcmd
> > 
> > As well as direct boot commands such as:
> >   run bootcmd_mmc0
> >   run bootcmd_usb
> >   run bootcmd_dhcp
> >   ...
> > 
> > Refer to doc/README.distro fo details.
> > 
> > While at it also re-order boot command macros as well as the
> > CONFIG_EXTRA_ENV_SETTINGS.
> > 
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > Acked-by: Max Krummenacher <max.krummenacher@toradex.com>
> > Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > 
> > ---
> > 
> > Changes in v2: None
> > 
> >  include/configs/colibri_vf.h | 75 ++++++++++++++++++++++--------
> > ------
> >  1 file changed, 46 insertions(+), 29 deletions(-)
> > 
> > diff --git a/include/configs/colibri_vf.h
> > b/include/configs/colibri_vf.h
> > index 9effa56539..83a33ff786 100644
> > --- a/include/configs/colibri_vf.h
> > +++ b/include/configs/colibri_vf.h
> > @@ -48,13 +48,15 @@
> >  /* We boot from the gfxRAM area of the OCRAM. */
> >  #define CONFIG_BOARD_SIZE_LIMIT                520192
> > 
> > -#define SD_BOOTCMD \
> > -       "sdargs=root=/dev/mmcblk0p2 rw rootwait\0"      \
> minor: "mmcblk0p2" (it's used in multiple places) can also be put
> into
> the separate variable in case if we decide to change the partition
> layout in the future.

With BSP 3.0 we anyway plan to transition to proper distroboot.

> > -       "sdboot=run setup; setenv bootargs ${defargs} ${sdargs}
> > ${mtdparts} " \
> > -       "${setupargs} ${vidargs}; echo Booting from MMC/SD card...;
> > " \
> > -       "load mmc 0:2 ${kernel_addr_r} /boot/${kernel_file} && " \
> > -       "load mmc 0:2 ${fdt_addr_r} /boot/${soc}-colibri-
> > ${fdt_board}.dtb && " \
> minor: I think we should unify (can be addressed in another patchset)
> the way how dtb names are concatenated (and for example use this
> scheme for all Toradex SoMs; particularly for Colibri iMX6DL the
> full-name is still hardcoded "imx6dl-colibri-eval-v3.dtb")

Yeah, that is another legacy in need of cleaning up.

> > -       "run fdt_fixup && bootz ${kernel_addr_r} - ${fdt_addr_r}\0"
> > \
> > +#define MEM_LAYOUT_ENV_SETTINGS \
> > +       "bootm_size=0x10000000\0" \
> > +       "fdt_addr_r=0x82000000\0" \
> > +       "fdt_high=0xffffffff\0" \
> > +       "initrd_high=0xffffffff\0" \
> > +       "kernel_addr_r=0x81000000\0" \
> > +       "pxefile_addr_r=0x87100000\0" \
> > +       "ramdisk_addr_r=0x82100000\0" \
> > +       "scriptaddr=0x87000000\0"
> > 
> >  #define NFS_BOOTCMD \
> >         "nfsargs=ip=:::::eth0: root=/dev/nfs\0" \
> > @@ -65,7 +67,15 @@
> >         "tftp ${fdt_addr_r} ${soc}-colibri-${fdt_board}.dtb && " \
> >         "run fdt_fixup && bootz ${kernel_addr_r} - ${fdt_addr_r}\0"
> > \
> > 
> > -#define UBI_BOOTCMD    \
> > +#define SD_BOOTCMD \
> > +       "sdargs=root=/dev/mmcblk0p2 rw rootwait\0"      \
> > +       "sdboot=run setup; setenv bootargs ${defargs} ${sdargs}
> > ${mtdparts} " \
> > +       "${setupargs} ${vidargs}; echo Booting from MMC/SD card...;
> > " \
> > +       "load mmc 0:2 ${kernel_addr_r} /boot/${kernel_file} && " \
> > +       "load mmc 0:2 ${fdt_addr_r} /boot/${soc}-colibri-
> > ${fdt_board}.dtb && " \
> > +       "run fdt_fixup && bootz ${kernel_addr_r} - ${fdt_addr_r}\0"
> > \
> > +
> > +#define UBI_BOOTCMD \
> >         "ubiargs=ubi.mtd=ubi root=ubi0:rootfs rootfstype=ubifs " \
> >         "ubi.fm_autoconvert=1\0" \
> >         "ubiboot=run setup; " \
> > @@ -76,36 +86,43 @@
> >         "ubi read ${fdt_addr_r} dtb && " \
> >         "run fdt_fixup && bootz ${kernel_addr_r} - ${fdt_addr_r}\0"
> > \
> > 
> > -#define CONFIG_BOOTCOMMAND "run ubiboot; run sdboot; run nfsboot"
> > +#define CONFIG_BOOTCOMMAND "run ubiboot; run distro_bootcmd;"
> > +
> > +#define BOOT_TARGET_DEVICES(func) \
> > +       func(MMC, mmc, 0) \
> > +       func(USB, usb, 0) \
> > +       func(DHCP, dhcp, na)
> > +#include <config_distro_bootcmd.h>
> > +#undef BOOTENV_RUN_NET_USB_START
> > +#define BOOTENV_RUN_NET_USB_START ""
> > 
> >  #define DFU_ALT_NAND_INFO "vf-bcb part 0,1;u-boot part 0,2;ubi
> > part 0,4"
> > 
> >  #define CONFIG_EXTRA_ENV_SETTINGS \
> > -       "kernel_addr_r=0x82000000\0" \
> > -       "fdt_addr_r=0x84000000\0" \
> > -       "kernel_file=zImage\0" \
> > -       "fdt_file=${soc}-colibri-${fdt_board}.dtb\0" \
> > +       BOOTENV \
> > +       MEM_LAYOUT_ENV_SETTINGS \
> > +       NFS_BOOTCMD \
> > +       SD_BOOTCMD \
> > +       UBI_BOOTCMD \
> > +       "console=ttyLP0\0" \
> > +       "defargs=\0" \
> > +       "dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
> >         "fdt_board=eval-v3\0" \
> > +       "fdt_file=${soc}-colibri-${fdt_board}.dtb\0" \
> >         "fdt_fixup=;\0" \
> > -       "defargs=\0" \
> > -       "console=ttyLP0\0" \
> > -       "setup=setenv setupargs " \
> > -       "console=tty1 console=${console}" \
> > -       ",${baudrate}n8 ${memargs}\0" \
> > +       "kernel_file=zImage\0" \
> > +       "mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0" \
> >         "setsdupdate=mmc rescan && set interface mmc && " \
> > -       "fatload ${interface} 0:1 ${loadaddr} flash_blk.img && " \
> > -       "source ${loadaddr}\0" \
> > -       "setusbupdate=usb start && set interface usb && " \
> > -       "fatload ${interface} 0:1 ${loadaddr} flash_blk.img && " \
> > -       "source ${loadaddr}\0" \
> > +               "fatload ${interface} 0:1 ${loadaddr} flash_blk.img
> > && " \
> > +               "source ${loadaddr}\0" \
> > +       "setup=setenv setupargs console=tty1 console=${console}" \
> what is the point providing console configuration two times?

That fist one is the frame buffer console vs. the later serial console.

> > +               ",${baudrate}n8 ${memargs}\0" \
> >         "setupdate=run setsdupdate || run setusbupdate\0" \
> > -       "mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0" \
> > -       "dfu_alt_info=" DFU_ALT_NAND_INFO "\0" \
> > -       "video-mode=dcufb:640x480-16 at 60,monitor=lcd\0" \
> > +       "setusbupdate=usb start && set interface usb && " \
> > +               "fatload ${interface} 0:1 ${loadaddr} flash_blk.img
> > && " \
> > +               "source ${loadaddr}\0" \
> >         "splashpos=m,m\0" \
> > -       SD_BOOTCMD \
> > -       NFS_BOOTCMD \
> > -       UBI_BOOTCMD
> > +       "video-mode=dcufb:640x480-16 at 60,monitor=lcd\0"
> > 
> >  /* Miscellaneous configurable options */
> >  #define CONFIG_SYS_CBSIZE              1024    /* Console I/O
> > Buffer Size */
> > --
> > 2.20.1
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

Cheers

Marcel