On 2024/1/25 06:55, Chris Morgan wrote:
On Fri, Jan 19, 2024 at 05:01:38PM +0800, Kever Yang wrote:
Hi Chris,
On 2024/1/18 23:06, Chris Morgan wrote:
On Thu, Jan 18, 2024 at 03:20:52PM +0800, Kever Yang wrote:
Hi Chris,
On 2024/1/2 23:46, Chris Morgan wrote:
From: Chris Morganmacromorgan@hotmail.com
Update the rockchip_dnl_key_pressed() so that it can run in SPL. Also change the ADC channel to a define that can be overridden by a board specific option.
I should ask this earlier.
Why you have to enable the download key in SPL?
This works for a long time in U-Boot proper, both mainline and downstream vendor U-Boot,
no one is detect this download key in SPL.
In theory the SPL can implement all the feature which is available in U-Boot proper,
but it would be better to make the SPL focus on the job it should be done and keep as small as possible.
I wanted this code to run as soon as possible in the event of a bad write. I have some devices which lack a way to bypass the eMMC in the boot path; the fear is that if an SPL stage is written with a good header but a bad payload (or if the A-TF does something wrong) the device will become unbootable. If we can't do it this way for all Rockchip boards that's fine, but I REALLY want to at least do it this way for the Powkiddy X55 board and the Anbernic RGxx3 board.
That's why rockchip platform has two download mode, loader mode(in U-Boot)
and maskRom mode, any error happen before U-Boot should get into maksRom mode.
Most of user/developer only use loader mode because the modules before U-Boot should
be more stable and have very little chance to update, for the module owners before U-Boot,
they are the users of maskRom mode.
If you enable this feature in SPL, the error still have chance to happen during dram init to SPL key detect.
Does these two board has no way to get into eMMC event with hardware rework? any hardware
signal of eMMC CMD/CLK/DATA is OK.
A few do not. The error is less likely to occur between RAM init and SPL key detect, but yes it's even possible at this stage.
From my testing using the latest BL31 file from the
rockchip-linux/rkbin github repository I was unable to reboot into maskrom mode after A-TF loaded, the device would just straight reboot. Do you know if A-TF does something that makes reboot into maskrom mode no longer work?
This is no reasonable, it should be able to reboot to maskrom mode any time we want, because
the vendor kernel also do this and the function works.
And another way to bypass eMMC is to use SDcard which has higher priority then eMMC in BootRom.
I see in the TRM under section 1.2 that the eMMC is higher in the boot path than SDMMC, and that matches my experience as well.
Yes, you are right about this, and I think rockchip vendor code is using SD card as the first priority in the SPL instead of "same-as SPL", and make the SPL small and simple enough to involve less bug, so that stable enough and no need to update. This patch is OK when the check for key press entry is in the board level instead of platform common code.
Thanks, - Kever
So this wouldn't be a solution sadly.
For SPL itself debug, I would suggest to RESET into bootrom download mode if any error happen
and a reset will need.
I pick up patches other than 2,3,4 in this patchset for now.
While I have you, I have a related question I hope you can help with. Is there any way to utilize the boot partitions of an eMMC device? I don't know if the BROM of an RK3568 or RK3588 is capable, but I'd love to have the SPL and U-Boot stages written to the eMMC boot partitions instead of the user partition if I can.
No, Rockchip SoCs does not use the boot partitions of eMMC, only use the user data partition.
That's unfortunate, but I appreciate the feedback.
Thank you, Chris
Thanks,
- Kever
Thank you.
Thanks,
- Kever
Signed-off-by: Chris Morganmacromorgan@hotmail.com
arch/arm/mach-rockchip/Makefile | 4 ++-- arch/arm/mach-rockchip/boot_mode.c | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-)diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile index 1dc92066bb..ff089ae949 100644 --- a/arch/arm/mach-rockchip/Makefile +++ b/arch/arm/mach-rockchip/Makefile @@ -15,13 +15,13 @@ obj-tpl-$(CONFIG_ROCKCHIP_PX30) += px30-board-tpl.o obj-spl-$(CONFIG_ROCKCHIP_RK3036) += rk3036-board-spl.o -ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),)
- # Always include boot_mode.o, as we bypass it (i.e. turn it off) # inside of boot_mode.c when CONFIG_ROCKCHIP_BOOT_MODE_REG is 0. This way, # we can have the preprocessor correctly recognise both 0x0 and 0 # meaning "turn it off". obj-y += boot_mode.o
+ifeq ($(CONFIG_SPL_BUILD)$(CONFIG_TPL_BUILD),) obj-$(CONFIG_ROCKCHIP_COMMON_BOARD) += board.o obj-$(CONFIG_MISC_INIT_R) += misc.o endif diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c index eb8f65ae4e..d2308768be 100644 --- a/arch/arm/mach-rockchip/boot_mode.c +++ b/arch/arm/mach-rockchip/boot_mode.c @@ -38,6 +38,10 @@ void set_back_to_bootrom_dnl_flag(void) #define KEY_DOWN_MIN_VAL 0 #define KEY_DOWN_MAX_VAL 30 +#ifndef RK_DNL_ADC_CHAN +#define RK_DNL_ADC_CHAN 1 +#endif
- __weak int rockchip_dnl_key_pressed(void) { unsigned int val;
@@ -52,7 +56,8 @@ __weak int rockchip_dnl_key_pressed(void) ret = -ENODEV; uclass_foreach_dev(dev, uc) { if (!strncmp(dev->name, "saradc", 6)) {
ret = adc_channel_single_shot(dev->name, 1, &val);
ret = adc_channel_single_shot(dev->name,RK_DNL_ADC_CHAN, &val); break; } }@@ -73,11 +78,13 @@ __weak int rockchip_dnl_key_pressed(void) void rockchip_dnl_mode_check(void) { +#if CONFIG_IS_ENABLED(ADC) if (rockchip_dnl_key_pressed()) { printf("download key pressed, entering download mode..."); set_back_to_bootrom_dnl_flag(); do_reset(NULL, 0, 0, NULL); } +#endif } int setup_boot_mode(void) @@ -90,6 +97,7 @@ int setup_boot_mode(void) boot_mode = readl(reg); debug("%s: boot mode 0x%08x\n", __func__, boot_mode); +#if !defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD) /* Clear boot mode */ writel(BOOT_NORMAL, reg); @@ -103,6 +111,7 @@ int setup_boot_mode(void) env_set("preboot", "setenv preboot; ums mmc 0"); break; } +#endif return 0; }