* [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
@ 2025-04-07 8:56 Christoph Niedermaier
2025-04-08 20:58 ` Tom Rini
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Niedermaier @ 2025-04-07 8:56 UTC (permalink / raw)
To: u-boot
Cc: Christoph Niedermaier, Tom Rini, Simon Glass, Michael Walle,
Quentin Schulz, Marek Vasut, Benedikt Spranger, Jerome Forissier,
John Ogness, Ilias Apalodimas
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.
+
config PANIC_HANG
bool "Do not reset the system on fatal error"
help
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index 0503c17341f..10db812567a 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -47,7 +47,7 @@ static void div_out(struct printf_info *info, unsigned long *num,
out_dgt(info, dgt);
}
-#ifdef CONFIG_SPL_NET
+#if defined(CONFIG_SPL_NET) || defined(CONFIG_NET_LWIP)
static void string(struct printf_info *info, char *s)
{
char ch;
@@ -141,7 +141,7 @@ static void ip4_addr_string(struct printf_info *info, u8 *addr)
string(info, ip4_addr);
}
-#endif
+#endif /* defined(CONFIG_SPL_NET) || defined(CONFIG_NET_LWIP) */
/*
* Show a '%p' thing. A kernel extension is that the '%p' is followed
@@ -157,18 +157,14 @@ static void ip4_addr_string(struct printf_info *info, u8 *addr)
* decimal).
*/
-static void __maybe_unused pointer(struct printf_info *info, const char *fmt,
- void *ptr)
+#if defined(CONFIG_XPL_USE_TINY_PRINTF_POINTER_SUPPORT) || _DEBUG
+static void pointer(struct printf_info *info, const char *fmt, void *ptr)
{
-#ifdef DEBUG
unsigned long num = (uintptr_t)ptr;
unsigned long div;
-#endif
switch (*fmt) {
-#ifdef DEBUG
case 'a':
-
switch (fmt[1]) {
case 'p':
default:
@@ -176,8 +172,7 @@ static void __maybe_unused pointer(struct printf_info *info, const char *fmt,
break;
}
break;
-#endif
-#ifdef CONFIG_SPL_NET
+#if defined(CONFIG_SPL_NET) || defined(CONFIG_NET_LWIP)
case 'm':
return mac_address_string(info, ptr, false);
case 'M':
@@ -189,12 +184,12 @@ static void __maybe_unused pointer(struct printf_info *info, const char *fmt,
default:
break;
}
-#ifdef DEBUG
+
div = 1UL << (sizeof(long) * 8 - 4);
for (; div; div /= 0x10)
div_out(info, &num, div);
-#endif
}
+#endif
static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
{
@@ -268,21 +263,18 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
div_out(info, &num, div);
}
break;
+#if defined(CONFIG_XPL_USE_TINY_PRINTF_POINTER_SUPPORT) || _DEBUG
case 'p':
- if (CONFIG_IS_ENABLED(NET) ||
- CONFIG_IS_ENABLED(NET_LWIP) || _DEBUG) {
- pointer(info, fmt, va_arg(va, void *));
- /*
- * Skip this because it pulls in _ctype which is
- * 256 bytes, and we don't generally implement
- * pointer anyway
- */
- while (isalnum(fmt[0]))
- fmt++;
- break;
- }
- islong = true;
- /* no break */
+ pointer(info, fmt, va_arg(va, void *));
+ /*
+ * Skip this because it pulls in _ctype which is
+ * 256 bytes, and we don't generally implement
+ * pointer anyway
+ */
+ while (isalnum(fmt[0]))
+ fmt++;
+ break;
+#endif
case 'x':
if (islong) {
num = va_arg(va, unsigned long);
@@ -307,6 +299,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
case '%':
out(info, '%');
default:
+ out(info, '?');
break;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-07 8:56 [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig Christoph Niedermaier
@ 2025-04-08 20:58 ` Tom Rini
2025-04-09 11:44 ` Christoph Niedermaier
0 siblings, 1 reply; 8+ messages in thread
From: Tom Rini @ 2025-04-08 20:58 UTC (permalink / raw)
To: Christoph Niedermaier
Cc: u-boot, Simon Glass, Michael Walle, Quentin Schulz, Marek Vasut,
Benedikt Spranger, Jerome Forissier, John Ogness,
Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 3421 bytes --]
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.
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.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-08 20:58 ` Tom Rini
@ 2025-04-09 11:44 ` Christoph Niedermaier
2025-04-09 12:33 ` Michael Walle
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Niedermaier @ 2025-04-09 11:44 UTC (permalink / raw)
To: Tom Rini
Cc: u-boot@lists.denx.de, Simon Glass, Michael Walle, Quentin Schulz,
Marek Vasut, Benedikt Spranger, Jerome Forissier, John Ogness,
Ilias Apalodimas
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-09 11:44 ` Christoph Niedermaier
@ 2025-04-09 12:33 ` Michael Walle
2025-04-09 15:22 ` Tom Rini
0 siblings, 1 reply; 8+ messages in thread
From: Michael Walle @ 2025-04-09 12:33 UTC (permalink / raw)
To: Christoph Niedermaier, Tom Rini
Cc: u-boot@lists.denx.de, Simon Glass, Quentin Schulz, Marek Vasut,
Benedikt Spranger, Jerome Forissier, John Ogness,
Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 4271 bytes --]
Hi,
> >> 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.
IIRC, the old one also enabled the pointer support if DEBUG is
enabled. I don't think this will work with Kconfig.
> 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.
FWIW, I'm also fine with enabling full printf support as long as the
tiny one doesn't print misleading values.
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-09 12:33 ` Michael Walle
@ 2025-04-09 15:22 ` Tom Rini
2025-04-10 10:44 ` Michael Walle
0 siblings, 1 reply; 8+ messages in thread
From: Tom Rini @ 2025-04-09 15:22 UTC (permalink / raw)
To: Michael Walle
Cc: Christoph Niedermaier, u-boot@lists.denx.de, Simon Glass,
Quentin Schulz, Marek Vasut, Benedikt Spranger, Jerome Forissier,
John Ogness, Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 4914 bytes --]
On Wed, Apr 09, 2025 at 02:33:08PM +0200, Michael Walle wrote:
> Hi,
>
> > >> 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.
>
> IIRC, the old one also enabled the pointer support if DEBUG is
> enabled. I don't think this will work with Kconfig.
I was looking around for, but didn't quite see, a good existing option
to "if .." around the prompt text for.
> > 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.
>
> FWIW, I'm also fine with enabling full printf support as long as the
> tiny one doesn't print misleading values.
I'm not sure if the one non-debug %pa print in pinctrl-single.c is
really triggered within SPL, and I do hope that the way this patch is
otherwise done will make it easier if someone needs %pa to work when
debugging a problem in SPL, and can't enable full printf due to space.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-09 15:22 ` Tom Rini
@ 2025-04-10 10:44 ` Michael Walle
2025-04-10 12:16 ` Christoph Niedermaier
0 siblings, 1 reply; 8+ messages in thread
From: Michael Walle @ 2025-04-10 10:44 UTC (permalink / raw)
To: Tom Rini
Cc: Christoph Niedermaier, u-boot@lists.denx.de, Simon Glass,
Quentin Schulz, Marek Vasut, Benedikt Spranger, Jerome Forissier,
John Ogness, Ilias Apalodimas
[-- Attachment #1: Type: text/plain, Size: 5554 bytes --]
On Wed Apr 9, 2025 at 5:22 PM CEST, Tom Rini wrote:
> On Wed, Apr 09, 2025 at 02:33:08PM +0200, Michael Walle wrote:
> > Hi,
> >
> > > >> 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.
> >
> > IIRC, the old one also enabled the pointer support if DEBUG is
> > enabled. I don't think this will work with Kconfig.
>
> I was looking around for, but didn't quite see, a good existing option
> to "if .." around the prompt text for.
What I meant was that you normally do -DDEBUG on a file (or
equally a #define DEBUG). Thus you cannot add it to Kconfig.
> > > 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.
> >
> > FWIW, I'm also fine with enabling full printf support as long as the
> > tiny one doesn't print misleading values.
>
> I'm not sure if the one non-debug %pa print in pinctrl-single.c is
> really triggered within SPL, and I do hope that the way this patch is
> otherwise done will make it easier if someone needs %pa to work when
> debugging a problem in SPL, and can't enable full printf due to space.
Yes, but the dev_dbg() triggered it because of the same reason as
above, DEBUG isn't defined if you do it on a per-file basis. Or I'm
getting that logic wrong, as I don't understand why _DEBUG (commit
c091f65234cf introduced it but doesn't tell the reason).
-michael
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-10 10:44 ` Michael Walle
@ 2025-04-10 12:16 ` Christoph Niedermaier
2025-04-29 14:45 ` Christoph Niedermaier
0 siblings, 1 reply; 8+ messages in thread
From: Christoph Niedermaier @ 2025-04-10 12:16 UTC (permalink / raw)
To: Michael Walle, Tom Rini
Cc: u-boot@lists.denx.de, Simon Glass, Quentin Schulz, Marek Vasut,
Benedikt Spranger, Jerome Forissier, John Ogness,
Ilias Apalodimas
From: Michael Walle <mwalle@kernel.org>
Sent: Thursday, April 10, 2025 12:44 PM
> On Wed Apr 9, 2025 at 5:22 PM CEST, Tom Rini wrote:
> > On Wed, Apr 09, 2025 at 02:33:08PM +0200, Michael Walle wrote:
> > > Hi,
> > >
> > > > >> 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.
> > >
> > > IIRC, the old one also enabled the pointer support if DEBUG is
> > > enabled. I don't think this will work with Kconfig.
> >
> > I was looking around for, but didn't quite see, a good existing option
> > to "if .." around the prompt text for.
>
> What I meant was that you normally do -DDEBUG on a file (or
> equally a #define DEBUG). Thus you cannot add it to Kconfig.
>
> > > > 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.
> > >
> > > FWIW, I'm also fine with enabling full printf support as long as the
> > > tiny one doesn't print misleading values.
> >
> > I'm not sure if the one non-debug %pa print in pinctrl-single.c is
> > really triggered within SPL, and I do hope that the way this patch is
> > otherwise done will make it easier if someone needs %pa to work when
> > debugging a problem in SPL, and can't enable full printf due to space.
>
> Yes, but the dev_dbg() triggered it because of the same reason as
> above, DEBUG isn't defined if you do it on a per-file basis. Or I'm
> getting that logic wrong, as I don't understand why _DEBUG (commit
> c091f65234cf introduced it but doesn't tell the reason).
Should I replace _DEBUG with defined(DEBUG) in the next version?
I also found something else:
[...]
@@ -307,6 +299,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
case '%':
out(info, '%');
default:
+ out(info, '?');
break;
}
[...]
If I add "out(info, '?');" in the default case for case '%' a break is missing.
So for "%%" the output will be "%?".
Regards
Christoph
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig
2025-04-10 12:16 ` Christoph Niedermaier
@ 2025-04-29 14:45 ` Christoph Niedermaier
0 siblings, 0 replies; 8+ messages in thread
From: Christoph Niedermaier @ 2025-04-29 14:45 UTC (permalink / raw)
To: Michael Walle, Tom Rini
Cc: u-boot@lists.denx.de, Simon Glass, Quentin Schulz, Marek Vasut,
Benedikt Spranger, Jerome Forissier, John Ogness,
Ilias Apalodimas
From: Christoph Niedermaier
Sent: Thursday, April 10, 2025 2:17 PM
> From: Michael Walle <mwalle@kernel.org>
> Sent: Thursday, April 10, 2025 12:44 PM
> > On Wed Apr 9, 2025 at 5:22 PM CEST, Tom Rini wrote:
> > > On Wed, Apr 09, 2025 at 02:33:08PM +0200, Michael Walle wrote:
> > > > Hi,
> > > >
> > > > > >> 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.
> > > >
> > > > IIRC, the old one also enabled the pointer support if DEBUG is
> > > > enabled. I don't think this will work with Kconfig.
> > >
> > > I was looking around for, but didn't quite see, a good existing option
> > > to "if .." around the prompt text for.
> >
> > What I meant was that you normally do -DDEBUG on a file (or
> > equally a #define DEBUG). Thus you cannot add it to Kconfig.
> >
> > > > > 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.
> > > >
> > > > FWIW, I'm also fine with enabling full printf support as long as the
> > > > tiny one doesn't print misleading values.
> > >
> > > I'm not sure if the one non-debug %pa print in pinctrl-single.c is
> > > really triggered within SPL, and I do hope that the way this patch is
> > > otherwise done will make it easier if someone needs %pa to work when
> > > debugging a problem in SPL, and can't enable full printf due to space.
> >
> > Yes, but the dev_dbg() triggered it because of the same reason as
> > above, DEBUG isn't defined if you do it on a per-file basis. Or I'm
> > getting that logic wrong, as I don't understand why _DEBUG (commit
> > c091f65234cf introduced it but doesn't tell the reason).
>
> Should I replace _DEBUG with defined(DEBUG) in the next version?
>
> I also found something else:
> [...]
> @@ -307,6 +299,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list
> va)
> case '%':
> out(info, '%');
> default:
> + out(info, '?');
> break;
> }
> [...]
> If I add "out(info, '?');" in the default case for case '%' a break is missing.
> So for "%%" the output will be "%?".
I will send a version 2 as a new basis.
Regards
Christoph
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-04-29 14:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 8:56 [PATCH] tiny-printf: Handle formatting of %p with an extra Kconfig Christoph Niedermaier
2025-04-08 20:58 ` Tom Rini
2025-04-09 11:44 ` Christoph Niedermaier
2025-04-09 12:33 ` Michael Walle
2025-04-09 15:22 ` Tom Rini
2025-04-10 10:44 ` Michael Walle
2025-04-10 12:16 ` Christoph Niedermaier
2025-04-29 14:45 ` Christoph Niedermaier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox