All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
       [not found] <20201112111859.7762-1-pragnesh.patel@sifive.com>
@ 2020-11-12 12:32 ` Heinrich Schuchardt
  2020-11-15 12:16   ` Pragnesh Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Heinrich Schuchardt @ 2020-11-12 12:32 UTC (permalink / raw)
  To: u-boot

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.

Can't we move initr_trace down in the code to after both initr_of_live
and initr_dm?

@Simon:
Please, advise.

Best regards

Heinrich

>  #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || \
>  	defined(CONFIG_SANDBOX)
>  	board_init,	/* Setup chipselects */
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
  2020-11-12 12:32 ` [PATCH] common/board_r: make sure to call initr_dm() before initr_trace() Heinrich Schuchardt
@ 2020-11-15 12:16   ` Pragnesh Patel
  2020-11-16 23:53     ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Pragnesh Patel @ 2020-11-15 12:16 UTC (permalink / raw)
  To: u-boot

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.

>
>Best regards
>
>Heinrich
>
>>  #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) ||
>defined(CONFIG_RISCV) || \
>>       defined(CONFIG_SANDBOX)
>>       board_init,     /* Setup chipselects */
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
  2020-11-15 12:16   ` Pragnesh Patel
@ 2020-11-16 23:53     ` Simon Glass
  2020-11-17  5:23       ` Pragnesh Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-11-16 23:53 UTC (permalink / raw)
  To: u-boot

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.

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
  2020-11-16 23:53     ` Simon Glass
@ 2020-11-17  5:23       ` Pragnesh Patel
  2020-11-18 14:37         ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Pragnesh Patel @ 2020-11-17  5:23 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
  2020-11-17  5:23       ` Pragnesh Patel
@ 2020-11-18 14:37         ` Simon Glass
  2020-11-19 10:42           ` Pragnesh Patel
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-11-18 14:37 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] common/board_r: make sure to call initr_dm() before initr_trace()
  2020-11-18 14:37         ` Simon Glass
@ 2020-11-19 10:42           ` Pragnesh Patel
  0 siblings, 0 replies; 6+ messages in thread
From: Pragnesh Patel @ 2020-11-19 10:42 UTC (permalink / raw)
  To: u-boot



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

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

>
>Regards,
>Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-19 10:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20201112111859.7762-1-pragnesh.patel@sifive.com>
2020-11-12 12:32 ` [PATCH] common/board_r: make sure to call initr_dm() before initr_trace() Heinrich Schuchardt
2020-11-15 12:16   ` Pragnesh Patel
2020-11-16 23:53     ` Simon Glass
2020-11-17  5:23       ` Pragnesh Patel
2020-11-18 14:37         ` Simon Glass
2020-11-19 10:42           ` Pragnesh Patel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.