Hi Lukasz,
I would have renamed the commit title to be something like
rockchip: px30-board-tpl: sync ifdef guards with full TPL
because CONFIG_TPL_BANNER_PRINT isn't really a flag but a Kconfig symbol
and the effect of this patch is to not print the banner when the symbol
is not selected.
Another option could have been (only for the v1, since you do more now):
rockchip: px30-board-tpl: print banner only if CONFIG_TPL_BANNER_PRINT
On 4/17/24 11:35, lukasz.czechow...@thaumatec.com wrote:
From: Lukasz Czechowski <lukasz.czechow...@thaumatec.com>
Display TPL init information message only when TPL_BANNER_PRINT
configuration entry is set. This allows to disable information
message in case logs on UART are unwanted.
Update parent ifdef condition to check also CONFIG_TPL_SERIAL
to match logic of the non-PX30 TPL implementation.
Signed-off-by: Lukasz Czechowski <lukasz.czechow...@thaumatec.com>
Cc: Tom Rini <tr...@konsulko.com>
Cc: Simon Glass <s...@chromium.org>
Cc: Philipp Tomsich <philipp.toms...@vrull.eu>
Cc: Kever Yang <kever.y...@rock-chips.com>
Small nitpick, the Cc would make it to the git history, which isn't
really necessary. I believe you could have those below the --- and
git-send-email still find them.
Additionally, git send-email allows you to provide --cc and --to manually.
---
Changes for v2:
- Updated parent ifdef condition
---
arch/arm/mach-rockchip/px30-board-tpl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..db368a7b8c 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -36,7 +36,7 @@ void board_init_f(ulong dummy)
{
int ret;
-#ifdef CONFIG_DEBUG_UART
+#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
Quite interestingly, CONFIG_TPL_SERIAL cannot be disabled on PX30, maybe
we should change arch/arm/mach-rockchip/Kconfig to use an `imply`
instead of `select`.
Anyway, no change required on my side. Only, if there's a v3 (for the Cc
and commit title, the Kconfig change would be a separate patch anyway).
Reviewed-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>
Thanks,
Quentin
