Skip to contentSkip to conversation

SPL: tiny-printf: avoid any BSS usage

PATCH Superseded Andre Przywara 4 participants 4 messages Jul 3, 2016 – Jul 5, 2016 Library code

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 SPL: tiny-printf: avoid any BSS usage

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. Please note that this is quite a hack (which may need to be extended to other source files) and more sustainable solutions are warmly welcomed.

---
Hi,

as mentioned before I don't like this hack very much, but as this fixes the WIP Pine64 SPL code I propose this solution for 2016.07.

Cheers,
Andre

 lib/tiny-printf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
lib/tiny-printf.c +10 -3
index 451f4f7..bc2995b 100644
@@ -13,11 +13,18 @@
 #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.
+ */
+#define NO_BSS __attribute__((section(".data")))
+
+static char *bf NO_BSS;
+static char zs NO_BSS;
 
 /* Current position in sprintf() output string */
-static char *outstr;
+static char *outstr NO_BSS;
 
 static void out(char c)
 {
Signature · 1 line
2.8.2
#1 lore raw

PATCH 1/1 SPL: tiny-printf: avoid any BSS usage

Quoted 27 lines · Andre Przywara

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. Please note that this is quite a hack (which may need to be extended to other source files) and more sustainable solutions are warmly welcomed.

---
Hi,

as mentioned before I don't like this hack very much, but as this fixes the WIP Pine64 SPL code I propose this solution for 2016.07.

Cheers,
Andre

 lib/tiny-printf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
lib/tiny-printf.c +0 -0
index 451f4f7..bc2995b 100644
@@ -13,11 +13,18 @@
 #include <stdarg.h>

lib/tiny-printf.c line 21

Quoted 10 lines
lib/tiny-printf.c +6 -2
 #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.
+ */
+#define NO_BSS __attribute__((section(".data")))

Please don't introduce new macros, it's really not necessary here, the comment is enough.

Quoted 10 lines
lib/tiny-printf.c +3 -1
+static char *bf NO_BSS;
+static char zs NO_BSS;
 
 /* Current position in sprintf() output string */
-static char *outstr;
+static char *outstr NO_BSS;
 
 static void out(char c)
 {
Signature · 2 lines
Best regards,
Marek Vasut
#2 lore raw

PATCH 1/1 SPL: tiny-printf: avoid any BSS usage

Quoted 30 lines · Marek Vasut
Quoted 29 lines · Andre Przywara

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. Please note that this is quite a hack (which may need to be extended to other source files) and more sustainable solutions are warmly welcomed.

---
Hi,

as mentioned before I don't like this hack very much, but as this fixes the WIP Pine64 SPL code I propose this solution for 2016.07.

Cheers,
Andre

lib/tiny-printf.c | 13 ++++++++++---

1 file changed, 10 insertions(+), 3 deletions(-)
lib/tiny-printf.c +0 -0
index 451f4f7..bc2995b 100644
@@ -13,11 +13,18 @@
 #include <stdarg.h>
 #include <serial.h>
 
Quoted 11 lines

lib/tiny-printf.c line 21

Quoted 8 lines
lib/tiny-printf.c +6 -2
-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.
+ */
+#define NO_BSS __attribute__((section(".data")))

Please don't introduce new macros, it's really not necessary here, the comment is enough.

And would also match the other areas where we have to do something "ugly" like this in order to function within constraints. Frankly, I don't see this exactly as a hack, but it should be tested on a few more platforms just to be safe.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160704/4a24901a/attachment.sig>
#3 lore raw

PATCH 1/1 SPL: tiny-printf: avoid any BSS usage

In reply to Andre Przywara
Quoted 10 lines · Andre Przywara

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. Please note that this is quite a hack (which may need to be extended to other source files) and more sustainable solutions are warmly welcomed.

---

There is atleast one place where I encountered same issue:
drivers/i2c/i2c-uclass-compact.c:
static int cur_busnum;

I could verify that the below macro helps in that case as well.

Quoted 31 lines
 lib/tiny-printf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)
lib/tiny-printf.c +10 -3
index 451f4f7..bc2995b 100644
@@ -13,11 +13,18 @@
 #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.
+ */
+#define NO_BSS __attribute__((section(".data")))
+
+static char *bf NO_BSS;
+static char zs NO_BSS;
 
 /* Current position in sprintf() output string */
-static char *outstr;
+static char *outstr NO_BSS;
 
 static void out(char c)
 {
Signature · 2 lines
Regards
Vignesh