[03/15] rockchip: rk3288: Print reset reason

Message ID 20190729074711.16988-4-jagan@amarulasolutions.com
State New
Headers show
Series
  • rk3399: Add redundant boot support
Related show

Commit Message

Jagan Teki July 29, 2019, 7:46 a.m. UTC
Print the reason for reset instead of storing it into
env variable in rk3288.

This would help to find the reset reason directly
on U-Boot proper logs.

Cc: Wadim Egorov <w.egorov@phytec.de>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/mach-rockchip/rk3288-board.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kever Yang Aug. 5, 2019, 12:30 p.m. UTC | #1
On 2019/7/29 下午3:46, Jagan Teki wrote:
> Print the reason for reset instead of storing it into
> env variable in rk3288.
>
> This would help to find the reset reason directly
> on U-Boot proper logs.
>
> Cc: Wadim Egorov <w.egorov@phytec.de>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>   arch/arm/mach-rockchip/rk3288-board.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c
> index d3ec141fea..613264d7ee 100644
> --- a/arch/arm/mach-rockchip/rk3288-board.c
> +++ b/arch/arm/mach-rockchip/rk3288-board.c
> @@ -72,7 +72,7 @@ static void rk3288_detect_reset_reason(void)
>   		reason = "unknown reset";
>   	}
>   
> -	env_set("reset_reason", reason);
> +	printf("Reset cause: %s\n", reason);


Why this need to set as env before? I didn't touch this code when I migrate

the code to use common board file. If this no need to set env, then this

call back can goto board_init() instead of board_late_init().


Thanks,

- Kever

>   
>   	/*
>   	 * Clear cru_glb_rst_st, so we can determine the last reset cause
Jagan Teki Aug. 14, 2019, 9:40 a.m. UTC | #2
On Mon, Aug 5, 2019 at 6:00 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>
>
> On 2019/7/29 下午3:46, Jagan Teki wrote:
> > Print the reason for reset instead of storing it into
> > env variable in rk3288.
> >
> > This would help to find the reset reason directly
> > on U-Boot proper logs.
> >
> > Cc: Wadim Egorov <w.egorov@phytec.de>
> > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > ---
> >   arch/arm/mach-rockchip/rk3288-board.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c
> > index d3ec141fea..613264d7ee 100644
> > --- a/arch/arm/mach-rockchip/rk3288-board.c
> > +++ b/arch/arm/mach-rockchip/rk3288-board.c
> > @@ -72,7 +72,7 @@ static void rk3288_detect_reset_reason(void)
> >               reason = "unknown reset";
> >       }
> >
> > -     env_set("reset_reason", reason);
> > +     printf("Reset cause: %s\n", reason);
>
>
> Why this need to set as env before? I didn't touch this code when I migrate

Don't know the proper reason for this, ie why I have CCed Wadim.

Wadim, any comments?
Wadim Egorov Aug. 14, 2019, 10:43 a.m. UTC | #3
On 14.08.19 11:40, Jagan Teki wrote:
> On Mon, Aug 5, 2019 at 6:00 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>>
>> On 2019/7/29 下午3:46, Jagan Teki wrote:
>>> Print the reason for reset instead of storing it into
>>> env variable in rk3288.
>>>
>>> This would help to find the reset reason directly
>>> on U-Boot proper logs.
>>>
>>> Cc: Wadim Egorov <w.egorov@phytec.de>
>>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
>>> ---
>>>   arch/arm/mach-rockchip/rk3288-board.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c
>>> index d3ec141fea..613264d7ee 100644
>>> --- a/arch/arm/mach-rockchip/rk3288-board.c
>>> +++ b/arch/arm/mach-rockchip/rk3288-board.c
>>> @@ -72,7 +72,7 @@ static void rk3288_detect_reset_reason(void)
>>>               reason = "unknown reset";
>>>       }
>>>
>>> -     env_set("reset_reason", reason);
>>> +     printf("Reset cause: %s\n", reason);
>>
>> Why this need to set as env before? I didn't touch this code when I migrate
> Don't know the proper reason for this, ie why I have CCed Wadim.
>
> Wadim, any comments?
If we put the reset_reason inside an environment variable we can later
reuse it from a script, e.g. changing the boot behavior for special
cases. Right now I have no usecase for it. So from my side you can just
print the reason instead of setting the environment. AFAIR I copied the
env-method from another board.
btw, board/xilinx/zynqmp/zynqmp.c is doing both.
Michael Nazzareno Trimarchi Aug. 14, 2019, 10:46 a.m. UTC | #4
Hi


On Wednesday, August 14, 2019, Wadim Egorov <w.egorov@phytec.de> wrote:

>
> On 14.08.19 11:40, Jagan Teki wrote:
> > On Mon, Aug 5, 2019 at 6:00 PM Kever Yang <kever.yang@rock-chips.com>
> wrote:
> >>
> >> On 2019/7/29 下午3:46, Jagan Teki wrote:
> >>> Print the reason for reset instead of storing it into
> >>> env variable in rk3288.
> >>>
> >>> This would help to find the reset reason directly
> >>> on U-Boot proper logs.
> >>>
> >>> Cc: Wadim Egorov <w.egorov@phytec.de>
> >>> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> >>> ---
> >>>   arch/arm/mach-rockchip/rk3288-board.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/arm/mach-rockchip/rk3288-board.c
> b/arch/arm/mach-rockchip/rk3288-board.c
> >>> index d3ec141fea..613264d7ee 100644
> >>> --- a/arch/arm/mach-rockchip/rk3288-board.c
> >>> +++ b/arch/arm/mach-rockchip/rk3288-board.c
> >>> @@ -72,7 +72,7 @@ static void rk3288_detect_reset_reason(void)
> >>>               reason = "unknown reset";
> >>>       }
> >>>
> >>> -     env_set("reset_reason", reason);
> >>> +     printf("Reset cause: %s\n", reason);
> >>
> >> Why this need to set as env before? I didn't touch this code when I
> migrate
> > Don't know the proper reason for this, ie why I have CCed Wadim.
> >
> > Wadim, any comments?
> If we put the reset_reason inside an environment variable we can later
> reuse it from a script, e.g. changing the boot behavior for special
> cases. Right now I have no usecase for it. So from my side you can just
> print the reason instead of setting the environment. AFAIR I copied the
> env-method from another board.
> btw, board/xilinx/zynqmp/zynqmp.c is doing both.


The reason is totally valid and we don't know even is already use in some
deploy. Add the print without remove the environment make more sense to me

Michael

Patch

diff --git a/arch/arm/mach-rockchip/rk3288-board.c b/arch/arm/mach-rockchip/rk3288-board.c
index d3ec141fea..613264d7ee 100644
--- a/arch/arm/mach-rockchip/rk3288-board.c
+++ b/arch/arm/mach-rockchip/rk3288-board.c
@@ -72,7 +72,7 @@  static void rk3288_detect_reset_reason(void)
 		reason = "unknown reset";
 	}
 
-	env_set("reset_reason", reason);
+	printf("Reset cause: %s\n", reason);
 
 	/*
 	 * Clear cru_glb_rst_st, so we can determine the last reset cause