[U-Boot] MX31: change return value of get_cpu_rev

Drop warnings in get_cpu_rev and changes the return value
(a u32 instead of char * is returned) of the function
to be coherent with other processors.

Signed-off-by: Stefano Babic <sbabic@denx.de>
CC: Detlev Zundev <dzu@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
---

This is a follow-up patch of "MX31: drop warnings in get_cpu_rev",
as this patch was already integrated in u-boot mainline, and
implements the comments discussed on ML.

- Detlev Zundev: be coherent with other architecture

 arch/arm/cpu/arm1136/mx31/generic.c       |   29 ++++++++++++++++-------------
 arch/arm/include/asm/arch-mx31/imx-regs.h |    2 +-
 2 files changed, 17 insertions(+), 14 deletions(-)

Hi Stefano,

> Drop warnings in get_cpu_rev and changes the return value
> (a u32 instead of char * is returned) of the function
> to be coherent with other processors.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> CC: Detlev Zundev <dzu@denx.de>

Can you please correct the spelling of my name?  Thanks.

> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>
> This is a follow-up patch of "MX31: drop warnings in get_cpu_rev",
> as this patch was already integrated in u-boot mainline, and
> implements the comments discussed on ML.
>
> - Detlev Zundev: be coherent with other architecture
>
>  arch/arm/cpu/arm1136/mx31/generic.c       |   29 ++++++++++++++++-------------
>  arch/arm/include/asm/arch-mx31/imx-regs.h |    2 +-
>  2 files changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
> index 18572b9..461d960 100644
> --- a/arch/arm/cpu/arm1136/mx31/generic.c
> +++ b/arch/arm/cpu/arm1136/mx31/generic.c
> @@ -107,18 +107,18 @@ void mx31_set_pad(enum iomux_pins pin, u32 config)
>  }
>  
>  struct mx3_cpu_type mx31_cpu_type[] = {
> -	{ .srev = 0x00,	.v = "1.0"  },
> -	{ .srev = 0x10,	.v = "1.1"  },
> -	{ .srev = 0x11,	.v = "1.1"  },
> -	{ .srev = 0x12,	.v = "1.15" },
> -	{ .srev = 0x13,	.v = "1.15" },
> -	{ .srev = 0x14,	.v = "1.2"  },
> -	{ .srev = 0x15,	.v = "1.2"  },
> -	{ .srev = 0x28,	.v = "2.0"  },
> -	{ .srev = 0x29,	.v = "2.0"  },
> +	{ .srev = 0x00, .v = 0x10 },
> +	{ .srev = 0x10, .v = 0x11 },
> +	{ .srev = 0x11, .v = 0x11 },
> +	{ .srev = 0x12, .v = 0x1F },
> +	{ .srev = 0x13, .v = 0x1F },
> +	{ .srev = 0x14, .v = 0x12 },
> +	{ .srev = 0x15, .v = 0x12 },
> +	{ .srev = 0x28, .v = 0x20 },
> +	{ .srev = 0x29, .v = 0x20 },
>  };
>  
> -char *get_cpu_rev(void)
> +u32 get_cpu_rev(void)
>  {
>  	u32 i, srev;
>  
> @@ -129,7 +129,7 @@ char *get_cpu_rev(void)
>  	for (i = 0; i < ARRAY_SIZE(mx31_cpu_type); i++)
>  		if (srev == mx31_cpu_type[i].srev)
>  			return mx31_cpu_type[i].v;
> -		return "unknown";
> +	return srev;

Hm, so we drop the "unknown" case and return the srev unchanged.

>  }
>  
>  char *get_reset_cause(void)
> @@ -161,8 +161,11 @@ char *get_reset_cause(void)
>  #if defined(CONFIG_DISPLAY_CPUINFO)
>  int print_cpuinfo (void)
>  {
> -	printf("CPU:   Freescale i.MX31 rev %s at %d MHz.",
> -			get_cpu_rev(), mx31_get_mcu_main_clk() / 1000000);
> +	u32 srev = get_cpu_rev();
> +
> +	printf("CPU:   Freescale i.MX31 rev %d.%d at %d MHz.",
> +			(srev & 0xF0) >> 4, (srev & 0x0F),
> +			mx31_get_mcu_main_clk() / 1000000);

And here we have no way of knowing if the output number is the result of
a correct translation or if our table is insufficient.  This is not
good.

Please provide a "unknown" case again for missing table entries.

Cheers
  Detlev

On 04/29/2011 01:36 PM, Detlev Zundel wrote:
> Hi Stefano,
> 
>> Drop warnings in get_cpu_rev and changes the return value
>> (a u32 instead of char * is returned) of the function
>> to be coherent with other processors.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> CC: Detlev Zundev <dzu@denx.de>
> 
> Can you please correct the spelling of my name?  Thanks.

Sorry, I should know how to write your name...


>> @@ -129,7 +129,7 @@ char *get_cpu_rev(void)
>>  	for (i = 0; i < ARRAY_SIZE(mx31_cpu_type); i++)
>>  		if (srev == mx31_cpu_type[i].srev)
>>  			return mx31_cpu_type[i].v;
>> -		return "unknown";
>> +	return srev;
> 
> Hm, so we drop the "unknown" case and return the srev unchanged.

Yes, I have changed this behavior. I thought, if the revision is not in
the table, it is better to know which is the value of srev register. I
do not know if it is better to print only an "unknown" or get directly
the value of the register, to check in some documentation which new
version was put on the board.

> And here we have no way of knowing if the output number is the result of
> a correct translation or if our table is insufficient.  This is not
> good.
> 
> Please provide a "unknown" case again for missing table entries.

Understood. I will change the behavior as set previously.

Best regards,
Stefano Babic

Hi Stefano,

[...]

>>> @@ -129,7 +129,7 @@ char *get_cpu_rev(void)
>>>  	for (i = 0; i < ARRAY_SIZE(mx31_cpu_type); i++)
>>>  		if (srev == mx31_cpu_type[i].srev)
>>>  			return mx31_cpu_type[i].v;
>>> -		return "unknown";
>>> +	return srev;
>> 
>> Hm, so we drop the "unknown" case and return the srev unchanged.
>
> Yes, I have changed this behavior. I thought, if the revision is not in
> the table, it is better to know which is the value of srev register. I
> do not know if it is better to print only an "unknown" or get directly
> the value of the register, to check in some documentation which new
> version was put on the board.

Well, I do not insist on "unknown" - all I want is some way that someone
reading the message can tell whether it is a translated or an
untranslated value.  I.e. adding a second (sensible) output string for
no translation is also ok for me.

Cheers
  Detlev