From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Wed, 18 Nov 2020 07:37:19 -0700
Subject: [PATCH] common/board_r: make sure to call initr_dm() before
 initr_trace()
In-Reply-To: <MN2PR13MB2797E781229173F01296C19DEBE20@MN2PR13MB2797.namprd13.prod.outlook.com>
References: <20201112111859.7762-1-pragnesh.patel@sifive.com>
 <e78560fe-c1a2-b51b-4a54-a67bb49d88df@gmx.de>
 <MN2PR13MB2797E72BE7F6B559D162B0DCEBE40@MN2PR13MB2797.namprd13.prod.outlook.com>
 <CAPnjgZ3X_biNg4mZm4gDm24piLAKHCe0JFCeBmLUgKvWu1nYkg@mail.gmail.com>
 <MN2PR13MB2797E781229173F01296C19DEBE20@MN2PR13MB2797.namprd13.prod.outlook.com>
Message-ID: <CAPnjgZ0664seowVo1jhUx9SmiBCF-uhx_YPpt7-iQtKPBLTQWA@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Pragnesh,

On Mon, 16 Nov 2020 at 22:23, Pragnesh Patel
<pragnesh.patel@openfive.com> wrote:
>
> 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.

How about we make TRACE select or depend on TIMER_EARLY?

Regards,
Simon