[PATCH v2] rockchip: px30-board-tpl: Use CONFIG_TPL_BANNER_PRINT flag

[Quentin Schulz]

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.czechowski at thaumatec.com wrote:
> From: Lukasz Czechowski <lukasz.czechowski at 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.czechowski at thaumatec.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Philipp Tomsich <philipp.tomsich at vrull.eu>
> Cc: Kever Yang <kever.yang at 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.schulz at theobroma-systems.com>

Thanks,
Quentin