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

login
register
mail settings
Submitter Fabio Estevam
Date April 12, 2011, 2:18 a.m.
Message ID <1302574692-28134-1-git-send-email-festevam@gmail.com>
Download mbox | patch
Permalink /patch/90692/
State Accepted
Commit 4adaf9bf097b57af12f9a598759f8c9990e3502e
Delegated to: Stefano Babic
Headers show

Comments

Fabio Estevam - April 12, 2011, 2:18 a.m.
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(-)
Albert ARIBAUD - April 14, 2011, 11:14 a.m.
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,
Stefano Babic - April 14, 2011, 8:53 p.m.
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
Stefano Babic - April 15, 2011, 6:07 p.m.
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
Stefano Babic - April 21, 2011, 4:01 p.m.
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
Detlev Zundel - April 27, 2011, 9:11 a.m.
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
Stefano Babic - April 27, 2011, 9:40 a.m.
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
Fabio Estevam - April 27, 2011, 1:16 p.m.
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
Stefano Babic - April 27, 2011, 2 p.m.
On 04/27/2011 03:16 PM, Fabio Estevam wrote:

Hi Fabio,

> Will send a patch soon.

Ok, thanks.

Regards,
Stefano Babic
Detlev Zundel - April 27, 2011, 2:57 p.m.
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
Stefano Babic - April 27, 2011, 4:01 p.m.
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

Patch

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 at %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)