Hi Andy,
On 13 September 2017 at 00:14, Andy Yan andy.yan@rock-chips.com wrote:
Hi Simon:
On 2017年09月13日 12:26, Simon Glass wrote:
Hi Andy,
On 12 September 2017 at 07:58, Andy Yan andy.yan@rock-chips.com wrote:
Enter download mode if the download key pressed.
Signed-off-by: Andy Yan andy.yan@rock-chips.com
arch/arm/mach-rockchip/boot_mode.c | 27 ++++++++++++++++++++++++++- board/rockchip/evb_rk3399/evb-rk3399.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/boot_mode.c b/arch/arm/mach-rockchip/boot_mode.c index 7b3cbc5..bff1cbc 100644 --- a/arch/arm/mach-rockchip/boot_mode.c +++ b/arch/arm/mach-rockchip/boot_mode.c @@ -8,11 +8,36 @@ #include <asm/io.h> #include <asm/arch/boot_mode.h>
+void set_back_to_bootrom_dnl_flag(void) +{
writel(BOOT_BROM_DOWNLOAD, CONFIG_ROCKCHIP_BOOT_MODE_REG);+}
+/*
- some boards use a adc key, but some use gpio
- */
+__weak int rockchip_dnl_key_pressed(void)
Can you please declare this in a header file and add a full comment as to what this does?
This function is only used in this file and can be override by otherboards. So I don't think this is a necessary to declare this in a header file.
The reason is that all exported functions should be declared somewhere. Otherwise the function can be used / overridden somewhere else without any argument checking.
Of course, I will add a full comment for it.+{
return false;+}
+void rockchip_dnl_mode_check(void) +{
if (rockchip_dnl_key_pressed()) {printf("download key pressed, enter download mode...");should that be 'entering'?
Okay, should be 'entering'
set_back_to_bootrom_dnl_flag();do_reset(NULL, 0, 0, NULL);}+}
- int setup_boot_mode(void) { void* reg = (void *)CONFIG_ROCKCHIP_BOOT_MODE_REG;
int boot_mode = readl(reg);
int boot_mode;rockchip_dnl_mode_check();boot_mode = readl(reg); debug("boot mode %x.\n", boot_mode); /* Clear boot mode */diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c b/board/rockchip/evb_rk3399/evb-rk3399.c index d50c59d..738d942 100644 --- a/board/rockchip/evb_rk3399/evb-rk3399.c +++ b/board/rockchip/evb_rk3399/evb-rk3399.c @@ -4,6 +4,7 @@
- SPDX-License-Identifier: GPL-2.0+
*/ #include <common.h> +#include <adc.h> #include <dm.h> #include <ram.h> #include <dm/pinctrl.h> @@ -13,6 +14,33 @@
DECLARE_GLOBAL_DATA_PTR;
+#define KEY_DOWN_MIN_VAL 1 +#define KEY_DOWN_MAX_VAL 20
+int rockchip_dnl_key_pressed(void) +{
unsigned int ret;unsigned int i;unsigned int val;int cnt = 0;Please document what is going on here - why the loop? What is the check against min/max value?
This if for voltage debounce, according to my experience on the rk3399evb board, in the 10 adc sample results, we will met 2 ~ 4 result vals are out of the MIN~MAX range.
OK great, please add a comment here about this.
for (i = 0; i < 10; i++) {ret = adc_channel_single_shot("saradc", 1, &val);if (ret) {printf("%s adc_channel_single_shot fail!\n",__func__);
break;}if ((val >= KEY_DOWN_MIN_VAL) && (val <=KEY_DOWN_MAX_VAL))
cnt++;}if (cnt >= 8)return true;elsereturn false;+}
- int board_init(void) { struct udevice *pinctrl, *regulator;
-- 2.7.4Regards,
Simon