[U-Boot] [PATCH 3/4] arm:exynos4: Set common code for trats and universal_c210 boards

[Piotr Wilczek]

Dear Minkyu Kang,

Thank you for your comments. My motivation to move the dram_init functions
was that they looked the same for every board. I keep them in the board
specific file. I'll fix all the other issues and prepare new version.

Best regards
Piotr Wilczek

-----Original Message-----
From: Minkyu Kang [mailto:promsoft at gmail.com] 
Sent: Thursday, August 16, 2012 11:10 AM
To: Piotr Wilczek
Cc: u-boot at lists.denx.de; Kyungmin Park
Subject: Re: [U-Boot] [PATCH 3/4] arm:exynos4: Set common code for trats and
universal_c210 boards

Dear Piotr Wilczek,

On 10 August 2012 19:12, Piotr Wilczek <p.wilczek at samsung.com> wrote:
> Boards trats and universal are very similar resulting in some common code.
> That common code is moved to exynos4_common/exynos4_common.c
>
> Signed-off-by: Piotr Wilczek <p.wilczek at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> CC: Minkyu Kang <mk7.kang at samsung.com>
> ---
>  board/samsung/exynos4_common/exynos4_common.c |  139
+++++++++++++++++++++++++
>  board/samsung/trats/trats.c                   |  104 +------------------
>  board/samsung/universal_c210/universal.c      |   98 +-----------------
>  3 files changed, 143 insertions(+), 198 deletions(-)  create mode 
> 100644 board/samsung/exynos4_common/exynos4_common.c
>
> diff --git a/board/samsung/exynos4_common/exynos4_common.c 
> b/board/samsung/exynos4_common/exynos4_common.c
> new file mode 100644
> index 0000000..f2413e5
> --- /dev/null
> +++ b/board/samsung/exynos4_common/exynos4_common.c

I'll reject your patch.
It's not a standard way, I think..

> +#ifdef CONFIG_REVISION_TAG
> +u32 get_board_rev(void)
> +{
> +       return board_rev;
> +}
> +#endif
> +
> +int dram_init(void)
> +{
> +       gd->ram_size    = get_ram_size((long *)PHYS_SDRAM_1,
PHYS_SDRAM_1_SIZE)
> +                       + get_ram_size((long *)PHYS_SDRAM_2, 
> +PHYS_SDRAM_2_SIZE); #if CONFIG_NR_DRAM_BANKS == 4
> +       gd->ram_size    += get_ram_size((long *)PHYS_SDRAM_3,
PHYS_SDRAM_3_SIZE)
> +                       + get_ram_size((long *)PHYS_SDRAM_4, 
> +PHYS_SDRAM_4_SIZE); #endif
> +       return 0;
> +}
> +
> +void dram_init_banksize(void)
> +{
> +       gd->bd->bi_dram[0].start = PHYS_SDRAM_1;
> +       gd->bd->bi_dram[0].size = get_ram_size((long *)PHYS_SDRAM_1, \
> +
PHYS_SDRAM_1_SIZE);
> +       gd->bd->bi_dram[1].start = PHYS_SDRAM_2;
> +       gd->bd->bi_dram[1].size = get_ram_size((long *)PHYS_SDRAM_2, \
> +                                                       
> +PHYS_SDRAM_2_SIZE); #if CONFIG_NR_DRAM_BANKS == 4
> +       gd->bd->bi_dram[2].start = PHYS_SDRAM_3;
> +       gd->bd->bi_dram[2].size = get_ram_size((long *)PHYS_SDRAM_3, \
> +
PHYS_SDRAM_3_SIZE);
> +       gd->bd->bi_dram[3].start = PHYS_SDRAM_4;
> +       gd->bd->bi_dram[3].size = get_ram_size((long *)PHYS_SDRAM_4, \
> +                                                       
> +PHYS_SDRAM_4_SIZE); #endif }

also, this is not a common function.
Although they are almost same, but I think it's a board specific.
Please don't move it.

> +
> +int exynos4_mmc_init(struct exynos4_gpio_part2 *gpio2)

How about using pinmux instead?

Thanks.
Minkyu Kang.
--
from. prom.
www.promsoft.net