On 09/07/16 15:38, Simon Glass wrote:
> On 8 July 2016 at 08:18, Andre Przywara <andre.przyw...@arm.com> wrote:
>> As printf calls may be executed quite early, we should avoid using any
>> BSS stored variables, since some boards put BSS in DRAM, which may not
>> have been initialised yet.
>> Explicitly mark those "static global" variables as belonging to the
>> .data section, to keep tiny-printf clear of any BSS usage.
>>
>> Signed-off-by: Andre Przywara <andre.przyw...@arm.com>
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> Another approach would be to put these vars in a struct, declare it as
> a local variable and pass it around. But this works OK too.
Yeah, I was thinking about this too. Actually fixing the issue that we
need global variables in the first place.
But a quick look revealed that this is not trivial, so I reverted to
this simpler approach for the quick fix. Also I am not sure this will
eventually work against this "tiny" idea, we will see.
To be honest I think the real proper fix(TM) would be to provide a
separate BSS section for any code that runs _before_ DRAM init. This
should be a few bytes only and could easily live in SRAM.
If the loading part of the SPL requires a bigger BSS, that's fine as it
could still live in DRAM.
I might bake a patch when I get bored ...
Cheers,
Andre.
>
>> ---
>> Changes v1 .. v2:
>> - remove NO_BSS macro, use __attribute__ ... directly instead
>>
>> lib/tiny-printf.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
>> index 451f4f7..b334f05 100644
>> --- a/lib/tiny-printf.c
>> +++ b/lib/tiny-printf.c
>> @@ -13,11 +13,16 @@
>> #include <stdarg.h>
>> #include <serial.h>
>>
>> -static char *bf;
>> -static char zs;
>> +/*
>> + * This code in here may execute before the DRAM is initialised, so
>> + * we should make sure that it doesn't touch BSS, which some boards
>> + * put in DRAM.
>> + */
>> +static char *bf __attribute__ ((section(".data")));
>> +static char zs __attribute__ ((section(".data")));
>>
>> /* Current position in sprintf() output string */
>> -static char *outstr;
>> +static char *outstr __attribute__ ((section(".data")));
>>
>> static void out(char c)
>> {
>> --
>> 2.8.2
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
