Hi Johan,

On 2022/4/4 07:06, Johan Jonker wrote:

Move ROCKCHIP_STIMER_BASE to Kconfig.

Signed-off-by: Johan Jonker jbx6244@gmail.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

Changed V4: rebase remove depends on rk3188/rk3066 must now disable ROCKCHIP_STIMER in defconfig with defconfig there is more flexibility for other SoC types

Changed V3: add ROCKCHIP_STIMER

---

arch/arm/mach-rockchip/Kconfig | 10 ++++++++++ arch/arm/mach-rockchip/px30/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3036/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3128/Kconfig | 3 +++ arch/arm/mach-rockchip/rk322x/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3288/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3308/Kconfig | 10 ++++++---- arch/arm/mach-rockchip/rk3328/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3368/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3399/Kconfig | 3 +++ arch/arm/mach-rockchip/rk3568/Kconfig | 3 +++ configs/rock_defconfig | 3 ++- include/configs/px30_common.h | 1 - include/configs/rk3036_common.h | 1 - include/configs/rk3128_common.h | 1 - include/configs/rk322x_common.h | 1 - include/configs/rk3288_common.h | 1 - include/configs/rk3308_common.h | 1 - include/configs/rk3328_common.h | 1 - include/configs/rk3368_common.h | 1 - include/configs/rk3399_common.h | 1 - include/configs/rk3568_common.h | 1 - 22 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 308dc09b03..811964973a 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -339,6 +339,16 @@ config ROCKCHIP_BOOT_MODE_REG The Soc will enter to different boot mode(defined in asm/arch-rockchip/boot_mode.h) according to the value from this register.

+config ROCKCHIP_STIMER

• bool "Rockchip STIMER support"
• default y
• help

•  Enable Rockchip STIMER support.
  

+config ROCKCHIP_STIMER_BASE

• hex
• depends on ROCKCHIP_STIMER
• config ROCKCHIP_SPL_RESERVE_IRAM hex "Size of IRAM reserved in SPL" default 0

diff --git a/arch/arm/mach-rockchip/px30/Kconfig b/arch/arm/mach-rockchip/px30/Kconfig index 145bf3591f..4886fe946e 100644 --- a/arch/arm/mach-rockchip/px30/Kconfig +++ b/arch/arm/mach-rockchip/px30/Kconfig @@ -38,6 +38,9 @@ config TARGET_PX30_CORE config ROCKCHIP_BOOT_MODE_REG default 0xff010200

+config ROCKCHIP_STIMER_BASE

• default 0xff220020
• config SYS_SOC default "px30"

diff --git a/arch/arm/mach-rockchip/rk3036/Kconfig b/arch/arm/mach-rockchip/rk3036/Kconfig index b746795d81..111531be1e 100644 --- a/arch/arm/mach-rockchip/rk3036/Kconfig +++ b/arch/arm/mach-rockchip/rk3036/Kconfig @@ -16,6 +16,9 @@ endchoice config ROCKCHIP_BOOT_MODE_REG default 0x200081c8

+config ROCKCHIP_STIMER_BASE

• default 0x200440a0
• config SYS_SOC default "rk3036"

diff --git a/arch/arm/mach-rockchip/rk3128/Kconfig b/arch/arm/mach-rockchip/rk3128/Kconfig index b867401c7f..9cc494eb40 100644 --- a/arch/arm/mach-rockchip/rk3128/Kconfig +++ b/arch/arm/mach-rockchip/rk3128/Kconfig @@ -16,6 +16,9 @@ endchoice config ROCKCHIP_BOOT_MODE_REG default 0x100a0038

+config ROCKCHIP_STIMER_BASE

• default 0x200440a0
• config SYS_SOC default "rk3128"

diff --git a/arch/arm/mach-rockchip/rk322x/Kconfig b/arch/arm/mach-rockchip/rk322x/Kconfig index 6458cd5581..058f848ddc 100644 --- a/arch/arm/mach-rockchip/rk322x/Kconfig +++ b/arch/arm/mach-rockchip/rk322x/Kconfig @@ -8,6 +8,9 @@ config TARGET_EVB_RK3229 config ROCKCHIP_BOOT_MODE_REG default 0x110005c8

+config ROCKCHIP_STIMER_BASE

• default 0x110d0020
• config SYS_SOC default "rk322x"

diff --git a/arch/arm/mach-rockchip/rk3288/Kconfig b/arch/arm/mach-rockchip/rk3288/Kconfig index f37b1bdfd5..dd8c7826fc 100644 --- a/arch/arm/mach-rockchip/rk3288/Kconfig +++ b/arch/arm/mach-rockchip/rk3288/Kconfig @@ -148,6 +148,9 @@ config ROCKCHIP_FAST_SPL config ROCKCHIP_BOOT_MODE_REG default 0xff730094

+config ROCKCHIP_STIMER_BASE

• default 0xff810020
• config SYS_SOC default "rk3288"

diff --git a/arch/arm/mach-rockchip/rk3308/Kconfig b/arch/arm/mach-rockchip/rk3308/Kconfig index 8fa536e15d..194353e4cd 100644 --- a/arch/arm/mach-rockchip/rk3308/Kconfig +++ b/arch/arm/mach-rockchip/rk3308/Kconfig @@ -8,6 +8,12 @@ config TARGET_ROC_RK3308_CC bool "Firefly roc-rk3308-cc" select BOARD_LATE_INIT

+config ROCKCHIP_BOOT_MODE_REG

• default 0xff000500

+config ROCKCHIP_STIMER_BASE

• default 0xff1b00a0
• config SYS_SOC default "rk3308"

@@ -17,10 +23,6 @@ config SYS_MALLOC_F_LEN config SPL_SERIAL default y

-config ROCKCHIP_BOOT_MODE_REG

• default 0xff000500
• source "board/rockchip/evb_rk3308/Kconfig" source "board/firefly/firefly-rk3308/Kconfig"

diff --git a/arch/arm/mach-rockchip/rk3328/Kconfig b/arch/arm/mach-rockchip/rk3328/Kconfig index d13a169022..f6f1e06a83 100644 --- a/arch/arm/mach-rockchip/rk3328/Kconfig +++ b/arch/arm/mach-rockchip/rk3328/Kconfig @@ -15,6 +15,9 @@ endchoice config ROCKCHIP_BOOT_MODE_REG default 0xff1005c8

+config ROCKCHIP_STIMER_BASE

• default 0xff1d0020
• config SYS_SOC default "rk3328"

diff --git a/arch/arm/mach-rockchip/rk3368/Kconfig b/arch/arm/mach-rockchip/rk3368/Kconfig index 78eb96df3d..104db36737 100644 --- a/arch/arm/mach-rockchip/rk3368/Kconfig +++ b/arch/arm/mach-rockchip/rk3368/Kconfig @@ -45,6 +45,9 @@ endchoice config ROCKCHIP_BOOT_MODE_REG default 0xff738200

+config ROCKCHIP_STIMER_BASE

• default 0xff830020
• config SYS_SOC default "rk3368"

diff --git a/arch/arm/mach-rockchip/rk3399/Kconfig b/arch/arm/mach-rockchip/rk3399/Kconfig index 0833e083d9..c1f251316c 100644 --- a/arch/arm/mach-rockchip/rk3399/Kconfig +++ b/arch/arm/mach-rockchip/rk3399/Kconfig @@ -125,6 +125,9 @@ endchoice config ROCKCHIP_BOOT_MODE_REG default 0xff320300

+config ROCKCHIP_STIMER_BASE

• default 0xff8680a0
• config SYS_SOC default "rk3399"

diff --git a/arch/arm/mach-rockchip/rk3568/Kconfig b/arch/arm/mach-rockchip/rk3568/Kconfig index 201c63c2a9..4e7c02cce0 100644 --- a/arch/arm/mach-rockchip/rk3568/Kconfig +++ b/arch/arm/mach-rockchip/rk3568/Kconfig @@ -9,6 +9,9 @@ config TARGET_EVB_RK3568 config ROCKCHIP_BOOT_MODE_REG default 0xfdc20200

+config ROCKCHIP_STIMER_BASE

• default 0xfdd1c020
• config SYS_SOC default "rk3568"

diff --git a/configs/rock_defconfig b/configs/rock_defconfig index 46df66994b..4aa4608f90 100644 --- a/configs/rock_defconfig +++ b/configs/rock_defconfig @@ -11,12 +11,13 @@ CONFIG_ENV_OFFSET=0x3F8000 CONFIG_DEFAULT_DEVICE_TREE="rk3188-radxarock" CONFIG_SPL_TEXT_BASE=0x10080800 CONFIG_ROCKCHIP_RK3188=y +# CONFIG_ROCKCHIP_STIMER is not set CONFIG_TARGET_ROCK=y CONFIG_SPL_STACK_R_ADDR=0x60080000 CONFIG_DEBUG_UART_BASE=0x20064000 CONFIG_DEBUG_UART_CLOCK=24000000 -CONFIG_DEBUG_UART=y CONFIG_SYS_LOAD_ADDR=0x60800800 +CONFIG_DEBUG_UART=y CONFIG_USE_PREBOOT=y CONFIG_DEFAULT_FDT_FILE="rk3188-radxarock.dtb" # CONFIG_DISPLAY_CPUINFO is not set diff --git a/include/configs/px30_common.h b/include/configs/px30_common.h index 0992387157..dc609013f3 100644 --- a/include/configs/px30_common.h +++ b/include/configs/px30_common.h @@ -12,7 +12,6 @@

#define CONFIG_SYS_NS16550_MEM32

-#define CONFIG_ROCKCHIP_STIMER_BASE 0xff220020 #define COUNTER_FREQUENCY 24000000

/* FIXME: ff020000 is pmu_mem (10k), while ff0e0000 is regular int_mem */ diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h index 00c453d739..5905518edf 100644 --- a/include/configs/rk3036_common.h +++ b/include/configs/rk3036_common.h @@ -10,7 +10,6 @@

#define CONFIG_SYS_CBSIZE 1024

-#define CONFIG_ROCKCHIP_STIMER_BASE 0x200440a0 #define COUNTER_FREQUENCY 24000000 #define CONFIG_SYS_HZ_CLOCK 24000000

diff --git a/include/configs/rk3128_common.h b/include/configs/rk3128_common.h index 97caceacfe..d77a7d7b09 100644 --- a/include/configs/rk3128_common.h +++ b/include/configs/rk3128_common.h @@ -11,7 +11,6 @@ #define CONFIG_SYS_MAXARGS 16 #define CONFIG_SYS_CBSIZE 1024

-#define CONFIG_ROCKCHIP_STIMER_BASE 0x200440a0 #define COUNTER_FREQUENCY 24000000 #define CONFIG_SYS_HZ_CLOCK 24000000

diff --git a/include/configs/rk322x_common.h b/include/configs/rk322x_common.h index ef55ef0a83..3258820fcd 100644 --- a/include/configs/rk322x_common.h +++ b/include/configs/rk322x_common.h @@ -11,7 +11,6 @@ #define CONFIG_SYS_CBSIZE 1024 #define CONFIG_SYS_BOOTM_LEN (64 << 20) /* 64M */

-#define CONFIG_ROCKCHIP_STIMER_BASE 0x110d0020 #define COUNTER_FREQUENCY 24000000 #define CONFIG_SYS_HZ_CLOCK 24000000

diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h index 490da7cb23..e2e0f70a70 100644 --- a/include/configs/rk3288_common.h +++ b/include/configs/rk3288_common.h @@ -13,7 +13,6 @@

#define CONFIG_SYS_CBSIZE 1024

-#define CONFIG_ROCKCHIP_STIMER_BASE 0xff810020 #define COUNTER_FREQUENCY 24000000 #define CONFIG_SYS_HZ_CLOCK 24000000

diff --git a/include/configs/rk3308_common.h b/include/configs/rk3308_common.h index 1664707ca6..9cda8d9c48 100644 --- a/include/configs/rk3308_common.h +++ b/include/configs/rk3308_common.h @@ -15,7 +15,6 @@

#define CONFIG_SYS_NS16550_MEM32

-#define CONFIG_ROCKCHIP_STIMER_BASE 0xff1b00a0 #define CONFIG_IRAM_BASE 0xfff80000 #define CONFIG_SYS_INIT_SP_ADDR 0x00800000 #define CONFIG_SPL_STACK 0x00400000 diff --git a/include/configs/rk3328_common.h b/include/configs/rk3328_common.h index c1e26a019b..8a5f0c8999 100644 --- a/include/configs/rk3328_common.h +++ b/include/configs/rk3328_common.h @@ -10,7 +10,6 @@

#define CONFIG_IRAM_BASE 0xff090000

-#define CONFIG_ROCKCHIP_STIMER_BASE 0xff1d0020 #define COUNTER_FREQUENCY 24000000

#define CONFIG_SYS_CBSIZE 1024 diff --git a/include/configs/rk3368_common.h b/include/configs/rk3368_common.h index 8b239ca07d..239296c1d2 100644 --- a/include/configs/rk3368_common.h +++ b/include/configs/rk3368_common.h @@ -15,7 +15,6 @@ #define SDRAM_MAX_SIZE 0xff000000 #define CONFIG_SYS_CBSIZE 1024

-#define CONFIG_ROCKCHIP_STIMER_BASE 0xff830020 #define COUNTER_FREQUENCY 24000000

#define CONFIG_IRAM_BASE 0xff8c0000 diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h index ed72c8bb6b..4037dba58c 100644 --- a/include/configs/rk3399_common.h +++ b/include/configs/rk3399_common.h @@ -11,7 +11,6 @@ #define CONFIG_SYS_CBSIZE 1024

#define COUNTER_FREQUENCY 24000000 -#define CONFIG_ROCKCHIP_STIMER_BASE 0xff8680a0

#define CONFIG_IRAM_BASE 0xff8c0000

diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h index 25d7c5cc8f..5649cd64e0 100644 --- a/include/configs/rk3568_common.h +++ b/include/configs/rk3568_common.h @@ -11,7 +11,6 @@ #define CONFIG_SYS_CBSIZE 1024

#define COUNTER_FREQUENCY 24000000 -#define CONFIG_ROCKCHIP_STIMER_BASE 0xfdd1c020

#define CONFIG_IRAM_BASE 0xfdcc0000

Hi Johan,

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip SoCs rk3066/rk3188 have no CONFIG_ROCKCHIP_STIMER_BASE defined. Currently only rk3188 has an exception. Make this more generic and call the function rockchip_stimer_init() only when CONFIG_ROCKCHIP_STIMER_BASE is available.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

---

Changed V3: use CONFIG_ROCKCHIP_STIMER

Changed V2: use IS_ENABLED add include kconfig.h move define location so that rockchip_stimer_init() is always visible to the compiler

---

arch/arm/mach-rockchip/spl.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c index 7a8db632b8..0017ca71b0 100644 --- a/arch/arm/mach-rockchip/spl.c +++ b/arch/arm/mach-rockchip/spl.c @@ -16,6 +16,7 @@ #include <asm/global_data.h> #include <asm/io.h> #include <linux/bitops.h> +#include <linux/kconfig.h>

DECLARE_GLOBAL_DATA_PTR;

@@ -71,7 +72,6 @@ u32 spl_mmc_boot_mode(const u32 boot_device) return MMCSD_MODE_RAW; }

-#if !defined(CONFIG_ROCKCHIP_RK3188) #define TIMER_LOAD_COUNT_L 0x00 #define TIMER_LOAD_COUNT_H 0x04 #define TIMER_CONTROL_REG 0x10 @@ -81,6 +81,7 @@ u32 spl_mmc_boot_mode(const u32 boot_device)

__weak void rockchip_stimer_init(void) { +#if defined(CONFIG_ROCKCHIP_STIMER_BASE) /* If Timer already enabled, don't re-init it */ u32 reg = readl(CONFIG_ROCKCHIP_STIMER_BASE + TIMER_CONTROL_REG);

@@ -95,8 +96,8 @@ __weak void rockchip_stimer_init(void) writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE + 4); writel(TIMER_EN | TIMER_FMODE, CONFIG_ROCKCHIP_STIMER_BASE + TIMER_CONTROL_REG); -} #endif +}

__weak int board_early_init_f(void) { @@ -133,9 +134,11 @@ void board_init_f(ulong dummy) hang(); } arch_cpu_init(); -#if !defined(CONFIG_ROCKCHIP_RK3188)

• rockchip_stimer_init();

-#endif

• /* Init secure timer */
• if (IS_ENABLED(CONFIG_ROCKCHIP_STIMER))

• rockchip_stimer_init();
  

The CONFIG_ROCKCHIP_STIMER and CONFIG_ROCKCHIP_STIMER_BASE are always show up at the same time,

so we don't need two ifdef with two different MACRO in this file, remove one of it.

I would prefer to just call rockchip_stimer_init() here, and it will be empty function for rk3066/rk3188.

Thanks,

- Kever

#ifdef CONFIG_SYS_ARCH_TIMER /* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */ timer_init();

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip SoCs rk3066/rk3188 have no CONFIG_ROCKCHIP_STIMER_BASE defined. Currently only rk3188 has an exception. Make this more generic and call the function rockchip_stimer_init() only when CONFIG_ROCKCHIP_STIMER_BASE is available.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

---

Changed V3: use CONFIG_ROCKCHIP_STIMER

Changed V2: use IS_ENABLED add include kconfig.h move define location so that rockchip_stimer_init() is always visible to the compiler

---

arch/arm/mach-rockchip/tpl.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c index 3c007bb450..2b561264a5 100644 --- a/arch/arm/mach-rockchip/tpl.c +++ b/arch/arm/mach-rockchip/tpl.c @@ -15,6 +15,7 @@ #include <asm/io.h> #include <asm/arch-rockchip/bootrom.h> #include <linux/bitops.h> +#include <linux/kconfig.h>

#if CONFIG_IS_ENABLED(BANNER_PRINT) #include <timestamp.h> @@ -29,6 +30,7 @@

__weak void rockchip_stimer_init(void) { +#if defined(CONFIG_ROCKCHIP_STIMER_BASE) /* If Timer already enabled, don't re-init it */ u32 reg = readl(CONFIG_ROCKCHIP_STIMER_BASE + TIMER_CONTROL_REG);

@@ -45,6 +47,7 @@ __weak void rockchip_stimer_init(void) writel(0xffffffff, CONFIG_ROCKCHIP_STIMER_BASE + 4); writel(TIMER_EN | TIMER_FMODE, CONFIG_ROCKCHIP_STIMER_BASE + TIMER_CONTROL_REG); +#endif }

void board_init_f(ulong dummy) @@ -74,7 +77,9 @@ void board_init_f(ulong dummy) }

/* Init secure timer */

• rockchip_stimer_init();

• if (IS_ENABLED(CONFIG_ROCKCHIP_STIMER))

• rockchip_stimer_init();
  

Same with 2/12 for SPL.

Thanks,

- Kever

• /* Init ARM arch timer in arch/arm/cpu/ */ timer_init();

On 2022/4/4 07:06, Johan Jonker wrote:

Not all Rockchip SoC models use the ARM arch timer. Call the function timer_init() only when CONFIG_SYS_ARCH_TIMER is available. Replace the ifdef call condition by IS_ENABLED to increase build coverage and make the code easier to read.

The commit message does not really describe the code change.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

---

arch/arm/mach-rockchip/spl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-rockchip/spl.c b/arch/arm/mach-rockchip/spl.c index 0017ca71b0..9c83b038a6 100644 --- a/arch/arm/mach-rockchip/spl.c +++ b/arch/arm/mach-rockchip/spl.c @@ -139,10 +139,10 @@ void board_init_f(ulong dummy) if (IS_ENABLED(CONFIG_ROCKCHIP_STIMER)) rockchip_stimer_init();

-#ifdef CONFIG_SYS_ARCH_TIMER

• /* Init ARM arch timer in arch/arm/cpu/armv7/arch_timer.c */
• timer_init();

-#endif

• /* Init ARM arch timer */
• if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))

• timer_init();
  

These two kind of code end up with the same compile output, isn't it?

This patch is no need for this series.

Thanks, - Kever

• #if !defined(CONFIG_TPL) || defined(CONFIG_SPL_RAM) debug("\nspl:init dram\n"); ret = dram_init();

On 2022/4/4 07:06, Johan Jonker wrote:

Not all Rockchip SoC models use the ARM arch timer. Call the function timer_init() only when CONFIG_SYS_ARCH_TIMER is available. Use the call condition IS_ENABLED to increase build coverage and make the code easier to read.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

arch/arm/mach-rockchip/tpl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-rockchip/tpl.c b/arch/arm/mach-rockchip/tpl.c index 2b561264a5..05deba3f84 100644 --- a/arch/arm/mach-rockchip/tpl.c +++ b/arch/arm/mach-rockchip/tpl.c @@ -80,8 +80,9 @@ void board_init_f(ulong dummy) if (IS_ENABLED(CONFIG_ROCKCHIP_STIMER)) rockchip_stimer_init();

• /* Init ARM arch timer in arch/arm/cpu/ */
• timer_init();

• /* Init ARM arch timer */

• if (IS_ENABLED(CONFIG_SYS_ARCH_TIMER))

• timer_init();
  
  

  ret = uclass_get_device(UCLASS_RAM, 0, &dev); if (ret) {

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip rk3066 SoC has 3 dw-apb-timer nodes. U-boot is compiled with OF_PLATDATA TPL/SPL options, so add OF_PLATDATA support for the dw-apb-timer. Also change driver name to be able to compile with U-boot scripts. No reset OF_PLATDATA support was added, because the rk3066 nodes don't need/have them.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

Changed V3: use if (CONFIG_IS_ENABLED(OF_REAL)) remove white space fix

Changed V2: use #if CONFIG_IS_ENABLED(OF_REAL)

---

drivers/timer/dw-apb-timer.c | 50 ++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index 9aed5dd217..f722697934 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -8,10 +8,12 @@ #include <common.h> #include <dm.h> #include <clk.h> +#include <dt-structs.h> #include <malloc.h> #include <reset.h> #include <timer.h> #include <dm/device_compat.h> +#include <linux/kconfig.h>

#include <asm/io.h> #include <asm/arch/timer.h> @@ -25,6 +27,12 @@ struct dw_apb_timer_priv { struct reset_ctl_bulk resets; };

+struct dw_apb_timer_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA)

• struct dtd_snps_dw_apb_timer dtplat;

+#endif +};

• static u64 dw_apb_timer_get_count(struct udevice *dev) { struct dw_apb_timer_priv *priv = dev_get_priv(dev);

@@ -43,20 +51,33 @@ static int dw_apb_timer_probe(struct udevice *dev) struct dw_apb_timer_priv *priv = dev_get_priv(dev); struct clk clk; int ret; +#if CONFIG_IS_ENABLED(OF_PLATDATA)

• struct dw_apb_timer_plat *plat = dev_get_plat(dev);
• struct dtd_snps_dw_apb_timer *dtplat = &plat->dtplat;

• ret = reset_get_bulk(dev, &priv->resets);
• if (ret)

• dev_warn(dev, "Can't get reset: %d\n", ret);
  

• else

• reset_deassert_bulk(&priv->resets);
  

• priv->regs = dtplat->reg[0];

• ret = clk_get_by_index(dev, 0, &clk);
• if (ret)

• ret = clk_get_by_phandle(dev, &dtplat->clocks[0], &clk);
• if (ret < 0) return ret;

• uc_priv->clock_rate = clk_get_rate(&clk);

• uc_priv->clock_rate = dtplat->clock_frequency;

+#endif

• if (CONFIG_IS_ENABLED(OF_REAL)) {

• ret = reset_get_bulk(dev, &priv->resets);
  

• if (ret)
  

• 	dev_warn(dev, "Can't get reset: %d\n", ret);
  

• else
  

• 	reset_deassert_bulk(&priv->resets);
  

• ret = clk_get_by_index(dev, 0, &clk);
  

• if (ret)
  

• 	return ret;
  
  

• clk_free(&clk);

• uc_priv->clock_rate = clk_get_rate(&clk);
  

• clk_free(&clk);
  

• }

  /* init timer */ writel(0xffffffff, priv->regs + DW_APB_LOAD_VAL);

@@ -68,9 +89,11 @@ static int dw_apb_timer_probe(struct udevice *dev)

static int dw_apb_timer_of_to_plat(struct udevice *dev) {

• struct dw_apb_timer_priv *priv = dev_get_priv(dev);

• if (CONFIG_IS_ENABLED(OF_REAL)) {

• struct dw_apb_timer_priv *priv = dev_get_priv(dev);
  
  

• priv->regs = dev_read_addr(dev);

• priv->regs = dev_read_addr(dev);
  

• }

  return 0; }

@@ -91,8 +114,8 @@ static const struct udevice_id dw_apb_timer_ids[] = { {} };

-U_BOOT_DRIVER(dw_apb_timer) = {

• .name = "dw_apb_timer",

+U_BOOT_DRIVER(snps_dw_apb_timer) = {

• .name = "snps_dw_apb_timer", .id = UCLASS_TIMER, .ops = &dw_apb_timer_ops, .probe = dw_apb_timer_probe,

@@ -100,4 +123,5 @@ U_BOOT_DRIVER(dw_apb_timer) = { .of_to_plat = dw_apb_timer_of_to_plat, .remove = dw_apb_timer_remove, .priv_auto = sizeof(struct dw_apb_timer_priv),

• .plat_auto = sizeof(struct dw_apb_timer_plat), };

On 2022/4/4 07:06, Johan Jonker wrote:

The line with .of_to_plat in the U_BOOT_DRIVER structure of dw-apb-timer.c is not aligned with the rest. Add an extra TAB to fix the whitespace.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

drivers/timer/dw-apb-timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/timer/dw-apb-timer.c b/drivers/timer/dw-apb-timer.c index f722697934..10f0a9f646 100644 --- a/drivers/timer/dw-apb-timer.c +++ b/drivers/timer/dw-apb-timer.c @@ -120,7 +120,7 @@ U_BOOT_DRIVER(snps_dw_apb_timer) = { .ops = &dw_apb_timer_ops, .probe = dw_apb_timer_probe, .of_match = dw_apb_timer_ids,

• .of_to_plat = dw_apb_timer_of_to_plat,

• .of_to_plat = dw_apb_timer_of_to_plat, .remove = dw_apb_timer_remove, .priv_auto = sizeof(struct dw_apb_timer_priv), .plat_auto = sizeof(struct dw_apb_timer_plat),

On 2022/4/4 07:06, Johan Jonker wrote:

The document rockchip-dw-mshc.yaml decribes a maximum of 4 clocks. In the rockchip_dw_mmc driver the clock name in use was "fixed" to "ciu" with index 1, but later reverted back to index 0. The clock drivers can handle both, but the calling driver should submit correct data as a standard practice. Fix the "ciu" clock index by setting it back to 1.

clock-names: minItems: 2 items: - const: biu - const: ciu - const: ciu-drive - const: ciu-sample

Signed-off-by: Johan Jonker jbx6244@gmail.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

drivers/mmc/rockchip_dw_mmc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index 7f8dea1e34..be065ec0c3 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -123,11 +123,11 @@ static int rockchip_dwmmc_probe(struct udevice *dev) priv->minmax[0] = 400000; /* 400 kHz */ priv->minmax[1] = dtplat->max_frequency;

• ret = clk_get_by_phandle(dev, dtplat->clocks, &priv->clk);

• ret = clk_get_by_phandle(dev, &dtplat->clocks[1], &priv->clk); if (ret < 0) return ret; #else

• ret = clk_get_by_index(dev, 0, &priv->clk);

• ret = clk_get_by_index(dev, 1, &priv->clk); if (ret < 0) return ret; #endif

Hi Johan,

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip SoCs rk3066/rk3188 have mmc DT nodes with as compatible string "rockchip,rk2928-dw-mshc". Add support to the existing driver with help of a DM_DRIVER_ALIAS.

This type needs a permanent enabled fifo. The other Rockchip SoCs not always have the property "fifo-mode" in the TPL/SPL DT nodes, so dtplat structures can't be used to switch it on.

There is an option "u-boot,spl-fifo-mode", which already used for many SoCs,

could you try with it?

Thanks,

- Kever

Add a data structure linked to the compatible string to enable.

Signed-off-by: Johan Jonker jbx6244@gmail.com

Changed V4: use boolean

---

drivers/mmc/rockchip_dw_mmc.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index be065ec0c3..45488dd7bd 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -19,6 +19,11 @@ #include <linux/delay.h> #include <linux/err.h>

+enum rockchip_dwmmc_type {

• RK2928_MSHC,
• RK3288_MSHC,

+};

• struct rockchip_mmc_plat { #if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3288_dw_mshc dtplat;

@@ -111,6 +116,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)

#if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3288_dw_mshc *dtplat = &plat->dtplat;

• enum rockchip_dwmmc_type type = dev_get_driver_data(dev);

  host->name = dev->name; host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]);

@@ -119,7 +125,10 @@ static int rockchip_dwmmc_probe(struct udevice *dev) host->priv = dev; host->dev_index = 0; priv->fifo_depth = dtplat->fifo_depth;

• priv->fifo_mode = 0;

• if (type == RK2928_MSHC)

• priv->fifo_mode = true;
  

• else

• priv->fifo_mode = false;
  

  priv->minmax[0] = 400000; /* 400 kHz */ priv->minmax[1] = dtplat->max_frequency;

@@ -163,8 +172,8 @@ static int rockchip_dwmmc_bind(struct udevice *dev) }

static const struct udevice_id rockchip_dwmmc_ids[] = {

• { .compatible = "rockchip,rk2928-dw-mshc" },
• { .compatible = "rockchip,rk3288-dw-mshc" },

• { .compatible = "rockchip,rk2928-dw-mshc", .data = RK2928_MSHC },
• { .compatible = "rockchip,rk3288-dw-mshc", .data = RK3288_MSHC }, { } };

@@ -180,5 +189,6 @@ U_BOOT_DRIVER(rockchip_rk3288_dw_mshc) = { .plat_auto = sizeof(struct rockchip_mmc_plat), };

+DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk2928_dw_mshc) DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3328_dw_mshc) DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3368_dw_mshc)

On 2022/4/6 23:29, Johan Jonker wrote:

Hi,

On 4/6/22 16:50, Kever Yang wrote:

...

Hi Johan,

On 2022/4/4 07:06, Johan Jonker wrote:

...

The Rockchip SoCs rk3066/rk3188 have mmc DT nodes with as compatible string "rockchip,rk2928-dw-mshc". Add support to the existing driver with help of a DM_DRIVER_ALIAS.

This type needs a permanent enabled fifo. The other Rockchip SoCs not always have the property "fifo-mode" in the TPL/SPL DT nodes, so dtplat structures can't be used to switch it on.

There is an option "u-boot,spl-fifo-mode", which already used for many SoCs,

could you try with it?

Rk3066 SPL makes use of dt-plat.c to describe the DT, because of memory size and it's BOOTROM doesn't have build-in SD card support like the later models.

u-boot-tpl.bin : 25,112 bytes u-boot-spl.bin : 39,280 bytes u-boot-dtb.img : 328,678 bytes

Example rockchip_dwmmc_probe function:

old: priv->fifo_mode = 0;

vs.

new: priv->fifo_mode = dtplat->u_boot_spl_fifo_mode

The use of u_boot_spl_fifo_mode in C code would make it a requirement for other SoC types to include it in "static struct dtd_rockchip_rk3288_dw_mshc" as well:

.u_boot_spl_fifo_mode = false,

Yes, extend the dt-plat with new fifo mode support will be OK.

BTW: rk3288 can also use fifo-mode in SPL in dt-plat mode, so you can re-use this structure

and no need to add a new one.

Thanks,

- Kever

Please advise how to get progress here as.

Kind regards,

Johan

===

static struct dtd_rockchip_rk3288_dw_mshc dtv_mmc_at_10214000 = { .bus_width = 0x4, .cap_mmc_highspeed = true, .cap_sd_highspeed = true, .clock_frequency = 0x2faf080, .clocks = { {0, {448}}, {0, {72}},}, .dma_names = "rx-tx", .dmas = {0xa, 0x1}, .fifo_depth = 0x100, .fifo_mode = true, .max_frequency = 0x3d0900, .reg = {0x10214000, 0x1000}, .reset_names = "reset", .resets = {0x2, 0x51},

.u_boot_spl_fifo_mode = true,

.vmmc_supply = 0xf, }; U_BOOT_DRVINFO(mmc_at_10214000) = { .name = "rockchip_rk3288_dw_mshc", .plat = &dtv_mmc_at_10214000, .plat_size = sizeof(dtv_mmc_at_10214000), .parent_idx = -1, };

...

Thanks,

• Kever

...

Add a data structure linked to the compatible string to enable.

Signed-off-by: Johan Jonker jbx6244@gmail.com

Changed V4:    use boolean

---

drivers/mmc/rockchip_dw_mmc.c | 16 +++++++++++++---   1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c index be065ec0c3..45488dd7bd 100644 --- a/drivers/mmc/rockchip_dw_mmc.c +++ b/drivers/mmc/rockchip_dw_mmc.c @@ -19,6 +19,11 @@   #include <linux/delay.h>   #include <linux/err.h>   +enum rockchip_dwmmc_type { +    RK2928_MSHC, +    RK3288_MSHC, +};

struct rockchip_mmc_plat {   #if CONFIG_IS_ENABLED(OF_PLATDATA)       struct dtd_rockchip_rk3288_dw_mshc dtplat; @@ -111,6 +116,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)     #if CONFIG_IS_ENABLED(OF_PLATDATA)       struct dtd_rockchip_rk3288_dw_mshc *dtplat = &plat->dtplat; +    enum rockchip_dwmmc_type type = dev_get_driver_data(dev);         host->name = dev->name;       host->ioaddr = map_sysmem(dtplat->reg[0], dtplat->reg[1]); @@ -119,7 +125,10 @@ static int rockchip_dwmmc_probe(struct udevice *dev)       host->priv = dev;       host->dev_index = 0;       priv->fifo_depth = dtplat->fifo_depth; -    priv->fifo_mode = 0; +    if (type == RK2928_MSHC) +        priv->fifo_mode = true; +    else +        priv->fifo_mode = false;       priv->minmax[0] = 400000;  /*  400 kHz */       priv->minmax[1] = dtplat->max_frequency;   @@ -163,8 +172,8 @@ static int rockchip_dwmmc_bind(struct udevice *dev)   }     static const struct udevice_id rockchip_dwmmc_ids[] = { -    { .compatible = "rockchip,rk2928-dw-mshc" }, -    { .compatible = "rockchip,rk3288-dw-mshc" }, +    { .compatible = "rockchip,rk2928-dw-mshc", .data = RK2928_MSHC }, +    { .compatible = "rockchip,rk3288-dw-mshc", .data = RK3288_MSHC },       { }   };   @@ -180,5 +189,6 @@ U_BOOT_DRIVER(rockchip_rk3288_dw_mshc) = {       .plat_auto    = sizeof(struct rockchip_mmc_plat),   };   +DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk2928_dw_mshc)   DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3328_dw_mshc)   DM_DRIVER_ALIAS(rockchip_rk3288_dw_mshc, rockchip_rk3368_dw_mshc)

On 2022/4/4 07:06, Johan Jonker wrote:

The ns16550.c driver has the following conditions for .of_match:

CONFIG_IS_ENABLED(OF_REAL)

For Rockchip SoCs with TPL/SPL and platform data that need serial support the serial_rockchip.c driver was made. It copies this data and then calls ns16550_serial_probe(). With the addition of yet an other SoC type this driver is in need for a little restyle. Simplify struct rockchip_uart_plat and add extra SoCs with DM_DRIVER_ALIAS(). Return -ENODEV when the ns16550.c driver probe function is available.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

Changed V4: add empty line

---

drivers/serial/serial_rockchip.c | 37 +++++++++++++------------------- 1 file changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/serial/serial_rockchip.c b/drivers/serial/serial_rockchip.c index 97d40869a2..f5ac705f4d 100644 --- a/drivers/serial/serial_rockchip.c +++ b/drivers/serial/serial_rockchip.c @@ -12,22 +12,20 @@ #include <asm/arch-rockchip/clock.h> #include <dm/device-internal.h>

-#if defined(CONFIG_ROCKCHIP_RK3188) -struct rockchip_uart_plat {

• struct dtd_rockchip_rk3188_uart dtplat;
• struct ns16550_plat plat;

-}; -struct dtd_rockchip_rk3188_uart *dtplat, s_dtplat; -#elif defined(CONFIG_ROCKCHIP_RK3288) struct rockchip_uart_plat { +#if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3288_uart dtplat; +#endif struct ns16550_plat plat; };

+#if CONFIG_IS_ENABLED(OF_PLATDATA) struct dtd_rockchip_rk3288_uart *dtplat, s_dtplat; #endif

static int rockchip_serial_probe(struct udevice *dev) { +#if CONFIG_IS_ENABLED(OF_PLATDATA) struct rockchip_uart_plat *plat = dev_get_plat(dev);

/* Create some new platform data for the standard driver */ @@ -38,24 +36,19 @@ static int rockchip_serial_probe(struct udevice *dev) dev_set_plat(dev, &plat->plat);

return ns16550_serial_probe(dev); +#else

• return -ENODEV;

+#endif }

-U_BOOT_DRIVER(rockchip_rk3188_uart) = {

• .name = "rockchip_rk3188_uart",
• .id = UCLASS_SERIAL,
• .priv_auto = sizeof(struct ns16550),
• .plat_auto = sizeof(struct rockchip_uart_plat),
• .probe = rockchip_serial_probe,
• .ops = &ns16550_serial_ops,
• .flags = DM_FLAG_PRE_RELOC,

-};

• U_BOOT_DRIVER(rockchip_rk3288_uart) = {
• .name = "rockchip_rk3288_uart",
• .id = UCLASS_SERIAL,

• .name = "rockchip_rk3288_uart",
• .id = UCLASS_SERIAL, .priv_auto = sizeof(struct ns16550), .plat_auto = sizeof(struct rockchip_uart_plat),

• .probe = rockchip_serial_probe,
• .ops = &ns16550_serial_ops,
• .flags = DM_FLAG_PRE_RELOC,

• .probe = rockchip_serial_probe,
• .ops = &ns16550_serial_ops,
• .flags = DM_FLAG_PRE_RELOC, };

+DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3066_uart) +DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3188_uart)

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip uart DT nodes have "snps,dw-apb-uart" as fall back string. The driver ns16550.c has CONFIG_IS_ENABLED(OF_REAL) as condition to of_match and does not copy dtplat data. For TPL/SPL the driver serial_rockchip.c is used. Move driver alias to correct driver.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

drivers/serial/ns16550.c | 2 -- drivers/serial/serial_rockchip.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 796ff1658c..a4220fd0ae 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -624,8 +624,6 @@ U_BOOT_DRIVER(ns16550_serial) = { #endif };

-DM_DRIVER_ALIAS(ns16550_serial, rockchip_rk3328_uart) -DM_DRIVER_ALIAS(ns16550_serial, rockchip_rk3368_uart) DM_DRIVER_ALIAS(ns16550_serial, ti_da830_uart) #endif #endif /* SERIAL_PRESENT */ diff --git a/drivers/serial/serial_rockchip.c b/drivers/serial/serial_rockchip.c index f5ac705f4d..10e731caa1 100644 --- a/drivers/serial/serial_rockchip.c +++ b/drivers/serial/serial_rockchip.c @@ -52,3 +52,5 @@ U_BOOT_DRIVER(rockchip_rk3288_uart) = { }; DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3066_uart) DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3188_uart) +DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3328_uart) +DM_DRIVER_ALIAS(rockchip_rk3288_uart, rockchip_rk3368_uart)

On 2022/4/4 07:06, Johan Jonker wrote:

The Rockchip serial driver depends on an enabled NS16550 driver, so add select SYS_NS16550 to config ROCKCHIP_SERIAL.

Signed-off-by: Johan Jonker jbx6244@gmail.com Reviewed-by: Simon Glass sjg@chromium.org

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

drivers/serial/Kconfig | 1 + 1 file changed, 1 insertion(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 76171e7146..06a69e78de 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -763,6 +763,7 @@ config PL01X_SERIAL config ROCKCHIP_SERIAL bool "Rockchip on-chip UART support" depends on DM_SERIAL && SPL_OF_PLATDATA

• select SYS_NS16550 help Select this to enable a debug UART for Rockchip devices when using CONFIG_SPL_OF_PLATDATA (i.e. a compiled-in device tree replacemenmt).