Message ID | 20240827142058.594556-3-lukasz.czechowski@thaumatec.com |
---|---|
State | Superseded |
Delegated to: | Kever Yang |
Headers | show |
Series | Rockchip: Allow to silent TPL/SPL debug console | expand |
Hi Lukasz, On 8/27/24 4:20 PM, Lukasz Czechowski wrote: > The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is > available, otherwise it won't have any effect. > Add negative dependency to TPL_SILENT_CONSOLE. > > Signed-off-by: Lukasz Czechowski <lukasz.czechowski@thaumatec.com> > --- > drivers/ram/rockchip/Kconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig > index 67c63ecba04..4a6a6328676 100644 > --- a/drivers/ram/rockchip/Kconfig > +++ b/drivers/ram/rockchip/Kconfig > @@ -15,6 +15,8 @@ if RAM_ROCKCHIP > > config RAM_ROCKCHIP_DEBUG > bool "Rockchip ram drivers debugging" > + depends on DEBUG_UART > + depends on !TPL_SILENT_CONSOLE I am wondering if we should also do this for TPL-less systems? i.e. the ones for which the dram is configured in SPL. Something like: depends on !SPL_SILENT_CONSOLE if SPL_RAM depends on !TPL_SILENT_CONSOLE if TPL_RAM maybe? Cheers, Quentin
Hi Quentin, Fair point. Unfortunately the syntax "depends on <symbol> if <expr>" seem to be not supported in Kconfig I can do something like depends on !TPL_SILENT_CONSOLE && TPL_RAM || !SPL_SILENT_CONSOLE && SPL_RAM instead. What do you think? Best regards, Łukasz czw., 29 sie 2024 o 12:19 Quentin Schulz <quentin.schulz@cherry.de> napisał(a): > > Hi Lukasz, > > On 8/27/24 4:20 PM, Lukasz Czechowski wrote: > > The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is > > available, otherwise it won't have any effect. > > Add negative dependency to TPL_SILENT_CONSOLE. > > > > Signed-off-by: Lukasz Czechowski <lukasz.czechowski@thaumatec.com> > > --- > > drivers/ram/rockchip/Kconfig | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig > > index 67c63ecba04..4a6a6328676 100644 > > --- a/drivers/ram/rockchip/Kconfig > > +++ b/drivers/ram/rockchip/Kconfig > > @@ -15,6 +15,8 @@ if RAM_ROCKCHIP > > > > config RAM_ROCKCHIP_DEBUG > > bool "Rockchip ram drivers debugging" > > + depends on DEBUG_UART > > + depends on !TPL_SILENT_CONSOLE > > I am wondering if we should also do this for TPL-less systems? i.e. the > ones for which the dram is configured in SPL. > > Something like: > > depends on !SPL_SILENT_CONSOLE if SPL_RAM > depends on !TPL_SILENT_CONSOLE if TPL_RAM > > maybe? > > Cheers, > Quentin
Hi Lukasz, On 9/2/24 4:57 PM, Łukasz Czechowski wrote: > Hi Quentin, > > Fair point. Unfortunately the syntax "depends on <symbol> if <expr>" > seem to be not supported in Kconfig Shoot, it's the ONLY Kconfig attribute that doesn't support if :) > I can do something like > > depends on !TPL_SILENT_CONSOLE && TPL_RAM || !SPL_SILENT_CONSOLE && SPL_RAM > > instead. What do you think? > Mmmm no, that wouldn't work I think. E.g. this would allow RAM_ROCKCHIP_DEBUG to be enabled in TPL even if TPL_SILENT_CONSOLE is enabled, if SPL_RAM=y and SPL_SILENT_CONSOLE=n. I think we have two options: 1) depends on (!SPL_SILENT_CONSOLE && SPL_RAM) || !SPL_RAM depends on (!TPL_SILENT_CONSOLE && TPL_RAM) || !TPL_RAM This means RAM_ROCKCHIP_DEBUG would be selectable for RAM=y, TPL_RAM=n, SPL_RAM=n. I don't think anyone would want TPL_RAM AND SPL_RAM set at the same time, so having SPL_SILENT_CONSOLE set but not TPL_SILENT_CONSOLE wouldn't be an issue here. 2) Create an SPL and TPL symbol for RAM_ROCKCHIP_DEBUG, i.e. SPL_RAM_ROCKCHIP_DEBUG and TPL_RAM_ROCKCHIP_DEBUG. This is a bit more involved in terms of changes but is closer to how U-Boot works when SPL/TPL is involved. We basically would need to change IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG) into CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG) and #ifdef CONFIG_RAM_ROCKCHIP_DEBUG into #if CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG). I assume we would want those new symbols to default to y too. If that's the case, then we need to manually disable those symbols in defconfigs that disable RAM_ROCMCHIP_DEBUG right now: configs/anbernic-rgxx3-rk3566_defconfig configs/neu2-io-rv1126_defconfig configs/roc-pc-mezzanine-rk3399_defconfig configs/roc-pc-rk3399_defconfig configs/rock-pi-n10-rk3399pro_defconfig configs/rock-pi-n8-rk3288_defconfig configs/sonoff-ihost-rv1126_defconfig Cheers, Quentin
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig index 67c63ecba04..4a6a6328676 100644 --- a/drivers/ram/rockchip/Kconfig +++ b/drivers/ram/rockchip/Kconfig @@ -15,6 +15,8 @@ if RAM_ROCKCHIP config RAM_ROCKCHIP_DEBUG bool "Rockchip ram drivers debugging" + depends on DEBUG_UART + depends on !TPL_SILENT_CONSOLE default y help This enables debugging ram driver API's for the platforms
The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is available, otherwise it won't have any effect. Add negative dependency to TPL_SILENT_CONSOLE. Signed-off-by: Lukasz Czechowski <lukasz.czechowski@thaumatec.com> --- drivers/ram/rockchip/Kconfig | 2 ++ 1 file changed, 2 insertions(+)