[U-Boot] [PATCH] common: board_f: Fix build error when CONFIG_TRACE is not defined

[Simon Glass]

Hi,

On Sun, Jul 7, 2013 at 9:10 PM, Tushar Behera <tushar.behera at linaro.org>wrote:

> On 07/06/2013 01:27 AM, Simon Glass wrote:
> > Hi Tushar,
> >
> > On Fri, Jul 5, 2013 at 1:40 AM, Tushar Behera <tushar.behera at linaro.org
> >wrote:
> >
> >> When CONFIG_TRACE is not defined, definition of trace_early_init is not
> >> compiled and we get following error.
> >>
> >> common/libcommon.o:(.data.init_sequence_f+0xc): undefined reference to
> >> `trace_early_init'
> >>
> >> While at it, also define reserve_trace only if CONFIG_TRACE is defined.
> >>
> >> CC: Simon Glass <sjg at chromium.org>
> >> Signed-off-by: Tushar Behera <tushar.behera at linaro.org>
> >> ---
> >>  common/board_f.c |    8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/common/board_f.c b/common/board_f.c
> >> index ab4242a..a685368 100644
> >> --- a/common/board_f.c
> >> +++ b/common/board_f.c
> >> @@ -501,17 +501,17 @@ static int reserve_lcd(void)
> >>  }
> >>  #endif /* CONFIG_LCD */
> >>
> >> +#ifdef CONFIG_TRACE
> >>  static int reserve_trace(void)
> >>  {
> >> -#ifdef CONFIG_TRACE
> >>         gd->relocaddr -= CONFIG_TRACE_BUFFER_SIZE;
> >>         gd->trace_buff = map_sysmem(gd->relocaddr,
> >> CONFIG_TRACE_BUFFER_SIZE);
> >>         debug("Reserving %dk for trace data at: %08lx\n",
> >>               CONFIG_TRACE_BUFFER_SIZE >> 10, gd->relocaddr);
> >> -#endif
> >>
> >>         return 0;
> >>  }
> >> +#endif
> >>
> >>  #if defined(CONFIG_VIDEO) && (!defined(CONFIG_PPC) ||
> >> defined(CONFIG_8xx)) \
> >>                 && !defined(CONFIG_ARM) && !defined(CONFIG_X86)
> >> @@ -833,7 +833,9 @@ static init_fnc_t init_sequence_f[] = {
> >>  #endif
> >>         setup_mon_len,
> >>         setup_fdt,
> >> +#ifdef CONFIG_TRACE
> >>         trace_early_init,
> >> +#endif
> >>  #if defined(CONFIG_MPC85xx) || defined(CONFIG_MPC86xx)
> >>         /* TODO: can this go into arch_cpu_init()? */
> >>         probecpu,
> >> @@ -977,7 +979,9 @@ static init_fnc_t init_sequence_f[] = {
> >>  #ifdef CONFIG_LCD
> >>         reserve_lcd,
> >>  #endif
> >> +#ifdef CONFIG_TRACE
> >>         reserve_trace,
> >> +#endif
> >>         /* TODO: Why the dependency on CONFIG_8xx? */
> >>  #if defined(CONFIG_VIDEO) && (!defined(CONFIG_PPC) ||
> >> defined(CONFIG_8xx)) \
> >>                 && !defined(CONFIG_ARM) && !defined(CONFIG_X86)
> >>
> >
> > Does the static inline not work for you? What toolchain are you using? I
> > assume this is ARM?
> >
>
> I was testing on EXYNOS5250 using 4.7.3 toolchain.
>
> My mistake: while testing this patch, I had disabled CONFIG_TRACE, but
> CONFIG_TRACE_EARLY was still enabled, hence the error. You may ignore
> the patch.
>
> IMHO, we should define the TRACE related config options only if TRACE is
> enabled. If that is ok for you, I will submit a patch for that.
>

I'm not sure it matters. The idea is that you can build U-Boot with 'make
FTRACE=1' and get tracing enabled, without having to modify the config
files.

Tracing IMO is something you turn on and off while developing, rather than
a board option. I didn't come up with a good way of enabling it globally
(since we need things like buffer size and the early trace address) but if
we could make it a 'make' option instead of a board option, that would be
my preference.



> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -45,11 +45,13 @@
>
>  /* Allow tracing to be enabled */
>  #define CONFIG_TRACE
> +#ifdef CONFIG_TRACE
>  #define CONFIG_CMD_TRACE
>  #define CONFIG_TRACE_BUFFER_SIZE       (16 << 20)
>  #define CONFIG_TRACE_EARLY_SIZE                (8 << 20)
>  #define CONFIG_TRACE_EARLY
>  #define CONFIG_TRACE_EARLY_ADDR                0x50000000
> +#endif
>

Regards,
Simon