[U-Boot] [PATCH 1/2] ARM: MX31: print WRSR to indicate the source of the last reset
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
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
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
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
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
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
participants (3)
-
Anatolij Gustschin -
Fabio Estevam -
Stefano Babic