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

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

[Stefano Babic]

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(-)

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;
 }
 
 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@%d MHz.",
+			(srev & 0xF0) >> 4, (srev & 0x0F),
+			mx31_get_mcu_main_clk() / 1000000);
 	printf("Reset cause: %s\n", get_reset_cause());
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index c830a03..306f966 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -105,7 +105,7 @@ struct iim_regs {
 
 struct mx3_cpu_type {
 	u8 srev;
-	char *v;
+	u32 v;
 };
 
 #define IOMUX_PADNUM_MASK	0x1ff
-- 
1.7.1

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

[Detlev Zundel]

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

-- 
Math and Alcohol don't mix, so...
PLEASE DON'T DRINK AND DERIVE

[Motto of the society: Mathematicians Against Drunk Deriving]
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

[Stefano Babic]

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

-- 
=====================================================================
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] MX31: change return value of get_cpu_rev

[Detlev Zundel]

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

-- 
Man is a fool, and woman, for tolerating him, is a damned fool
                       -- Mark Twain
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

[Stefano Babic]

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 Zundel <dzu@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
---

Changes:
- commit message
- "unknown" will be printed together with the
	revision register if the value of the register
	cannot be translated.

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

diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
index 18572b9..af6da91 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,8 @@ 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 | 0x8000;
 }
 
 char *get_reset_cause(void)
@@ -161,8 +162,12 @@ 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 %s@%d MHz.",
+			(srev & 0xF0) >> 4, (srev & 0x0F),
+			((srev & 0x8000) ? "unknown" : ""),
+			mx31_get_mcu_main_clk() / 1000000);
 	printf("Reset cause: %s\n", get_reset_cause());
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index c830a03..306f966 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -105,7 +105,7 @@ struct iim_regs {
 
 struct mx3_cpu_type {
 	u8 srev;
-	char *v;
+	u32 v;
 };
 
 #define IOMUX_PADNUM_MASK	0x1ff
-- 
1.7.1

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

[Detlev Zundel]

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 Zundel <dzu@denx.de>
> CC: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>
> Changes:
> - commit message
> - "unknown" will be printed together with the
> 	revision register if the value of the register
> 	cannot be translated.
>
>  arch/arm/cpu/arm1136/mx31/generic.c       |   31 ++++++++++++++++------------
>  arch/arm/include/asm/arch-mx31/imx-regs.h |    2 +-
>  2 files changed, 19 insertions(+), 14 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
> index 18572b9..af6da91 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,8 @@ 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 | 0x8000;
>  }
>  
>  char *get_reset_cause(void)
> @@ -161,8 +162,12 @@ 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 %s at %d MHz.",

Use a space less here, i.e. "rev %d.%d%s at %d"...

> +			(srev & 0xF0) >> 4, (srev & 0x0F),
> +			((srev & 0x8000) ? "unknown" : ""),

.. and " unknown" here, so we always get only one space.

Ok, ok, I'm getting carried away ;)

Cheers
  Detlev

-- 
SWYM XYZ (Sympathize with your machinery).  [..] In fact it is often
called a no-op, because it performs no operation.  It does, however,
keep the machine running smoothly, just as real-world swimming helps
to keep programmers healthy.             -- Donald Knuth, Fascicle 1
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

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

[Stefano Babic]

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 Zundel <dzu@denx.de>
CC: Fabio Estevam <fabio.estevam@freescale.com>
---

Changed since V2:

- Formatting of cpu revision in print_cpuinfo

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

diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
index 18572b9..fccd2cd 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,8 @@ 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 | 0x8000;
 }
 
 char *get_reset_cause(void)
@@ -161,8 +162,12 @@ 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%s@%d MHz.",
+			(srev & 0xF0) >> 4, (srev & 0x0F),
+			((srev & 0x8000) ? " unknown" : ""),
+			mx31_get_mcu_main_clk() / 1000000);
 	printf("Reset cause: %s\n", get_reset_cause());
 	return 0;
 }
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index c830a03..306f966 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -105,7 +105,7 @@ struct iim_regs {
 
 struct mx3_cpu_type {
 	u8 srev;
-	char *v;
+	u32 v;
 };
 
 #define IOMUX_PADNUM_MASK	0x1ff
-- 
1.7.1