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

[Pragnesh Patel]

>-----Original Message-----
>From: Simon Glass <sjg at chromium.org>
>Sent: 18 November 2020 20:07
>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 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?

I am okay with this. Better to make TRACE select TIMER_EARLY.

>
>Regards,
>Simon