Patchwork [U-Boot,v2,22/22] omap: spl: add more debug traces

login
register
mail settings
Submitter Aneesh V
Date May 15, 2011, 3:21 p.m.
Message ID <1305472900-4004-23-git-send-email-aneesh@ti.com>
Download mbox | patch
Permalink /patch/95648/
State Changes Requested
Headers show

Comments

Aneesh V - May 15, 2011, 3:21 p.m.
In SPL console is enabled very early where as in U-Boot
it's not. So, SPL can have traces in early init code
while U-Boot can not have it in the same shared code.

Adding a debug print macro that will be defined in SPL
but compiled out in U-Boot.

Also add some useful debug traces throughout SPL

Signed-off-by: Aneesh V <aneesh@ti.com>
---
 arch/arm/cpu/armv7/omap4/clocks.c |   10 ++++++++--
 arch/arm/cpu/armv7/omap4/emif.c   |   35 +++++++++++++++++++++++++++--------
 arch/arm/include/asm/utils.h      |    6 ++++++
 spl/board/ti/spl-omap.c           |   13 ++++++++++---
 4 files changed, 51 insertions(+), 13 deletions(-)
Wolfgang Denk - May 15, 2011, 8:21 p.m.
Dear Aneesh V,

In message <1305472900-4004-23-git-send-email-aneesh@ti.com> you wrote:
> In SPL console is enabled very early where as in U-Boot
> it's not. So, SPL can have traces in early init code

Console should _always_ be enabled as early as possible,

> while U-Boot can not have it in the same shared code.
> 
> Adding a debug print macro that will be defined in SPL
> but compiled out in U-Boot.

Can we not rather change the code so both configurations behave the
same?

> --- a/arch/arm/cpu/armv7/omap4/clocks.c
> +++ b/arch/arm/cpu/armv7/omap4/clocks.c
> @@ -379,7 +379,7 @@ u32 omap4_ddr_clk(void)
>  
>  	core_dpll_params = get_core_dpll_params();
>  
> -	debug("sys_clk %d\n ", sys_clk_khz * 1000);
> +	spl_debug("sys_clk %d\n ", sys_clk_khz * 1000);

Do we really need a new macro name?  Can this not be the same debug()
macro, just generating different code (if really needed) when building
the SPL code?

> @@ -1318,4 +1328,13 @@ void sdram_init(void)
>  
>  	/* for the shadow registers to take effect */
>  	freq_update_core();
> +
> +	/* Do some basic testing */
> +	writel(0xDEADBEEF, CONFIG_SYS_SDRAM_BASE);
> +	if (readl(CONFIG_SYS_SDRAM_BASE) == 0xDEADBEEF)
> +		spl_debug("SDRAM Init success!\n");
> +	else
> +		printf("SDRAM Init failed!!\n");
> +
> +	spl_debug("<<sdram_init()\n");

This is beyond the scope of "adding debug traces".  it must be split
into separate patch.

Also, please do not mess witrhout need with the RAM content - at the
very least, restore the previous values.

But then - I wonder why this is needed at all. Are you not using
get_ram_size()?  Maybe you should fix your code to using it!

> diff --git a/arch/arm/include/asm/utils.h b/arch/arm/include/asm/utils.h
> index d581539..3e847c1 100644
> --- a/arch/arm/include/asm/utils.h
> +++ b/arch/arm/include/asm/utils.h
> @@ -25,6 +25,12 @@
>  #ifndef	_UTILS_H_
>  #define	_UTILS_H_
>  
> +#if defined(DEBUG) && defined(CONFIG_PRELOADER)
> +#define spl_debug(fmt, args...)	printf(fmt, ##args)
> +#else
> +#define spl_debug(fmt, args...)
> +#endif

NAK.  This is neither the right place for such a definition, nor do I
want to see this addressed like that.

I recommend to unify the code, so both SPL and non-SPL configurations
can use teh same early console behaviour.

>  void board_init_f(ulong dummy)
>  {
> +	debug(">>board_init_f()\n");
>  	relocate_code(CONFIG_SYS_SPL_STACK, &gdata, CONFIG_SYS_SPL_TEXT_BASE);
> +	debug("<<board_init_f()\n");

This is overkill, isn't it?

> @@ -166,6 +172,7 @@ void board_init_r(gd_t *id, ulong dummy)
>  	switch (boot_device) {
>  	case BOOT_DEVICE_MMC1:
>  	case BOOT_DEVICE_MMC2:
> +		spl_debug("boot device - MMC2\n");
>  		mmc_load_uboot(boot_device - BOOT_DEVICE_MMC1);

This is wrong in the BOOT_DEVICE_MMC1 case.

Best regards,

Wolfgang Denk

Patch

diff --git a/arch/arm/cpu/armv7/omap4/clocks.c b/arch/arm/cpu/armv7/omap4/clocks.c
index c405b5c..720e60f 100644
--- a/arch/arm/cpu/armv7/omap4/clocks.c
+++ b/arch/arm/cpu/armv7/omap4/clocks.c
@@ -379,7 +379,7 @@  u32 omap4_ddr_clk(void)
 
 	core_dpll_params = get_core_dpll_params();
 
-	debug("sys_clk %d\n ", sys_clk_khz * 1000);
+	spl_debug("sys_clk %d\n ", sys_clk_khz * 1000);
 
 	/* Find Core DPLL locked frequency first */
 	ddr_clk = sys_clk_khz * 2 * core_dpll_params->m /
@@ -391,7 +391,7 @@  u32 omap4_ddr_clk(void)
 	ddr_clk = ddr_clk / 4 / core_dpll_params->m2;
 
 	ddr_clk *= 1000;	/* convert to Hz */
-	debug("ddr_clk %d\n ", ddr_clk);
+	spl_debug("ddr_clk %d\n ", ddr_clk);
 
 	return ddr_clk;
 }
@@ -400,6 +400,7 @@  static void setup_dplls(void)
 {
 	u32 sysclk_ind, temp;
 	const struct dpll_params *params;
+	spl_debug("setup_dplls\n");
 
 	sysclk_ind = get_sys_clk_index();
 
@@ -416,10 +417,12 @@  static void setup_dplls(void)
 	    (CLKSEL_L3_CORE_DIV_2 << CLKSEL_L3_SHIFT) |
 	    (CLKSEL_L4_L3_DIV_2 << CLKSEL_L4_SHIFT);
 	writel(temp, CM_CLKSEL_CORE);
+	spl_debug("Core DPLL configured\n");
 
 	/* lock PER dpll */
 	do_setup_dpll(CM_CLKMODE_DPLL_PER,
 			&per_dpll_params_1536mhz[sysclk_ind], DPLL_LOCK);
+	spl_debug("PER DPLL locked\n");
 
 	/* MPU dpll */
 	if (omap4_revision() == OMAP4430_ES1_0)
@@ -427,6 +430,7 @@  static void setup_dplls(void)
 	else
 		params = &mpu_dpll_params_1ghz[sysclk_ind];
 	do_setup_dpll(CM_CLKMODE_DPLL_MPU, params, DPLL_LOCK);
+	spl_debug("MPU DPLL locked\n");
 }
 
 static void setup_non_essential_dplls(void)
@@ -551,6 +555,7 @@  static void scale_vcores(void)
 
 static void enable_clock_domain(u32 clkctrl_reg, u32 enable_mode)
 {
+	spl_debug("Enable clock domain - 0x%08x\n", clkctrl_reg);
 	modify_reg_32(clkctrl_reg, CD_CLKCTRL_CLKTRCTRL_SHIFT,
 		      CD_CLKCTRL_CLKTRCTRL_MASK, enable_mode);
 }
@@ -570,6 +575,7 @@  static inline void wait_for_clk_enable(u32 clkctrl_addr)
 static void enable_clock_module(u32 clkctrl_addr, u32 enable_mode,
 				u32 wait_for_enable)
 {
+	spl_debug("Enable clock module - 0x%08x\n", clkctrl_addr);
 	modify_reg_32(clkctrl_addr, MODULE_CLKCTRL_MODULEMODE_SHIFT,
 			MODULE_CLKCTRL_MODULEMODE_MASK, enable_mode);
 	if (wait_for_enable)
diff --git a/arch/arm/cpu/armv7/omap4/emif.c b/arch/arm/cpu/armv7/omap4/emif.c
index 699a545..fcb6d47 100644
--- a/arch/arm/cpu/armv7/omap4/emif.c
+++ b/arch/arm/cpu/armv7/omap4/emif.c
@@ -34,7 +34,7 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define print_timing_reg(reg) debug(#reg" - 0x%08x\n", (reg))
+#define print_timing_reg(reg) spl_debug(#reg" - 0x%08x\n", (reg))
 
 static u32 *const T_num = (u32 *)OMAP4_SRAM_SCRATCH_EMIF_T_NUM;
 static u32 *const T_den = (u32 *)OMAP4_SRAM_SCRATCH_EMIF_T_DEN;
@@ -237,7 +237,7 @@  s8 addressing_table_index(u8 type, u8 density, u8 width)
 	else
 		index = density;
 
-	debug("emif: addressing table index %d\n", index);
+	spl_debug("emif: addressing table index %d\n", index);
 
 	return index;
 }
@@ -272,7 +272,7 @@  static const struct lpddr2_ac_timings *get_timings_table(const struct
 			timings = device_timings[i];
 		}
 	}
-	debug("emif: timings table: %d\n", freq_nearest);
+	spl_debug("emif: timings table: %d\n", freq_nearest);
 	return timings;
 }
 
@@ -637,7 +637,7 @@  static inline u32 get_mr(u32 base, u32 cs, u32 mr_addr)
 		mr = readl(&emif->emif_lpddr2_mode_reg_data_es2);
 	else
 		mr = readl(&emif->emif_lpddr2_mode_reg_data);
-	debug("get_mr: EMIF%d cs %d mr %08x val 0x%x\n", emif_num(base),
+	spl_debug("get_mr: EMIF%d cs %d mr %08x val 0x%x\n", emif_num(base),
 	      cs, mr_addr, mr);
 	return mr;
 }
@@ -790,10 +790,10 @@  static void display_sdram_details(u32 emif_nr, u32 cs,
 	char density_str[10];
 	u32 density;
 
-	debug("EMIF%d CS%d\t", emif_nr, cs);
+	spl_debug("EMIF%d CS%d\t", emif_nr, cs);
 
 	if (!device) {
-		debug("None\n");
+		spl_debug("None\n");
 		return;
 	}
 
@@ -807,7 +807,7 @@  static void display_sdram_details(u32 emif_nr, u32 cs,
 	} else
 		sprintf(density_str, "%d MB", density);
 	if (mfg_str && type_str)
-		debug("%s\t\t%s\t%s\n", mfg_str, type_str, density_str);
+		spl_debug("%s\t\t%s\t%s\n", mfg_str, type_str, density_str);
 }
 
 static u8 is_lpddr2_sdram_present(u32 base, u32 cs,
@@ -948,6 +948,8 @@  static void lpddr2_init(u32 base, const struct emif_regs *regs)
 {
 	struct emif_reg_struct *emif = (struct emif_reg_struct *)base;
 
+	spl_debug(">>lpddr2_init() %x\n", base);
+
 	/* Not NVM */
 	modify_reg_32(&emif->emif_lpddr2_nvm_config,
 		OMAP44XX_REG_CS1NVMEN_SHIFT, OMAP44XX_REG_CS1NVMEN_MASK, 0);
@@ -978,6 +980,7 @@  static void lpddr2_init(u32 base, const struct emif_regs *regs)
 		OMAP44XX_REG_INITREF_DIS_SHIFT,
 		OMAP44XX_REG_INITREF_DIS_MASK, 0);
 
+	spl_debug("<<lpddr2_init()\n");
 }
 
 static void emif_update_timings(u32 base, const struct emif_regs *regs)
@@ -1023,6 +1026,8 @@  static void do_sdram_init(u32 base)
 	const struct emif_regs *regs;
 	u32 in_sdram, emif_nr;
 
+	spl_debug(">>do_sdram_init() %x\n", base);
+
 	in_sdram = running_from_sdram();
 	emif_nr = (base == OMAP44XX_EMIF1) ? 1 : 2;
 
@@ -1115,6 +1120,8 @@  static void do_sdram_init(u32 base)
 
 	/* Write to the shadow registers */
 	emif_update_timings(base, regs);
+
+	spl_debug("<<do_sdram_init() %x\n", base);
 }
 
 void sdram_init_pads(void)
@@ -1187,7 +1194,7 @@  static void dmm_init(u32 base)
 	sys_addr = CONFIG_SYS_SDRAM_BASE;
 	emif1_size = emif_sizes[0];
 	emif2_size = emif_sizes[1];
-	debug("emif1_size 0x%x emif2_size 0x%x\n", emif1_size, emif2_size);
+	spl_debug("emif1_size 0x%x emif2_size 0x%x\n", emif1_size, emif2_size);
 
 	if (!emif1_size && !emif2_size)
 		return;
@@ -1296,11 +1303,14 @@  static void dmm_init(u32 base)
  */
 void sdram_init(void)
 {
+	spl_debug(">>sdram_init()\n");
+
 	if (omap4_hw_init_context() == OMAP_INIT_CONTEXT_UBOOT_LOADED_BY_SPL)
 		return;
 
 	u32 in_sdram;
 	in_sdram = running_from_sdram();
+	spl_debug("in_sdram = %d\n", in_sdram);
 
 	if (!in_sdram) {
 		sdram_init_pads();
@@ -1318,4 +1328,13 @@  void sdram_init(void)
 
 	/* for the shadow registers to take effect */
 	freq_update_core();
+
+	/* Do some basic testing */
+	writel(0xDEADBEEF, CONFIG_SYS_SDRAM_BASE);
+	if (readl(CONFIG_SYS_SDRAM_BASE) == 0xDEADBEEF)
+		spl_debug("SDRAM Init success!\n");
+	else
+		printf("SDRAM Init failed!!\n");
+
+	spl_debug("<<sdram_init()\n");
 }
diff --git a/arch/arm/include/asm/utils.h b/arch/arm/include/asm/utils.h
index d581539..3e847c1 100644
--- a/arch/arm/include/asm/utils.h
+++ b/arch/arm/include/asm/utils.h
@@ -25,6 +25,12 @@ 
 #ifndef	_UTILS_H_
 #define	_UTILS_H_
 
+#if defined(DEBUG) && defined(CONFIG_PRELOADER)
+#define spl_debug(fmt, args...)	printf(fmt, ##args)
+#else
+#define spl_debug(fmt, args...)
+#endif
+
 /* extract a bit field from a bit vector */
 #define get_bit_field(nr, start, mask)\
 	(((nr) & (mask)) >> (start))
diff --git a/spl/board/ti/spl-omap.c b/spl/board/ti/spl-omap.c
index 81ca0d3..6db3675 100644
--- a/spl/board/ti/spl-omap.c
+++ b/spl/board/ti/spl-omap.c
@@ -27,6 +27,7 @@ 
  */
 #include <common.h>
 #include <asm/u-boot.h>
+#include <asm/utils.h>
 #include <asm/arch/sys_proto.h>
 #include <mmc.h>
 #include <fat.h>
@@ -45,7 +46,9 @@  typedef void (*u_boot_entry_t)(void)__attribute__ ((noreturn));
 
 void board_init_f(ulong dummy)
 {
+	debug(">>board_init_f()\n");
 	relocate_code(CONFIG_SYS_SPL_STACK, &gdata, CONFIG_SYS_SPL_TEXT_BASE);
+	debug("<<board_init_f()\n");
 }
 
 inline void hang(void)
@@ -145,11 +148,13 @@  static void mmc_load_uboot(u32 mmc_dev)
 	}
 
 	boot_mode = omap_boot_mode();
-	if (boot_mode == MMCSD_MODE_RAW)
+	if (boot_mode == MMCSD_MODE_RAW) {
+		spl_debug("boot mode - RAW\n");
 		mmc_load_uboot_raw(mmc, mmc_dev);
-	else if (boot_mode == MMCSD_MODE_FAT)
+	} else if (boot_mode == MMCSD_MODE_FAT) {
+		spl_debug("boot mode - FAT\n");
 		mmc_load_uboot_fat(mmc);
-	else {
+	} else {
 		puts("spl: wrong MMC boot mode\n");
 		hang();
 	}
@@ -159,6 +164,7 @@  void board_init_r(gd_t *id, ulong dummy)
 {
 	u32 boot_device;
 	u_boot_entry_t u_boot_entry = (u_boot_entry_t) CONFIG_SYS_TEXT_BASE;
+	spl_debug(">>spl:board_init_r()\n");
 
 	timer_init();
 	i2c_init(CONFIG_SYS_I2C_SPEED, CONFIG_SYS_I2C_SLAVE);
@@ -166,6 +172,7 @@  void board_init_r(gd_t *id, ulong dummy)
 	switch (boot_device) {
 	case BOOT_DEVICE_MMC1:
 	case BOOT_DEVICE_MMC2:
+		spl_debug("boot device - MMC2\n");
 		mmc_load_uboot(boot_device - BOOT_DEVICE_MMC1);
 		break;
 	default: