Patchwork [U-Boot,V3,1/2] MX5: factor out boot cause funciton to common code

login
register
mail settings
Submitter Liu Hui-R64343
Date April 15, 2011, 7:08 a.m.
Message ID <1302851307-22244-1-git-send-email-r64343@freescale.com>
Download mbox | patch
Permalink /patch/91322/
State Superseded
Headers show

Comments

Liu Hui-R64343 - April 15, 2011, 7:08 a.m.
From: Liu Hui-R64343 <r64343@freescale.com>

factor out boot cause funciton to common code to avoid
the duplicate code in each board support package

Signed-off-by: Jason Liu <r64343@freescale.com>
---
 arch/arm/cpu/armv7/mx5/soc.c              |   18 ++++++++++++++++++
 arch/arm/include/asm/arch-mx5/sys_proto.h |    1 +
 2 files changed, 19 insertions(+), 0 deletions(-)
Fabio Estevam - April 15, 2011, 12:47 p.m.
Hi Jason,

--- On Fri, 4/15/11, Jason Liu <r64343@freescale.com> wrote:
...
> +char *get_reset_cause(void)
> +{
> +    u32 cause;
> +    struct src *src_regs = (struct src
> *)SRC_BASE_ADDR;
> +
> +    cause = readl(&src_regs->srsr);

You need to mask the 7 LSB of SRSR register.

If you don´t bit 16 can still affect its result.

Regards,

Fabio Estevam
Stefano Babic - April 15, 2011, 6:35 p.m.
On 04/15/2011 02:47 PM, Fabio Estevam wrote:
>> +char *get_reset_cause(void)
>> +{
>> +    u32 cause;
>> +    struct src *src_regs = (struct src
>> *)SRC_BASE_ADDR;
>> +
>> +    cause = readl(&src_regs->srsr);
> 
> You need to mask the 7 LSB of SRSR register.
> 
> If you don´t bit 16 can still affect its result.

Why ? As this becomes a general function for i.MX5, should we not
provide a way to check all significant bits ? Why should we exclude the
"warm boot" bit to be checked and printed out ?

Regards,
Stefano Babic
Detlev Zundel - April 18, 2011, 9:19 a.m.
Hi Stefano,

> On 04/15/2011 02:47 PM, Fabio Estevam wrote:
>>> +char *get_reset_cause(void)
>>> +{
>>> +    u32 cause;
>>> +    struct src *src_regs = (struct src
>>> *)SRC_BASE_ADDR;
>>> +
>>> +    cause = readl(&src_regs->srsr);
>> 
>> You need to mask the 7 LSB of SRSR register.
>> 
>> If you don´t bit 16 can still affect its result.
>
> Why ? As this becomes a general function for i.MX5, should we not
> provide a way to check all significant bits ? Why should we exclude the
> "warm boot" bit to be checked and printed out ?

And _please_ (as indictated in my i.MX31 mail) use the code for _all_
iMX51 boards withoput the need for them to call a function and print the
result.

Thanks!
  Detlev
Stefano Babic - April 20, 2011, 9:54 a.m.
On 04/18/2011 11:19 AM, Detlev Zundel wrote:
> Hi Stefano,
> 
>> On 04/15/2011 02:47 PM, Fabio Estevam wrote:
>>>> +char *get_reset_cause(void)
>>>> +{
>>>> +    u32 cause;
>>>> +    struct src *src_regs = (struct src
>>>> *)SRC_BASE_ADDR;
>>>> +
>>>> +    cause = readl(&src_regs->srsr);
>>>
>>> You need to mask the 7 LSB of SRSR register.
>>>
>>> If you don´t bit 16 can still affect its result.
>>
>> Why ? As this becomes a general function for i.MX5, should we not
>> provide a way to check all significant bits ? Why should we exclude the
>> "warm boot" bit to be checked and printed out ?
> 
> And _please_ (as indictated in my i.MX31 mail) use the code for _all_
> iMX51 boards withoput the need for them to call a function and print the
> result.

Jason,

I noted only now that this comment was not directly addressed to you,
but it is related to your patch. As you see for the i.MX31, the result
of the discussion was to call the get_reset_cause() inside the
print_cpuinfo() function, to make automatically this function available
for all MX5 boards.

Regards,
Stefano
Jason Liu - April 20, 2011, 10:50 a.m.
Hi, Stefano,

2011/4/20 Stefano Babic <sbabic@denx.de>:
> On 04/18/2011 11:19 AM, Detlev Zundel wrote:
>> Hi Stefano,
>>
>>> On 04/15/2011 02:47 PM, Fabio Estevam wrote:
>>>>> +char *get_reset_cause(void)
>>>>> +{
>>>>> +    u32 cause;
>>>>> +    struct src *src_regs = (struct src
>>>>> *)SRC_BASE_ADDR;
>>>>> +
>>>>> +    cause = readl(&src_regs->srsr);
>>>>
>>>> You need to mask the 7 LSB of SRSR register.
>>>>
>>>> If you don´t bit 16 can still affect its result.
>>>
>>> Why ? As this becomes a general function for i.MX5, should we not
>>> provide a way to check all significant bits ? Why should we exclude the
>>> "warm boot" bit to be checked and printed out ?
>>
>> And _please_ (as indictated in my i.MX31 mail) use the code for _all_
>> iMX51 boards withoput the need for them to call a function and print the
>> result.
>
> Jason,
>
> I noted only now that this comment was not directly addressed to you,
> but it is related to your patch. As you see for the i.MX31, the result
> of the discussion was to call the get_reset_cause() inside the
> print_cpuinfo() function, to make automatically this function available
> for all MX5 boards.

Yes, I have send out the v5 patch for it. please review it. Thanks,

Jason

>
> 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@denx.de
> =====================================================================
>

Patch

diff --git a/arch/arm/cpu/armv7/mx5/soc.c b/arch/arm/cpu/armv7/mx5/soc.c
index 09500b3..dddda08 100644
--- a/arch/arm/cpu/armv7/mx5/soc.c
+++ b/arch/arm/cpu/armv7/mx5/soc.c
@@ -77,6 +77,24 @@  u32 get_cpu_rev(void)
 	return system_rev;
 }
 
+char *get_reset_cause(void)
+{
+	u32 cause;
+	struct src *src_regs = (struct src *)SRC_BASE_ADDR;
+
+	cause = readl(&src_regs->srsr);
+	switch (cause) {
+	case 0x0001:
+		return "POR";
+	case 0x0009:
+		return "RST";
+	case 0x0010:
+	case 0x0011:
+		return "WDOG";
+	default:
+		return "unknown reset";
+	}
+}
 
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo(void)
diff --git a/arch/arm/include/asm/arch-mx5/sys_proto.h b/arch/arm/include/asm/arch-mx5/sys_proto.h
index f687503..00c2c5d 100644
--- a/arch/arm/include/asm/arch-mx5/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx5/sys_proto.h
@@ -27,5 +27,6 @@ 
 u32 get_cpu_rev(void);
 #define is_soc_rev(rev)	((get_cpu_rev() & 0xFF) - rev)
 void sdelay(unsigned long);
+char *get_reset_cause(void);
 
 #endif