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: [EMAIL PROTECTED]", 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: [EMAIL PROTECTED]
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users
