[U-Boot] [RESENT PATCH v3] rockchip: update emmc/sd index for distro boot order

[Philipp Tomsich]

Kever,

> On 29.11.2018, at 02:48, Kever Yang <kever.yang at rock-chips.com> wrote:
> 
> On 11/28/2018 05:15 PM, Philipp Tomsich wrote:
>> Kever,
>> 
>>> On 28.11.2018, at 03:06, Kever Yang <kever.yang at rock-chips.com> wrote:
>>> 
>>> According to the emmc/sdcard index in dts alias, emmc is always 0 and
>>> sdcard index is 1, let's update to using correct mmc number for distro
>>> boot order in common header.
>>> 
>>> SD card suppost to have higher priority so that people can boot into
>>> the firmware in SD card, this is very convenient for developer try with
>>> distro img from SUSE, Fedora and etc. Developer only need to 'dd' the
>>> Distro image(which id download from OS vendor release) into SD card without
>>> any modify and then we can boot it up directly.
>> You never addressed the review comment from Klaus (from the review in May):
> I do address the review comment and that's why this patch has been V3
> but not a RESEND for V1. The source code does not change, while the commit
> message has update to make it more clear why this patch is needed.
> 
> I'm sorry to Klaus for not sure if I have reply to him directly, but I
> do this
> because I always not get response after I send out my comments in this
> channel,
> then I thought send a new patch with necessary update may be more fast to
> make patch merged.
> 
>> 
>>> Also prioritising SD card over eMMC does not make any sense to me. At least on
>>> RK3399 and RK3368 the default ROM boot order is first eMMC then SD card. So 
>>> starting U-Boot from eMMC and then loading the Kernel from SD-card doesn’t sound
>>> right for me. 
>> This will change default behaviour and may break things for users in the field.
>> Before we can move forward, we really need to establish a consensus on this
>> and how users will be affected.
>> 
>> While this doesn’t matter much for our boards, as we have logic to rewrite the
>> default boot during boot-up anyway, I expect a lot of trouble for mainline users
>> with their own boards...
> First we have to understand what we want and what we should do, I think the
> origin comment "First try to boot from SD (index 0), then eMMC (index 1)" is
> what we want, but the index is wrong, so we have to correct it, and my first
> commit message is about "index fix”.

Your patch clearly changes both the comment _and_ the boot order.
So we need to find a consensus on the boot order as part of this discussion.

> And I do explain in latest commit message about why we need boot SD first,
> but I'm not sure if you have read it.

I have read the message, but just because there is one use-case for this we
shouldn't change the boot order for everyone.

That said, I didn’t find the rationale from the commit message very compelling:
(1) if there isn’t any firmware in the MMC, then the SD card will be booted anyway;
(2) if there is firmware in the MMC, then U-Boot can be instructed to continue booting from SD (i.e. the user breaks onto the commandline and overrides the boottarget for distroboot)
(3) if the firmware in the MMC is broken, then a USB-recovery can be attempted.

Furthermore, if we always boot from SD, there’s issues for people that have
another fixed storage connected to the SD controller (e.g. a second eMMC) or 
if a user inserts a SD card that inadvertently has a loadable image.

> Rockchip private loaders always make SD as higher priority then eMMC,
> because
> we may need use SD to update the firmware for eMMC or just a temporary test
> only image to check if the PCB is good or not.
> I think most of SBC developers prefer to use SD card instead of eMMC,
> because
> write a SD is easier then update eMMC firmware with vendor tools, and we can
> use SD card just like we try a Ubuntu in a Udisk without install it in
> hard disk for PC,
> just like I write in the commit message.

There’s some interesting information here, as I had not been aware that your
bootloader prioritised SD cards over eMMC.  This may be an issue for most
industrial embedded applications, as these frequently don’t want surprising
changes to the boot order … however, I’d expect that most of these have the
boot-order fixed in their board-files anyway.

You might be surprised to hear that our RK3399 module has the eMMC on
the SDHCI controller and the SD/SDIO on the DWMMC.
As I said: my team doesn’t really have any skin in the game, as we have a
'u-boot,spl-boot-order’ to select where and how SPL searches for its FIT
images… and we have a board-specific setup_boottargets() function to
perform appropriate reordering of the boottargets before distroboot.

> If we keep eMMC as higher priority, then people have no chance to use
> firmware
> in SD card if there already have firmware in eMMC.
> 
> Then let's check if this "break things for users in the field ", this
> patch only affect the
> case both eMMC and SD have available firmware(boot.img), right?
> I think people only write firmware to SD when they want to boot from it,
> or else
> people would never do it, the SD will be a normal external storage if
> they want to use
> the firmware in eMMC. I don't think  there will be "a lot of trouble for
> mainline users “.

The unexpected behaviour (a.k.a. “a lot of trouble”) would occur for users that
have firmware on their SD card (and may not even be aware of this).
Again: we need to establish a consensus here, as we are changing something
that has always been like this in our tree.

> With this patch, people can use firmware in SD card if they want;
> Without this patch, people never able to use firmware in SD card if eMMC
> firmware exist (even if it's broken).

Most boards I know have the ability to bypass the internal eMMC.

Thanks,
Philipp.