mx5: drop boot cause code from board support code

PATCH New Jason Liu 4 participants 17 messages Apr 20, 2011 – Apr 22, 2011

Subsystems (2)

Arm (32-bit) Arm i.MX and Layerscape

lore

Share & tools

lore

Jason Liu

author

2011-04-20 10:47 UTC #0 lore raw

PATCH 1/1 mx5: drop boot cause code from board support code

The boot cause code has been factor out to soc common code,we need drop the part from the board support code

Signed-off-by Jason Liu <[email protected]>

---
 board/efikamx/efikamx.c           |   30 ++++++------------------------
 board/freescale/mx51evk/mx51evk.c |   26 ++++++--------------------
 board/freescale/mx53evk/mx53evk.c |   21 +--------------------
 board/ttcontrol/vision2/vision2.c |   28 ++++++----------------------
 4 files changed, 19 insertions(+), 86 deletions(-)

board/efikamx/efikamx.c +6 ~~-24~~

index f735260..0aef654 100644
@@ -644,46 +644,28 @@ int board_late_init(void)
 int checkboard(void)
 {
 	u32 system_rev = get_cpu_rev();
-	u32 cause;
-	struct src *src_regs = (struct src *)SRC_BASE_ADDR;
 
 	puts("Board: Efika MX ");
 
 	switch (system_rev & 0xff) {
 	case CHIP_REV_3_0:
-		puts("3.0 [");
+		puts("3.0");
 		break;
 	case CHIP_REV_2_5:
-		puts("2.5 [");
+		puts("2.5");
 		break;
 	case CHIP_REV_2_0:
-		puts("2.0 [");
+		puts("2.0");
 		break;
 	case CHIP_REV_1_1:
-		puts("1.1 [");
+		puts("1.1");
 		break;
 	case CHIP_REV_1_0:
 	default:
-		puts("1.0 [");
+		puts("1.0");
 		break;
 	}
 
-	cause = src_regs->srsr;
-	switch (cause) {
-	case 0x0001:
-		puts("POR");
-		break;
-	case 0x0009:
-		puts("RST");
-		break;
-	case 0x0010:
-	case 0x0011:
-		puts("WDOG");
-		break;
-	default:
-		printf("unknown 0x%x", cause);
-	}
-	puts("]\n");
-
+	puts("\n");
 	return 0;
 }

board/freescale/mx51evk/mx51evk.c +6 ~~-20~~

index 02a765d..cff2ff1 100644
@@ -435,37 +435,23 @@ int checkboard(void)
 
 	switch (system_rev & 0xff) {
 	case CHIP_REV_3_0:
-		puts("3.0 [");
+		puts("3.0");
 		break;
 	case CHIP_REV_2_5:
-		puts("2.5 [");
+		puts("2.5");
 		break;
 	case CHIP_REV_2_0:
-		puts("2.0 [");
+		puts("2.0");
 		break;
 	case CHIP_REV_1_1:
-		puts("1.1 [");
+		puts("1.1");
 		break;
 	case CHIP_REV_1_0:
 	default:
-		puts("1.0 [");
+		puts("1.0");
 		break;
 	}
 
-	switch (__raw_readl(SRC_BASE_ADDR + 0x8)) {
-	case 0x0001:
-		puts("POR");
-		break;
-	case 0x0009:
-		puts("RST");
-		break;
-	case 0x0010:
-	case 0x0011:
-		puts("WDOG");
-		break;
-	default:
-		puts("unknown");
-	}
-	puts("]\n");
+	puts("\n");
 	return 0;
 }

board/freescale/mx53evk/mx53evk.c +1 ~~-20~~

index e71701b..a89aa25 100644
@@ -372,26 +372,7 @@ int board_late_init(void)
 
 int checkboard(void)
 {
-	u32 cause;
-	struct src *src_regs = (struct src *)SRC_BASE_ADDR;
+	puts("Board: MX53EVK\n");
 
-	puts("Board: MX53EVK [");
-
-	cause = src_regs->srsr;
-	switch (cause) {
-	case 0x0001:
-		printf("POR");
-		break;
-	case 0x0009:
-		printf("RST");
-		break;
-	case 0x0010:
-	case 0x0011:
-		printf("WDOG");
-		break;
-	default:
-		printf("unknown");
-	}
-	printf("]\n");
 	return 0;
 }

board/ttcontrol/vision2/vision2.c +6 ~~-22~~

index f8ef4fc..8423110 100644
@@ -708,40 +708,24 @@ int checkboard(void)
 
 	switch (system_rev & 0xff) {
 	case CHIP_REV_3_0:
-		puts("3.0 [");
+		puts("3.0");
 		break;
 	case CHIP_REV_2_5:
-		puts("2.5 [");
+		puts("2.5");
 		break;
 	case CHIP_REV_2_0:
-		puts("2.0 [");
+		puts("2.0");
 		break;
 	case CHIP_REV_1_1:
-		puts("1.1 [");
+		puts("1.1");
 		break;
 	case CHIP_REV_1_0:
 	default:
-		puts("1.0 [");
+		puts("1.0");
 		break;
 	}
 
-	cause = src_regs->srsr;
-	switch (cause) {
-	case 0x0001:
-		puts("POR");
-		break;
-	case 0x0009:
-		puts("RST");
-		break;
-	case 0x0010:
-	case 0x0011:
-		puts("WDOG");
-		break;
-	default:
-		printf("unknown 0x%x", cause);
-	}
-	puts("]\n");
-
+	puts("\n");
 	return 0;
 }

Signature · 1 line

1.7.1

Jason Liu

author

2011-04-20 10:47 UTC #1 lore raw

PATCH v5 1/2 MX5: factor out boot cause funciton to common code

factor out boot cause funciton to common code to avoid the duplicate code in each board support package

Signed-off-by Jason Liu <[email protected]>

---
change since v4:
- make common code soc specific
changes since v3:
- add full boot reset cause

---
 arch/arm/cpu/armv7/mx5/soc.c |   28 ++++++++++++++++++++++++++++
 1 files changed, 28 insertions(+), 0 deletions(-)

arch/arm/cpu/armv7/mx5/soc.c +28 -0

index 09500b3..6f4e8db 100644
@@ -77,6 +77,33 @@ u32 get_cpu_rev(void)
 	return system_rev;
 }
 
+static char *get_reset_cause(void)
+{
+	u32 cause;
+	struct src *src_regs = (struct src *)SRC_BASE_ADDR;
+
+	cause = readl(&src_regs->srsr);
+	writel(cause, &src_regs->srsr);
+
+	switch (cause) {
+	case 0x00001:
+		return "POR";
+	case 0x00004:
+		return "CSU";
+	case 0x00008:
+		return "IPP USER";
+	case 0x00010:
+		return "WDOG";
+	case 0x00020:
+		return "JTAG HIGH-Z";
+	case 0x00040:
+		return "JTAG SW";
+	case 0x10000:
+		return "WARM BOOT";
+	default:
+		return "unknown reset";
+	}
+}
 
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo(void)

@@ -89,6 +116,7 @@ int print_cpuinfo(void)
 		(cpurev & 0x000F0) >> 4,
 		(cpurev & 0x0000F) >> 0,
 		mxc_get_clock(MXC_ARM_CLK) / 1000000);
+	printf("Reset cause: %s\n", get_reset_cause());
 	return 0;
 }
 #endif

Signature · 1 line

1.7.1

Stefano Babic

2011-04-21 17:53 UTC #2 lore raw

PATCH v5 1/2 MX5: factor out boot cause funciton to common code

Quoted 10 lines · Jason Liu

factor out boot cause funciton to common code to avoid the duplicate code in each board support package
Signed-off-by Jason Liu <[email protected]>
---
change since v4:
- make common code soc specific
changes since v3:
- add full boot reset cause
---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines

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

Jason Liu

author

2011-04-20 10:47 UTC #3 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

In reply to Jason Liu

This patch add initial support for freescale MX53LOCO board. Network(FEC),SD/MMC, UART have been supported by this patch.

Signed-off-by Jason Liu <[email protected]>

---
changes since v4:
- remove the boot reset cause from board support changes since v3:
- include other two small patch into this commit,
mx53loco: set mmc env to MMC slot1
MX5: Enable flat-device-tree support on mx53 loco board changes since V2:
-factor out the boot cause function to common code, -fix gd->ram_size with full memory size
-remove the '1' from all #defines that just enable features -correct memory test end address value

---
 MAINTAINERS                           |    1 +
 arch/arm/cpu/armv7/mx5/soc.c          |    2 +-
 board/freescale/mx53loco/Makefile     |   47 +++++
 board/freescale/mx53loco/imximage.cfg |   96 +++++++++++
 board/freescale/mx53loco/mx53loco.c   |  301 +++++++++++++++++++++++++++++++++
 boards.cfg                            |    1 +
 include/configs/mx53loco.h            |  199 ++++++++++++++++++++++
 7 files changed, 646 insertions(+), 1 deletions(-)

MAINTAINERS +1 -0

index 4756f14..7c311f7 100644
@@ -567,6 +567,7 @@ Stefano Babic <[email protected]>
 Jason Liu <[email protected]>
 
 	mx53evk         i.MX53
+	mx53loco        i.MX53
 
 Enric Balletbo i Serra <[email protected]>

arch/arm/cpu/armv7/mx5/soc.c +1 -1

index 6f4e8db..9c03474 100644
@@ -116,7 +116,7 @@ int print_cpuinfo(void)
 		(cpurev & 0x000F0) >> 4,
 		(cpurev & 0x0000F) >> 0,
 		mxc_get_clock(MXC_ARM_CLK) / 1000000);
-	printf("Reset cause: %s\n", get_reset_cause());
+	printf("Reset  cause: %s\n", get_reset_cause());
 	return 0;
 }
 #endif

board/freescale/mx53loco/Makefile new file +47 -0

47 lines not on this page · raw message

board/freescale/mx53loco/imximage.cfg new file +96 -0

96 lines not on this page · raw message

board/freescale/mx53loco/mx53loco.c new file +301 -0

301 lines not on this page · raw message

boards.cfg +1 -0

index b78e321..ce6e2e9 100644
@@ -110,6 +110,7 @@ ca9x4_ct_vxp                 arm         armv7       vexpress            armltd
 efikamx                      arm         armv7       efikamx             -              mx5
 mx51evk                      arm         armv7       mx51evk             freescale      mx5		mx51evk:IMX_CONFIG=board/freescale/mx51evk/imximage.cfg
 mx53evk                      arm         armv7       mx53evk             freescale      mx5		mx53evk:IMX_CONFIG=board/freescale/mx53evk/imximage.cfg
+mx53loco                     arm         armv7       mx53loco            freescale      mx5		mx53loco:IMX_CONFIG=board/freescale/mx53loco/imximage.cfg
 vision2                      arm         armv7       vision2             ttcontrol      mx5
 cm_t35                       arm         armv7       cm_t35              -              omap3
 omap3_overo                  arm         armv7       overo               -              omap3

include/configs/mx53loco.h new file +199 -0

199 lines not on this page · raw message

Signature · 1 line

1.7.1

Stefano Babic

2011-04-21 17:43 UTC #4 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

board/freescale/mx53loco/mx53loco.c line 153

Quoted 11 lines · Jason Liu

board/freescale/mx53loco/mx53loco.c new file +11 -0

+int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+{
+	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+
+	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
+		*cd = mxc_gpio_get(77); /*GPIO3_13*/
+	else
+		*cd = mxc_gpio_get(75); /*GPIO3_11*/
+
+	return 0;
+}

Rereading the thread regarding this board I see there is an open question issued by Wolfgang about this code, if accessors should be used in this case. IMHO the code is correct, using the GPIO get function to acquire the current value of a pin. Wolfgang, is it enough to answer your question ? Do you see an open issue ?

include/configs/mx53loco.h line 62

Quoted 4 lines

include/configs/mx53loco.h new file +4 -0

+/* PMIC Configs */
+#define CONFIG_FSL_PMIC
+#define CONFIG_FSL_PMIC_I2C
+#define CONFIG_SYS_FSL_PMIC_I2C_ADDR    8

If I am not confused, this setup is not valid as you already sent a patchset to drop these lines and adding support for the DA9053 pmic.

PMIC: Add dialog pmic support
MX53: loco: Add power init support

As this patchset is based on the LOCO patch (this one), and it does not apply anymore, please drop these lines and rebase your patches to add pmic support on the actual state of the current patch.
IMHO it should be better if you resend them as a single patchset, as they are strictly related, merging "Add power init support" inside this patch.

include/configs/mx53loco.h line 161

Quoted 2 lines

include/configs/mx53loco.h new file +2 -0

+#define CONFIG_SYS_MEMTEST_START       0x70000000
+#define CONFIG_SYS_MEMTEST_END         0x70010000

There is still an open question about this range. Can you answer to Wolfgang's question ?

Best regards,
Stefano

Signature · 5 lines

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

Wolfgang Denk

2011-04-21 23:24 UTC #5 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

Dear Stefano Babic,

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 9 lines · Jason Liu

board/freescale/mx53loco/mx53loco.c new file +9 -0

+int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+{
+	struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+
+	if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
+		*cd = mxc_gpio_get(77); /*GPIO3_13*/
+	else
+		*cd = mxc_gpio_get(75); /*GPIO3_11*/
+

Quoted 8 lines

Review
Quoted 2 lines
(quoted diff) +2 -0
+	return 0;
+}
Rereading the thread regarding this board I see there is an open question issued by Wolfgang about this code, if accessors should be used in this case. IMHO the code is correct, using the GPIO get function to acquire the current value of a pin. Wolfgang, is it enough to answer your question ? Do you see an open issue ?

I was just asking.

Best regards,

Wolfgang Denk

Signature · 4 lines

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
When all else fails, read the instructions.

Jason Hui

author

2011-04-22 04:36 UTC #6 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

In reply to Stefano Babic

Hi, Stefano,

Quoted 31 lines · Stefano Babic

Review
Quoted 11 lines · Jason Liu
(quoted diff) +11 -0
+int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+{
+ ? ? struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+
+ ? ? if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
+ ? ? ? ? ? ? *cd = mxc_gpio_get(77); /*GPIO3_13*/
+ ? ? else
+ ? ? ? ? ? ? *cd = mxc_gpio_get(75); /*GPIO3_11*/
+
+ ? ? return 0;
+}
Rereading the thread regarding this board I see there is an open question issued by Wolfgang about this code, if accessors should be used in this case. IMHO the code is correct, using the GPIO get function to acquire the current value of a pin. Wolfgang, is it enough to answer your question ? Do you see an open issue ?
include/configs/mx53loco.h
Quoted 4 lines
include/configs/mx53loco.h new file +4 -0
+/* PMIC Configs */
+#define CONFIG_FSL_PMIC
+#define CONFIG_FSL_PMIC_I2C
+#define CONFIG_SYS_FSL_PMIC_I2C_ADDR ? ?8
If I am not confused, this setup is not valid as you already sent a patchset to drop these lines and adding support for the DA9053 pmic.
? ? ? ?PMIC: Add dialog pmic support
? ? ? ?MX53: loco: Add power init support

Quoted 6 lines

As this patchset is based on the LOCO patch (this one), and it does not apply anymore, please drop these lines and rebase your patches to add pmic support on the actual state of the current patch.
IMHO it should be better if you resend them as a single patchset, as they are strictly related, merging "Add power init support" inside this patch.

OK, I will resend the patchset as the following layout:

MX5: clock: Add clock config interface
MX5: factor out boot cause funciton to common code
PMIC: Add dialog pmic support
MX53: support for freescale MX53LOCO board (merge "Add power init support")

I will rebase on the u-boot-imx git, do you think is it OK?

Quoted 7 lines

Review
Quoted 4 lines
(quoted diff) +2 -0
+#define CONFIG_SYS_MEMTEST_START ? ? ? 0x70000000
+#define CONFIG_SYS_MEMTEST_END ? ? ? ? 0x70010000
There is still an open question about this range. Can you answer to Wolfgang's question ?

In fact, I want to give one simple test range when use mtest, I don't know what you are worrying about, please tell it clearly?

Quoted 11 lines

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

Jason Hui

author

2011-04-22 05:10 UTC #7 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

Hi, Stefano,

Quoted 41 lines · Jason Hui

Hi, Stefano,
Quoted 31 lines · Stefano Babic
Review
Quoted 11 lines · Jason Liu
(quoted diff) +11 -0
+int board_mmc_getcd(u8 *cd, struct mmc *mmc)
+{
+ ? ? struct fsl_esdhc_cfg *cfg = (struct fsl_esdhc_cfg *)mmc->priv;
+
+ ? ? if (cfg->esdhc_base == MMC_SDHC1_BASE_ADDR)
+ ? ? ? ? ? ? *cd = mxc_gpio_get(77); /*GPIO3_13*/
+ ? ? else
+ ? ? ? ? ? ? *cd = mxc_gpio_get(75); /*GPIO3_11*/
+
+ ? ? return 0;
+}
Rereading the thread regarding this board I see there is an open question issued by Wolfgang about this code, if accessors should be used in this case. IMHO the code is correct, using the GPIO get function to acquire the current value of a pin. Wolfgang, is it enough to answer your question ? Do you see an open issue ?
include/configs/mx53loco.h
Quoted 4 lines
include/configs/mx53loco.h new file +4 -0
+/* PMIC Configs */
+#define CONFIG_FSL_PMIC
+#define CONFIG_FSL_PMIC_I2C
+#define CONFIG_SYS_FSL_PMIC_I2C_ADDR ? ?8
If I am not confused, this setup is not valid as you already sent a patchset to drop these lines and adding support for the DA9053 pmic.
? ? ? ?PMIC: Add dialog pmic support
? ? ? ?MX53: loco: Add power init support
Quoted 6 lines
As this patchset is based on the LOCO patch (this one), and it does not apply anymore, please drop these lines and rebase your patches to add pmic support on the actual state of the current patch.
IMHO it should be better if you resend them as a single patchset, as they are strictly related, merging "Add power init support" inside this patch.

Quoted 8 lines

OK, I will resend the patchset as the following layout:
MX5: clock: Add clock config interface
MX5: factor out boot cause funciton to common code
PMIC: Add dialog pmic support
MX53: support for freescale MX53LOCO board (merge "Add power init support")
I will rebase on the u-boot-imx git, do you think is it OK?

I just noticed that MX5: factor out boot cause funciton to common code has been applied.
So, I will send out the following,

MX5: clock: Add clock config interface
PMIC: Add dialog pmic support
MX53: support for freescale MX53LOCO board (merge "Add power init support")

Any comments?

Jason

Quoted 24 lines

Quoted 8 lines
Review
Quoted 5 lines
(quoted diff) +2 -0
+#define CONFIG_SYS_MEMTEST_START ? ? ? 0x70000000
+#define CONFIG_SYS_MEMTEST_END ? ? ? ? 0x70010000
There is still an open question about this range. Can you answer to Wolfgang's question ?
In fact, I want to give one simple test range when use mtest, I don't know what you are worrying about, please tell it clearly?
Quoted 11 lines
Best 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
=====================================================================

Stefano Babic

2011-04-22 06:57 UTC #8 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

In reply to Jason Hui

Quoted 9 lines · Jason Hui

OK, I will resend the patchset as the following layout:
MX5: clock: Add clock config interface
MX5: factor out boot cause funciton to common code
PMIC: Add dialog pmic support
MX53: support for freescale MX53LOCO board (merge "Add power init support")
I will rebase on the u-boot-imx git, do you think is it OK?

Yes, thanks.

Quoted 8 lines

Quoted 5 lines
include/configs/mx53loco.h line 161
Quoted 2 lines
include/configs/mx53loco.h new file +2 -0
+#define CONFIG_SYS_MEMTEST_START       0x70000000
+#define CONFIG_SYS_MEMTEST_END         0x70010000
There is still an open question about this range. Can you answer to Wolfgang's question ?
In fact, I want to give one simple test range when use mtest, I don't know what you are worrying about, please tell it clearly?

I think the only issue is if there is some reasons to set the test range to only 64KB, except the fact this was the value set for the mx51evk. If you do not see any special reasons, you could increase the CONFIG_SYS_MEMTEST_END to a larger value.

Best regards,
Stefano

Signature · 5 lines

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

Jason Liu

2011-04-22 08:33 UTC #9 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

Hi, Stefano,

2011/4/22 Stefano Babic <[email protected]>:

Quoted 19 lines · Jason Hui

Quoted 9 lines
OK, I will resend the patchset as the following layout:
MX5: clock: Add clock config interface
MX5: factor out boot cause funciton to common code
PMIC: Add dialog pmic support
MX53: support for freescale MX53LOCO board (merge "Add power init support")
I will rebase on the u-boot-imx git, do you think is it OK?
Yes, thanks.
Quoted 6 lines
Quoted 5 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+#define CONFIG_SYS_MEMTEST_START ? ? ? 0x70000000
+#define CONFIG_SYS_MEMTEST_END ? ? ? ? 0x70010000
There is still an open question about this range. Can you answer to Wolfgang's question ?

Quoted 7 lines

Quoted 2 lines
In fact, I want to give one simple test range when use mtest, I don't know what you are worrying about, please tell it clearly?
I think the only issue is if there is some reasons to set the test range to only 64KB, except the fact this was the value set for the mx51evk. If you do not see any special reasons, you could increase the CONFIG_SYS_MEMTEST_END to a larger value.

Is there any rule that we need must follow to set the CONFIG_SYS_MEMTEST_START CONFIG_SYS_MEMTEST_END ??

If no, I think the value we set is free. Here I set for 64KB is just for simple and quick
test, there is no other reasons.

In fact, mtest command line can override the value.

What the value you think I need set?

Jason

Quoted 15 lines

Best 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 mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Stefano Babic

2011-04-22 09:11 UTC #10 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

Quoted 4 lines · Jason Liu

Is there any rule that we need must follow to set the CONFIG_SYS_MEMTEST_START CONFIG_SYS_MEMTEST_END ??

There is no rule, it was only a question if there are some specific reasons.

Quoted 6 lines

If no, I think the value we set is free. Here I set for 64KB is just for simple and quick
test, there is no other reasons.
In fact, mtest command line can override the value.

That is correct.

Quoted 2 lines

What the value you think I need set?

As I said, there is no rule. You choose the value you think it is better for your board as default values for mtest.

Best regards,
Stefano Babic

Signature · 5 lines

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

Wolfgang Denk

2011-04-22 12:45 UTC #11 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

In reply to Jason Liu

Dear Jason Liu,

Quoted 9 lines · In message <[email protected]> you wrote:

Quoted 5 lines
I think the only issue is if there is some reasons to set the test range to only 64KB, except the fact this was the value set for the mx51evk. If you do not see any special reasons, you could increase the CONFIG_SYS_MEMTEST_END to a larger value.
Is there any rule that we need must follow to set the CONFIG_SYS_MEMTEST_START CONFIG_SYS_MEMTEST_END ??

The only strict rule is that the test must work with the defined area.

Quoted 7 lines

If no, I think the value we set is free. Here I set for 64KB is just for simple and quick
test, there is no other reasons.
In fact, mtest command line can override the value.
What the value you think I need set?

My recommendation is to set a range that is as big as possible for the given system, so the default call of the mtest command will actually test all (well, most of) the present RAM.

Best regards,

Wolfgang Denk

Signature · 6 lines

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
On our campus the UNIX system has proved to be not only an  effective
software tool, but an agent of technical and social change within the
University.                          - John Lions (U. of Toronto (?))

Jason Liu

2011-04-22 13:18 UTC #12 lore raw

PATCH v5 2/2 MX53: support for freescale MX53LOCO board

Hi, Wolfgang,

2011/4/22 Wolfgang Denk <[email protected]>:

Quoted 9 lines

Dear Jason Liu,
Quoted 6 lines · In message <[email protected]> you wrote:
Quoted 5 lines
I think the only issue is if there is some reasons to set the test range to only 64KB, except the fact this was the value set for the mx51evk. If you do not see any special reasons, you could increase the CONFIG_SYS_MEMTEST_END to a larger value.

Quoted 5 lines

Quoted 3 lines
Is there any rule that we need must follow to set the CONFIG_SYS_MEMTEST_START CONFIG_SYS_MEMTEST_END ??
The only strict rule is that the test must work with the defined area.

Agree, I have tested it and it works.

Quoted 5 lines

Quoted 5 lines
If no, I think the value we set is free. Here I set for 64KB is just for simple and quick
test, there is no other reasons.

Quoted 7 lines

Quoted 3 lines
In fact, mtest command line can override the value.
What the value you think I need set?
My recommendation is to set a range that is as big as possible for the given system, so the default call of the mtest command will actually test all (well, most of) the present RAM.

As I said, I just want the user to do one quick test for the memory. I don't think it need be set as big as possible.

Since there is no other rule I need follow for setting such value, So I will keep as it's.

Jason

Quoted 13 lines

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 On our campus the UNIX system has proved to be not only an ?effective software tool, but an agent of technical and social change within the University. ? ? ? ? ? ? ? ? ? ? ? ? ?- John Lions (U. of Toronto (?))

Stefano Babic

2011-04-21 17:18 UTC #13 lore raw

PATCH 1/1 mx5: drop boot cause code from board support code

In reply to Jason Liu

Quoted 4 lines · Jason Liu

The boot cause code has been factor out to soc common code,we need drop the part from the board support code
Signed-off-by Jason Liu <[email protected]>

Hi Jason,

Quoted 12 lines

---
 board/efikamx/efikamx.c           |   30 ++++++------------------------
 board/freescale/mx51evk/mx51evk.c |   26 ++++++--------------------
 board/freescale/mx53evk/mx53evk.c |   21 +--------------------
 board/ttcontrol/vision2/vision2.c |   28 ++++++----------------------
 4 files changed, 19 insertions(+), 86 deletions(-)

board/efikamx/efikamx.c +0 -0

index f735260..0aef654 100644

board/efikamx/efikamx.c int board_late_init(void) · line 648

Quoted 5 lines

board/efikamx/efikamx.c +0 -2

@@ -644,46 +644,28 @@ int board_late_init(void)
 int checkboard(void)
 {
 	u32 system_rev = get_cpu_rev();
-	u32 cause;
-	struct src *src_regs = (struct src *)SRC_BASE_ADDR;

This seems to me not the best solution. If we have now factored out code to print the reset cause and the silicon version (inside print_cpuinfo), why do we need to repeat this code for each board ? Calling get_cpu_rev seems to me redundant (then each board should only set CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and this is redundant.

board/efikamx/efikamx.c int board_late_init(void) · line 650

Quoted 4 lines

board/efikamx/efikamx.c +0 -0

 	puts("Board: Efika MX ");
 
 	switch (system_rev & 0xff) {

As I see in code, in system_rev & 0xff we can find the cpu revision, and the output is already part of print_cpuinfo.

I think we need more clean up, removing all part related to CPU revision and leaving (if any) only the output related to the board revision.

Best regards,
Stefano Babic

Signature · 5 lines

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

Jason Hui

author

2011-04-22 05:45 UTC #14 lore raw

PATCH 1/1 mx5: drop boot cause code from board support code

Hi, Stefano,

Quoted 24 lines · Stefano Babic

Quoted 4 lines · Jason Liu
The boot cause code has been factor out to soc common code,we need drop the part from the board support code
Signed-off-by Jason Liu <[email protected]>
Hi Jason,
Quoted 16 lines
---
?board/efikamx/efikamx.c ? ? ? ? ? | ? 30 ++++++------------------------
?board/freescale/mx51evk/mx51evk.c | ? 26 ++++++--------------------
?board/freescale/mx53evk/mx53evk.c | ? 21 +--------------------
?board/ttcontrol/vision2/vision2.c | ? 28 ++++++----------------------
?4 files changed, 19 insertions(+), 86 deletions(-)
board/efikamx/efikamx.c +0 -1
index f735260..0aef654 100644
@@ -644,46 +644,28 @@ int board_late_init(void)
 ?int checkboard(void)
 ?{
 ? ? ? u32 system_rev = get_cpu_rev();
- ? ? u32 cause;

Quoted 8 lines

board/efikamx/efikamx.c int board_late_init(void) · line 648
Quoted 1 line
board/efikamx/efikamx.c +0 -1
- ? ? struct src *src_regs = (struct src *)SRC_BASE_ADDR;
This seems to me not the best solution. If we have now factored out code to print the reset cause and the silicon version (inside print_cpuinfo), why do we need to repeat this code for each board ? Calling get_cpu_rev seems to me redundant (then each board should only set CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and this is redundant.

The purpose for this patch is to remove the boot cause code and and don't change any cpu rev code. The cpu rev part of code is as it is as before.

Quoted 8 lines

board/efikamx/efikamx.c int board_late_init(void) · line 649
Quoted 5 lines
board/efikamx/efikamx.c +0 -0
 ? ? ? puts("Board: Efika MX ");
 
 ? ? ? switch (system_rev & 0xff) {
As I see in code, in system_rev & 0xff we can find the cpu revision, and the output is already part of print_cpuinfo.

Ditto, as I only remove the boot cause part of code as the patch tile said.

Quoted 3 lines

I think we need more clean up, removing all part related to CPU revision and leaving (if any) only the output related to the board revision.

If that, I need change the patch tile, and include more clean up in the patch and send again.

Jason

Quoted 11 lines

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

Stefano Babic

2011-04-22 07:06 UTC #15 lore raw

PATCH 1/1 mx5: drop boot cause code from board support code

Quoted 1 line · Jason Hui

Hi, Stefano,

Hi Jason,

Quoted 13 lines

Quoted 6 lines
Review
Quoted 5 lines
int checkboard(void)
 {
      u32 system_rev = get_cpu_rev();
(quoted diff) +0 -2
-     u32 cause;
-     struct src *src_regs = (struct src *)SRC_BASE_ADDR;
Quoted 6 lines
This seems to me not the best solution. If we have now factored out code to print the reset cause and the silicon version (inside print_cpuinfo), why do we need to repeat this code for each board ? Calling get_cpu_rev seems to me redundant (then each board should only set CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and this is redundant.

Quoted 2 lines

The purpose for this patch is to remove the boot cause code and and don't change any cpu rev code. The cpu rev part of code is as it is as before.

However, it seems to me a half-way clean up. As we have already factorize function for printing the cpu revision and the reset cause, I will see in the checkboard only board related information. If there is no revision number for board, printing only the board name as you make for the LOCO is correct.

Taking as example the efika board (all boards make the same):

Quoted 6 lines



      puts("Board: Efika MX ");

      switch (system_rev & 0xff) {

The only new information is the board name. If I am not wrong, system_rev & 0xff contains only the cpu revision, and the switch prints out the silicon version. Everything already done in print_cpuinfo as well.

Quoted 2 lines

Ditto, as I only remove the boot cause part of code as the patch tile said.

Yes, but this it is redundant with the print_cpuinfo(). As you plan to clean up the code, this part should be cleaned up as well. Or do you think there is something not covered by the common code ?

Quoted 5 lines

Quoted 2 lines
I think we need more clean up, removing all part related to CPU revision and leaving (if any) only the output related to the board revision.
If that, I need change the patch tile, and include more clean up in the patch and send again.

Agree.

Best regards,
Stefano Babic

Signature · 5 lines

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

Jason Liu

2011-04-22 08:34 UTC #16 lore raw

PATCH 1/1 mx5: drop boot cause code from board support code

Hi, Stefano,

2011/4/22 Stefano Babic <[email protected]>:

Quoted 47 lines · Jason Hui

Quoted 1 line
Hi, Stefano,
Hi Jason,
Quoted 13 lines
Quoted 6 lines
Quoted 5 lines
?int checkboard(void)
?{
? ? ? u32 system_rev = get_cpu_rev();
- ? ? u32 cause;
- ? ? struct src *src_regs = (struct src *)SRC_BASE_ADDR;
Quoted 6 lines
This seems to me not the best solution. If we have now factored out code to print the reset cause and the silicon version (inside print_cpuinfo), why do we need to repeat this code for each board ? Calling get_cpu_rev seems to me redundant (then each board should only set CONFIG_DISPLAY_CPUINFO). And then the CPU revision is printed again, and this is redundant.
Quoted 2 lines
The purpose for this patch is to remove the boot cause code and and don't change any cpu rev code. The cpu rev part of code is as it is as before.
However, it seems to me a half-way clean up. As we have already factorize function for printing the cpu revision and the reset cause, I will see in the checkboard only board related information. If there is no revision number for board, printing only the board name as you make for the LOCO is correct.
Taking as example the efika board (all boards make the same):
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
? ? ? puts("Board: Efika MX ");
? ? ? switch (system_rev & 0xff) {
The only new information is the board name. If I am not wrong, system_rev & 0xff contains only the cpu revision, and the switch prints out the silicon version. Everything already done in print_cpuinfo as well.
Quoted 2 lines
Ditto, as I only remove the boot cause part of code as the patch tile said.
Yes, but this it is redundant with the print_cpuinfo(). As you plan to clean up the code, this part should be cleaned up as well. Or do you think there is something not covered by the common code ?

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
I think we need more clean up, removing all part related to CPU revision and leaving (if any) only the output related to the board revision.
If that, I need change the patch tile, and include more clean up in the patch and send again.
Agree.

OK, I will re-send the patch with more clean-up.

Jason

Quoted 15 lines

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 mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot