arm, davinci: Remove board specific code from da850_lowlevel.c

RFC PATCH New Christian Riesch 4 participants 6 messages Nov 8, 2011 – Nov 9, 2011

Subsystems (2)

Arm (32-bit) Arm/actions semi architecture

lore

Share & tools

lore

Christian Riesch

author

2011-11-08 09:39 UTC #0 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

Signed-off-by Christian Riesch <[email protected]>

---

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(-)

arch/arm/cpu/arm926ejs/davinci/da850_lowlevel.c +0 ~~-71~~

index 6f72491..04dfa34 100644
@@ -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
 }

Signature · 1 line

1.7.0.4

Heiko Schocher

2011-11-09 05:37 UTC #1 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

Hello Christian,

Quoted 5 lines · Christian Riesch

Signed-off-by Christian Riesch <[email protected]>
---
Hello Heiko,

Quoted 8 lines

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

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Wolfgang Denk

2011-11-09 06:47 UTC #2 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

Dear Heiko Schocher,

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

Quoted 9 lines
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().

Quoted 5 lines

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

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
The following statement is not true.  The previous statement is true.

Christian Riesch

author

2011-11-09 08:18 UTC #3 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

In reply to Heiko Schocher

Hello Heiko,
Thank you for your reply!

Quoted 17 lines · Heiko Schocher

Hello Christian,
Quoted 5 lines · Christian Riesch
Signed-off-by Christian Riesch <[email protected]>
---
Hello Heiko,
Quoted 8 lines
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().

Quoted 7 lines

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

Heiko Schocher

2011-11-09 08:33 UTC #4 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

Hello Christian,

Quoted 30 lines · Christian Riesch

Hello Heiko,
Thank you for your reply!
Quoted 16 lines · Heiko Schocher
Hello Christian,
Quoted 5 lines · Christian Riesch
Signed-off-by Christian Riesch <[email protected]>
---
Hello Heiko,
Quoted 8 lines
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().
Quoted 7 lines
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

Quoted 7 lines

__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 ...

Quoted 3 lines

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?

Quoted 1 line

I'll submit a patch for adding the weak attribute.

Thanks!

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Paulraj

2011-11-09 21:30 UTC #5 lore raw

RFC PATCH 1/1 arm, davinci: Remove board specific code from da850_lowlevel.c

Quoted 7 lines

Why? They are in sync with the doc ...
Quoted 3 lines
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?

Quoted 7 lines

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

Quoted 4 lines

Quoted 2 lines
I'll submit a patch for adding the weak attribute.
Thanks!