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

From: Pragnesh Patel <pragnesh.patel@openfive.com>
To: u-boot@lists.denx.de
Subject: [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
Date: Tue, 17 Nov 2020 05:23:00 +0000	[thread overview]
Message-ID: <MN2PR13MB2797E781229173F01296C19DEBE20@MN2PR13MB2797.namprd13.prod.outlook.com> (raw)
In-Reply-To: <CAPnjgZ3X_biNg4mZm4gDm24piLAKHCe0JFCeBmLUgKvWu1nYkg@mail.gmail.com>

Hi,

>-----Original Message-----
>From: Simon Glass <sjg@chromium.org>
>Sent: 17 November 2020 05:23
>To: Pragnesh Patel <pragnesh.patel@openfive.com>
>Cc: Heinrich Schuchardt <xypron.glpk@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@openfive.com>
>wrote:
>>
>> Hi Heinrich,
>>
>> >-----Original Message-----
>> >From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> >Sent: 12 November 2020 18:02
>> >To: Pragnesh Patel <pragnesh.patel@openfive.com>
>> >Cc: U-Boot Mailing List <u-boot@lists.denx.de>; Simon Glass
>> ><sjg@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@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.

>
>Regards,
>Simon