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

Fri Nov 30 10:26:50 UTC 2018

Hi Kever,

Am 30.11.18 um 02:27 schrieb Kever Yang:
> + Andreas from SUSE and Thomas from Armbian
>
> Hi Wadim,
>
>
> On 11/29/2018 05:54 PM, Wadim Egorov 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.
> This need a updated of U-Boot into there eMMC, right?

yes, you are right. But this should not be an issue for SBCs shipped
with rockchip private loaders since - as you mentioned before - they
already prioritize the SD card.

> Most of boards with Rockchip SoC will ship with rockchip default U-Boot,
> what I want to do
> is the default behavior, developers can boot the system from SD card by
> default.
> But, please note that every one wants to play with U-Boot.
>>
>>> 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.
> No, not for all the products like box and tablet, and many SBC only have
> test point instead of jumper which is not easy to do it without hardware
> workaround.
> And any kind of software or hardware workaround is not what I want.

I understand why you want to change the default boot order. It makes
things much more easier for people who want to try new distros without
ever touching the eMMC content.

Our SoM is not really affected by the change because we redefine the
boot order in the board includes.
Anyway, if you are going to pick this commit I would like to see a note
in the documentation. The bootrom order was default since forever.

Regards,
Wadim

>
> Thanks,
> - Kever
>> 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
>>> https://lists.denx.de/listinfo/u-boot
>