
On Sat, 9 Aug 2008, Jean-Christophe PLAGNIOL-VILLARD wrote:
+void dcache_disable (void) +{
- ulong reg;
- reg = read_p15_c1 ();
- cp_delay ();
- reg &= ~C1_DC;
- write_p15_c1 (reg);
why not as the other implementation?
ok
/*printf("Calculated %lu timer_load_val\n", timer_load_val);*/
please remove if not need
ok
please add some empty line to be more readable
- /* load value for 10 ms timeout */
- lastdec = timers->TCNTB4 = timer_load_val;
- /* auto load, manual update of Timer 4 */
- timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO |
TCON_4_UPDATE;
- /* auto load, start Timer 4 */
- timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
- timestamp = 0;
ok
+static ulong get_PLLCLK(int pllreg)
please not uppercase
These are s3c* common functions declared in include/common.h, chenging their declarations is outside of the scope of this patchset (see below)
- if (pllreg == APLL)
r = APLL_CON_REG;
- else if (pllreg == MPLL)
r = MPLL_CON_REG;
- else if (pllreg == EPLL)
r = EPLL_CON0_REG;
- else
hang();
please move to switch implementation
ok
- printf("\nCPU: S3C6400@%luMHz\n", get_ARMCLK() / 1000000);
- printf(" Fclk = %luMHz, Hclk = %luMHz, Pclk = %luMHz ",
get_FCLK() / 1000000, get_HCLK() / 1000000,
get_PCLK() / 1000000);
maybe a macro like HZ_TO_MHZ(x) could be helpfull to avoid typo
Don't think such metric conversions deserve a macro.
please add space between parameter
- mrs r0,cpsr
- bic r0,r0,#0x3f
- orr r0,r0,#0xd3
- msr cpsr,r0
ok
diff --git a/include/common.h b/include/common.h index 06ed278..ba87322 100644 --- a/include/common.h +++ b/include/common.h @@ -491,7 +491,8 @@ int prt_mpc8220_clks (void); ulong get_OPB_freq (void); ulong get_PCI_freq (void); #endif -#if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) || defined(CONFIG_LH7A40X) +#if defined(CONFIG_S3C2400) || defined(CONFIG_S3C2410) || \
- defined(CONFIG_LH7A40X) || defined(CONFIG_S3C6400)
Is it possible to have a better and simpler ifdef?
It is possible and desirable to remove these declarations
void s3c2410_irq(void); #define ARM920_IRQ_CALLBACK s3c2410_irq ulong get_FCLK (void);
from this header completely. Don't understand what they are doing in include/common.h. However, this is outside of the scope of this patchset. Please, if you will be fixing this, do this after this patchset.
+#define NFADDR (ELFIN_NAND_BASE+NFADDR_OFFSET)
^^^^^^^^^^^
please remove whitesapce
ok
btw on all macro please add space beetwen operator like
+#define NFCONF_REG __REG(ELFIN_NAND_BASE+NFCONF_OFFSET)
to #define NFCONF_REG __REG(ELFIN_NAND_BASE + NFCONF_OFFSET)
ok
+#define Startup_AMDIV 400
for macro I'll prefer upercase
+#define Startup_MDIV 400 +#define Startup_PDIV 6 +#define Startup_SDIV 1
ok
+typedef vu_char S3C64XX_REG8; +typedef vu_short S3C64XX_REG16; +typedef vu_long S3C64XX_REG32;
I'll prefer you use the type directly
ok
+} /*__attribute__((__packed__))*/ s3c64xx_uart;
why do you remove the packed attribute?
put it back. Makes no _practical_ difference in this case.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de