[PATCH v2 3/5] ram: rockchip: Add separate RAM_ROCKCHIP_DEBUG config for TPL/SPL

Łukasz Czechowski lukasz.czechowski at thaumatec.com
Tue Sep 10 17:17:44 CEST 2024


Hi Kever,
Thanks for the review.
This patch is indeed intended to disallow enabling RAM_ROCKCHIP_DEBUG
when SILENT_CONSOLE is enabled and control what is printed on the
debug console more independently. Patch was created in response to
review of patchset v1, and implements change suggested by Quentin
Schulz <quentin.schulz at cherry.de>. The additional complexity with new
TPL/SPL symbols had been added because we couldn't simply add i.e.
`depends on !TPL_SILENT_CONSOLE if TPL` and  `depends on
!SPL_SILENT_CONSOLE if SPL`.
@Quentin, is it fine with you that we can drop this change?

Best regards,
Lukasz


wt., 10 wrz 2024 o 11:51 Kever Yang <kever.yang at rock-chips.com> napisał(a):
>
> Hi Lukasz,
>
> On 2024/9/4 00:38, Lukasz Czechowski wrote:
>
> Introduce new config symbols TPL_RAM_ROCKCHIP_DEBUG and
> SPL_RAM_ROCKCHIP_DEBUG to allow for better dependencies control
> of RAM driver debugging configuration.
>
> The RAM_ROCKCHIP_DEBUG should enough because this only happen when ram driver
>
> running the initialization, and this init process always only run only once for all SoCs.
>
> RAM_ROCKCHIP_DEBUG depends on DEBUG_UART is the correct and no need to add more
>
> other dependency.
>
> Add negative dependencies to TPL_SILENT_CONSOLE and
> SPL_SILENT_CONSOLE, respectively.
>
> I believe this is he main target, you want to control the UART output by SILENT_CONTROL,
>
> but the RAM_DEBUG should follow the DEBUG_UART.
>
> So I think this patch is no need, please drop it.
>
>
> Thanks,
> - Kever
>
> Replace IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG) directives with
> CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG) to utilize the configuration
> changes.
>
> Update defconfigs of affected boards so that behavior
> is not changed.
>
> Signed-off-by: Lukasz Czechowski <lukasz.czechowski at thaumatec.com>
> ---
>  configs/anbernic-rgxx3-rk3566_defconfig   |  1 +
>  configs/neu2-io-rv1126_defconfig          |  2 ++
>  configs/roc-pc-mezzanine-rk3399_defconfig |  2 ++
>  configs/roc-pc-rk3399_defconfig           |  2 ++
>  configs/rock-pi-n10-rk3399pro_defconfig   |  2 ++
>  configs/sonoff-ihost-rv1126_defconfig     |  2 ++
>  drivers/ram/rockchip/Kconfig              | 24 +++++++++++++++++++++++
>  drivers/ram/rockchip/sdram_common.c       |  2 +-
>  drivers/ram/rockchip/sdram_rk3399.c       |  4 ++--
>  drivers/ram/rockchip/sdram_rv1126.c       | 10 +++++-----
>  10 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/configs/anbernic-rgxx3-rk3566_defconfig b/configs/anbernic-rgxx3-rk3566_defconfig
> index a03509bf467..4392bb5af00 100644
> --- a/configs/anbernic-rgxx3-rk3566_defconfig
> +++ b/configs/anbernic-rgxx3-rk3566_defconfig
> @@ -68,6 +68,7 @@ CONFIG_REGULATOR_RK8XX=y
>  CONFIG_PWM_ROCKCHIP=y
>  CONFIG_SPL_RAM=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
>  # CONFIG_RNG_SMCCC_TRNG is not set
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
> diff --git a/configs/neu2-io-rv1126_defconfig b/configs/neu2-io-rv1126_defconfig
> index 2a4c9b45a04..1bdf99887b3 100644
> --- a/configs/neu2-io-rv1126_defconfig
> +++ b/configs/neu2-io-rv1126_defconfig
> @@ -46,6 +46,8 @@ CONFIG_MMC_DW_ROCKCHIP=y
>  CONFIG_REGULATOR_PWM=y
>  CONFIG_PWM_ROCKCHIP=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_TPL_RAM_ROCKCHIP_DEBUG is not set
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYSRESET=y
> diff --git a/configs/roc-pc-mezzanine-rk3399_defconfig b/configs/roc-pc-mezzanine-rk3399_defconfig
> index a57899bfdfa..45c4975e452 100644
> --- a/configs/roc-pc-mezzanine-rk3399_defconfig
> +++ b/configs/roc-pc-mezzanine-rk3399_defconfig
> @@ -66,6 +66,8 @@ CONFIG_SPL_DM_REGULATOR_FIXED=y
>  CONFIG_REGULATOR_RK8XX=y
>  CONFIG_PWM_ROCKCHIP=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_TPL_RAM_ROCKCHIP_DEBUG is not set
>  CONFIG_RAM_ROCKCHIP_LPDDR4=y
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
> diff --git a/configs/roc-pc-rk3399_defconfig b/configs/roc-pc-rk3399_defconfig
> index b45f0e0a899..3f2d5650249 100644
> --- a/configs/roc-pc-rk3399_defconfig
> +++ b/configs/roc-pc-rk3399_defconfig
> @@ -63,6 +63,8 @@ CONFIG_SPL_DM_REGULATOR_FIXED=y
>  CONFIG_REGULATOR_RK8XX=y
>  CONFIG_PWM_ROCKCHIP=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_TPL_RAM_ROCKCHIP_DEBUG is not set
>  CONFIG_RAM_ROCKCHIP_LPDDR4=y
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
> diff --git a/configs/rock-pi-n10-rk3399pro_defconfig b/configs/rock-pi-n10-rk3399pro_defconfig
> index ec995a54a0e..d4ba628e428 100644
> --- a/configs/rock-pi-n10-rk3399pro_defconfig
> +++ b/configs/rock-pi-n10-rk3399pro_defconfig
> @@ -52,6 +52,8 @@ CONFIG_PMIC_RK8XX=y
>  CONFIG_REGULATOR_RK8XX=y
>  CONFIG_PWM_ROCKCHIP=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_TPL_RAM_ROCKCHIP_DEBUG is not set
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYS_NS16550_MEM32=y
> diff --git a/configs/sonoff-ihost-rv1126_defconfig b/configs/sonoff-ihost-rv1126_defconfig
> index 4890644c7e6..1b10cded8fd 100644
> --- a/configs/sonoff-ihost-rv1126_defconfig
> +++ b/configs/sonoff-ihost-rv1126_defconfig
> @@ -47,6 +47,8 @@ CONFIG_MMC_DW_ROCKCHIP=y
>  CONFIG_REGULATOR_PWM=y
>  CONFIG_PWM_ROCKCHIP=y
>  # CONFIG_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_SPL_RAM_ROCKCHIP_DEBUG is not set
> +# CONFIG_TPL_RAM_ROCKCHIP_DEBUG is not set
>  CONFIG_BAUDRATE=1500000
>  CONFIG_DEBUG_UART_SHIFT=2
>  CONFIG_SYSRESET=y
> diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig
> index d707d09c1c8..5e069dca3be 100644
> --- a/drivers/ram/rockchip/Kconfig
> +++ b/drivers/ram/rockchip/Kconfig
> @@ -24,6 +24,30 @@ config RAM_ROCKCHIP_DEBUG
>    This is an option for developers to understand the ram drivers
>    initialization, configurations and etc.
>
> +config SPL_RAM_ROCKCHIP_DEBUG
> + bool "Rockchip ram drivers debugging in SPL"
> + depends on DEBUG_UART
> + depends on SPL && SPL_RAM && !SPL_SILENT_CONSOLE
> + default y
> + help
> +  This enables debugging ram driver API's for the platforms
> +  based on Rockchip SoCs.
> +
> +  This is an option for developers to understand the ram drivers
> +  initialization, configurations and etc.
> +
> +config TPL_RAM_ROCKCHIP_DEBUG
> + bool "Rockchip ram drivers debugging in TPL"
> + depends on DEBUG_UART
> + depends on TPL && TPL_RAM && !TPL_SILENT_CONSOLE
> + default y
> + help
> +  This enables debugging ram driver API's for the platforms
> +  based on Rockchip SoCs.
> +
> +  This is an option for developers to understand the ram drivers
> +  initialization, configurations and etc.
> +
>  config RAM_ROCKCHIP_DDR4
>   bool "DDR4 support for Rockchip SoCs"
>   help
> diff --git a/drivers/ram/rockchip/sdram_common.c b/drivers/ram/rockchip/sdram_common.c
> index b7a8fce607c..c2a6310388c 100644
> --- a/drivers/ram/rockchip/sdram_common.c
> +++ b/drivers/ram/rockchip/sdram_common.c
> @@ -10,7 +10,7 @@
>  #include <asm/arch-rockchip/sdram.h>
>  #include <asm/arch-rockchip/sdram_common.h>
>
> -#ifdef CONFIG_RAM_ROCKCHIP_DEBUG
> +#if CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)
>  void sdram_print_dram_type(unsigned char dramtype)
>  {
>   switch (dramtype) {
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 45270e27184..99a1cc6c174 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -2558,7 +2558,7 @@ static int lpddr4_set_rate(struct dram_info *dram,
>   lpddr4_set_ctl(dram, params, ctl_fn,
>         dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG))
>   printf("%s: change freq to %dMHz %d, %d\n", __func__,
>         dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
>         ctl_fn, phy_fn);
> @@ -2980,7 +2980,7 @@ static int sdram_init(struct dram_info *dram,
>   continue;
>   }
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG)) {
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)) {
>   printf("Channel ");
>   printf(channel ? "1: " : "0: ");
>   }
> diff --git a/drivers/ram/rockchip/sdram_rv1126.c b/drivers/ram/rockchip/sdram_rv1126.c
> index 4fbb088a8d9..14e1df71ae4 100644
> --- a/drivers/ram/rockchip/sdram_rv1126.c
> +++ b/drivers/ram/rockchip/sdram_rv1126.c
> @@ -3375,7 +3375,7 @@ static void ddr_set_rate_for_fsp(struct dram_info *dram,
>   if (get_wrlvl_val(dram, sdram_params))
>   printascii("get wrlvl value fail\n");
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG)) {
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)) {
>   printascii("change to: ");
>   printdec(f1);
>   printascii("MHz\n");
> @@ -3383,21 +3383,21 @@ static void ddr_set_rate_for_fsp(struct dram_info *dram,
>   ddr_set_rate(&dram_info, sdram_params, f1,
>       sdram_params->base.ddr_freq, 1, 1, 1);
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG)) {
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)) {
>   printascii("change to: ");
>   printdec(f2);
>   printascii("MHz\n");
>   }
>   ddr_set_rate(&dram_info, sdram_params, f2, f1, 2, 0, 1);
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG)) {
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)) {
>   printascii("change to: ");
>   printdec(f3);
>   printascii("MHz\n");
>   }
>   ddr_set_rate(&dram_info, sdram_params, f3, f2, 3, 1, 1);
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG)) {
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG)) {
>   printascii("change to: ");
>   printdec(f0);
>   printascii("MHz(final freq)\n");
> @@ -3493,7 +3493,7 @@ static int rv1126_dmc_init(struct udevice *dev)
>   save_rw_trn_result_to_ddr(&rw_trn_result);
>  #endif
>
> - if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> + if (CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG))
>   printascii("out\n");
>
>   return ret;


More information about the U-Boot mailing list