Hi Igor

On Fri, 2019-04-12 at 11:15 +0000, Igor Opaniuk wrote:
> Hi Marcel,
> 
> Thanks for your comments.
> 
> On Fri, Apr 12, 2019 at 11:32 AM Marcel Ziswiler
> <marcel.ziswi...@toradex.com> wrote:
> > Hi Igor
> > 
> > Thanks for cleaning that up.
> > 
> > On Fri, 2019-04-05 at 15:51 +0200, Igor Opaniuk wrote:
> > > Replace usage of "/dev/mmcblk*p*" with a proper UUID of rootfs
> > > partition.
> > > This fixes the issue, when MMC controllers are probed in a
> > > different
> > > order
> > > in U-boot and Linux kernel.
> > 
> > Basically a dream come true (;-p). Looks fine and works with latest
> > 2.8b6 as well as today's linux-next. Just a few minor comments
> > below.
> > 
> > > Signed-off-by: Igor Opaniuk <igor.opan...@toradex.com>
> > 
> > With that addressed you may add my
> > 
> > Tested-by: Marcel Ziswiler <marcel.ziswi...@toradex.com>
> > 
> > And tackle all the other boards as well.
> > 
> > > ---
> > >  configs/colibri_imx6_defconfig |  1 +
> > >  include/configs/colibri_imx6.h | 55 +++++++++++++++++++++++-----
> > > ----
> > > --
> > >  2 files changed, 38 insertions(+), 18 deletions(-)
> > > 
> > > diff --git a/configs/colibri_imx6_defconfig
> > > b/configs/colibri_imx6_defconfig
> > > index bf05c68807..083588e02d 100644
> > > --- a/configs/colibri_imx6_defconfig
> > > +++ b/configs/colibri_imx6_defconfig
> > > @@ -40,6 +40,7 @@ CONFIG_CMD_GPIO=y
> > >  CONFIG_CMD_GPT=y
> > >  CONFIG_CMD_I2C=y
> > >  CONFIG_CMD_MMC=y
> > > +CONFIG_CMD_UUID=y
> > >  CONFIG_CMD_USB=y
> > >  CONFIG_CMD_USB_SDP=y
> > >  CONFIG_CMD_USB_MASS_STORAGE=y
> > > diff --git a/include/configs/colibri_imx6.h
> > > b/include/configs/colibri_imx6.h
> > > index 803c9be064..6603698a66 100644
> > > --- a/include/configs/colibri_imx6.h
> > > +++ b/include/configs/colibri_imx6.h
> > > @@ -115,16 +115,22 @@
> > >       "imx6dl-colibri-cam-eval-v3.dtb fat 0 1"
> > > 
> > >  #define EMMC_BOOTCMD \
> > > -     "emmcargs=ip=off root=/dev/mmcblk0p2 rw,noatime
> > > rootfstype=ext4
> > > " \
> > > +     "emmcdev=0\0" \
> > > +     "emmcbootpart=1\0" \
> > > +     "emmcrootpart=2\0" \
> > > +     "emmcfinduuid=part uuid mmc ${mmcdev}:${emmcrootpart}
> > > uuid\0" \
> > > +     "set_emmcargs=setenv emmcargs ip=off root=PARTUUID=${uuid}
> > > "\
> > > +             "rw,noatime rootfstype=ext4 " \
> > >               "rootwait\0" \
> > > -     "emmcboot=run setup; " \
> > > +     "emmcboot=run setup; run emmcfinduuid; run set_emmcargs; "
> > > \
> > >               "setenv bootargs ${defargs} ${emmcargs}
> > > ${setupargs} "
> > > \
> > >               "${vidargs}; echo Booting from internal eMMC
> > > chip...; "
> > >       \
> > > -             "run emmcdtbload; load mmc 0:1 ${kernel_addr_r} " \
> > > -             "${boot_file} && run fdt_fixup && " \
> > > +             "run emmcdtbload; load mmc
> > > ${emmcdev}:${emmcbootpart} "
> > > \
> > > +             "${kernel_addr_r} ${boot_file} && run fdt_fixup &&
> > > " \
> > >               "bootz ${kernel_addr_r} ${dtbparam}\0" \
> > > -     "emmcdtbload=setenv dtbparam; load mmc 0:1 ${fdt_addr_r} "
> > > \
> > > -             "${fdt_file} && setenv dtbparam \" -
> > > ${fdt_addr_r}\" &&
> > > true\0"
> > > +     "emmcdtbload=setenv dtbparam; load mmc
> > > ${emmcdev}:${emmcbootpart} " \
> > > +             "${fdt_addr_r} ${fdt_file} && " \
> > > +             "setenv dtbparam \" - ${fdt_addr_r}\" && true\0"
> > > 
> > >  #define MEM_LAYOUT_ENV_SETTINGS \
> > >       "bootm_size=0x10000000\0" \
> > 
> > Looks great, except I would sort that stuff alphabetically again
> > (applies to sdboot and usbboot as well). Just makes comparing
> > things so
> > much easier.
> 
> This is what I actually didn't get, which particular stuff did you
> mean? defines?

Well, yes. But also e.g. emmcbootpart coming before emmcdev etc.

> > > @@ -147,26 +153,38 @@
> > >               "&& setenv dtbparam \" - ${fdt_addr_r}\" && true\0"
> > > 
> > >  #define SD_BOOTCMD \
> > > -     "sdargs=ip=off root=/dev/mmcblk1p2 rw,noatime
> > > rootfstype=ext4 "
> > > \
> > > -             "rootwait\0" \
> > > -     "sdboot=run setup; " \
> > > +     "sddev=1\0" \
> > > +     "sdbootpart=1\0" \
> > > +     "sdrootpart=2\0" \
> > > +     "sdfinduuid=part uuid mmc ${sddev}:${sdrootpart} uuid\0" \
> > > +     "set_sdargs=setenv sdargs ip=off root=PARTUUID=${uuid}
> > > rw,noatime " \
> > > +             "rootfstype=ext4 rootwait\0" \
> > > +     "sdboot=run setup; run sdfinduuid; run set_sdargs; " \
> > >               "setenv bootargs ${defargs} ${sdargs} ${setupargs}
> > > " \
> > >               "${vidargs}; echo Booting from SD card; " \
> > > -             "run sddtbload; load mmc 1:1 ${kernel_addr_r} " \
> > > -             "${boot_file} && run fdt_fixup && " \
> > > +             "run sddtbload; load mmc ${sddev}:${sdbootpart} "\
> > > +             "${kernel_addr_r} ${boot_file} && run fdt_fixup &&
> > > " \
> > >               "bootz ${kernel_addr_r} ${dtbparam}\0" \
> > > -     "sddtbload=setenv dtbparam; load mmc 1:1 ${fdt_addr_r} " \
> > > -             "${fdt_file} && setenv dtbparam \" -
> > > ${fdt_addr_r}\" &&
> > > true\0"
> > > +     "sddtbload=setenv dtbparam; load mmc ${sddev}:${sdbootpart}
> > > " \
> > > +             "${fdt_addr_r} ${fdt_file} && setenv dtbparam \" -
> > > " \
> > > +             "${fdt_addr_r}\" && true\0"
> > > 
> > >  #define USB_BOOTCMD \
> > > -     "usbargs=ip=off root=/dev/sda2 rw,noatime rootfstype=ext4 "
> > > \
> > > -             "rootwait\0" \
> > > -     "usbboot=run setup; setenv bootargs ${defargs} ${setupargs}
> > > " \
> > > +     "usbdev=0\0" \
> > > +     "usbbootpart=1\0" \
> > > +     "usbrootpart=2\0" \
> > > +     "usbfinduuid=part uuid usb ${usbdev}:${usbrootpart} uuid\0"
> > > \
> > > +     "set_usbargs=setenv usbargs ip=off root=PARTUUID=${uuid}
> > > rw,noatime " \
> > > +             "rootfstype=ext4 rootwait\0" \
> > > +     "usbboot=run setup; usb start; run usbfinduuid; run
> > > set_usbargs; " \
> > > +             "setenv bootargs ${defargs} ${setupargs} " \
> > >               "${usbargs} ${vidargs}; echo Booting from USB
> > > stick...;
> > > " \
> > > -             "usb start && run usbdtbload; load usb 0:1
> > > ${kernel_addr_r} " \
> > > +             "run usbdtbload; " \
> > > +             "load usb ${usbdev}:${usbbootpart} ${kernel_addr_r}
> > > " \
> > >               "${boot_file} && run fdt_fixup && " \
> > >               "bootz ${kernel_addr_r} ${dtbparam}\0" \
> > > -     "usbdtbload=setenv dtbparam; load usb 0:1 ${fdt_addr_r} " \
> > > +     "usbdtbload=setenv dtbparam; load usb
> > > ${usbdev}:${usbbootpart}
> > > " \
> > > +             "${fdt_addr_r} " \
> > >               "${fdt_file} && setenv dtbparam \" -
> > > ${fdt_addr_r}\" &&
> > > true\0"
> > > 
> > >  #define FDT_FILE "imx6dl-colibri-eval-v3.dtb"
> > > @@ -186,6 +204,7 @@
> > >       MEM_LAYOUT_ENV_SETTINGS \
> > >       NFS_BOOTCMD \
> > >       SD_BOOTCMD \
> > > +     USB_BOOTCMD \
> > 
> > Yeah, that is a good one (;-p). Maybe you could add a note to the
> > commit message that this indeed also fixes the legacy usbboot
> > command.
> > Actually, the commit message probably should mention sdboot and
> > usbboot
> > as well anyway.
> 
> I'll also check it with the fix from Heinrich [1], which fixes the
> data abort in the load command.

Yeah, I manually just used fatload but that would not work e.g. if the
media is formatted with a file system other than FAT.

> > >       "setethupdate=if env exists ethaddr; then; else setenv
> > > ethaddr
> > > " \
> > >               "00:14:2d:00:00:00; fi; tftpboot ${loadaddr} " \
> > >               "flash_eth.img && source ${loadaddr}\0" \
> > 
> > Cheers
> > 
> > Marcel
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
> 
> [1]: https://patchwork.ozlabs.org/patch/1082249/
> 
> -- 
> Best regards - Freundliche Grüsse - Meilleures salutations
> 
> Senior Development Engineer,
> Igor Opaniuk

Best regards - Mit freundlichen Grüssen - Meilleures salutations

Marcel Ziswiler
Platform Manager Embedded Linux

Toradex AG
Altsagenstrasse 5 | 6048 Horw/Luzern | Switzerland | T: +41 41 500 48 00
(main line) | Direct: +41 41 500 48 10

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to