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 Morgan<macromor...@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 Morgan<macromor...@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;
    }

Reply via email to