RE: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig

[Christoph Niedermaier <cniedermaier@dh-electronics.com>]

From: Tom Rini <trini@konsulko.com>
Sent: Tuesday, April 8, 2025 10:58 PM
> To: Christoph Niedermaier <cniedermaier@dh-electronics.com>
> Cc: u-boot@lists.denx.de; Simon Glass <sjg@chromium.org>; Michael Walle
> <mwalle@kernel.org>; Quentin Schulz <quentin.schulz@cherry.de>; Marek Vasut
> <marex@denx.de>; Benedikt Spranger <b.spranger@linutronix.de>; Jerome Forissier
> <jerome.forissier@linaro.org>; John Ogness <john.ogness@linutronix.de>; Ilias Apalodimas
> <ilias.apalodimas@linaro.org>
> Subject: Re: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
> 
> On Mon, Apr 07, 2025 at 10:56:14AM +0200, Christoph Niedermaier wrote:
> 
>> The formatting with %pa / %pap behaves like %x, which results in an
>> incorrect value being output. To improve this, a new fine-tuning
>> Kconfig XPL_USE_TINY_PRINTF_POINTER_SUPPORT for pointer formatting
>> has been added. If it is enabled, the output of %pa / %pap should
>> be correct, and if it is disabled, the pointer formatting is
>> completely unsupported. In addition to indicate unsupported formatting,
>> '?' will be output. This allows enabling pointer formatting only
>> when needed. For SPL_NET and NET_LWIP it is selected by default.
>> Then it also supports the formatting with %pm, %pM and %pI4.
>>
>> Signed-off-by: Christoph Niedermaier <cniedermaier@dh-electronics.com>
>> ---
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Michael Walle <mwalle@kernel.org>
>> Cc: Quentin Schulz <quentin.schulz@cherry.de>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Benedikt Spranger <b.spranger@linutronix.de>
>> Cc: Jerome Forissier <jerome.forissier@linaro.org>
>> Cc: John Ogness <john.ogness@linutronix.de>
>> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> ---
>>  Kconfig            |  1 +
>>  common/spl/Kconfig |  1 +
>>  lib/Kconfig        |  8 ++++++++
>>  lib/tiny-printf.c  | 45 +++++++++++++++++++--------------------------
>>  4 files changed, 29 insertions(+), 26 deletions(-)
>>
>> diff --git a/Kconfig b/Kconfig
>> index 6379a454166..4d13717294c 100644
>> --- a/Kconfig
>> +++ b/Kconfig
>> @@ -757,6 +757,7 @@ config NET
>>
>>  config NET_LWIP
>>  	bool "Use lwIP for networking stack"
>> +	select XPL_USE_TINY_PRINTF_POINTER_SUPPORT if SPL_USE_TINY_PRINTF || TPL_USE_TINY_PRINTF || VPL_USE_TINY_PRINTF
>>  	imply NETDEVICES
>>  	help
>>  	  Include networking support based on the lwIP (lightweight IP)
>> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
>> index 94e118f8465..72736dbecf5 100644
>> --- a/common/spl/Kconfig
>> +++ b/common/spl/Kconfig
>> @@ -1096,6 +1096,7 @@ config SPL_DM_SPI_FLASH
>>  config SPL_NET
>>  	bool "Support networking"
>>  	depends on !NET_LWIP
>> +	select XPL_USE_TINY_PRINTF_POINTER_SUPPORT if SPL_USE_TINY_PRINTF || TPL_USE_TINY_PRINTF || VPL_USE_TINY_PRINTF
>>  	help
>>  	  Enable support for network devices (such as Ethernet) in SPL.
>>  	  This permits SPL to load U-Boot over a network link rather than
>> diff --git a/lib/Kconfig b/lib/Kconfig
>> index 1a683dea670..62e28d4a1f3 100644
>> --- a/lib/Kconfig
>> +++ b/lib/Kconfig
>> @@ -253,6 +253,14 @@ config VPL_USE_TINY_PRINTF
>>
>>  	  The supported format specifiers are %c, %s, %u/%d and %x.
>>
>> +config XPL_USE_TINY_PRINTF_POINTER_SUPPORT
>> +	bool "Extend tiny printf with the pointer formatting %p"
>> +	depends on SPL_USE_TINY_PRINTF || TPL_USE_TINY_PRINTF || VPL_USE_TINY_PRINTF
>> +	help
>> +	  This option enables the formatting of pointers %p. It supports
>> +	  %p and %pa / %pap. If this option is selected by SPL_NET or NET_LWIP
>> +	  it also supports the formatting with %pm, %pM and %pI4.
> 
> This isn't quite what I'd like to see. I don't want to start using the
> literal XPL namespace as that will lead to confusion down the line.

OK, in V2 I will only support SPL.

> Since we only have SPL_NET, I think we should name this symbol
> SPL_USE_TINY_PRINTF_POINTER_SUPPORT, not ask about it (so bool without
> "prompt text" following), and select from SPL_NET if
> SPL_USE_TINY_PRINTF.

Now you will get the output '?' when using formatting with %p or %pa.
If someone wants to use the pointer support e.g. %pa in pinctrl-single.c
and is restricted to use tiny printf, then it would be good to have
the option to enable it manually and not be forced to enable SPL_NET or
NET_LWIP to have the pointer support enabled. In this case, it makes
sense to allow switching it on in menuconfig.

Regards
Christoph