Dear Macpaul Lin,
In message 1276248884-21492-2-git-send-email-macpaul@andestech.com you wrote:
--===============1276623933==
Add nds32 architecture with generic cpu core support. Add nds32 architecture with n1213s cpu core support.
Please do not send patches as attachments, send them inline. Try using "git format-patch" to create the patches and "git send-email" to send them.
Don't indent the commit messages like that.
diff --git a/arch/nds32/config.mk b/arch/nds32/config.mk new file mode 100644 index 0000000..e88e516 --- /dev/null +++ b/arch/nds32/config.mk @@ -0,0 +1,32 @@ +# +# (C) Copyright 2000-2002 +# Wolfgang Denk, DENX Software Engineering, wd@denx.de. +# +# (C) Copyright 2006 +# Shawn Lin, Andes Technology Corporation nobuhiro@andestech.com +# Macpaul Lin, Andes Technology Corporation macpaul@andestech.com
So no work has been done on this in the last 4 years? I guess you want to update your Copyright messages - all of them.
...
+START = start.o +COBJS = interrupts.o cpu.o
Please keep all such lists sorted. 'c' < 'i'.
...
+void do_reset (cmd_tbl_t *cmdtp, bd_t *bd, int flag, int argc, char *argv[]) +{
- extern void reset_cpu(ulong addr);
- disable_interrupts();
+// reset_cpu(0); FIXME: -by Shawn, currently no ROM loader at addr 0
No C++ comments allowd. Please fix globally.
+void flush_cache (unsigned long dummy1, unsigned long dummy2) +{ +/*
- unsigned long u32IfRun = 0;
- if(u32IfRun) return;
- u32IfRun = 1;
We do not allow CamelCase identifiers in U-Boot. Please fix globally. Also, pay attentiuon to indentation rules, the "return" belongs on a new line, indented by a TAB.
- reset_cpu((unsigned long)flush_cache);
- return;
+*/
Please do not add dead code. Remove it. Please fix globally.
--- /dev/null +++ b/arch/nds32/cpu/interrupts.c
...
+//#include <board/AndesTech/include/porting.h> +#include "../../../board/AndesTech/include/porting.h"
This is fundeamentally flawed. Global code must never include any board specific header files.
+/*
- TimerBase[0] is a NULL entry
- */
+fLib_TimerReg *TimerBase[] ={0, (fLib_TimerReg *) NDS32_COMMON_TIMER1_BASE,
- (fLib_TimerReg *) NDS32_COMMON_TIMER2_BASE,(fLib_TimerReg *)NDS32_COMMON_TIMER3_BASE};
Lines too long. Please fix globally.
+ulong get_timer_masked(void) +{
- ulong now = Read_Timer_Counter(1);;
- if (lastdec >= now)
- {
/* normal mode */timestamp += lastdec - now;- } else {
/* we have an overflow ... */timestamp += lastdec + TIMER_LOAD_VAL - now;- }
Incorrect brace style. Please fix globally.
Please re-read the CodingStyle document!!!
...
+uart_init: +#! 1. fLib_SetSerialMode(DebugSerialPort, SERIAL_MDR_UART);
- li $r0, #0x99600000 !; CPE_UART4_BASE = 0x99600000
- lwi $r1, [$r0+#0x20] !; mdr = mdr = inw(CPE_UART4_BASE + SERIAL_MDR)
- li $r2, #0xfffffffc !; mdr &= ~(0x3) ; SERIAL_MDR_MODE_SEL = 0x3
- and $r1, $r1, $r2
- swi $r1, [$r0+#0x20] !; outw(CPE_UART4_BASE + SERIAL_MDR, mdr | mode);
+#! 2. fLib_SerialInit(DebugSerialPort, (int)DEFAULT_HOST_BAUD, PARITY_NONE, 0, 8); +#! dgbserialport = 0x99600000; baud = (UART_CLOCK / 614400) = 24; PARITY_NONE = 0; num = 0; len = 8;
- li $r0, #0x99600000 !;lcr = inw(port + SERIAL_LCR) & ~SERIAL_LCR_DLAB; SERIAL_LCR = 0x0c
- lwi $r1, [$r0+#0x0c]
- li $r2, #0xffffff7f !; SERIAL_LCR_DLAB = 0x80
- and $r1, $r1, $r2
- li $r2, #0x80 !; outw(port + SERIAL_LCR,SERIAL_LCR_DLAB);
- swi $r2, [$r0+#0x0c]
- li $r2, #0 !; outw(port + SERIAL_DLM, ((baudrate & 0xf00) >> 8));
- swi $r2, [$r0+#0x4]
+#ifdef __NDS32_N1213_43U1H__ /* AG101 */
- li $r2, #60 !; outw(port + SERIAL_DLL, (baudrate & 0xff));
+#else
- li $r2, #24 !; outw(port + SERIAL_DLL, (baudrate & 0xff));
+#endif
- swi $r2, [$r0+#0x0]
- andi $r1, $r1, #0xc0 !; lcr &= 0xc0;
- li $r2, #3 !; len-=5;
- or $r1, $r1, $r2 !; lcr|=len;
- swi $r1, [$r0+#0x0c] !; outw(port+SERIAL_LCR,lcr);
+#! 3. fLib_SetSerialFifoCtrl(DebugSerialPort, 0, 0, ENABLE(1), ENABLE(1));
- li $r0, #0x99600000
- li $r1, #0x7 !; fcr = 0x7
- swi $r1, [$r0+#0x8] !; SERIAL_FCR = 0x08; outw(port+SERIAL_FCR,fcr);
- ret
Lines too long - why don't you implement uart_init() in C?
+void cleanup_before_linux(void) +{
- /*
* this function is called just before we call linux* it prepares the processor for linux** we disable interrupt and caches.*/+#ifdef CONFIG_MMU
- unsigned long i;
+#endif
Please move such block comments outside the functions. Functions should normally start with declarations. Please apply globally.
diff --git a/arch/nds32/cpu/n1213s/config.mk b/arch/nds32/cpu/n1213s/config.mk new file mode 100644 index 0000000..316332e --- /dev/null +++ b/arch/nds32/cpu/n1213s/config.mk
...
+#
+PLATFORM_CPPFLAGS +=
This is a noop - why do we need this file?
diff --git a/arch/nds32/cpu/n1213s/interrupts.c b/arch/nds32/cpu/n1213s/interrupts.c new file mode 100644 index 0000000..a115a72 --- /dev/null +++ b/arch/nds32/cpu/n1213s/interrupts.c
...
- */
+#include "../interrupts.c"
Is this all? Then why do we need this file?
diff --git a/arch/nds32/cpu/n1213s/lowlevel_init.S b/arch/nds32/cpu/n1213s/lowlevel_init.S new file mode 100644 index 0000000..26e0727 --- /dev/null +++ b/arch/nds32/cpu/n1213s/lowlevel_init.S
...
+#include "../lowlevel_init.S"
Ditto?
diff --git a/arch/nds32/cpu/n1213s/start.S b/arch/nds32/cpu/n1213s/start.S new file mode 100644 index 0000000..e93d048 --- /dev/null +++ b/arch/nds32/cpu/n1213s/start.S
...
+#include "../start.S"
And again.
This makes no sense.
Please find a better way to use common code.
...
+__start_andesboot: .word start_andesboot
+!=========================================================================
Too many blank lines. absolute maximum is 2. Please fix globally.
Best regards,
Wolfgang Denk