[U-Boot] [PATCH] Kirkwood: print_cpuinfo fixed for valid devid revid

[U-Boot] [PATCH] Kirkwood: print_cpuinfo fixed for valid devid revid

[Albert ARIBAUD]

Hi Prafulla,

Le 16/09/2010 16:52, Prafulla Wadaskar a ?crit :

 > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c 
b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
 > index 82c978b..5feb0b5 100644
 > --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
 > +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c

 > @@ -271,20 +272,32 @@ static void kw_sysrst_check(void)
 >   #if defined(CONFIG_DISPLAY_CPUINFO)
 >   int print_cpuinfo(void)
 >   {
 > -	char *name = "Unknown";
 > +	char *rev, name[6];
 > +	u16 devid = (readl(KW_REG_PCIE_DEVID)>>  16)&  0xffff;
 > +	u8 revid = readl(KW_REG_PCIE_REVID)&  0xff;
 >
 > -	switch (readl(KW_REG_DEVICE_ID)&  0x03) {
 > -	case 1:
 > -		name = "88F6192_A0";
 > +	if ((readl(KW_REG_DEVICE_ID)&  0x03)>  2) {
 > +		debug("Error.. %s:Unsupported Kirkwood SoC\n", __FUNCTION__);

Maybe you could add the actual content of the device ID register so that 
the operator can read it on the console?

 > +		return -1;
 > +	}
 > +
 > +	switch (revid) {
 > +	case 0:
 > +		rev = "Z0";
 >   		break;
 >   	case 2:
 > -		name = "88F6281_A0";
 > +		rev = "A0";
 > +		break;
 > +	case 3:
 > +		rev = "A1";
 >   		break;
 >   	default:
 > -		printf("SoC:   Unsupported Kirkwood\n");
 > -		return -1;
 > +		rev = "??";

Same here with this "??": replacing it with the actual revision ID 
register value would allow the operator to read it directly on the console.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH] Kirkwood: print_cpuinfo fixed for valid devid revid

[Wolfgang Denk]

Dear Prafulla Wadaskar,

In message <1284648724-6871-1-git-send-email-prafulla@marvell.com> you wrote:
> Earlier Device Identification register was used to detect
> the type for SoC, considering 88F6282 support to be added,
> It is not possible to detect the same using current
> algorithm.
> 
> With this patch, device ID is being read using PCIE devid
> register, also valid chip revision ID will also be read and
> displayed
> 
> Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
...
> +	if ((readl(KW_REG_DEVICE_ID) & 0x03) > 2) {
> +		debug("Error.. %s:Unsupported Kirkwood SoC\n", __FUNCTION__);
> +		return -1;

Please use a printf() instead of debug() here - error messages should
always be printed.

> +	sprintf(name, "%04x", devid);
> +	printf("SoC:   Kirkwood 88F%s_%s\n", name, rev);

Why do you need a separate sprintf() here?  Why not simply:

	printf("SoC:   Kirkwood 88F%04x_%s\n", devid, rev);

?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Am besten betrachten Sie Fehlermeldungen als eine  Art  Psycho-Test,
mit  dem  herausgefunden  werden soll, wie belastbar Sie sind."
 - Dr. R. Wonneberger, Kompaktf?hrer LaTeX, Kap. 1.6: Fehlermeldungen

[U-Boot] [PATCH] Kirkwood: print_cpuinfo fixed for valid devid revid

[Prafulla Wadaskar]

Earlier Device Identification register was used to detect
the type for SoC, considering 88F6282 support to be added,
It is not possible to detect the same using current
algorithm.

With this patch, device ID is being read using PCIE devid
register, also valid chip revision ID will also be read and
displayed

Signed-off-by: Prafulla Wadaskar <prafulla@marvell.com>
---
 arch/arm/cpu/arm926ejs/kirkwood/cpu.c    |   35 ++++++++++++++++++++---------
 arch/arm/include/asm/arch-kirkwood/cpu.h |    2 +
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
index 82c978b..5feb0b5 100644
--- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
+++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
@@ -54,10 +54,11 @@ unsigned char get_random_hex(void)
 	u8 outbuf[BUFLEN];
 
 	/*
-	 * in case of 88F6281/88F6192 A0,
+	 * in case of 88F6281/88F6282/88F6192 A0,
 	 * Bit7 need to reset to generate random values in KW_REG_UNDOC_0x1470
-	 * Soc reg offsets KW_REG_UNDOC_0x1470 and KW_REG_UNDOC_0x1478 are reserved regs and
-	 * Does not have names at this moment (no errata available)
+	 * Soc reg offsets KW_REG_UNDOC_0x1470 and KW_REG_UNDOC_0x1478 are
+	 * reserved regs and does not have names at this moment
+	 * (no errata available)
 	 */
 	writel(readl(KW_REG_UNDOC_0x1478) & ~(1 << 7), KW_REG_UNDOC_0x1478);
 	for (i = 0; i < BUFLEN; i++) {
@@ -271,20 +272,32 @@ static void kw_sysrst_check(void)
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo(void)
 {
-	char *name = "Unknown";
+	char *rev, name[6];
+	u16 devid = (readl(KW_REG_PCIE_DEVID) >> 16) & 0xffff;
+	u8 revid = readl(KW_REG_PCIE_REVID) & 0xff;
 
-	switch (readl(KW_REG_DEVICE_ID) & 0x03) {
-	case 1:
-		name = "88F6192_A0";
+	if ((readl(KW_REG_DEVICE_ID) & 0x03) > 2) {
+		debug("Error.. %s:Unsupported Kirkwood SoC\n", __FUNCTION__);
+		return -1;
+	}
+
+	switch (revid) {
+	case 0:
+		rev = "Z0";
 		break;
 	case 2:
-		name = "88F6281_A0";
+		rev = "A0";
+		break;
+	case 3:
+		rev = "A1";
 		break;
 	default:
-		printf("SoC:   Unsupported Kirkwood\n");
-		return -1;
+		rev = "??";
+		break;
 	}
-	printf("SoC:   Kirkwood %s\n", name);
+
+	sprintf(name, "%04x", devid);
+	printf("SoC:   Kirkwood 88F%s_%s\n", name, rev);
 	return 0;
 }
 #endif /* CONFIG_DISPLAY_CPUINFO */
diff --git a/arch/arm/include/asm/arch-kirkwood/cpu.h b/arch/arm/include/asm/arch-kirkwood/cpu.h
index b3022a3..d28c51a 100644
--- a/arch/arm/include/asm/arch-kirkwood/cpu.h
+++ b/arch/arm/include/asm/arch-kirkwood/cpu.h
@@ -35,6 +35,8 @@
 #define KWGBE_PORT_SERIAL_CONTROL1_REG(_x)	\
 		((_x ? KW_EGIGA0_BASE : KW_EGIGA1_BASE) + 0x44c)
 
+#define KW_REG_PCIE_DEVID		(KW_REG_PCIE_BASE + 0x00)
+#define KW_REG_PCIE_REVID		(KW_REG_PCIE_BASE + 0x08)
 #define KW_REG_DEVICE_ID		(KW_MPP_BASE + 0x34)
 #define KW_REG_SYSRST_CNT		(KW_MPP_BASE + 0x50)
 #define SYSRST_CNT_1SEC_VAL		(25*1000000)
-- 
1.5.3.3