| Message ID | 4F0D3192.7000708@denx.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | Tom Rini |
| Headers | show |
hi Heiko, On Wed Jan 11, 2012 at 07:52:02AM +0100, Heiko Schocher wrote: > Hello Sughosh, > > Sughosh Ganu wrote: > > This patch moves hawkboard to the new spl infrastructure from the > > older nand_spl one. > > > > Removed the hawkboard_nand_config build option -- The spl code now > > gets compiled with hawkboard_config, after building the main u-boot > > image, using the CONFIG_SPL_TEXT_BASE. Modified the README.hawkboard > > to reflect the same. > > > > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> > > Cc: Heiko Schocher <hs@denx.de> > > Cc: Christian Riesch <christian.riesch@omicron.at> > > Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com> > > Cc: Tom Rini <trini@ti.com> > > --- > > > [...] > > diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > > index a532f8a..a4778b8 100644 > > --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > > +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > > @@ -32,6 +32,7 @@ > > #include <asm/arch/emif_defs.h> > > #include <asm/arch/pll_defs.h> > > > > +#if !defined(CONFIG_MACH_DAVINCI_HAWK) > > Please no board specific defines. Ok, will replace with the arch specific define that you suggest. > > > void da850_waitloop(unsigned long loopcnt) > > { > > unsigned long i; > > @@ -235,6 +236,7 @@ int da850_ddr_setup(void) > > > > return 0; > > } > > +#endif /* CONFIG_MACH_DAVINCI_HAWK */ > > > > __attribute__((weak)) > > void board_gpio_init(void) > > @@ -242,10 +244,6 @@ void board_gpio_init(void) > > return; > > } > > > > -/* pinmux_resource[] vector is defined in the board specific file */ > > -extern const struct pinmux_resource pinmuxes[]; > > -extern const int pinmuxes_size; > > - > > int arch_cpu_init(void) > > { > > /* Unlock kick registers */ > > @@ -259,6 +257,7 @@ int arch_cpu_init(void) > > if (davinci_configure_pin_mux_items(pinmuxes, pinmuxes_size)) > > return 1; > > > > +#if defined(CONFIG_MACH_DAVINCI_DA850_EVM) > > here too. I propose here a CONFIG_SYS_DA850_PLL_INIT ... Ok. > > > /* PLL setup */ > > da850_pll_init(davinci_pllc0_regs, CONFIG_SYS_DA850_PLL0_PLLM); > > da850_pll_init(davinci_pllc1_regs, CONFIG_SYS_DA850_PLL1_PLLM); > > @@ -275,6 +274,12 @@ int arch_cpu_init(void) > > #endif > > > > lpsc_on(CONFIG_SYS_DA850_LPSC_UART); > > + > > + da850_ddr_setup(); > > and around the ddr_setup a CONFIG_SYS_DA850_DDR_INIT ... > > > +#elif defined(CONFIG_MACH_DAVINCI_HAWK) > > + da8xx_configure_lpsc_items(lpsc, lpsc_size); > > +#endif > > and we should use da8xx_configure_lpsc_items() for all da850 boards. Ok. > > This patch breaks current enbw_cmc board compile > > [hs@pollux u-boot]$ ./MAKEALL enbw_cmc > Configuring for enbw_cmc board... > enbw_cmc.c:52:35: error: static declaration of 'lpsc' follows non-static declaration > /work/hs/u-boot/include/asm/arch/da850_lowlevel.h:33:35: note: previous declaration of 'lpsc' was here > make[1]: *** [enbw_cmc.o] Fehler 1 > make: *** [board/enbw/enbw_cmc/libenbw_cmc.o] Fehler 2 > arm-linux-gnueabi-size: './u-boot': No such file Oops, sorry about that. <snip> > diff --git a/include/configs/enbw_cmc.h b/include/configs/enbw_cmc.h > index c427dc7..804846d 100644 > --- a/include/configs/enbw_cmc.h > +++ b/include/configs/enbw_cmc.h > @@ -48,6 +48,8 @@ > #define CONFIG_SKIP_LOWLEVEL_INIT > #define CONFIG_DA850_LOWLEVEL > #define CONFIG_ARCH_CPU_INIT > +#define CONFIG_SYS_DA850_PLL_INIT > +#define CONFIG_SYS_DA850_DDR_INIT I think we'll need to add this for da850evm.h too. > #define CONFIG_DA8XX_GPIO > #define CONFIG_HOSTNAME enbw_cmc > #define CONFIG_DISPLAY_CPUINFO > -- > 1.7.7.4 > > [...] > > diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h > > index a21d448..8e3a4d2 100644 > > --- a/include/configs/cam_enc_4xx.h > > +++ b/include/configs/cam_enc_4xx.h > > @@ -205,6 +205,7 @@ > > > > /* Defines for SPL */ > > #define CONFIG_SPL > > +#define CONFIG_DM365_SPL > > Why we need this define? Yeah, missed out cleaning these defines that i had used in V1. Will clean up in next version for all the cases. -sughosh
Hello Sughosh, I reviewed your patch and I agree with Heiko's comments. I am looking forward to v3 :-) Regards, Christian On Wed, Jan 11, 2012 at 8:53 AM, Sughosh Ganu <urwithsughosh@gmail.com> wrote: > hi Heiko, > > On Wed Jan 11, 2012 at 07:52:02AM +0100, Heiko Schocher wrote: >> Hello Sughosh, >> >> Sughosh Ganu wrote: >> > This patch moves hawkboard to the new spl infrastructure from the >> > older nand_spl one. >> > >> > Removed the hawkboard_nand_config build option -- The spl code now >> > gets compiled with hawkboard_config, after building the main u-boot >> > image, using the CONFIG_SPL_TEXT_BASE. Modified the README.hawkboard >> > to reflect the same. >> > >> > Signed-off-by: Sughosh Ganu <urwithsughosh@gmail.com> >> > Cc: Heiko Schocher <hs@denx.de> >> > Cc: Christian Riesch <christian.riesch@omicron.at> >> > Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com> >> > Cc: Tom Rini <trini@ti.com> >> > --- >> > >> [...] >> > diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c >> > index a532f8a..a4778b8 100644 >> > --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c >> > +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c >> > @@ -32,6 +32,7 @@ >> > #include <asm/arch/emif_defs.h> >> > #include <asm/arch/pll_defs.h> >> > >> > +#if !defined(CONFIG_MACH_DAVINCI_HAWK) >> >> Please no board specific defines. > > Ok, will replace with the arch specific define that you suggest. > >> >> > void da850_waitloop(unsigned long loopcnt) >> > { >> > unsigned long i; >> > @@ -235,6 +236,7 @@ int da850_ddr_setup(void) >> > >> > return 0; >> > } >> > +#endif /* CONFIG_MACH_DAVINCI_HAWK */ >> > >> > __attribute__((weak)) >> > void board_gpio_init(void) >> > @@ -242,10 +244,6 @@ void board_gpio_init(void) >> > return; >> > } >> > >> > -/* pinmux_resource[] vector is defined in the board specific file */ >> > -extern const struct pinmux_resource pinmuxes[]; >> > -extern const int pinmuxes_size; >> > - >> > int arch_cpu_init(void) >> > { >> > /* Unlock kick registers */ >> > @@ -259,6 +257,7 @@ int arch_cpu_init(void) >> > if (davinci_configure_pin_mux_items(pinmuxes, pinmuxes_size)) >> > return 1; >> > >> > +#if defined(CONFIG_MACH_DAVINCI_DA850_EVM) >> >> here too. I propose here a CONFIG_SYS_DA850_PLL_INIT ... > > Ok. > >> >> > /* PLL setup */ >> > da850_pll_init(davinci_pllc0_regs, CONFIG_SYS_DA850_PLL0_PLLM); >> > da850_pll_init(davinci_pllc1_regs, CONFIG_SYS_DA850_PLL1_PLLM); >> > @@ -275,6 +274,12 @@ int arch_cpu_init(void) >> > #endif >> > >> > lpsc_on(CONFIG_SYS_DA850_LPSC_UART); >> > + >> > + da850_ddr_setup(); >> >> and around the ddr_setup a CONFIG_SYS_DA850_DDR_INIT ... >> >> > +#elif defined(CONFIG_MACH_DAVINCI_HAWK) >> > + da8xx_configure_lpsc_items(lpsc, lpsc_size); >> > +#endif >> >> and we should use da8xx_configure_lpsc_items() for all da850 boards. > > Ok. > >> >> This patch breaks current enbw_cmc board compile >> >> [hs@pollux u-boot]$ ./MAKEALL enbw_cmc >> Configuring for enbw_cmc board... >> enbw_cmc.c:52:35: error: static declaration of 'lpsc' follows non-static declaration >> /work/hs/u-boot/include/asm/arch/da850_lowlevel.h:33:35: note: previous declaration of 'lpsc' was here >> make[1]: *** [enbw_cmc.o] Fehler 1 >> make: *** [board/enbw/enbw_cmc/libenbw_cmc.o] Fehler 2 >> arm-linux-gnueabi-size: './u-boot': No such file > > Oops, sorry about that. > > <snip> > >> diff --git a/include/configs/enbw_cmc.h b/include/configs/enbw_cmc.h >> index c427dc7..804846d 100644 >> --- a/include/configs/enbw_cmc.h >> +++ b/include/configs/enbw_cmc.h >> @@ -48,6 +48,8 @@ >> #define CONFIG_SKIP_LOWLEVEL_INIT >> #define CONFIG_DA850_LOWLEVEL >> #define CONFIG_ARCH_CPU_INIT >> +#define CONFIG_SYS_DA850_PLL_INIT >> +#define CONFIG_SYS_DA850_DDR_INIT > > I think we'll need to add this for da850evm.h too. > >> #define CONFIG_DA8XX_GPIO >> #define CONFIG_HOSTNAME enbw_cmc >> #define CONFIG_DISPLAY_CPUINFO >> -- >> 1.7.7.4 >> >> [...] >> > diff --git a/include/configs/cam_enc_4xx.h b/include/configs/cam_enc_4xx.h >> > index a21d448..8e3a4d2 100644 >> > --- a/include/configs/cam_enc_4xx.h >> > +++ b/include/configs/cam_enc_4xx.h >> > @@ -205,6 +205,7 @@ >> > >> > /* Defines for SPL */ >> > #define CONFIG_SPL >> > +#define CONFIG_DM365_SPL >> >> Why we need this define? > > Yeah, missed out cleaning these defines that i had used in V1. Will > clean up in next version for all the cases. > > -sughosh > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot
Hello Heiko, Sughosh, On Wed, Jan 11, 2012 at 7:52 AM, Heiko Schocher <hs@denx.de> wrote: > please remove the da8xx_configure_lpsc_items() in board_gpio_init() > in the ./board/enbw/enbw_cmc/enbw_cmc.c() file, and also move ... > before I write here a lot of text, here the patch, based on yours, > please add it to your patch, and add my > > Signed-off-by: Heiko Schocher <hs@denx.de> > > and tested on the enbw_cmc board, so: > Tested-by: Heiko Schocher <hs@denx.de> > > to your patch, here the patch: > > diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > index a4778b8..a404916 100644 > --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c [....] > - lpsc_on(CONFIG_SYS_DA850_LPSC_UART); Now we can also remove CONFIG_SYS_DA850_LPSC_UART from include/configs/da850evm.h and include/configs/enbw_cmc.h. Regards, Christian
Hello Christian, Christian Riesch wrote: > Hello Heiko, Sughosh, > > On Wed, Jan 11, 2012 at 7:52 AM, Heiko Schocher <hs@denx.de> wrote: >> please remove the da8xx_configure_lpsc_items() in board_gpio_init() >> in the ./board/enbw/enbw_cmc/enbw_cmc.c() file, and also move ... >> before I write here a lot of text, here the patch, based on yours, >> please add it to your patch, and add my >> >> Signed-off-by: Heiko Schocher <hs@denx.de> >> >> and tested on the enbw_cmc board, so: >> Tested-by: Heiko Schocher <hs@denx.de> >> >> to your patch, here the patch: >> >> diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c >> index a4778b8..a404916 100644 >> --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c >> +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c > [....] >> - lpsc_on(CONFIG_SYS_DA850_LPSC_UART); > > Now we can also remove CONFIG_SYS_DA850_LPSC_UART from > include/configs/da850evm.h and include/configs/enbw_cmc.h. correct. Good catch! bye, Heiko
diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c index a4778b8..a404916 100644 --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c @@ -32,7 +32,7 @@ #include <asm/arch/emif_defs.h> #include <asm/arch/pll_defs.h> -#if !defined(CONFIG_MACH_DAVINCI_HAWK) +#if defined(CONFIG_SYS_DA850_DDR_INIT) void da850_waitloop(unsigned long loopcnt) { unsigned long i; @@ -236,7 +236,7 @@ int da850_ddr_setup(void) return 0; } -#endif /* CONFIG_MACH_DAVINCI_HAWK */ +#endif /* CONFIG_SYS_DA850_DDR_INIT */ __attribute__((weak)) void board_gpio_init(void) @@ -257,14 +257,11 @@ int arch_cpu_init(void) if (davinci_configure_pin_mux_items(pinmuxes, pinmuxes_size)) return 1; -#if defined(CONFIG_MACH_DAVINCI_DA850_EVM) +#if defined(CONFIG_SYS_DA850_PLL_INIT) /* PLL setup */ da850_pll_init(davinci_pllc0_regs, CONFIG_SYS_DA850_PLL0_PLLM); da850_pll_init(davinci_pllc1_regs, CONFIG_SYS_DA850_PLL1_PLLM); - - /* GPIO setup */ - board_gpio_init(); - +#endif /* setup CSn config */ #if defined(CONFIG_SYS_DA850_CS2CFG) writel(CONFIG_SYS_DA850_CS2CFG, &davinci_emif_regs->ab1cr); @@ -273,13 +270,15 @@ int arch_cpu_init(void) writel(CONFIG_SYS_DA850_CS3CFG, &davinci_emif_regs->ab2cr); #endif - lpsc_on(CONFIG_SYS_DA850_LPSC_UART); - - da850_ddr_setup(); -#elif defined(CONFIG_MACH_DAVINCI_HAWK) da8xx_configure_lpsc_items(lpsc, lpsc_size); +#if defined(CONFIG_SYS_DA850_DDR_INIT) + da850_ddr_setup(); #endif + /* GPIO setup */ + board_gpio_init(); + + NS16550_init((NS16550_t)(CONFIG_SYS_NS16550_COM1), CONFIG_SYS_NS16550_CLK / 16 / CONFIG_BAUDRATE); diff --git a/board/davinci/da8xxevm/da850evm.c b/board/davinci/da8xxevm/da850evm.c index 9bd3e71..34ef53d 100644 --- a/board/davinci/da8xxevm/da850evm.c +++ b/board/davinci/da8xxevm/da850evm.c @@ -137,7 +137,7 @@ const struct pinmux_resource pinmuxes[] = { const int pinmuxes_size = ARRAY_SIZE(pinmuxes); -static const struct lpsc_resource lpsc[] = { +const struct lpsc_resource lpsc[] = { { DAVINCI_LPSC_AEMIF }, /* NAND, NOR */ { DAVINCI_LPSC_SPI1 }, /* Serial Flash */ { DAVINCI_LPSC_EMAC }, /* image download */ @@ -145,6 +145,8 @@ static const struct lpsc_resource lpsc[] = { { DAVINCI_LPSC_GPIO }, }; +const int lpsc_size = ARRAY_SIZE(lpsc); + #ifndef CONFIG_DA850_EVM_MAX_CPU_CLK #define CONFIG_DA850_EVM_MAX_CPU_CLK 300000000 #endif diff --git a/board/enbw/enbw_cmc/enbw_cmc.c b/board/enbw/enbw_cmc/enbw_cmc.c index 5cd5357..f6c3dd9 100644 --- a/board/enbw/enbw_cmc/enbw_cmc.c +++ b/board/enbw/enbw_cmc/enbw_cmc.c @@ -49,7 +49,7 @@ DECLARE_GLOBAL_DATA_PTR; -static const struct lpsc_resource lpsc[] = { +const struct lpsc_resource lpsc[] = { { DAVINCI_LPSC_AEMIF }, { DAVINCI_LPSC_SPI1 }, { DAVINCI_LPSC_ARM_RAM_ROM }, @@ -65,6 +65,8 @@ static const struct lpsc_resource lpsc[] = { { DAVINCI_LPSC_USB11 }, }; +const int lpsc_size = ARRAY_SIZE(lpsc); + static const struct pinmux_config enbw_pins[] = { { pinmux(0), 8, 0 }, { pinmux(0), 8, 1 }, @@ -549,15 +551,6 @@ void board_gpio_init(void) struct davinci_gpio *gpio = davinci_gpio_bank01; /* - * Power on required peripherals - * ARM does not have access by default to PSC0 and PSC1 - * assuming here that the DSP bootloader has set the IOPU - * such that PSC access is available to ARM - */ - if (da8xx_configure_lpsc_items(lpsc, ARRAY_SIZE(lpsc))) - return; - - /* * set LED (gpio Interface not usable here) * set LED pins to output and state 0 */ diff --git a/include/configs/enbw_cmc.h b/include/configs/enbw_cmc.h index c427dc7..804846d 100644 --- a/include/configs/enbw_cmc.h +++ b/include/configs/enbw_cmc.h @@ -48,6 +48,8 @@ #define CONFIG_SKIP_LOWLEVEL_INIT #define CONFIG_DA850_LOWLEVEL #define CONFIG_ARCH_CPU_INIT +#define CONFIG_SYS_DA850_PLL_INIT +#define CONFIG_SYS_DA850_DDR_INIT #define CONFIG_DA8XX_GPIO #define CONFIG_HOSTNAME enbw_cmc #define CONFIG_DISPLAY_CPUINFO