[U-Boot] [RFC PATCH] arm, davinci: Remove board specific code from da850_lowlevel.c
Signed-off-by: Christian Riesch christian.riesch@omicron.at ---
Hello Heiko,
On my board I cannot use your code in arch_cpu_init() in da850_lowlevel.c since I have different versions of my board with different input clock frequencies. Here u-boot should first determine the board revision number and then configure the SoC accordingly. Therefore I would like to move all board-specific parts (and PLL and memory configuration is board-specific since it depends on the memory chips and oscillators deployed on the board) to board_early_init_f which is called right after arch_cpu_init() and keep only a few initializiation steps in arch_cpu_init().
This patch applies on top of
[U-Boot,v3,1/2] arm, davinci: Rename AM1808 lowlevel functions to DA850 http://patchwork.ozlabs.org/patch/124291/
[U-Boot,v3,2/2] arm, davinci: Remove the duplication of LPSC functions in da850_lowlevel.c http://patchwork.ozlabs.org/patch/124290/
[U-Boot] arm, davinci: Fix setting of the SDRAM configuration register http://patchwork.ozlabs.org/patch/124289/
Thank you for your comments! Best regards, Christian
arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c | 71 ----------------------- 1 files changed, 0 insertions(+), 71 deletions(-)
diff --git a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c index 6f72491..04dfa34 100644 --- a/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +++ b/arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c @@ -23,7 +23,6 @@ */ #include <common.h> #include <nand.h> -#include <ns16550.h> #include <post.h> #include <asm/arch/da850_lowlevel.h> #include <asm/arch/hardware.h> @@ -231,19 +230,6 @@ int da850_ddr_setup(unsigned int freq) return 0; }
-void da850_pinmux_ctl(unsigned long offset, unsigned long mask, - unsigned long value) -{ - clrbits_le32(&davinci_syscfg_regs->pinmux[offset], mask); - setbits_le32(&davinci_syscfg_regs->pinmux[offset], (mask & value)); -} - -__attribute__((weak)) -void board_gpio_init(void) -{ - return; -} - #if defined(CONFIG_NAND_SPL) void nand_boot(void) { @@ -260,11 +246,7 @@ void nand_boot(void) } #endif
-#if defined(CONFIG_NAND_SPL) -void board_init_f(ulong bootflag) -#else int arch_cpu_init(void) -#endif { /* * copied from arch/arm/cpu/arm926ejs/start.S @@ -296,58 +278,5 @@ int arch_cpu_init(void) dv_maskbits(&davinci_syscfg_regs->suspsrc, ((1 << 27) | (1 << 22) | (1 << 20) | (1 << 5) | (1 << 16)));
- /* Setup Pinmux */ - da850_pinmux_ctl(0, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX0); - da850_pinmux_ctl(1, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX1); - da850_pinmux_ctl(2, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX2); - da850_pinmux_ctl(3, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX3); - da850_pinmux_ctl(4, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX4); - da850_pinmux_ctl(5, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX5); - da850_pinmux_ctl(6, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX6); - da850_pinmux_ctl(7, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX7); - da850_pinmux_ctl(8, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX8); - da850_pinmux_ctl(9, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX9); - da850_pinmux_ctl(10, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX10); - da850_pinmux_ctl(11, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX11); - da850_pinmux_ctl(12, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX12); - da850_pinmux_ctl(13, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX13); - da850_pinmux_ctl(14, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX14); - da850_pinmux_ctl(15, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX15); - da850_pinmux_ctl(16, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX16); - da850_pinmux_ctl(17, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX17); - da850_pinmux_ctl(18, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX18); - da850_pinmux_ctl(19, 0xFFFFFFFF, CONFIG_SYS_DA850_PINMUX19); - - /* 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(); - - /* setup CSn config */ - writel(CONFIG_SYS_DA850_CS2CFG, &davinci_emif_regs->ab1cr); - writel(CONFIG_SYS_DA850_CS3CFG, &davinci_emif_regs->ab2cr); - - lpsc_on(DAVINCI_LPSC_UART2); - NS16550_init((NS16550_t)(CONFIG_SYS_NS16550_COM1), - CONFIG_SYS_NS16550_CLK / 16 / CONFIG_BAUDRATE); - - /* - * Fix Power and Emulation Management Register - * see sprufw3a.pdf page 37 Table 24 - */ - writel(readl((CONFIG_SYS_NS16550_COM1 + 0x30)) | 0x00006001, - (CONFIG_SYS_NS16550_COM1 + 0x30)); -#if defined(CONFIG_NAND_SPL) - puts("ddr init\n"); - da850_ddr_setup(132); - - puts("boot u-boot ...\n"); - - nand_boot(); -#else - da850_ddr_setup(132); return 0; -#endif }
Hello Christian,
Christian Riesch wrote:
Signed-off-by: Christian Riesch christian.riesch@omicron.at
Hello Heiko,
On my board I cannot use your code in arch_cpu_init() in da850_lowlevel.c since I have different versions of my board with different input clock frequencies. Here u-boot should first determine the board revision number and then configure the SoC accordingly. Therefore I would like to move all board-specific parts (and PLL and memory configuration is board-specific since it depends on the memory chips and oscillators deployed on the board) to board_early_init_f which is called right after arch_cpu_init() and keep only a few initializiation steps in arch_cpu_init().
Ok, if you have other needs on your board we must change something. But I want to prevent, that all the code you remove in your patch is moved to board code, so all boards must (copy?) it. Instead you should move it to a weak function, which you can replace for your specific needs. (I thought the code was common enough ...)
What do you think?
bye, Heiko
Dear Heiko Schocher,
In message 4EBA11A9.6060905@denx.de you wrote:
On my board I cannot use your code in arch_cpu_init() in da850_lowlevel.c since I have different versions of my board with different input clock frequencies. Here u-boot should first determine the board revision number and then configure the SoC accordingly. Therefore I would like to move all board-specific parts (and PLL and memory configuration is board-specific since it depends on the memory chips and oscillators deployed on the board) to board_early_init_f which is called right after arch_cpu_init() and keep only a few initializiation steps in arch_cpu_init().
Ok, if you have other needs on your board we must change something. But I want to prevent, that all the code you remove in your patch is moved to board code, so all boards must (copy?) it. Instead you should move it to a weak function, which you can replace for your specific needs. (I thought the code was common enough ...)
I guess in most cases no different code will be needed, but just different configuration parameters. It should be possible to handle these through settings in the board config header file.
In any case, a duplication of that coede shall be avoided.
Best regards,
Wolfgang Denk
Hello Heiko, Thank you for your reply!
On Wed, Nov 9, 2011 at 6:37 AM, Heiko Schocher hs@denx.de wrote:
Hello Christian,
Christian Riesch wrote:
Signed-off-by: Christian Riesch christian.riesch@omicron.at
Hello Heiko,
On my board I cannot use your code in arch_cpu_init() in da850_lowlevel.c since I have different versions of my board with different input clock frequencies. Here u-boot should first determine the board revision number and then configure the SoC accordingly. Therefore I would like to move all board-specific parts (and PLL and memory configuration is board-specific since it depends on the memory chips and oscillators deployed on the board) to board_early_init_f which is called right after arch_cpu_init() and keep only a few initializiation steps in arch_cpu_init().
Ok, if you have other needs on your board we must change something. But I want to prevent, that all the code you remove in your patch is moved to board code, so all boards must (copy?) it. Instead you should move it to a weak function, which you can replace for your specific needs. (I thought the code was common enough ...)
What do you think?
That's fine for me, I tried it now with
__attribute__((weak)) int arch_cpu_init(void)
and added my own arch_cpu_init() to my board specific file.
Of course I had to add all those CONFIG_SYS_DA850_PINMUXn with dummy values to my board config file to make it build... I don't like those defines because the values are difficult to read and we already have code for pinmuxing in board/davinci/common/davinci_pinmux.c so it is duplicate code. Couldn't we move this davinci_pinmux.c to arch/arm/cpu/arm926ejs/davinci and use it instead? What do you think?
I'll submit a patch for adding the weak attribute.
Regards, Christian
Hello Christian,
Christian Riesch wrote:
Hello Heiko, Thank you for your reply!
On Wed, Nov 9, 2011 at 6:37 AM, Heiko Schocher hs@denx.de wrote:
Hello Christian,
Christian Riesch wrote:
Signed-off-by: Christian Riesch christian.riesch@omicron.at
Hello Heiko,
On my board I cannot use your code in arch_cpu_init() in da850_lowlevel.c since I have different versions of my board with different input clock frequencies. Here u-boot should first determine the board revision number and then configure the SoC accordingly. Therefore I would like to move all board-specific parts (and PLL and memory configuration is board-specific since it depends on the memory chips and oscillators deployed on the board) to board_early_init_f which is called right after arch_cpu_init() and keep only a few initializiation steps in arch_cpu_init().
Ok, if you have other needs on your board we must change something. But I want to prevent, that all the code you remove in your patch is moved to board code, so all boards must (copy?) it. Instead you should move it to a weak function, which you can replace for your specific needs. (I thought the code was common enough ...)
What do you think?
That's fine for me, I tried it now with
__attribute__((weak)) int arch_cpu_init(void)
and added my own arch_cpu_init() to my board specific file.
Of course I had to add all those CONFIG_SYS_DA850_PINMUXn with dummy values to my board config file to make it build... I don't like those defines because the values are difficult to read and we already have
Why? They are in sync with the doc ...
code for pinmuxing in board/davinci/common/davinci_pinmux.c so it is duplicate code. Couldn't we move this davinci_pinmux.c to arch/arm/cpu/arm926ejs/davinci and use it instead? What do you think?
Hmm.. if you use the http://www-s.ti.com/sc/techlit/spraba2.zip utility from ti, you exactly get the values for the defines ... so I prefered to go this way, but have no real objections against using the code from board/davinci/common/davinci_pinmux.c (Of course, it should be moved to arch/arm/cpu/arm926ejs/davinci)
added Sandeep to cc. Sandeep, what do you think?
I'll submit a patch for adding the weak attribute.
Thanks!
bye, Heiko
Why? They are in sync with the doc ...
code for pinmuxing in board/davinci/common/davinci_pinmux.c so it is duplicate code. Couldn't we move this davinci_pinmux.c to arch/arm/cpu/arm926ejs/davinci and use it instead? What do you think?
Hmm.. if you use the http://www-s.ti.com/sc/techlit/spraba2.zip utility from ti, you exactly get the values for the defines ... so I prefered to go this way, but have no real objections against using the code from board/davinci/common/davinci_pinmux.c (Of course, it should be moved to arch/arm/cpu/arm926ejs/davinci)
added Sandeep to cc. Sandeep, what do you think?
That is fine Should be moved to arch/arm/cpu/arm926ejs/davinci
I'll submit a patch for adding the weak attribute.
Thanks!
participants (4)
-
Christian Riesch -
Heiko Schocher -
Paulraj, Sandeep -
Wolfgang Denk