
Dear Lars,
In message 1357663926-15937-2-git-send-email-larsi@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