[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Anatolij Gustschin]

Add output of the WRSR register content while booting so that
we can see the source of the last reset.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 arch/arm/cpu/arm1136/mx31/generic.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
index 1415d6c..788faed 100644
--- a/arch/arm/cpu/arm1136/mx31/generic.c
+++ b/arch/arm/cpu/arm1136/mx31/generic.c
@@ -93,8 +93,10 @@ void mx31_gpio_mux(unsigned long mode)
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo (void)
 {
-	printf("CPU:   Freescale i.MX31 at %d MHz\n",
-		mx31_get_mcu_main_clk() / 1000000);
+	struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE;
+
+	printf("CPU:   Freescale i.MX31 at %d MHz (WRSR=0x%04x)\n",
+		mx31_get_mcu_main_clk() / 1000000, wdog->wrsr);
 	return 0;
 }
 #endif
-- 
1.7.1

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Fabio Estevam]

Hi Anatolij,

--- On Thu, 3/10/11, Anatolij Gustschin <agust@denx.de> wrote:
....?? ???
> mx31_get_mcu_main_clk() / 1000000);
> +??? struct wdog_regs *wdog = (struct> wdog_regs *)WDOG_BASE;
> +
> +??? printf("CPU:???Freescale i.MX31 at %d MHz (WRSR=0x%04x)\n",
> +??
> mx31_get_mcu_main_clk() / 1000000, wdog->wrsr);
>  ??? return 0;

Wouldn?t it be better to show a string for the reset source, rather than a pure register value?

I sent a similar patch yesterday for MX31PDK: http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/95615

,but I agree that moving it to the generic code is a better idea.

Regards,

Fabio Estevam



      

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Stefano Babic]

On 03/10/2011 11:53 AM, Anatolij Gustschin wrote:
> Add output of the WRSR register content while booting so that
> we can see the source of the last reset.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>

Hi Antolij,

> ---
>  arch/arm/cpu/arm1136/mx31/generic.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
> index 1415d6c..788faed 100644
> --- a/arch/arm/cpu/arm1136/mx31/generic.c
> +++ b/arch/arm/cpu/arm1136/mx31/generic.c
> @@ -93,8 +93,10 @@ void mx31_gpio_mux(unsigned long mode)
>  #if defined(CONFIG_DISPLAY_CPUINFO)
>  int print_cpuinfo (void)
>  {
> -	printf("CPU:   Freescale i.MX31 at %d MHz\n",
> -		mx31_get_mcu_main_clk() / 1000000);
> +	struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE;
> +
> +	printf("CPU:   Freescale i.MX31 at %d MHz (WRSR=0x%04x)\n",
> +		mx31_get_mcu_main_clk() / 1000000, wdog->wrsr);

Why is it better to use the wrsr register instead of the rcsr register ?
We are actually using the rcsr register for other i.MX processors
(MX51/MX53/MX35). And if we want to print the reset cause, I think it
should be better to write directly the cause as string instead of the
register value.

I do not think printing the reset cause should be implemented in
print_cpuinfo(), because it manages a different issue (reset cause
against CPU information). The print_cpuinfo() should have only CPU
related values, as clock values, and so on, as it is implemented now for
this and other processors. Better to add a different function for the
reset cause.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Anatolij Gustschin]

On Thu, 10 Mar 2011 16:07:49 +0100
Stefano Babic <sbabic@denx.de> wrote:

> On 03/10/2011 11:53 AM, Anatolij Gustschin wrote:
> > Add output of the WRSR register content while booting so that
> > we can see the source of the last reset.
> > 
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> 
> Hi Antolij,

Hi Stefano,

...
> > +	printf("CPU:   Freescale i.MX31 at %d MHz (WRSR=0x%04x)\n",
> > +		mx31_get_mcu_main_clk() / 1000000, wdog->wrsr);
> 
> Why is it better to use the wrsr register instead of the rcsr register ?
> We are actually using the rcsr register for other i.MX processors
> (MX51/MX53/MX35). And if we want to print the reset cause, I think it
> should be better to write directly the cause as string instead of the
> register value.

The reason for using wrsr register is that when reading this register,
we can also recognize the software reset cause (SWFT, bit 0 is set).
In our use case we have this requirement. U-Boot and Linux for iMX31
assert system reset by clearing SRS bit in wcr. This software reset
is reported by SWFT in the wrsr register. When I read rcsr after a
software reset, I always get watchdog timeout reset cause (probably
because the reset was performed by writing to the watchdog module
register wcr). But actually this reset was a software reset.

I'm fine with using strings for reset cause if there is no objection.

> I do not think printing the reset cause should be implemented in
> print_cpuinfo(), because it manages a different issue (reset cause
> against CPU information). The print_cpuinfo() should have only CPU
> related values, as clock values, and so on, as it is implemented now for
> this and other processors. Better to add a different function for the
> reset cause.

Were should we call this different function? Should be put it
into init_sequence[]?

Best regards,
Anatolij

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Anatolij Gustschin]

Hi Fabio,

On Thu, 10 Mar 2011 05:47:15 -0800 (PST)
Fabio Estevam <fabioestevam@yahoo.com> wrote:

> Hi Anatolij,
> 
> --- On Thu, 3/10/11, Anatolij Gustschin <agust@denx.de> wrote:
> ....?? ???
> > mx31_get_mcu_main_clk() / 1000000);
> > +??? struct wdog_regs *wdog = (struct> wdog_regs *)WDOG_BASE;
> > +
> > +??? printf("CPU:???Freescale i.MX31 at %d MHz (WRSR=0x%04x)\n",
> > +??
> > mx31_get_mcu_main_clk() / 1000000, wdog->wrsr);
> >  ??? return 0;
> 
> Wouldn?t it be better to show a string for the reset source, rather than a pure register value?

I'm fine with using a string.

Viele Gr??e,
Anatolij

[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset

[Stefano Babic]

On 03/12/2011 02:15 PM, Anatolij Gustschin wrote:

> The reason for using wrsr register is that when reading this register,
> we can also recognize the software reset cause (SWFT, bit 0 is set).
> In our use case we have this requirement.

Ok, understood.

>> I do not think printing the reset cause should be implemented in
>> print_cpuinfo(), because it manages a different issue (reset cause
>> against CPU information). The print_cpuinfo() should have only CPU
>> related values, as clock values, and so on, as it is implemented now for
>> this and other processors. Better to add a different function for the
>> reset cause.
> 
> Were should we call this different function? Should be put it
> into init_sequence[]?

Well, I have not thought as a general function to be inserted in the
init_sequence, I do not know if other ARM processor can export this
function. On other i.MX processors, the reset cause is printed inside
the checkboard function, if CONFIG_DISPLAY_BOARDINFO is set, and not
inside print_cpuinfo(). I understand that the cpuinfo reports only how
to identify the CPU, and the reset cause should be handled separately.

IMHO should be enough to have a mxc_get_reset_cause() (or something like
this) that the board maintainer can call if for some reason he must
perform different actions according to the last reset cause. As name, I
would prefer to identify not static functions starting with mxc_, as I
am trying to uniform all i.MX exponing the same set of functions.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================