[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Fabio Estevam]

Use the same method of the Linux kernel to print the MX31 silicon version on 
boot.

Tested on a MX31PDK with a 2.0 silicon, where it shows:

CPU:   Freescale i.MX31 rev 2.0 at 531 MHz

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>

---
Changes since v4:
- Get rid of imx_soc_revision.h and its macro
Changes since v3:
- Keep consistency with other i.MX processors and print the silicon version 
in the same line as the CPU name
- Remove unneeded blank line in imx_soc_revision.h
Changes since v2:
- Use macro instead of defines for IMX_CHIP_REVISION
Changes since v1:
- rename the CPU detect function name to get_cpu_rev
- Use struct to access iim register
 arch/arm/cpu/arm1136/mx31/generic.c       |   30 +++++++++++++++++++++++++++-
 arch/arm/include/asm/arch-mx31/imx-regs.h |   25 ++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/arm1136/mx31/generic.c b/arch/arm/cpu/arm1136/mx31/generic.c
index fa07fec..9b7a7a2 100644
--- a/arch/arm/cpu/arm1136/mx31/generic.c
+++ b/arch/arm/cpu/arm1136/mx31/generic.c
@@ -106,11 +106,37 @@ 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"  },
+};
+
+char *get_cpu_rev(void)
+{
+	u32 i, srev;
+
+	/* read SREV register from IIM module */
+	struct iim_regs *iim = (struct iim_regs *)MX31_IIM_BASE_ADDR;
+	srev = readl(&iim->iim_srev);
+
+	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";
+}
+
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo (void)
 {
-	printf("CPU:   Freescale i.MX31 at %d MHz\n",
-		mx31_get_mcu_main_clk() / 1000000);
+	printf("CPU:   Freescale i.MX31 rev %s@%d MHz\n",
+			get_cpu_rev(), mx31_get_mcu_main_clk() / 1000000);
 	return 0;
 }
 #endif
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index 37337f2..0eeaf39 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -84,6 +84,29 @@ struct wdog_regs {
 	u16 wrsr;	/* Reset Status */
 };
 
+/* IIM Control Registers */
+struct iim_regs {
+	u32 iim_stat;
+	u32 iim_statm;
+	u32 iim_err;
+	u32 iim_emask;
+	u32 iim_fctl;
+	u32 iim_ua;
+	u32 iim_la;
+	u32 iim_sdat;
+	u32 iim_prev;
+	u32 iim_srev;
+	u32 iim_prog_p;
+	u32 iim_scs0;
+	u32 iim_scs1;
+	u32 iim_scs2;
+	u32 iim_scs3;
+};
+
+struct mx3_cpu_type {
+	u8 srev;
+	const char *v;
+};
 
 #define IOMUX_PADNUM_MASK	0x1ff
 #define IOMUX_PIN(gpionum, padnum) ((padnum) & IOMUX_PADNUM_MASK)
@@ -480,6 +503,8 @@ enum iomux_pins {
 #define CCMR_FPM	(1 << 1)
 #define CCMR_CKIH	(2 << 1)
 
+#define MX31_IIM_BASE_ADDR	0x5001C000
+
 #define PDR0_CSI_PODF(x)	(((x) & 0x1ff) << 23)
 #define PDR0_PER_PODF(x)	(((x) & 0x1f) << 16)
 #define PDR0_HSP_PODF(x)	(((x) & 0x7) << 11)
-- 
1.6.0.4

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Albert ARIBAUD]

Le 12/04/2011 04:18, Fabio Estevam a ?crit :
> Use the same method of the Linux kernel to print the MX31 silicon version on
> boot.
>
> Tested on a MX31PDK with a 2.0 silicon, where it shows:
>
> CPU:   Freescale i.MX31 rev 2.0 at 531 MHz
>
> Signed-off-by: Fabio Estevam<fabio.estevam@freescale.com>
>
> ---
> Changes since v4:
> - Get rid of imx_soc_revision.h and its macro
> Changes since v3:
> - Keep consistency with other i.MX processors and print the silicon version
> in the same line as the CPU name
> - Remove unneeded blank line in imx_soc_revision.h
> Changes since v2:
> - Use macro instead of defines for IMX_CHIP_REVISION
> Changes since v1:
> - rename the CPU detect function name to get_cpu_rev
> - Use struct to access iim register
>   arch/arm/cpu/arm1136/mx31/generic.c       |   30 +++++++++++++++++++++++++++-
>   arch/arm/include/asm/arch-mx31/imx-regs.h |   25 ++++++++++++++++++++++++
>   2 files changed, 53 insertions(+), 2 deletions(-)

Seems this is ok.

Stefano, will you take this patch in and send a pull request, or do you 
want me to pick it?

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[stefano babic]

Am 14/04/2011 13:14, schrieb Albert ARIBAUD:

> 
> Seems this is ok.

It is for me ok, too.

> 
> Stefano, will you take this patch in and send a pull request, or do you
> want me to pick it?

I will take this patch and I wiil send you a pull request.

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
=====================================================================

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Stefano Babic]

On 04/12/2011 04:18 AM, Fabio Estevam wrote:
> Use the same method of the Linux kernel to print the MX31 silicon version on 
> boot.
> 
> Tested on a MX31PDK with a 2.0 silicon, where it shows:
> 
> CPU:   Freescale i.MX31 rev 2.0 at 531 MHz
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> 

Applied to u-boot-imx, thanks.

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 v5] ARM: mx31: Print the silicon version

[Stefano Babic]

On 04/15/2011 08:07 PM, Stefano Babic wrote:
> On 04/12/2011 04:18 AM, Fabio Estevam wrote:
>> Use the same method of the Linux kernel to print the MX31 silicon version on 
>> boot.
>>
>> Tested on a MX31PDK with a 2.0 silicon, where it shows:
>>
>> CPU:   Freescale i.MX31 rev 2.0 at 531 MHz
>>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>>
> 
> Applied to u-boot-imx, thanks.
> 

Fabio,

I have not noted before that your patch introduce a warning:

generic.c: In function 'get_cpu_rev':
generic.c:131: warning: return discards qualifiers from pointer target type

This is due to the usage of the const in the mx3_cpu_type:

struct mx3_cpu_type {
        u8 srev;
       const char *v;

Do you agree if I drop myself the const attribute on u-boot-imx before
pulling your patch to the arm tree ?

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
=====================================================================

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Detlev Zundel]

Hi Stefano,

> On 04/15/2011 08:07 PM, Stefano Babic wrote:
>> On 04/12/2011 04:18 AM, Fabio Estevam wrote:
>>> Use the same method of the Linux kernel to print the MX31 silicon version on 
>>> boot.
>>>
>>> Tested on a MX31PDK with a 2.0 silicon, where it shows:
>>>
>>> CPU:   Freescale i.MX31 rev 2.0 at 531 MHz
>>>
>>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>>>
>> 
>> Applied to u-boot-imx, thanks.
>> 
>
> Fabio,
>
> I have not noted before that your patch introduce a warning:
>
> generic.c: In function 'get_cpu_rev':
> generic.c:131: warning: return discards qualifiers from pointer target type
>
> This is due to the usage of the const in the mx3_cpu_type:
>
> struct mx3_cpu_type {
>         u8 srev;
>        const char *v;
>
> Do you agree if I drop myself the const attribute on u-boot-imx before
> pulling your patch to the arm tree ?

Sorry to jump in late, but why not change get_cpu_rev to 'const char *'
which it actually is?

Cheers
  Detlev

-- 
Emacs is the way to purify your soul using garbage collection.
--
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 v5] ARM: mx31: Print the silicon version

[Stefano Babic]

On 04/27/2011 11:11 AM, Detlev Zundel wrote:

>>>> This is due to the usage of the const in the mx3_cpu_type:
>>
>> struct mx3_cpu_type {
>>         u8 srev;
>>        const char *v;
>>
>> Do you agree if I drop myself the const attribute on u-boot-imx before
>> pulling your patch to the arm tree ?
> 
> Sorry to jump in late, 

.. not so late, we can change it...

>but why not change get_cpu_rev to 'const char *'
> which it actually is?

This is correct. However, I have not noted before that the last
introduced get_cpu_rev() in MX31 is an exception in u-boot. For all
other processors, it returns a u32 and it is defined as u32
get_cpu_rev(void).

Fabio's patch introduces a variant, because it is defined as const char
*get_cpu_rev(void). I propose to change its name (get_cpu_rev_string ?)
and add the static attribute, to make clear it is different as the
get_cpu_rev() already implemented in u-boot.

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
=====================================================================

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Fabio Estevam]

Hi Stefano,

On 4/27/2011 6:40 AM, Stefano Babic wrote:
...
>> but why not change get_cpu_rev to 'const char *'
>> which it actually is?
> 
> This is correct. However, I have not noted before that the last
> introduced get_cpu_rev() in MX31 is an exception in u-boot. For all
> other processors, it returns a u32 and it is defined as u32
> get_cpu_rev(void).
> 
> Fabio's patch introduces a variant, because it is defined as const char
> *get_cpu_rev(void). I propose to change its name (get_cpu_rev_string ?)
> and add the static attribute, to make clear it is different as the
> get_cpu_rev() already implemented in u-boot.

Ok, I can change it as per your suggestion.

Will send a patch soon.

Regards,

Fabio Estevam

[U-Boot] [PATCH v5] ARM: mx31: Print the silicon version

[Stefano Babic]

On 04/27/2011 03:16 PM, Fabio Estevam wrote:

Hi Fabio,

> Will send a patch soon.

Ok, thanks.

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 v5] ARM: mx31: Print the silicon version

[Detlev Zundel]

Hi Stefano,

> On 04/27/2011 11:11 AM, Detlev Zundel wrote:
>
>>>>> This is due to the usage of the const in the mx3_cpu_type:
>>>
>>> struct mx3_cpu_type {
>>>         u8 srev;
>>>        const char *v;
>>>
>>> Do you agree if I drop myself the const attribute on u-boot-imx before
>>> pulling your patch to the arm tree ?
>> 
>> Sorry to jump in late, 
>
> .. not so late, we can change it...
>
>>but why not change get_cpu_rev to 'const char *'
>> which it actually is?
>
> This is correct. However, I have not noted before that the last
> introduced get_cpu_rev() in MX31 is an exception in u-boot. For all
> other processors, it returns a u32 and it is defined as u32
> get_cpu_rev(void).
>
> Fabio's patch introduces a variant, because it is defined as const char
> *get_cpu_rev(void). I propose to change its name (get_cpu_rev_string ?)
> and add the static attribute, to make clear it is different as the
> get_cpu_rev() already implemented in u-boot.

I see.  Can't we consolidate all those functions into more general
functions higher up in the specialization graph?  I.e. why have static
non-conforming functions when we could have one generic function to
handle all users?

Cheers
  Detlev

-- 
System going down at 1:45 this afternoon for disk crashing.
--
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 v5] ARM: mx31: Print the silicon version

[Stefano Babic]

On 04/27/2011 04:57 PM, Detlev Zundel wrote:
> I see.  Can't we consolidate all those functions into more general
> functions higher up in the specialization graph?

As I can see, the get_cpu_rev() is already a generalized function, used
in more processors. IMHO we should switch back to the usual prototype
u32 get_cpu_rev(), and print_cpuinfo() should transform the revision
number in a human readable way.

>  I.e. why have static
> non-conforming functions when we could have one generic function to
> handle all users?

Yes, agree. Really I have not seen that get_cpu_rev() changed returning
a string, instead of u32 as usual. This must be fixed.

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
=====================================================================