[U-Boot] [PATCH V4 8/9] SMDK5250: Enable EMMC booting

Amarendra Reddy
2013-01-15T09:14:03Z

Hi Simon,

Thanks for review comments.
Please find my responses below.

Thanks & Regards
Amarendra

On 10 January 2013 22:09, Simon Glass <sjg@chromium.org> wrote:

> Hi Amar,
>
> On Fri, Jan 4, 2013 at 1:34 AM, Amar <amarendra.xt@samsung.com> wrote:
> > This patch adds support for EMMC booting on SMDK5250.
> >
> > Changes from V1:
> > 1)Updated spl_boot.c file to maintain irom pointer table
> > instead of using the #define values defined in header file.
> >
> > Changes from V2:
> > 1)Updation of commit message and resubmition of proper patch set.
> >
> > Changes from V3:
> > No change.
> >
> > Signed-off-by: Amar <amarendra.xt@samsung.com>
> > ---
> > board/samsung/smdk5250/clock_init.c | 15 +++++++++++
> > board/samsung/smdk5250/clock_init.h | 5 ++++
> > board/samsung/smdk5250/spl_boot.c | 52
> ++++++++++++++++++++++++++++++++-----
> > 3 files changed, 65 insertions(+), 7 deletions(-)
> >
> > diff --git a/board/samsung/smdk5250/clock_init.c
> b/board/samsung/smdk5250/clock_init.c
> > index c009ae5..154993c 100644
> > --- a/board/samsung/smdk5250/clock_init.c
> > +++ b/board/samsung/smdk5250/clock_init.c
> > @@ -28,6 +28,7 @@
> > #include <asm/arch/clk.h>
> > #include <asm/arch/clock.h>
> > #include <asm/arch/spl.h>
> > +#include <asm/arch/dwmmc.h>
> >
> > #include "clock_init.h"
> > #include "setup.h"
> > @@ -664,3 +665,17 @@ void clock_init_dp_clock(void)
> > /* We run DP at 267 Mhz */
> > setbits_le32(&clk->div_disp1_0, CLK_DIV_DISP1_0_FIMD1);
> > }
> > +
> > +/*
> > + * Set clock divisor value for booting from EMMC.
> > + * Set DWMMC channel-0 clk div to operate mmc0 device at 50MHz.
> > + */
> > +void emmc_boot_clk_div_set(void)
> > +{
> > + struct exynos5_clock *clk = (struct exynos5_clock
> *)EXYNOS5_CLOCK_BASE;
> > + unsigned int div_mmc;
> > +
> > + div_mmc = readl((unsigned int) &clk->div_fsys1) &
> ~FSYS1_MMC0_DIV_MASK;
> > + div_mmc |= FSYS1_MMC0_DIV_VAL;
> > + writel(div_mmc, (unsigned int) &clk->div_fsys1);
> > +}
> > diff --git a/board/samsung/smdk5250/clock_init.h
> b/board/samsung/smdk5250/clock_init.h
> > index f751bcb..20a1d47 100644
> > --- a/board/samsung/smdk5250/clock_init.h
> > +++ b/board/samsung/smdk5250/clock_init.h
> > @@ -146,4 +146,9 @@ struct mem_timings *clock_get_mem_timings(void);
> > * Initialize clock for the device
> > */
> > void system_clock_init(void);
> > +
> > +/*
> > + * Set clock divisor value for booting from EMMC.
> > + */
> > +void emmc_boot_clk_div_set(void);
> > #endif
> > diff --git a/board/samsung/smdk5250/spl_boot.c
> b/board/samsung/smdk5250/spl_boot.c
> > index d8f3c1e..906e197 100644
> > --- a/board/samsung/smdk5250/spl_boot.c
> > +++ b/board/samsung/smdk5250/spl_boot.c
> > @@ -23,16 +23,38 @@
> > #include<common.h>
> > #include<config.h>
> >
> > +#include <asm/arch-exynos/dmc.h>
> > +#include <asm/arch/clock.h>
> > +#include <asm/arch/clk.h>
> > +
> > +#include "clock_init.h"
> > +
> > +/* Index into irom ptr table */
> > +enum index {
> > + MMC_INDEX,
> > + EMMC44_INDEX,
> > + EMMC44_END_INDEX,
> > + SPI_INDEX,
> > +};
> > +
> > +/* IROM Function Pointers Table */
> > +u32 irom_ptr_table[] = {
> > + [MMC_INDEX] = 0x02020030, /* iROM Function Pointer-SDMMC
> boot */
> > + [EMMC44_INDEX] = 0x02020044, /* iROM Function Pointer-EMMC4.4
> boot*/
> > + [EMMC44_END_INDEX] = 0x02020048,/* iROM Function Pointer
> > + -EMMC4.4 end boot
> operation */
> > + [SPI_INDEX] = 0x02020058, /* iROM Function Pointer-SPI
> boot */
> > + };
> > +
> > enum boot_mode {
> > BOOT_MODE_MMC = 4,
> > BOOT_MODE_SERIAL = 20,
> > + BOOT_MODE_EMMC = 8, /* EMMC4.4 */
> > /* Boot based on Operating Mode pin settings */
> > BOOT_MODE_OM = 32,
> > BOOT_MODE_USB, /* Boot using USB download */
> > };
> >
> > - typedef u32 (*spi_copy_func_t)(u32 offset, u32 nblock, u32 dst);
> > -
> > /*
> > * Copy U-boot from mmc to RAM:
> > * COPY_BL2_FNPTR_ADDR: Address in iRAM, which Contains
> > @@ -40,23 +62,39 @@ enum boot_mode {
> > */
> > void copy_uboot_to_ram(void)
> > {
> > - spi_copy_func_t spi_copy;
> > enum boot_mode bootmode;
> > - u32 (*copy_bl2)(u32, u32, u32);
> > -
> > + u32 (*spi_copy)(u32 offset, u32 nblock, u32 dst);
> > + u32 (*copy_bl2)(u32 offset, u32 nblock, u32 dst);
> > + u32 (*copy_bl2_from_emmc)(u32 nblock, u32 dst);
> > + void (*end_bootop_from_emmc)(void);
> > + /* read Operation Mode ststus register to find the bootmode */
> > bootmode = readl(EXYNOS5_POWER_BASE) & OM_STAT;
> >
> > switch (bootmode) {
> > case BOOT_MODE_SERIAL:
> > - spi_copy = *(spi_copy_func_t
> *)EXYNOS_COPY_SPI_FNPTR_ADDR;
> > + spi_copy = (void *) *(u32 *)irom_ptr_table[SPI_INDEX];
>
> This looks OK to me. My only suggestion is to put the lookup in a function
> like:
>
> static void *get_irom_func(int index)
>
> and avoid all the casting here.
>
> Ok will implement in next patch set.

> > spi_copy(SPI_FLASH_UBOOT_POS, CONFIG_BL2_SIZE,
> > CONFIG_SYS_TEXT_BASE);
> > break;
>
> Re SPI, the IROM performance is quite slow - do you plan to send up
> the U-Boot SPI code version? with a 32KB SPL there might be room to
> just use the normal driver.
>
Ok shall be taken in a new patch set.

>
> > case BOOT_MODE_MMC:
> > - copy_bl2 = (void *) *(u32 *)COPY_BL2_FNPTR_ADDR;
> > + copy_bl2 = (void *) *(u32 *)irom_ptr_table[MMC_INDEX];
> > copy_bl2(BL2_START_OFFSET, BL2_SIZE_BLOC_COUNT,
> > CONFIG_SYS_TEXT_BASE);
> > break;
> > + case BOOT_MODE_EMMC:
> > + /* Set the FSYS1 clock divisor value for EMMC boot */
> > + emmc_boot_clk_div_set();
> > +
> > + copy_bl2_from_emmc =
> > + (void *) *(u32 *)irom_ptr_table[EMMC44_INDEX];
> > + end_bootop_from_emmc =
> > + (void *) *(u32
> *)irom_ptr_table[EMMC44_END_INDEX];
> > +
> > + copy_bl2_from_emmc(BL2_SIZE_BLOC_COUNT,
> CONFIG_SYS_TEXT_BASE);
> > + end_bootop_from_emmc();
> > +
> > + break;
> > +
> > default:
> > break;
> > }
> > --
> > 1.8.0
> >
>
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>