[U-Boot] [PATCH v1 1/1] lib/display_options: Fix print_freq
Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz
I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build.
Signed-off-by: Suriyan Ramasami suriyan.r@gmail.com --- lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/lib/display_options.c b/lib/display_options.c index df134cd..80316a4 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -26,9 +26,7 @@ int display_options (void) void print_freq(uint64_t freq, const char *s) { unsigned long m = 0; -#if defined(CONFIG_SPL_SERIAL_SUPPORT) unsigned long n; -#endif uint32_t f; static const char names[] = {'G', 'M', 'K'}; unsigned long d = 1e9; @@ -48,9 +46,7 @@ void print_freq(uint64_t freq, const char *s) }
f = do_div(freq, d); -#if defined(CONFIG_SPL_SERIAL_SUPPORT) n = freq; -#endif
/* If there's a remainder, show the first few digits */ if (f) { @@ -63,9 +59,7 @@ void print_freq(uint64_t freq, const char *s) m = (m / 10) + (m % 100 >= 50); }
-#if defined(CONFIG_SPL_SERIAL_SUPPORT) printf("%lu", n); -#endif if (m) printf(".%ld", m); printf(" %cHz%s", c, s);
Hi,
On 18 August 2015 at 10:25, Suriyan Ramasami suriyan.r@gmail.com wrote:
Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz
I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build.
Signed-off-by: Suriyan Ramasami suriyan.r@gmail.com
lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
That's strange. Your patch looks correct to me.
Regards, Simon
Hello Simon, Suriyan Ramasami,
Am 22.08.2015 um 02:36 schrieb Simon Glass:
Hi,
On 18 August 2015 at 10:25, Suriyan Ramasami suriyan.r@gmail.com wrote:
Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz
I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build.
Signed-off-by: Suriyan Ramasami suriyan.r@gmail.com
lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
That's strange. Your patch looks correct to me.
Yes, strange, how this slipped me ...
This patch leads in the following compiler warning for the smartweb board:
CC spl/lib/display_options.o /home/hs/zug/u-boot/lib/display_options.c: In function 'print_freq': /home/hs/zug/u-boot/lib/display_options.c:29:16: warning: variable 'n' set but not used [-Wunused-but-set-variable]
@Suriyan Ramasami: Could you add to your patch the following:
diff --git a/lib/display_options.c b/lib/display_options.c index 80316a4..a4a5032 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -23,6 +23,8 @@ int display_options (void) return 0; }
+#if !defined(CONFIG_SPL_BUILD) || \ + defined(CONFIG_SPL_SERIAL_SUPPORT) void print_freq(uint64_t freq, const char *s) { unsigned long m = 0; @@ -185,3 +187,4 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
return 0; } +#endif
Thanks!
bye, Heiko
Hello Heiko/Simon,
On Sun, Aug 23, 2015 at 10:37 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon, Suriyan Ramasami,
Am 22.08.2015 um 02:36 schrieb Simon Glass:
Hi,
On 18 August 2015 at 10:25, Suriyan Ramasami suriyan.r@gmail.com wrote:
Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz
I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build.
Signed-off-by: Suriyan Ramasami suriyan.r@gmail.com
lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
That's strange. Your patch looks correct to me.
Yes, strange, how this slipped me ...
This patch leads in the following compiler warning for the smartweb board:
CC spl/lib/display_options.o /home/hs/zug/u-boot/lib/display_options.c: In function 'print_freq': /home/hs/zug/u-boot/lib/display_options.c:29:16: warning: variable 'n' set but not used [-Wunused-but-set-variable]
@Suriyan Ramasami: Could you add to your patch the following:
diff --git a/lib/display_options.c b/lib/display_options.c index 80316a4..a4a5032 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -23,6 +23,8 @@ int display_options (void) return 0; }
+#if !defined(CONFIG_SPL_BUILD) || \
- defined(CONFIG_SPL_SERIAL_SUPPORT)
void print_freq(uint64_t freq, const char *s) { unsigned long m = 0; @@ -185,3 +187,4 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
return 0;} +#endif
I am trying to understand this a bit. The compiler warning seems to stem
from the fact that printf() is eradicated with CONFIG_SPL_BUILD being set, and hence results in the 'n being set but not used' situation. As its just one variable 'n' causing this, would it be more readable if we do not have the #ifdef you suggested, but rather just have this instead: 1. We get rid of: unsigned long n; 2. We substitute this: n = freq; printf("%lu", n); with: printf("%lu", (unsigned long) freq);
Comments and thoughts welcome! Thanks!
Thanks!
bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hello Suriyan,
Am 25.08.2015 um 00:14 schrieb Suriyan Ramasami:
Hello Heiko/Simon,
On Sun, Aug 23, 2015 at 10:37 PM, Heiko Schocher <hs@denx.de mailto:hs@denx.de> wrote:
Hello Simon, Suriyan Ramasami, Am 22.08.2015 um 02:36 schrieb Simon Glass: Hi, On 18 August 2015 at 10:25, Suriyan Ramasami <suriyan.r@gmail.com <mailto:suriyan.r@gmail.com>> wrote: Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build. Signed-off-by: Suriyan Ramasami <suriyan.r@gmail.com <mailto:suriyan.r@gmail.com>> --- lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-) Acked-by: Simon Glass <sjg@chromium.org <mailto:sjg@chromium.org>> That's strange. Your patch looks correct to me. Yes, strange, how this slipped me ... This patch leads in the following compiler warning for the smartweb board: CC spl/lib/display_options.o /home/hs/zug/u-boot/lib/display_options.c: In function 'print_freq': /home/hs/zug/u-boot/lib/display_options.c:29:16: warning: variable 'n' set but not used [-Wunused-but-set-variable] @Suriyan Ramasami: Could you add to your patch the following: diff --git a/lib/display_options.c b/lib/display_options.c index 80316a4..a4a5032 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -23,6 +23,8 @@ int display_options (void) return 0; } +#if !defined(CONFIG_SPL_BUILD) || \ + defined(CONFIG_SPL_SERIAL_SUPPORT) void print_freq(uint64_t freq, const char *s) { unsigned long m = 0; @@ -185,3 +187,4 @@ int print_buffer(ulong addr, const void *data, uint width, uint count, return 0; } +#endifI am trying to understand this a bit. The compiler warning seems to stem from the fact that printf() is eradicated with CONFIG_SPL_BUILD being set, and hence results in the 'n being set but not used' situation. As its just one variable 'n' causing this, would it be more readable if we do not have the #ifdef you suggested, but rather just have this instead:
- We get rid of:
unsigned long n; 2. We substitute this: n = freq; printf("%lu", n); with: printf("%lu", (unsigned long) freq);
Comments and thoughts welcome! Thanks!
Sounds better!
bye, Heiko
Thanks! bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Hi,
On 24 August 2015 at 16:14, Suriyan Ramasami suriyan.r@gmail.com wrote:
Hello Heiko/Simon,
On Sun, Aug 23, 2015 at 10:37 PM, Heiko Schocher hs@denx.de wrote:
Hello Simon, Suriyan Ramasami,
Am 22.08.2015 um 02:36 schrieb Simon Glass:
Hi,
On 18 August 2015 at 10:25, Suriyan Ramasami suriyan.r@gmail.com wrote:
Build without CONFIG_SPL_SERIAL_SUPPORT does not print the cpu freq. I have seen this in the odroid U3 board, where on boot one sees this: CPU: Exynos4412 @ GHz instead of: CPU: Exynos4412 @ 1 GHz
I am assuming that this change was done to get rid of compiler warnings related to unused variables when building with CONFIG_SPL_SERIAL_SUPPORT not being defined in an SPL build.
Signed-off-by: Suriyan Ramasami suriyan.r@gmail.com
lib/display_options.c | 6 ------ 1 file changed, 6 deletions(-)
Acked-by: Simon Glass sjg@chromium.org
That's strange. Your patch looks correct to me.
Yes, strange, how this slipped me ...
This patch leads in the following compiler warning for the smartweb board:
CC spl/lib/display_options.o /home/hs/zug/u-boot/lib/display_options.c: In function 'print_freq': /home/hs/zug/u-boot/lib/display_options.c:29:16: warning: variable 'n' set but not used [-Wunused-but-set-variable]
@Suriyan Ramasami: Could you add to your patch the following:
diff --git a/lib/display_options.c b/lib/display_options.c index 80316a4..a4a5032 100644 --- a/lib/display_options.c +++ b/lib/display_options.c @@ -23,6 +23,8 @@ int display_options (void) return 0; }
+#if !defined(CONFIG_SPL_BUILD) || \
- defined(CONFIG_SPL_SERIAL_SUPPORT)
void print_freq(uint64_t freq, const char *s) { unsigned long m = 0; @@ -185,3 +187,4 @@ int print_buffer(ulong addr, const void *data, uint width, uint count,
return 0;} +#endif
I am trying to understand this a bit. The compiler warning seems to stem from the fact that printf() is eradicated with CONFIG_SPL_BUILD being set, and hence results in the 'n being set but not used' situation. As its just one variable 'n' causing this, would it be more readable if we do not have the #ifdef you suggested, but rather just have this instead:
- We get rid of:
unsigned long n; 2. We substitute this: n = freq; printf("%lu", n); with: printf("%lu", (unsigned long) freq);
Comments and thoughts welcome! Thanks!
Yes you are right - I don't think we need that variable at all.
Regards, Simon
participants (3)
-
Heiko Schocher -
Simon Glass -
Suriyan Ramasami