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

2024-04-17 Thread Łukasz Czechowski
Hi Quentin,

Thanks for the hints. Matching the parent ifdef with non-PX30 TPL
implementation is indeed a good idea. I'll send the updated version of
the patch.

Best regards,
Lukasz

wt., 16 kwi 2024 o 15:22 Quentin Schulz
 napisał(a):
>
> Hi Lukasz,
>
> Please use scripts/get_maintainer.pl to set the Cc and To recipients of
> your mail to make sure it reaches the appropriate people explicitly.
>
> $ scripts/get_maintainer.pl arch/arm/mach-rockchip/px30-board-tpl.c
> Tom Rini  (maintainer:ARM)
> Simon Glass  (maintainer:ARM ROCKCHIP)
> Philipp Tomsich  (maintainer:ARM ROCKCHIP)
> Kever Yang  (maintainer:ARM ROCKCHIP)
> u-boot@lists.denx.de (open list)
>
> (one can use scripts/get_maintainer.pl on patches instead of files, and
> it'll return whatever is needed).
>
> Plugging `b4` here as well, because it's a pretty nice tool to use:
> https://git.kernel.org/pub/scm/utils/b4/b4.git/ One can install it with
> pip. I use it for Linux kernel and U-Boot contributions for a couple of
> years now and I'm not going back to manual workflow :)
>
> b4 prep --auto-to-cc would set everything up properly for you.
>
> On 4/16/24 14:47, Lukasz Czechowski wrote:
> > 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.
> >
> > Signed-off-by: Lukasz Czechowski 
>
> This matches Rockchip's non-PX30 TPL, so:
>
> Reviewed-by: Quentin Schulz 
>
> > ---
> >   arch/arm/mach-rockchip/px30-board-tpl.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
> > b/arch/arm/mach-rockchip/px30-board-tpl.c
> > index 637a5e1b18..a660816db0 100644
> > --- a/arch/arm/mach-rockchip/px30-board-tpl.c
> > +++ b/arch/arm/mach-rockchip/px30-board-tpl.c
> > @@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
> >* printhex8(0x1234);
> >* printascii("string");
> >*/
> > +#if CONFIG_TPL_BANNER_PRINT
> >   printascii("U-Boot TPL board init\n");
> > +#endif
>
> I'm wondering if we shouldn't have the parent ifdef also match the logic
> of the non-PX30 TPL?
>
> #if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)
>
> instead of
>
> #if defined(CONFIG_DEBUG_UART)
>
> ?
>
> Thanks,
> Quentin


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

2024-04-16 Thread Quentin Schulz

Hi Lukasz,

Please use scripts/get_maintainer.pl to set the Cc and To recipients of 
your mail to make sure it reaches the appropriate people explicitly.


$ scripts/get_maintainer.pl arch/arm/mach-rockchip/px30-board-tpl.c
Tom Rini  (maintainer:ARM)
Simon Glass  (maintainer:ARM ROCKCHIP)
Philipp Tomsich  (maintainer:ARM ROCKCHIP)
Kever Yang  (maintainer:ARM ROCKCHIP)
u-boot@lists.denx.de (open list)

(one can use scripts/get_maintainer.pl on patches instead of files, and 
it'll return whatever is needed).


Plugging `b4` here as well, because it's a pretty nice tool to use:
https://git.kernel.org/pub/scm/utils/b4/b4.git/ One can install it with 
pip. I use it for Linux kernel and U-Boot contributions for a couple of 
years now and I'm not going back to manual workflow :)


b4 prep --auto-to-cc would set everything up properly for you.

On 4/16/24 14:47, Lukasz Czechowski wrote:

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.

Signed-off-by: Lukasz Czechowski 


This matches Rockchip's non-PX30 TPL, so:

Reviewed-by: Quentin Schulz 


---
  arch/arm/mach-rockchip/px30-board-tpl.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..a660816db0 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
 * printhex8(0x1234);
 * printascii("string");
 */
+#if CONFIG_TPL_BANNER_PRINT
printascii("U-Boot TPL board init\n");
+#endif


I'm wondering if we shouldn't have the parent ifdef also match the logic 
of the non-PX30 TPL?


#if defined(CONFIG_DEBUG_UART) && defined(CONFIG_TPL_SERIAL)

instead of

#if defined(CONFIG_DEBUG_UART)

?

Thanks,
Quentin


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

2024-04-16 Thread Lukasz Czechowski
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.

Signed-off-by: Lukasz Czechowski 
---
 arch/arm/mach-rockchip/px30-board-tpl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-rockchip/px30-board-tpl.c 
b/arch/arm/mach-rockchip/px30-board-tpl.c
index 637a5e1b18..a660816db0 100644
--- a/arch/arm/mach-rockchip/px30-board-tpl.c
+++ b/arch/arm/mach-rockchip/px30-board-tpl.c
@@ -46,7 +46,9 @@ void board_init_f(ulong dummy)
 * printhex8(0x1234);
 * printascii("string");
 */
+#if CONFIG_TPL_BANNER_PRINT
printascii("U-Boot TPL board init\n");
+#endif
 #endif
 
secure_timer_init();
-- 
2.34.1