[U-Boot] [PATCH v3] rockchip: rk3288: Change method of loading u-boot

[Ziyuan Xu <xzy.xu@rock-chips.com>]

hi Simon,

On 2016?07?12? 07:29, Simon Glass wrote:
> Hi Ziyuan,
>
> On 27 June 2016 at 02:30, Ziyuan Xu <xzy.xu@rock-chips.com> wrote:
>> From: Xu Ziyuan <xzy.xu@rock-chips.com>
>>
>> If we would like to boot from SD card, we have to implement mmc driver
>> in SPL stage, and get a slightly large spl binrary. RK3288's bootrom code
>> has the ability to load spl and u-boot, then boot.
>>
>> If CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM is enabled, the spl will
>> return to bootrom in board_init_f(), then bootrom load u-boot binrary.
>>
>> This patch does that.
>>
>> Loading sequence after rework:
>> bootrom ==> spl ==> bootrom ==> u-boot
>>
>> Signed-off-by: Ziyuan Xu <xzy.xu@rock-chips.com>
>> ---
>>
>> Changes in v3:
>> - Add CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM for enabling this feature
>> - Update doc/README.rockchip to instruct how to use it
>> - Detailed commit message
>>
>> Changes in v2:
>> - Add sdcard iomux initlization in board_init() to fix sdmmc command
>> sending timeout issue when booting from eMMC
> Acked-by: Simon Glass <sjg@chromium.org>
>
> I'm going to apply this, but please can you send a follow-up patch (to
> u-boot-rockchip/testing) to fix the nits below?
This patch had already apply to u-boot/rockchip, I can't revise commit' 
nits.
I had revise the glitches according to your opinion. May I send patch 
V4, and you revert the former commit?
>
>>   arch/arm/mach-rockchip/Makefile                    |  1 +
>>   arch/arm/mach-rockchip/board.c                     | 33 ++++++++++++++++++++++
>>   arch/arm/mach-rockchip/rk3036/Makefile             |  1 -
>>   arch/arm/mach-rockchip/rk3288-board-spl.c          |  5 +++-
>>   .../mach-rockchip/{rk3036 => }/save_boot_param.S   |  2 +-
>>   doc/README.rockchip                                | 14 +++++++++
>>   include/configs/rk3288_common.h                    |  4 +++
>>   7 files changed, 57 insertions(+), 3 deletions(-)
>>   rename arch/arm/mach-rockchip/{rk3036 => }/save_boot_param.S (90%)
>>
>> diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile
>> index 55567cb..8e0c0ab 100644
>> --- a/arch/arm/mach-rockchip/Makefile
>> +++ b/arch/arm/mach-rockchip/Makefile
>> @@ -7,6 +7,7 @@
>>   ifdef CONFIG_SPL_BUILD
>>   obj-$(CONFIG_ROCKCHIP_RK3288) += rk3288-board-spl.o
>>   obj-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o
>> +obj-y += save_boot_param.o
>>   else
>>   obj-$(CONFIG_ROCKCHIP_RK3288) += board.o
>>   endif
>> diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
>> index 816540e..bcb2c9e 100644
>> --- a/arch/arm/mach-rockchip/board.c
>> +++ b/arch/arm/mach-rockchip/board.c
>> @@ -10,12 +10,45 @@
>>   #include <ram.h>
>>   #include <asm/io.h>
>>   #include <asm/arch/clock.h>
>> +#include <asm/arch/periph.h>
>> +#include <asm/gpio.h>
>> +#include <dm/pinctrl.h>
>>
>>   DECLARE_GLOBAL_DATA_PTR;
>>
>>   int board_init(void)
>>   {
>> +#ifdef CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM
> How about CONFIG_ROCKCHIP_SPL_BACK_TO_BROM?
>
> I think this could be implemented for any SoC.
>
> Also please add this to Kconfig as we should not be adding new CONFIG
> options that are not in Kconfig.
>
>> +       struct udevice *pinctrl;
>> +       int ret;
>> +
>> +    /*
>> +     * We need to implement sdcard iomux here for the further
>> +     * initlization, otherwise, it'll hit sdcard command sending
>> +     * timeout exception.
>> +     */
>> +       ret = uclass_get_device(UCLASS_PINCTRL, 0, &pinctrl);
>> +       if (ret) {
>> +               debug("%s: Cannot find pinctrl device\n", __func__);
>> +               goto err;
>> +       }
>> +       ret = pinctrl_request_noflags(pinctrl, PERIPH_ID_SDCARD);
>> +       if (ret) {
>> +               debug("%s: Failed to set up SD card\n", __func__);
>> +               goto err;
>> +       }
>> +
>> +       return 0;
>> +err:
>> +       printf("board_init: Error %d\n", ret);
>> +
>> +       /* No way to report error here */
>> +       hang();
>> +
>> +       return -1;
>> +#else
>>          return 0;
>> +#endif
>>   }
>>
>>   int dram_init(void)
>> diff --git a/arch/arm/mach-rockchip/rk3036/Makefile b/arch/arm/mach-rockchip/rk3036/Makefile
>> index 97d299d..6095777 100644
>> --- a/arch/arm/mach-rockchip/rk3036/Makefile
>> +++ b/arch/arm/mach-rockchip/rk3036/Makefile
>> @@ -10,4 +10,3 @@ obj-y += syscon_rk3036.o
>>   endif
>>
>>   obj-y += sdram_rk3036.o
>> -obj-y += save_boot_param.o
>> diff --git a/arch/arm/mach-rockchip/rk3288-board-spl.c b/arch/arm/mach-rockchip/rk3288-board-spl.c
>> index e133cca..5de060c 100644
>> --- a/arch/arm/mach-rockchip/rk3288-board-spl.c
>> +++ b/arch/arm/mach-rockchip/rk3288-board-spl.c
>> @@ -149,7 +149,7 @@ static int configure_emmc(struct udevice *pinctrl)
>>          return 0;
>>   }
>>   #endif
>> -
>> +extern void back_to_bootrom(void);
>>   void board_init_f(ulong dummy)
>>   {
>>          struct udevice *pinctrl;
>> @@ -204,6 +204,9 @@ void board_init_f(ulong dummy)
>>                  debug("DRAM init failed: %d\n", ret);
>>                  return;
>>          }
>> +#ifdef CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM
>> +       back_to_bootrom();
>> +#endif
>>   }
>>
>>   static int setup_led(void)
>> diff --git a/arch/arm/mach-rockchip/rk3036/save_boot_param.S b/arch/arm/mach-rockchip/save_boot_param.S
>> similarity index 90%
>> rename from arch/arm/mach-rockchip/rk3036/save_boot_param.S
>> rename to arch/arm/mach-rockchip/save_boot_param.S
>> index 778ec83..85b407b 100644
>> --- a/arch/arm/mach-rockchip/rk3036/save_boot_param.S
>> +++ b/arch/arm/mach-rockchip/save_boot_param.S
>> @@ -1,5 +1,5 @@
>>   /*
>> - * (C) Copyright 2015 Google, Inc
>> + * (C) Copyright 2016 Rockchip Electronics Co., Ltd
>>    *
>>    * SPDX-License-Identifier:     GPL-2.0+
>>    */
>> diff --git a/doc/README.rockchip b/doc/README.rockchip
>> index e0572c8..4c6a04e 100644
>> --- a/doc/README.rockchip
>> +++ b/doc/README.rockchip
>> @@ -119,6 +119,20 @@ something like:
>>      Hit any key to stop autoboot:  0
>>      =>
>>
>> +The rockchip bootrom could load and boot an initial spl, then continue to
> s/could/can/
>
>> +load a second level bootloader(ie. u-boot) as soon as it return to bootrom.
> second-level
>
> U-Boot
>
> s/return/returns
>
>> +Therefore RK3288 has another loading sequence like RK3036. The option of
>> +U-Boot is controlled with this setting in U-Boot:
>> +
>> +       #define CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM
> If you make this a generic Rockchip option then it should be enabled
> for RK3036 by default.
>
>> +
>> +You can create the image via the following operations:
>> +
>> +   ./firefly-rk3288/tools/mkimage -n rk3288 -T rksd -d \
>> +       firefly-rk3288/spl/u-boot-spl-dtb.bin out && \
>> +   cat firefly-rk3288/u-boot-dtb.bin >> out && \
>> +   sudo dd if=out of=/dev/sdc seek=64
>> +
>>   If you have an HDMI cable attached you should see a video console.
>>
>>   For evb_rk3036 board:
>> diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
>> index 9d50d83..3574d57 100644
>> --- a/include/configs/rk3288_common.h
>> +++ b/include/configs/rk3288_common.h
>> @@ -33,7 +33,11 @@
>>   #define CONFIG_SYS_NS16550_MEM32
>>   #define CONFIG_SPL_BOARD_INIT
>>
>> +#ifndef CONFIG_ROCKCHIP_RK3288_SPL_BACKTO_BROM
>>   #define CONFIG_SYS_TEXT_BASE           0x00100000
>> +#else
>> +#define CONFIG_SYS_TEXT_BASE           0x00000000
>> +#endif
> Can you flip that to an ifdef?
>
> Also please add a comment as to why it needs to be 0.
>
>>   #define CONFIG_SYS_INIT_SP_ADDR                0x00100000
>>   #define CONFIG_SYS_LOAD_ADDR           0x00800800
>>   #define CONFIG_SPL_STACK               0xff718000
>> --
>> 1.9.1
>>
>>
> Regards,
> Simon
>
>
>