[U-Boot] [PATCH v1 1/1] lib/display_options: Fix print_freq

[Heiko Schocher <hs@denx.de>]

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 at 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 at gmail.com <mailto:suriyan.r@gmail.com>>
>             ---
>                lib/display_options.c | 6 ------
>                1 file changed, 6 deletions(-)
>
>
>         Acked-by: Simon Glass <sjg at 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;
>       }
>     +#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!

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
>
>

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany