Patchwork [U-Boot,v3] arm: printf() is not available in some SPL configurations

login
register
mail settings
Submitter Christian Riesch
Date Nov. 22, 2011, 8:26 a.m.
Message ID <1321950379-25403-1-git-send-email-christian.riesch@omicron.at>
Download mbox | patch
Permalink /patch/127012/
State Superseded
Headers show

Comments

Christian Riesch - Nov. 22, 2011, 8:26 a.m.
This patch avoids build breakage for SPLs that do not support printf.

Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Tom Rini <trini@ti.com>
Acked-by: Tom Rini <trini@ti.com>
---
Changes for v3:
- Removed extra white space
- Separated patch from patchset [1]

[1] http://lists.denx.de/pipermail/u-boot/2011-November/110635.html

 arch/arm/lib/eabi_compat.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Andreas Bießmann - Nov. 22, 2011, 8:49 a.m.
Dear Christian,

sorry I haven't followed the discussion so far. I have a little pointer too.

Am 22.11.2011 09:26, schrieb Christian Riesch:
> This patch avoids build breakage for SPLs that do not support printf.
> 
> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Tom Rini <trini@ti.com>
> Acked-by: Tom Rini <trini@ti.com>
> ---
> Changes for v3:
> - Removed extra white space
> - Separated patch from patchset [1]
> 
> [1] http://lists.denx.de/pipermail/u-boot/2011-November/110635.html
> 
>  arch/arm/lib/eabi_compat.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
> index eb3e26d..e1b87be 100644
> --- a/arch/arm/lib/eabi_compat.c
> +++ b/arch/arm/lib/eabi_compat.c
> @@ -13,7 +13,9 @@
>  
>  int raise (int signum)
>  {
> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>  	printf("raise: Signal # %d caught\n", signum);
> +#endif

Well, you can do it that way but I guess there are several places where
printf() will be needed by some drivers in SPL. Therefore I think it is
better to provide printf() for SPL than to ifdef out all the printf's.

So how about providing an empty

#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
#define printf(x)
#endif

somewhere in the SPL code?

best regards

Andreas Bießmann
Christian Riesch - Nov. 22, 2011, 9:08 a.m.
Hi Andreas,
Thank you for your comments!

On Tue, Nov 22, 2011 at 9:49 AM, Andreas Bießmann
<andreas.devel@googlemail.com> wrote:
> Dear Christian,
>
> sorry I haven't followed the discussion so far. I have a little pointer too.
>
> Am 22.11.2011 09:26, schrieb Christian Riesch:
>> This patch avoids build breakage for SPLs that do not support printf.
>>
>> Signed-off-by: Christian Riesch <christian.riesch@omicron.at>
>> Cc: Wolfgang Denk <wd@denx.de>
>> Cc: Tom Rini <trini@ti.com>
>> Acked-by: Tom Rini <trini@ti.com>
>> ---
>> Changes for v3:
>> - Removed extra white space
>> - Separated patch from patchset [1]
>>
>> [1] http://lists.denx.de/pipermail/u-boot/2011-November/110635.html
>>
>>  arch/arm/lib/eabi_compat.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
>> index eb3e26d..e1b87be 100644
>> --- a/arch/arm/lib/eabi_compat.c
>> +++ b/arch/arm/lib/eabi_compat.c
>> @@ -13,7 +13,9 @@
>>
>>  int raise (int signum)
>>  {
>> +#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
>>       printf("raise: Signal # %d caught\n", signum);
>> +#endif
>
> Well, you can do it that way but I guess there are several places where
> printf() will be needed by some drivers in SPL. Therefore I think it is
> better to provide printf() for SPL than to ifdef out all the printf's.
>
> So how about providing an empty
>
> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> #define printf(x)
> #endif
>
> somewhere in the SPL code?

If others agree I will do something like this. But I don't like this
approach. I can imagine that this could be a real PITA when you are
debugging an SPL, see a printf in the code, expect an output on
console, but this output never appears because printf has been
replaced globally in some obscure header file.

Regards, Christian
Scott Wood - Nov. 22, 2011, 8:20 p.m.
On 11/22/2011 02:49 AM, Andreas Bießmann wrote:
> Well, you can do it that way but I guess there are several places where
> printf() will be needed by some drivers in SPL. Therefore I think it is
> better to provide printf() for SPL than to ifdef out all the printf's.
> 
> So how about providing an empty
> 
> #if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
> #define printf(x)
> #endif
> 
> somewhere in the SPL code?

Use an inline function instead so the arguments are consumed and it
doesn't cause unused-variable warnings.

-Scott

Patch

diff --git a/arch/arm/lib/eabi_compat.c b/arch/arm/lib/eabi_compat.c
index eb3e26d..e1b87be 100644
--- a/arch/arm/lib/eabi_compat.c
+++ b/arch/arm/lib/eabi_compat.c
@@ -13,7 +13,9 @@ 
 
 int raise (int signum)
 {
+#if !defined(CONFIG_SPL_BUILD) || defined(CONFIG_SPL_LIBCOMMON_SUPPORT)
 	printf("raise: Signal # %d caught\n", signum);
+#endif
 	return 0;
 }