public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT
@ 2025-11-05 16:21 Anshul Dalal
  2025-11-05 18:09 ` Andrew Davis
  0 siblings, 1 reply; 4+ messages in thread
From: Anshul Dalal @ 2025-11-05 16:21 UTC (permalink / raw)
  To: u-boot
  Cc: Tom Rini, Andrew Davis, Judith Mendez, Udit Kumar,
	Hrushikesh Salunke, Neha Malcom Francis, Vignesh R,
	Christoph Niedermaier, Anshul Dalal

Since the commit 1e24e84db41a ("tiny-printf: Handle formatting of %p
with an extra Kconfig"), SPL_USE_TINY_PRINTF_POINTER_SUPPORT has been
made mandatory in order to use %p which would earlier have defaulted to
a 'long' print.

Without this config symbol, k3_sysfw_dfu_download fails to set the
correct value for the DFU string with:

 sprintf(dfu_str, "sysfw.itb ram 0x%p 0x%x", addr,
   CONFIG_K3_SYSFW_IMAGE_SIZE_MAX);

The value we get "sysfw.itb ram 0x? 0x41c29d40" causes a boot failure.

Therefore this patch sets SPL_USE_TINY_PRINTF_POINTER_SUPPORT for the
usbdfu defconfig.

Signed-off-by: Anshul Dalal <anshuld@ti.com>
---
Not sure if a 'Fixes' tag is appropriate here since the commit
1e24e84db41a ("tiny-printf: Handle formatting of %p with an extra
Kconfig") did cause a regression though I think the problem existed with
the usage of "%p" with TINY_PRINTF set to begin with.
---
Changes in v2:
- Remove unrelated changes
- Link to v1: https://lore.kernel.org/r/20251105-am65_dfu_fix-v1-1-acc29111dccf@ti.com
---
 configs/am65x_evm_r5_usbdfu_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/am65x_evm_r5_usbdfu_defconfig b/configs/am65x_evm_r5_usbdfu_defconfig
index f9161f1fe111ff97f7aa79f10b21c3b0ab1dad32..add03dce002b2abd913770cd836d2cd8c852ed5e 100644
--- a/configs/am65x_evm_r5_usbdfu_defconfig
+++ b/configs/am65x_evm_r5_usbdfu_defconfig
@@ -6,6 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x57000
 CONFIG_SPL_GPIO=y
 CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_SPL_USE_TINY_PRINTF_POINTER_SUPPORT=y
 CONFIG_SOC_K3_AM654=y
 CONFIG_K3_EARLY_CONS=y
 CONFIG_TARGET_AM654_R5_EVM=y

---
base-commit: 1c250e444ad3b15315ee8b0fcb3fc3acc26449e2
change-id: 20251105-am65_dfu_fix-1144eec06b19

Best regards,
-- 
Anshul Dalal <anshuld@ti.com>


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT
  2025-11-05 16:21 [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT Anshul Dalal
@ 2025-11-05 18:09 ` Andrew Davis
  2025-12-04  9:35   ` Anshul Dalal
  0 siblings, 1 reply; 4+ messages in thread
From: Andrew Davis @ 2025-11-05 18:09 UTC (permalink / raw)
  To: Anshul Dalal, u-boot
  Cc: Tom Rini, Judith Mendez, Udit Kumar, Hrushikesh Salunke,
	Neha Malcom Francis, Vignesh R, Christoph Niedermaier

On 11/5/25 10:21 AM, Anshul Dalal wrote:
> Since the commit 1e24e84db41a ("tiny-printf: Handle formatting of %p
> with an extra Kconfig"), SPL_USE_TINY_PRINTF_POINTER_SUPPORT has been
> made mandatory in order to use %p which would earlier have defaulted to
> a 'long' print.
> 
> Without this config symbol, k3_sysfw_dfu_download fails to set the
> correct value for the DFU string with:
> 
>   sprintf(dfu_str, "sysfw.itb ram 0x%p 0x%x", addr,
>     CONFIG_K3_SYSFW_IMAGE_SIZE_MAX);
> 
> The value we get "sysfw.itb ram 0x? 0x41c29d40" causes a boot failure.
> 
> Therefore this patch sets SPL_USE_TINY_PRINTF_POINTER_SUPPORT for the
> usbdfu defconfig.
> 
> Signed-off-by: Anshul Dalal <anshuld@ti.com>
> ---
> Not sure if a 'Fixes' tag is appropriate here since the commit
> 1e24e84db41a ("tiny-printf: Handle formatting of %p with an extra
> Kconfig") did cause a regression though I think the problem existed with
> the usage of "%p" with TINY_PRINTF set to begin with.

So I do think this fix is valid, but it would be nice to not have to fix
this in each defconfig that might fall into this trap. Instead is there
some way to bake this into the Kconfig logic so that when K3, DFU, and
USE_TINY_PRINTF are set, the SPL_USE_TINY_PRINTF_POINTER_SUPPORT symbol
is also set automatically?

Or better yet, since this looks to be specific to k3_sysfw_dfu_download()
using 0x%p in a place where USE_TINY might be enabled we could just switch
to using 0x%x with a cast there.

Although the best solution IMHO would be to fix 1e24e84db41a which
doesn't seem right to me. We used to have it so TINY_PRINTF would
simply fall though with %p and use the %x handling. The commit
does point out that %pa / %pap would be wrong but it doesn't actually
fix those anyway, just removes the ability for %p to work without
that new flag..

Andrew

> ---
> Changes in v2:
> - Remove unrelated changes
> - Link to v1: https://lore.kernel.org/r/20251105-am65_dfu_fix-v1-1-acc29111dccf@ti.com
> ---
>   configs/am65x_evm_r5_usbdfu_defconfig | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/configs/am65x_evm_r5_usbdfu_defconfig b/configs/am65x_evm_r5_usbdfu_defconfig
> index f9161f1fe111ff97f7aa79f10b21c3b0ab1dad32..add03dce002b2abd913770cd836d2cd8c852ed5e 100644
> --- a/configs/am65x_evm_r5_usbdfu_defconfig
> +++ b/configs/am65x_evm_r5_usbdfu_defconfig
> @@ -6,6 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x57000
>   CONFIG_SPL_GPIO=y
>   CONFIG_SPL_LIBCOMMON_SUPPORT=y
>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
> +CONFIG_SPL_USE_TINY_PRINTF_POINTER_SUPPORT=y
>   CONFIG_SOC_K3_AM654=y
>   CONFIG_K3_EARLY_CONS=y
>   CONFIG_TARGET_AM654_R5_EVM=y
> 
> ---
> base-commit: 1c250e444ad3b15315ee8b0fcb3fc3acc26449e2
> change-id: 20251105-am65_dfu_fix-1144eec06b19
> 
> Best regards,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT
  2025-11-05 18:09 ` Andrew Davis
@ 2025-12-04  9:35   ` Anshul Dalal
  2025-12-04 10:31     ` Christoph Niedermaier
  0 siblings, 1 reply; 4+ messages in thread
From: Anshul Dalal @ 2025-12-04  9:35 UTC (permalink / raw)
  To: Andrew Davis, Anshul Dalal, u-boot
  Cc: Tom Rini, Judith Mendez, Udit Kumar, Hrushikesh Salunke,
	Neha Malcom Francis, Vignesh R, Christoph Niedermaier

On Wed Nov 5, 2025 at 11:39 PM IST, Andrew Davis wrote:
> On 11/5/25 10:21 AM, Anshul Dalal wrote:
>> Since the commit 1e24e84db41a ("tiny-printf: Handle formatting of %p
>> with an extra Kconfig"), SPL_USE_TINY_PRINTF_POINTER_SUPPORT has been
>> made mandatory in order to use %p which would earlier have defaulted to
>> a 'long' print.
>> 
>> Without this config symbol, k3_sysfw_dfu_download fails to set the
>> correct value for the DFU string with:
>> 
>>   sprintf(dfu_str, "sysfw.itb ram 0x%p 0x%x", addr,
>>     CONFIG_K3_SYSFW_IMAGE_SIZE_MAX);
>> 
>> The value we get "sysfw.itb ram 0x? 0x41c29d40" causes a boot failure.
>> 
>> Therefore this patch sets SPL_USE_TINY_PRINTF_POINTER_SUPPORT for the
>> usbdfu defconfig.
>> 
>> Signed-off-by: Anshul Dalal <anshuld@ti.com>
>> ---
>> Not sure if a 'Fixes' tag is appropriate here since the commit
>> 1e24e84db41a ("tiny-printf: Handle formatting of %p with an extra
>> Kconfig") did cause a regression though I think the problem existed with
>> the usage of "%p" with TINY_PRINTF set to begin with.
>
> So I do think this fix is valid, but it would be nice to not have to fix
> this in each defconfig that might fall into this trap. Instead is there
> some way to bake this into the Kconfig logic so that when K3, DFU, and
> USE_TINY_PRINTF are set, the SPL_USE_TINY_PRINTF_POINTER_SUPPORT symbol
> is also set automatically?
>
> Or better yet, since this looks to be specific to k3_sysfw_dfu_download()
> using 0x%p in a place where USE_TINY might be enabled we could just switch
> to using 0x%x with a cast there.
>
> Although the best solution IMHO would be to fix 1e24e84db41a which
> doesn't seem right to me. We used to have it so TINY_PRINTF would
> simply fall though with %p and use the %x handling. The commit
> does point out that %pa / %pap would be wrong but it doesn't actually
> fix those anyway, just removes the ability for %p to work without
> that new flag..
>

I think handling it in the Kconfig might be the best way forwards
although I'm not opposed to fixing 1e24e84db41a to begin with.

SPL_USE_TINY_PRINTF_POINTER_SUPPORT only adds ~100 bytes to our SPL
size, if that's an acceptable increase we can enable it by default for
K3.

> Andrew
>
>> ---
>> Changes in v2:
>> - Remove unrelated changes
>> - Link to v1: https://lore.kernel.org/r/20251105-am65_dfu_fix-v1-1-acc29111dccf@ti.com
>> ---
>>   configs/am65x_evm_r5_usbdfu_defconfig | 1 +
>>   1 file changed, 1 insertion(+)
>> 
>> diff --git a/configs/am65x_evm_r5_usbdfu_defconfig b/configs/am65x_evm_r5_usbdfu_defconfig
>> index f9161f1fe111ff97f7aa79f10b21c3b0ab1dad32..add03dce002b2abd913770cd836d2cd8c852ed5e 100644
>> --- a/configs/am65x_evm_r5_usbdfu_defconfig
>> +++ b/configs/am65x_evm_r5_usbdfu_defconfig
>> @@ -6,6 +6,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x57000
>>   CONFIG_SPL_GPIO=y
>>   CONFIG_SPL_LIBCOMMON_SUPPORT=y
>>   CONFIG_SPL_LIBGENERIC_SUPPORT=y
>> +CONFIG_SPL_USE_TINY_PRINTF_POINTER_SUPPORT=y
>>   CONFIG_SOC_K3_AM654=y
>>   CONFIG_K3_EARLY_CONS=y
>>   CONFIG_TARGET_AM654_R5_EVM=y
>> 
>> ---
>> base-commit: 1c250e444ad3b15315ee8b0fcb3fc3acc26449e2
>> change-id: 20251105-am65_dfu_fix-1144eec06b19
>> 
>> Best regards,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT
  2025-12-04  9:35   ` Anshul Dalal
@ 2025-12-04 10:31     ` Christoph Niedermaier
  0 siblings, 0 replies; 4+ messages in thread
From: Christoph Niedermaier @ 2025-12-04 10:31 UTC (permalink / raw)
  To: Anshul Dalal, Andrew Davis, u-boot@lists.denx.de
  Cc: Tom Rini, Judith Mendez, Udit Kumar, Hrushikesh Salunke,
	Neha Malcom Francis, Vignesh R

From: Anshul Dalal <anshuld@ti.com>
Sent: Thursday, December 4, 2025 10:36 AM
> On Wed Nov 5, 2025 at 11:39 PM IST, Andrew Davis wrote:
>> On 11/5/25 10:21 AM, Anshul Dalal wrote:
>>> Since the commit 1e24e84db41a ("tiny-printf: Handle formatting of %p
>>> with an extra Kconfig"), SPL_USE_TINY_PRINTF_POINTER_SUPPORT has been
>>> made mandatory in order to use %p which would earlier have defaulted to
>>> a 'long' print.
>>>
>>> Without this config symbol, k3_sysfw_dfu_download fails to set the
>>> correct value for the DFU string with:
>>>
>>>   sprintf(dfu_str, "sysfw.itb ram 0x%p 0x%x", addr,
>>>     CONFIG_K3_SYSFW_IMAGE_SIZE_MAX);
>>>
>>> The value we get "sysfw.itb ram 0x? 0x41c29d40" causes a boot failure.
>>>
>>> Therefore this patch sets SPL_USE_TINY_PRINTF_POINTER_SUPPORT for the
>>> usbdfu defconfig.
>>>
>>> Signed-off-by: Anshul Dalal <anshuld@ti.com>
>>> ---
>>> Not sure if a 'Fixes' tag is appropriate here since the commit
>>> 1e24e84db41a ("tiny-printf: Handle formatting of %p with an extra
>>> Kconfig") did cause a regression though I think the problem existed with
>>> the usage of "%p" with TINY_PRINTF set to begin with.
>>
>> So I do think this fix is valid, but it would be nice to not have to fix
>> this in each defconfig that might fall into this trap. Instead is there
>> some way to bake this into the Kconfig logic so that when K3, DFU, and
>> USE_TINY_PRINTF are set, the SPL_USE_TINY_PRINTF_POINTER_SUPPORT symbol
>> is also set automatically?
>>
>> Or better yet, since this looks to be specific to k3_sysfw_dfu_download()
>> using 0x%p in a place where USE_TINY might be enabled we could just switch
>> to using 0x%x with a cast there.
>>
>> Although the best solution IMHO would be to fix 1e24e84db41a which
>> doesn't seem right to me. We used to have it so TINY_PRINTF would
>> simply fall though with %p and use the %x handling. The commit
>> does point out that %pa / %pap would be wrong but it doesn't actually
>> fix those anyway, just removes the ability for %p to work without
>> that new flag..
>>
> 
> I think handling it in the Kconfig might be the best way forwards
> although I'm not opposed to fixing 1e24e84db41a to begin with.
> 
> SPL_USE_TINY_PRINTF_POINTER_SUPPORT only adds ~100 bytes to our SPL
> size, if that's an acceptable increase we can enable it by default for
> K3.

The idea is to either support pointer nearly completely or not at all.
Only partial support (fall through) always causes problems. But now if
you enable SPL_USE_TINY_PRINTF_POINTER_SUPPORT pointer support is handled
correctly by the pointer() function. Before it was only active with
NET support. So there is nothing to fix. The idea was developed and
discussed here in a previous patch:
https://patchwork.ozlabs.org/project/uboot/patch/20250320102346.13564-1-cniedermaier@dh-electronics.com/#3482590

Regards
Christoph

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-12-04 10:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-05 16:21 [PATCH v2] configs: am65x_usbdfu: add SPL_USE_TINY_PRINTF_POINTER_SUPPORT Anshul Dalal
2025-11-05 18:09 ` Andrew Davis
2025-12-04  9:35   ` Anshul Dalal
2025-12-04 10:31     ` Christoph Niedermaier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox