[PATCH] common/board_r: make sure to call initr_dm() before initr_trace()

[Simon Glass]

Hi Pragnesh,

On Mon, 16 Nov 2020 at 22:23, Pragnesh Patel
<pragnesh.patel at openfive.com> wrote:
>
> Hi,
>
> >-----Original Message-----
> >From: Simon Glass <sjg at chromium.org>
> >Sent: 17 November 2020 05:23
> >To: Pragnesh Patel <pragnesh.patel at openfive.com>
> >Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>; U-Boot Mailing List <u-
> >boot at lists.denx.de>
> >Subject: Re: [PATCH] common/board_r: make sure to call initr_dm() before
> >initr_trace()
> >
> >[External Email] Do not click links or attachments unless you recognize the
> >sender and know the content is safe
> >
> >Hi,
> >
> >On Sun, 15 Nov 2020 at 05:16, Pragnesh Patel <pragnesh.patel at openfive.com>
> >wrote:
> >>
> >> Hi Heinrich,
> >>
> >> >-----Original Message-----
> >> >From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> >Sent: 12 November 2020 18:02
> >> >To: Pragnesh Patel <pragnesh.patel at openfive.com>
> >> >Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Simon Glass
> >> ><sjg at chromium.org>
> >> >Subject: Re: [PATCH] common/board_r: make sure to call initr_dm()
> >> >before
> >> >initr_trace()
> >> >
> >> >[External Email] Do not click links or attachments unless you
> >> >recognize the sender and know the content is safe
> >> >
> >> >On 11/12/20 12:18 PM, Pragnesh Patel wrote:
> >> >> Tracing need timer ticks and initr_dm() will make gd->timer and
> >> >> gd->dm_root is equal to NULL, so make sure that initr_dm() to
> >> >> call before tracing got enabled.
> >> >>
> >> >> Signed-off-by: Pragnesh Patel <pragnesh.patel at sifive.com>
> >> >> ---
> >> >>  common/board_r.c | 6 +++---
> >> >>  1 file changed, 3 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/common/board_r.c b/common/board_r.c index
> >> >> 29dd7d26d9..7140a39947 100644
> >> >> --- a/common/board_r.c
> >> >> +++ b/common/board_r.c
> >> >> @@ -693,6 +693,9 @@ static int run_main_loop(void)
> >> >>   * TODO: perhaps reset the watchdog in the initcall function after each call?
> >> >>   */
> >> >>  static init_fnc_t init_sequence_r[] = {
> >> >> +#ifdef CONFIG_DM
> >> >> +     initr_dm,
> >> >> +#endif
> >> >>       initr_trace,
> >> >>       initr_reloc,
> >> >>       /* TODO: could x86/PPC have this also perhaps? */ @@ -718,9
> >> >> +721,6 @@ static init_fnc_t init_sequence_r[] = {
> >> >>       initr_noncached,
> >> >>  #endif
> >> >>       initr_of_live,
> >> >> -#ifdef CONFIG_DM
> >> >> -     initr_dm,
> >> >> -#endif
> >> >
> >> >You are moving initr_of_live before initr_of_live. I doubt this will
> >> >work for boards that have CONFIG_OF_LIVE=y.
> >>
> >> yes you are right. It will not work for CONFIG_OF_LIVE.
> >>
> >> >
> >> >Can't we move initr_trace down in the code to after both
> >> >initr_of_live and initr_dm?
> >> >
> >> >@Simon:
> >> >Please, advise.
> >>
> >> I am okay with this suggestion.
> >
> >Actually can we use the early timer for this case?
> >
> >DM init is a part of U-Boot and not being able to trace it would be unfortunate.
>
> Got it.
>
> If someone wants to use tracing without TIMER_EARLY then
>
> - initr_dm() will make gd->dm_root = NULL; and gd->timer = NULL; and
> __cyg_profile_func_enter () will call timer_get_us() -> get_ticks() -> dm_timer_init().
>
> dm_timer_init() will not able to initialize timer and return an error.
>
> We need to find any solution for this.

How about we make TRACE select or depend on TIMER_EARLY?

Regards,
Simon