Dear "HeungJun, Kim",
In message <1325751248-23125-1-git-send-email-riverful....@samsung.com> you
wrote:
> This patch adds support for Samsung TRATS board
>
> Signed-off-by: HeungJun, Kim <riverful....@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.p...@samsung.com>
...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a56ca10..c856c59 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -700,6 +700,10 @@ Minkyu Kang <mk7.k...@samsung.com>
> s5p_goni ARM ARMV7 (S5PC110 SoC)
> s5pc210_universal ARM ARMV7 (EXYNOS4210 SoC)
>
> +Heungjun Kim <riverful....@samsung.com>
> +
> + trats ARM ARMV7 (EXYNOS4210 SoC)
> +
> Chander Kashyap <k.chan...@samsung.com>
Please keep list sorted ("Ki..." goes after "Ka...").
> +++ b/board/samsung/trats/Makefile
> @@ -0,0 +1,44 @@
> +#
> +# Copyright (C) 2010 Samsung Electronics
> +# Heungjun Kim <riverful....@samsung.com>
> +#
> +# See file CREDITS for list of people who contributed to this
> +# project.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License version 2 as
> +# published by the Free Software Foundation.
Please see http://www.denx.de/wiki/view/U-Boot/Patches#Notes
We need GPLv2+ for all new files. Please fix globally.
> + */
> +uart_asm_init:
...
> +system_clock_init:
...
> +system_power_init:
...
What are the compelling reasons to write thes functionms in assembler,
and not in C?
> +int board_init(void)
> +{
> + gpio1 = (struct exynos4_gpio_part1 *) EXYNOS4_GPIO_PART1_BASE;
> + gpio2 = (struct exynos4_gpio_part2 *) EXYNOS4_GPIO_PART2_BASE;
> +
> + gd->bd->bi_boot_params = PHYS_SDRAM_1 + 0x100;
> +
> + check_hw_revision();
> + printf("HW Revision:\t0x%x\n", board_rev);
> +
> +#if defined(CONFIG_PMIC)
> + pmic_init();
> +#endif
> +
> + return 0;
> +}
Does this code build free wof warnings? I would expect to see
warnings here because of unused variables gpio1 and gpio2?
...
> +/*
> + * High Level Configuration Options
> + * (easy to change)
> + */
> +#define CONFIG_SAMSUNG 1 /* in a SAMSUNG core */
> +#define CONFIG_S5P 1 /* which is in a S5P Family */
> +#define CONFIG_EXYNOS4210 1 /* which is in a EXYNOS4210 */
> +#define CONFIG_TRATS 1 /* working with TRATS */
Please do not define values for variables that select features only.
Please fix globally.
> +#include <asm/arch/cpu.h> /* get chip and board defs */
> +
> +#define CONFIG_ARCH_CPU_INIT
> +#define CONFIG_DISPLAY_CPUINFO
> +#define CONFIG_DISPLAY_BOARDINFO
> +
> +/* Keep L2 Cache Disabled */
> +#define CONFIG_SYS_L2CACHE_OFF 1
> +
> +#define CONFIG_SYS_SDRAM_BASE 0x40000000
> +#define CONFIG_SYS_TEXT_BASE 0x44800000
> +
> +/* input clock of PLL: TRATS has 24MHz input clock at EXYNOS4210 */
> +#define CONFIG_SYS_CLK_FREQ_C210 24000000
> +
> +#define CONFIG_SETUP_MEMORY_TAGS
> +#define CONFIG_CMDLINE_TAG
> +#define CONFIG_INITRD_TAG
> +#define CONFIG_REVISION_TAG
> +#define CONFIG_CMDLINE_EDITING
> +
> +/* MACH_TYPE_TRATS macro will be removed once added to mach-types */
> +#define MACH_TYPE_TRATS 3928
> +#define CONFIG_MACH_TYPE MACH_TYPE_TRATS
> +
> +/* Size of malloc() pool */
> +#define CONFIG_SYS_MALLOC_LEN (CONFIG_ENV_SIZE + (1 << 20))
> +
> +/* select serial console configuration */
> +#define CONFIG_SERIAL_MULTI 1
> +#define CONFIG_SERIAL2 1 /* use SERIAL 2 */
> +#define CONFIG_BAUDRATE 115200
> +
> +/* MMC */
> +#define CONFIG_GENERIC_MMC 1
> +#define CONFIG_MMC 1
> +#define CONFIG_S5P_MMC 1
> +
> +/* PWM */
> +#define CONFIG_PWM 1
...
> +#define MBRPARTS_DEFAULT "20M(permanent)"\
> + ",20M(boot)"\
> + ",1G(system)"\
> + ",100M(swap)"\
> + ",-(UMS)\0"
...
> + "mbrparts=" MBRPARTS_DEFAULT \
MBR ??? Is this a typo and you actually mean MTD ?
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
Es ist nicht genug zu wissen, man muß auch anwenden; es ist nicht ge-
nug zu wollen, man muß auch tun. -- Goethe, Maximen und Reflexionen
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
