Dear Lars,
In message <1357663926-15937-2-git-send-email-la...@wh2.tu-dresden.de> you
wrote:
...
> arch/arm/include/asm/arch-am33xx/ddr_defs.h | 18 ++
> board/phytec/pcm051/Makefile | 46 ++++
> board/phytec/pcm051/board.c | 271 +++++++++++++++++++++++
> board/phytec/pcm051/board.h | 33 +++
> board/phytec/pcm051/mux.c | 133 ++++++++++++
> boards.cfg | 1 +
> include/configs/pcm051.h | 308
> +++++++++++++++++++++++++++
Please add an entry to the MAINTAINERS file.
Also, please run your patch through checkpatch - it complains about a
number of style errors:
WARNING: Whitespace before semicolon
> +/* DDR RAM defines */
> +#define DDR_CLK_MHZ 303
Is this really correct? 303 ??
> +static void rtc32k_enable(void)
> +{
> + struct rtc_regs *rtc = (struct rtc_regs *)AM335X_RTC_BASE;
> +
> + /*
> + * Unlock the RTC's registers. For more details please see the
> + * RTC_SS section of the TRM. In order to unlock we need to
> + * write these specific values (keys) in this order.
> + */
> + writel(0x83e70b13, &rtc->kick0r);
> + writel(0x95a4f1e0, &rtc->kick1r);
These magic numbers should probbly be moved to some header file ?
> +void s_init(void)
> +{
> + /* WDT1 is already running when the bootloader gets control
> + * Disable it to avoid "random" resets
> + */
Incorrect multiline comment style.
> + writel(0xAAAA, &wdtimer->wdtwspr);
More hard-wired magic values?
> + while (readl(&wdtimer->wdtwwps) != 0x0)
> + ;
No timeout?
> + writel(0x5555, &wdtimer->wdtwspr);
More hard-wired magic values?
> + while (readl(&wdtimer->wdtwwps) != 0x0)
> + ;
No timeout?
> + while ((readl(&uart_base->uartsyssts) &
> + UART_CLK_RUNNING_MASK) != UART_CLK_RUNNING_MASK)
> + ;
Braces needed for multiline statement.
> +#ifdef CONFIG_BOARD_LATE_INIT
> +int board_late_init(void)
> +{
> + return 0;
> +}
> +#endif
Do you plan to use this? Otherwise please just omit such dead code.
> +#ifdef CONFIG_DRIVER_TI_CPSW
> +static void cpsw_control(int enabled)
> +{
> + /* VTP can be added here */
> +
> + return;
> +}
Ditto...
> + if (!eth_getenv_enetaddr("ethaddr", mac_addr)) {
> + debug("<ethaddr> not set. Reading from E-fuse\n");
This should be a printf(). Any such automatic changes are always
worth to be told explicitly.
> + goto try_usbether;
...
> +#endif
> +try_usbether:
Did you ever try building without CONFIG_DRIVER_TI_CPSW set? I bet
this causes a few compiler warnings. [Hint: You define a label, but
don't use it anywhere.]
> +#define CONFIG_ENV_SIZE (128 << 10) /* 128 KiB */
Does this really make sense? I doubt you will ever need more than 10%
of this size - so you are just wasting a lot of time for checksumming
unused memory...
> +#define CONFIG_ENV_OVERWRITE 1
Please do not define values for logical variables like this one;
please fix globally.
> +#define CONFIG_ENV_IS_NOWHERE
Really?
Thanks!
Best regards,
Wolfgang Denk
--
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: w...@denx.de
Crash programs fail because they are based on the theory that, with
nine women pregnant, you can get a baby a month. - Wernher von Braun
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
