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

[Philipp Tomsich]

+Vagrant

Let’s also hear the opinion of the Debian project on this...

> On 29.11.2018, at 10:54, Wadim Egorov <w.egorov at phytec.de> wrote:
> 
> Hi,
> 
> Am 29.11.18 um 02:48 schrieb Kever Yang:
>> 
>> 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. 
> 
> +1 for keeping the ROM boot order also for u-boot.
> 
> 
>>> 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".
>> 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.
>> 
>> 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.
> 
> If that is the case, then the SBC developers can override
> BOOT_TARGET_DEVICES to their needs in the specific board include file.
> 
> 
>> 
>> 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.
> 
> AFAIK most of the rockchip boards provide a jumper to somehow cut the
> eMMC clock and bypass the fixed ROM boot order.
> 
> Regards,
> Wadim
> 
>> 
>> 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 ".
>> 
>> 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).
>>  
>> 
>> Hi Klaus,
>>     How do you think?
>> 
>> Thanks,
>> - Kever
>>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>>>> ---
>>>> 
>>>> Changes in v3:
>>>> - update the commit message
>>>> Series-changes: 2
>>>> - update the commit message
>>>> 
>>>> include/configs/rockchip-common.h | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/include/configs/rockchip-common.h b/include/configs/rockchip-common.h
>>>> index 68e1105a4b..8a72613e52 100644
>>>> --- a/include/configs/rockchip-common.h
>>>> +++ b/include/configs/rockchip-common.h
>>>> @@ -11,11 +11,11 @@
>>>> 
>>>> #ifndef CONFIG_SPL_BUILD
>>>> 
>>>> -/* First try to boot from SD (index 0), then eMMC (index 1) */
>>>> +/* First try to boot from SD (index 1), then eMMC (index 0) */
>>>> #if CONFIG_IS_ENABLED(CMD_MMC)
>>>> 	#define BOOT_TARGET_MMC(func) \
>>>> -		func(MMC, mmc, 0) \
>>>> -		func(MMC, mmc, 1)
>>>> +		func(MMC, mmc, 1) \
>>>> +		func(MMC, mmc, 0)
>>>> #else
>>>> 	#define BOOT_TARGET_MMC(func)
>>>> #endif
>>>> -- 
>>>> 2.18.0
>>>> 
>> 
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de <mailto:U-Boot at lists.denx.de>
>> https://lists.denx.de/listinfo/u-boot <https://lists.denx.de/listinfo/u-boot>