[U-Boot] [PATCH 6/9] SMDK5250: Enable DWMMC

[Amarendra Reddy]

Hi Simon,

Thanks for the review comments.
Please find the responses below.

Thanks & Regards
Amarendra Reddy

On 20 December 2012 07:59, Simon Glass <sjg at chromium.org> wrote:

> Hi Amar,
>
> On Mon, Dec 17, 2012 at 3:19 AM, Amar <amarendra.xt at samsung.com> wrote:
>
> > This patch enables DWMMC for SMDK5250.
> > Support both dt and non-dt versions.
> >
> > Signed-off-by: Amar <amarendra.xt at samsung.com>
> > ---
> >  board/samsung/smdk5250/smdk5250.c |   36
> > ++++++++++++++++++++++++++++++++----
> >  include/configs/exynos5250-dt.h   |    9 +++++++++
> >  2 files changed, 41 insertions(+), 4 deletions(-)
> >
> > diff --git a/board/samsung/smdk5250/smdk5250.c
> > b/board/samsung/smdk5250/smdk5250.c
> > index 4d24978..7a9c8f6 100644
> > --- a/board/samsung/smdk5250/smdk5250.c
> > +++ b/board/samsung/smdk5250/smdk5250.c
> > @@ -27,6 +27,7 @@
> >  #include <netdev.h>
> >  #include <spi.h>
> >  #include <asm/arch/cpu.h>
> > +#include <asm/arch/dwmmc.h>
> >  #include <asm/arch/gpio.h>
> >  #include <asm/arch/mmc.h>
> >  #include <asm/arch/pinmux.h>
> > @@ -192,16 +193,43 @@ int checkboard(void)
> >  #ifdef CONFIG_GENERIC_MMC
> >  int board_mmc_init(bd_t *bis)
> >  {
> > -       int err;
> > +       int err = 0, ret = 0;
> >
> > +#ifdef CONFIG_OF_CONTROL
> > +       /* dwmmc initializattion for available channels */
> > +       err = exynos_dwmmc_init(gd->fdt_blob);
> > +       if (err) {
> > +               debug("dwmmc init failed\n");
> > +       }
> > +       ret |= err;
> > +#else
> >         err = exynos_pinmux_config(PERIPH_ID_SDMMC0,
> > PINMUX_FLAG_8BIT_MODE);
> >         if (err) {
> >                 debug("SDMMC0 not configured\n");
> > -               return err;
> >         }
> > +       ret |= err;
> >
>
> Perhaps we need an exynos5-dt.c board file instead of using smdk5250? It
> seems like you will end up with lots of #ifdefs otherwise.
>
> OK.
>


> >
> > -       err = s5p_mmc_init(0, 8);
> > -       return err;
> > +       /*eMMC: dwmmc Channel-0 with 8 bit bus width */
> > +       err = exynos_dwmmc_init(0, 8);
> > +       if (err) {
> > +               debug("dwmmc Channel-0 init failed\n");
> >
>
> Don't need {} here.
> OK.
>
> > +       }
> > +       ret |= err;
> > +
> > +       err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
> > +       if (err) {
> > +               debug("SDMMC2 not configured\n");
> >
>
> and here
> OK.
>
> > +       }
> > +       ret |= err;
> > +
> > +       /*SD: dwmmc Channel-2 with 4 bit bus width */
> > +       err = exynos_dwmmc_init(2, 4);
> > +       if (err) {
> > +               debug("dwmmc Channel-2 init failed\n");
> > +       }
> > +       ret |= err;
> > +#endif
> > +       return ret;
> >  }
> >  #endif
> >
> > diff --git a/include/configs/exynos5250-dt.h
> > b/include/configs/exynos5250-dt.h
> > index 12f555c..3b89e20 100644
> > --- a/include/configs/exynos5250-dt.h
> > +++ b/include/configs/exynos5250-dt.h
> > @@ -84,6 +84,8 @@
> >  #define CONFIG_MMC
> >  #define CONFIG_SDHCI
> >  #define CONFIG_S5P_SDHCI
> > +#define CONFIG_DWMMC
> >
>
> What does this config enable? Is it in a README somewhere?
> CONFIG_DWMMC is for enaling DWMMC. It has to be added into README.
>

> > +#define CONFIG_EXYNOS_DWMMC
> >
> >  #define CONFIG_BOARD_EARLY_INIT_F
> >
> > @@ -116,6 +118,13 @@
> >  #define CONFIG_SPL
> >  #define COPY_BL2_FNPTR_ADDR    0x02020030
> >
> > +/* eMMC4.4 SPL */
> > +#define EMMC44_COPY_BL2_FNPTR_ADDR     0x02020044
> > +#define EMMC44_END_BOOTOP_FNPTR_ADDR   0x02020048
> >
>
> What are these for?
>
> These are the IROM function pointers used druing SPL boot.
>  Any how these #defines wii be removed, as a table for all iROM function
> pointers will be maintained in spl_boot.c.
>
> +
> > +#define FSYS1_MMC0_DIV_MASK            0xff0f
> >
>
> This seems like something for the SOC headers, not the board header.
>
> FSYS1_MMC0_DIV_MASK will be moved into include/asm/arch/clk.h
>


> > +
> > +
> >  /* specific .lds file */
> >  #define CONFIG_SPL_LDSCRIPT
> >  "board/samsung/smdk5250/smdk5250-uboot-spl.lds"
> >  #define CONFIG_SPL_TEXT_BASE   0x02023400
> > --
> > 1.7.0.4
> >
> >
> Regards,
> Simon
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>