Patchwork [U-Boot,V5,04/10] EXYNOS5: DWMMC: Added FDT support for DWMMC

login
register
mail settings
Submitter Amar
Date Jan. 21, 2013, 11:43 a.m.
Message ID <1358768638-14187-5-git-send-email-amarendra.xt@samsung.com>
Download mbox | patch
Permalink /patch/214098/
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Comments

Amar - Jan. 21, 2013, 11:43 a.m.
This patch adds FDT support for DWMMC, by reading the DWMMC node data
from the device tree and initialising DWMMC channels as per data
obtained from the node.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
Signed-off-by: Amar <amarendra.xt@samsung.com>
---
Changes since V1:
        1)Updated code to have same signature for the function
        exynos_dwmci_init() for both FDT and non-FDT versions.
        2)Updated code to pass device_id parameter to the function
        exynos5_mmc_set_clk_div() instead of index.
        3)Updated code to decode the value of "samsung,width" from FDT.
        4)Channel index is computed instead of getting from FDT.

Changes since V2:
        1)Updation of commit message and resubmition of proper patch set.

Changes since V3:
        1)Replaced the new function exynos5_mmc_set_clk_div() with the
        existing function set_mmc_clk(). set_mmc_clk() will do the purpose.
        2)Computation of FSYS block clock divisor (pre-ratio) is added.

Changes since V4:
        1)Replaced "unsigned int exynos_dwmmc_init(int index, int bus_width)" with
        int exynos_dwmci_add_port(int, u32, inth, u32)
                i)exynos_dwmmc_add_port() will be used by non-FDT boards.
                ii)In FDT case, exynos_dwmmc_init(const void *blob) will use
                exynos_dwmmc_add_port() for every channel enabled in device node.
        2)Changed the computation method of mmc clock divisor.
        3)Updated exynos_dwmmc_init() to compute the 'clksel_val' within the function.

 arch/arm/include/asm/arch-exynos/dwmmc.h |  11 +--
 drivers/mmc/exynos_dw_mmc.c              | 127 ++++++++++++++++++++++++++++---
 include/dwmmc.h                          |   3 +
 3 files changed, 124 insertions(+), 17 deletions(-)
Simon Glass - Jan. 26, 2013, 8:08 p.m.
Hi,

On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra.xt@samsung.com> wrote:
> This patch adds FDT support for DWMMC, by reading the DWMMC node data
> from the device tree and initialising DWMMC channels as per data
> obtained from the node.
>
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> Signed-off-by: Amar <amarendra.xt@samsung.com>

Acked-by: Simon Glass <sjg@chromium.org>

See below for nit/question.

> ---
> Changes since V1:
>         1)Updated code to have same signature for the function
>         exynos_dwmci_init() for both FDT and non-FDT versions.
>         2)Updated code to pass device_id parameter to the function
>         exynos5_mmc_set_clk_div() instead of index.
>         3)Updated code to decode the value of "samsung,width" from FDT.
>         4)Channel index is computed instead of getting from FDT.
>
> Changes since V2:
>         1)Updation of commit message and resubmition of proper patch set.
>
> Changes since V3:
>         1)Replaced the new function exynos5_mmc_set_clk_div() with the
>         existing function set_mmc_clk(). set_mmc_clk() will do the purpose.
>         2)Computation of FSYS block clock divisor (pre-ratio) is added.
>
> Changes since V4:
>         1)Replaced "unsigned int exynos_dwmmc_init(int index, int bus_width)" with
>         int exynos_dwmci_add_port(int, u32, inth, u32)
>                 i)exynos_dwmmc_add_port() will be used by non-FDT boards.
>                 ii)In FDT case, exynos_dwmmc_init(const void *blob) will use
>                 exynos_dwmmc_add_port() for every channel enabled in device node.
>         2)Changed the computation method of mmc clock divisor.
>         3)Updated exynos_dwmmc_init() to compute the 'clksel_val' within the function.
>
>  arch/arm/include/asm/arch-exynos/dwmmc.h |  11 +--
>  drivers/mmc/exynos_dw_mmc.c              | 127 ++++++++++++++++++++++++++++---
>  include/dwmmc.h                          |   3 +
>  3 files changed, 124 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h
> index 8acdf9b..3b147b8 100644
> --- a/arch/arm/include/asm/arch-exynos/dwmmc.h
> +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
> @@ -27,10 +27,7 @@
>  #define DWMCI_SET_DRV_CLK(x)   ((x) << 16)
>  #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
>
> -int exynos_dwmci_init(u32 regbase, int bus_width, int index);
> -
> -static inline unsigned int exynos_dwmmc_init(int index, int bus_width)
> -{
> -       unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
> -       return exynos_dwmci_init(base, bus_width, index);
> -}
> +#ifdef CONFIG_OF_CONTROL
> +int exynos_dwmmc_init(const void *blob);
> +#endif
> +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel);
> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> index 72a31b7..1d09e56 100644
> --- a/drivers/mmc/exynos_dw_mmc.c
> +++ b/drivers/mmc/exynos_dw_mmc.c
> @@ -19,39 +19,146 @@
>   */
>
>  #include <common.h>
> -#include <malloc.h>
>  #include <dwmmc.h>
> +#include <fdtdec.h>
> +#include <libfdt.h>
> +#include <malloc.h>
>  #include <asm/arch/dwmmc.h>
>  #include <asm/arch/clk.h>
> +#include <asm/arch/pinmux.h>
>
> -static char *EXYNOS_NAME = "EXYNOS DWMMC";
> +#define        DWMMC_MAX_CH_NUM                4
> +#define        DWMMC_MAX_FREQ                  52000000
> +#define        DWMMC_MIN_FREQ                  400000
> +#define        DWMMC_MMC0_CLKSEL_VAL           0x03030001
> +#define        DWMMC_MMC2_CLKSEL_VAL           0x03020001
>
> +/*
> + * Function used as callback function to initialise the
> + * CLKSEL register for every mmc channel.
> + */
>  static void exynos_dwmci_clksel(struct dwmci_host *host)
>  {
> -       u32 val;
> -       val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> -               DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
> +       dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
> +}
>
> -       dwmci_writel(host, DWMCI_CLKSEL, val);
> +unsigned int exynos_dwmci_get_clk(int dev_index)
> +{
> +       return get_mmc_clk(dev_index);
>  }
>
> -int exynos_dwmci_init(u32 regbase, int bus_width, int index)
> +/*
> + * This function adds the mmc channel to be registered with mmc core.
> + * index -     mmc channel number.
> + * regbase -   register base address of mmc channel specified in 'index'.
> + * bus_width - operating bus width of mmc channel specified in 'index'.
> + * clksel -    value to be written into CLKSEL register in case of FDT.
> + *             NULL in case od non-FDT.
> + */
> +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel)
>  {
>         struct dwmci_host *host = NULL;
> +       unsigned int div;
> +       unsigned long freq, sclk;
>         host = malloc(sizeof(struct dwmci_host));
>         if (!host) {
>                 printf("dwmci_host malloc fail!\n");
>                 return 1;
>         }
> +       /* request mmc clock vlaue of 50MHz.  */
> +       freq = 50000000;

Should this be 52MHz?

> +       sclk = get_mmc_clk(index);
> +       div = DIV_ROUND_UP(sclk, freq);
> +       /* set the clock divisor for mmc */
> +       set_mmc_clk(index, div);
>
> -       host->name = EXYNOS_NAME;
> +       host->name = "EXYNOS DWMMC";
>         host->ioaddr = (void *)regbase;
>         host->buswidth = bus_width;
> +
> +       if (clksel)

{} around if() part since you have it for else.

> +               host->clksel_val = clksel;
> +       else {
> +               if (0 == index)
> +                       host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
> +               if (2 == index)
> +                       host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
> +       }
> +
>         host->clksel = exynos_dwmci_clksel;
>         host->dev_index = index;
> +       host->mmc_clk = exynos_dwmci_get_clk;
> +       /* Add the mmc channel to be registered with mmc core */
> +       if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
> +               printf("dwmmc%d registration failed\n", index);

Should this be debug()?

> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +#ifdef CONFIG_OF_CONTROL
> +int exynos_dwmmc_init(const void *blob)
> +{
> +       int index, bus_width;
> +       int node_list[DWMMC_MAX_CH_NUM];
> +       int err = 0, dev_id, flag, count, i;
> +       u32 clksel_val, base, timing[3];
> +
> +       count = fdtdec_find_aliases_for_id(blob, "mmc",
> +                               COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,
> +                               DWMMC_MAX_CH_NUM);
> +
> +       for (i = 0; i < count; i++) {
> +               int node = node_list[i];
> +
> +               if (node <= 0)
> +                       continue;
>
> -       add_dwmci(host, 52000000, 400000);
> +               /* Extract device id for each mmc channel */
> +               dev_id = pinmux_decode_periph_id(blob, node);
>
> +               /* Get the bus width from the device node */
> +               bus_width = fdtdec_get_int(blob, node, "samsung,bus-width", 0);
> +               if (bus_width <= 0) {
> +                       debug("DWMMC: Can't get bus-width\n");
> +                       return -1;
> +               }
> +               if (8 == bus_width)
> +                       flag = PINMUX_FLAG_8BIT_MODE;
> +               else
> +                       flag = PINMUX_FLAG_NONE;
> +
> +               /* config pinmux for each mmc channel */
> +               err = exynos_pinmux_config(dev_id, flag);
> +               if (err) {
> +                       debug("DWMMC not configured\n");
> +                       return err;
> +               }
> +
> +               index = dev_id - PERIPH_ID_SDMMC0;
> +
> +               /* Get the base address from the device node */
> +               base = fdtdec_get_addr(blob, node, "reg");
> +               if (!base) {
> +                       debug("DWMMC: Can't get base address\n");
> +                       return -1;
> +               }
> +               /* Extract the timing info from the node */
> +               err = fdtdec_get_int_array(blob, node, "samsung,timing",
> +                                       timing, 3);
> +               if (err) {
> +                       debug("Can't get sdr-timings for divider\n");
> +                       return -1;
> +               }
> +
> +               clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |
> +                               DWMCI_SET_DRV_CLK(timing[1]) |
> +                               DWMCI_SET_DIV_RATIO(timing[2]));
> +               /* Initialise each mmc channel */
> +               err = exynos_dwmci_add_port(index, base, bus_width, clksel_val);
> +               if (err)
> +                       debug("dwmmc Channel-%d init failed\n", index);
> +       }
>         return 0;
>  }
> -
> +#endif
> diff --git a/include/dwmmc.h b/include/dwmmc.h
> index c8b1d40..e142f3e 100644
> --- a/include/dwmmc.h
> +++ b/include/dwmmc.h
> @@ -123,6 +123,8 @@
>  #define MSIZE(x)               ((x) << 28)
>  #define RX_WMARK(x)            ((x) << 16)
>  #define TX_WMARK(x)            (x)
> +#define RX_WMARK_SHIFT         16
> +#define RX_WMARK_MASK          (0xfff << RX_WMARK_SHIFT)
>
>  #define DWMCI_IDMAC_OWN                (1 << 31)
>  #define DWMCI_IDMAC_CH         (1 << 4)
> @@ -144,6 +146,7 @@ struct dwmci_host {
>         unsigned int bus_hz;
>         int dev_index;
>         int buswidth;
> +       u32 clksel_val;
>         u32 fifoth_val;
>         struct mmc *mmc;
>
> --
> 1.8.0
>

Regards,
Simon
Amarendra Reddy - Jan. 28, 2013, 9:31 a.m.
Hi Simon,

Please find the responses below.

Thanks & Regards
Amarendra

On 27 January 2013 01:38, Simon Glass <sjg@chromium.org> wrote:

> Hi,
>
> On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra.xt@samsung.com> wrote:
> > This patch adds FDT support for DWMMC, by reading the DWMMC node data
> > from the device tree and initialising DWMMC channels as per data
> > obtained from the node.
> >
> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> > Signed-off-by: Amar <amarendra.xt@samsung.com>
>
> Acked-by: Simon Glass <sjg@chromium.org>
>
> See below for nit/question.
>
> > ---
> > Changes since V1:
> >         1)Updated code to have same signature for the function
> >         exynos_dwmci_init() for both FDT and non-FDT versions.
> >         2)Updated code to pass device_id parameter to the function
> >         exynos5_mmc_set_clk_div() instead of index.
> >         3)Updated code to decode the value of "samsung,width" from FDT.
> >         4)Channel index is computed instead of getting from FDT.
> >
> > Changes since V2:
> >         1)Updation of commit message and resubmition of proper patch set.
> >
> > Changes since V3:
> >         1)Replaced the new function exynos5_mmc_set_clk_div() with the
> >         existing function set_mmc_clk(). set_mmc_clk() will do the
> purpose.
> >         2)Computation of FSYS block clock divisor (pre-ratio) is added.
> >
> > Changes since V4:
> >         1)Replaced "unsigned int exynos_dwmmc_init(int index, int
> bus_width)" with
> >         int exynos_dwmci_add_port(int, u32, inth, u32)
> >                 i)exynos_dwmmc_add_port() will be used by non-FDT boards.
> >                 ii)In FDT case, exynos_dwmmc_init(const void *blob) will
> use
> >                 exynos_dwmmc_add_port() for every channel enabled in
> device node.
> >         2)Changed the computation method of mmc clock divisor.
> >         3)Updated exynos_dwmmc_init() to compute the 'clksel_val' within
> the function.
> >
> >  arch/arm/include/asm/arch-exynos/dwmmc.h |  11 +--
> >  drivers/mmc/exynos_dw_mmc.c              | 127
> ++++++++++++++++++++++++++++---
> >  include/dwmmc.h                          |   3 +
> >  3 files changed, 124 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h
> b/arch/arm/include/asm/arch-exynos/dwmmc.h
> > index 8acdf9b..3b147b8 100644
> > --- a/arch/arm/include/asm/arch-exynos/dwmmc.h
> > +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
> > @@ -27,10 +27,7 @@
> >  #define DWMCI_SET_DRV_CLK(x)   ((x) << 16)
> >  #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
> >
> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index);
> > -
> > -static inline unsigned int exynos_dwmmc_init(int index, int bus_width)
> > -{
> > -       unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
> > -       return exynos_dwmci_init(base, bus_width, index);
> > -}
> > +#ifdef CONFIG_OF_CONTROL
> > +int exynos_dwmmc_init(const void *blob);
> > +#endif
> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
> clksel);
> > diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> > index 72a31b7..1d09e56 100644
> > --- a/drivers/mmc/exynos_dw_mmc.c
> > +++ b/drivers/mmc/exynos_dw_mmc.c
> > @@ -19,39 +19,146 @@
> >   */
> >
> >  #include <common.h>
> > -#include <malloc.h>
> >  #include <dwmmc.h>
> > +#include <fdtdec.h>
> > +#include <libfdt.h>
> > +#include <malloc.h>
> >  #include <asm/arch/dwmmc.h>
> >  #include <asm/arch/clk.h>
> > +#include <asm/arch/pinmux.h>
> >
> > -static char *EXYNOS_NAME = "EXYNOS DWMMC";
> > +#define        DWMMC_MAX_CH_NUM                4
> > +#define        DWMMC_MAX_FREQ                  52000000
> > +#define        DWMMC_MIN_FREQ                  400000
> > +#define        DWMMC_MMC0_CLKSEL_VAL           0x03030001
> > +#define        DWMMC_MMC2_CLKSEL_VAL           0x03020001
> >
> > +/*
> > + * Function used as callback function to initialise the
> > + * CLKSEL register for every mmc channel.
> > + */
> >  static void exynos_dwmci_clksel(struct dwmci_host *host)
> >  {
> > -       u32 val;
> > -       val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> > -               DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
> DWMCI_SET_DIV_RATIO(0);
> > +       dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
> > +}
> >
> > -       dwmci_writel(host, DWMCI_CLKSEL, val);
> > +unsigned int exynos_dwmci_get_clk(int dev_index)
> > +{
> > +       return get_mmc_clk(dev_index);
> >  }
> >
> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index)
> > +/*
> > + * This function adds the mmc channel to be registered with mmc core.
> > + * index -     mmc channel number.
> > + * regbase -   register base address of mmc channel specified in
> 'index'.
> > + * bus_width - operating bus width of mmc channel specified in 'index'.
> > + * clksel -    value to be written into CLKSEL register in case of FDT.
> > + *             NULL in case od non-FDT.
> > + */
> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
> clksel)
> >  {
> >         struct dwmci_host *host = NULL;
> > +       unsigned int div;
> > +       unsigned long freq, sclk;
> >         host = malloc(sizeof(struct dwmci_host));
> >         if (!host) {
> >                 printf("dwmci_host malloc fail!\n");
> >                 return 1;
> >         }
> > +       /* request mmc clock vlaue of 50MHz.  */
> > +       freq = 50000000;
>
> Should this be 52MHz?
>
Ok shall change it to 52MHz.

>
> > +       sclk = get_mmc_clk(index);
> > +       div = DIV_ROUND_UP(sclk, freq);
> > +       /* set the clock divisor for mmc */
> > +       set_mmc_clk(index, div);
> >
> > -       host->name = EXYNOS_NAME;
> > +       host->name = "EXYNOS DWMMC";
> >         host->ioaddr = (void *)regbase;
> >         host->buswidth = bus_width;
> > +
> > +       if (clksel)
>
> {} around if() part since you have it for else.
>
>
Ok will put {} around if() part.


> > +               host->clksel_val = clksel;
> > +       else {
> > +               if (0 == index)
> > +                       host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
> > +               if (2 == index)
> > +                       host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
> > +       }
> > +
> >         host->clksel = exynos_dwmci_clksel;
> >         host->dev_index = index;
> > +       host->mmc_clk = exynos_dwmci_get_clk;
> > +       /* Add the mmc channel to be registered with mmc core */
> > +       if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
> > +               printf("dwmmc%d registration failed\n", index);
>
> Should this be debug()?
>
I think it should be printf(), because if the mmc channel registration
fails, then this failure information needs to be conveyed to the user.
This will happen if printf() is used.
If debug() is used, then the statement will be printed only when the DEBUG
is defined.

> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +#ifdef CONFIG_OF_CONTROL
> +int exynos_dwmmc_init(const void *blob)
> +{
> +       int index, bus_width;
> +       int node_list[DWMMC_MAX_CH_NUM];
> +       int err = 0, dev_id, flag, count, i;
> +       u32 clksel_val, base, timing[3];
> +
> +       count = fdtdec_find_aliases_for_id(blob, "mmc",
> +                               COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,
> +                               DWMMC_MAX_CH_NUM);
> +
> +       for (i = 0; i < count; i++) {
> +               int node = node_list[i];
> +
> +               if (node <= 0)
> +                       continue;
>
> -       add_dwmci(host, 52000000, 400000);
> +               /* Extract device id for each mmc channel */
> +               dev_id = pinmux_decode_periph_id(blob, node);
>
> +               /* Get the bus width from the device node */
> +               bus_width = fdtdec_get_int(blob, node,
"samsung,bus-width", 0);
> +               if (bus_width <= 0) {
> +                       debug("DWMMC: Can't get bus-width\n");
> +                       return -1;
> +               }
> +               if (8 == bus_width)
> +                       flag = PINMUX_FLAG_8BIT_MODE;
> +               else
> +                       flag = PINMUX_FLAG_NONE;
> +
> +               /* config pinmux for each mmc channel */
> +               err = exynos_pinmux_config(dev_id, flag);
> +               if (err) {
> +                       debug("DWMMC not configured\n");
> +                       return err;
> +               }
> +
> +               index = dev_id - PERIPH_ID_SDMMC0;
> +
> +               /* Get the base address from the device node */
> +               base = fdtdec_get_addr(blob, node, "reg");
> +               if (!base) {
> +                       debug("DWMMC: Can't get base address\n");
> +                       return -1;
> +               }
> +               /* Extract the timing info from the node */
> +               err = fdtdec_get_int_array(blob, node, "samsung,timing",
> +                                       timing, 3);
> +               if (err) {
> +                       debug("Can't get sdr-timings for divider\n");
> +                       return -1;
> +               }
> +
> +               clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |
> +                               DWMCI_SET_DRV_CLK(timing[1]) |
> +                               DWMCI_SET_DIV_RATIO(timing[2]));
> +               /* Initialise each mmc channel */
> +               err = exynos_dwmci_add_port(index, base, bus_width,
clksel_val);
> +               if (err)
> +                       debug("dwmmc Channel-%d init failed\n", index);
> +       }
>         return 0;
>  }
> -
> +#endif
> diff --git a/include/dwmmc.h b/include/dwmmc.h
> index c8b1d40..e142f3e 100644
> --- a/include/dwmmc.h
> +++ b/include/dwmmc.h
> @@ -123,6 +123,8 @@
>  #define MSIZE(x)               ((x) << 28)
>  #define RX_WMARK(x)            ((x) << 16)
>  #define TX_WMARK(x)            (x)
> +#define RX_WMARK_SHIFT         16
> +#define RX_WMARK_MASK          (0xfff << RX_WMARK_SHIFT)
>
>  #define DWMCI_IDMAC_OWN                (1 << 31)
>  #define DWMCI_IDMAC_CH         (1 << 4)
> @@ -144,6 +146,7 @@ struct dwmci_host {
>         unsigned int bus_hz;
>         int dev_index;
>         int buswidth;
> +       u32 clksel_val;
>         u32 fifoth_val;
>         struct mmc *mmc;
>
> --
> 1.8.0
>

Regards,
> Simon
>  _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Simon Glass - Feb. 9, 2013, 4:54 p.m.
Hi Amarendra,

On Mon, Jan 28, 2013 at 1:31 AM, Amarendra Reddy
<amar.lavanuru@gmail.com> wrote:
> Hi Simon,
>
> Please find the responses below.
>
> Thanks & Regards
> Amarendra
>
> On 27 January 2013 01:38, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi,
>>
>> On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra.xt@samsung.com> wrote:
>> > This patch adds FDT support for DWMMC, by reading the DWMMC node data
>> > from the device tree and initialising DWMMC channels as per data
>> > obtained from the node.
>> >
>> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> > Signed-off-by: Amar <amarendra.xt@samsung.com>
>>
>> Acked-by: Simon Glass <sjg@chromium.org>
>>
>> See below for nit/question.
>>
>> > ---
>> > Changes since V1:
>> >         1)Updated code to have same signature for the function
>> >         exynos_dwmci_init() for both FDT and non-FDT versions.
>> >         2)Updated code to pass device_id parameter to the function
>> >         exynos5_mmc_set_clk_div() instead of index.
>> >         3)Updated code to decode the value of "samsung,width" from FDT.
>> >         4)Channel index is computed instead of getting from FDT.
>> >
>> > Changes since V2:
>> >         1)Updation of commit message and resubmition of proper patch
>> > set.
>> >
>> > Changes since V3:
>> >         1)Replaced the new function exynos5_mmc_set_clk_div() with the
>> >         existing function set_mmc_clk(). set_mmc_clk() will do the
>> > purpose.
>> >         2)Computation of FSYS block clock divisor (pre-ratio) is added.
>> >
>> > Changes since V4:
>> >         1)Replaced "unsigned int exynos_dwmmc_init(int index, int
>> > bus_width)" with
>> >         int exynos_dwmci_add_port(int, u32, inth, u32)
>> >                 i)exynos_dwmmc_add_port() will be used by non-FDT
>> > boards.
>> >                 ii)In FDT case, exynos_dwmmc_init(const void *blob) will
>> > use
>> >                 exynos_dwmmc_add_port() for every channel enabled in
>> > device node.
>> >         2)Changed the computation method of mmc clock divisor.
>> >         3)Updated exynos_dwmmc_init() to compute the 'clksel_val' within
>> > the function.
>> >
>> >  arch/arm/include/asm/arch-exynos/dwmmc.h |  11 +--
>> >  drivers/mmc/exynos_dw_mmc.c              | 127
>> > ++++++++++++++++++++++++++++---
>> >  include/dwmmc.h                          |   3 +
>> >  3 files changed, 124 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h
>> > b/arch/arm/include/asm/arch-exynos/dwmmc.h
>> > index 8acdf9b..3b147b8 100644
>> > --- a/arch/arm/include/asm/arch-exynos/dwmmc.h
>> > +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
>> > @@ -27,10 +27,7 @@
>> >  #define DWMCI_SET_DRV_CLK(x)   ((x) << 16)
>> >  #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
>> >
>> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index);
>> > -
>> > -static inline unsigned int exynos_dwmmc_init(int index, int bus_width)
>> > -{
>> > -       unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
>> > -       return exynos_dwmci_init(base, bus_width, index);
>> > -}
>> > +#ifdef CONFIG_OF_CONTROL
>> > +int exynos_dwmmc_init(const void *blob);
>> > +#endif
>> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
>> > clksel);
>> > diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
>> > index 72a31b7..1d09e56 100644
>> > --- a/drivers/mmc/exynos_dw_mmc.c
>> > +++ b/drivers/mmc/exynos_dw_mmc.c
>> > @@ -19,39 +19,146 @@
>> >   */
>> >
>> >  #include <common.h>
>> > -#include <malloc.h>
>> >  #include <dwmmc.h>
>> > +#include <fdtdec.h>
>> > +#include <libfdt.h>
>> > +#include <malloc.h>
>> >  #include <asm/arch/dwmmc.h>
>> >  #include <asm/arch/clk.h>
>> > +#include <asm/arch/pinmux.h>
>> >
>> > -static char *EXYNOS_NAME = "EXYNOS DWMMC";
>> > +#define        DWMMC_MAX_CH_NUM                4
>> > +#define        DWMMC_MAX_FREQ                  52000000
>> > +#define        DWMMC_MIN_FREQ                  400000
>> > +#define        DWMMC_MMC0_CLKSEL_VAL           0x03030001
>> > +#define        DWMMC_MMC2_CLKSEL_VAL           0x03020001
>> >
>> > +/*
>> > + * Function used as callback function to initialise the
>> > + * CLKSEL register for every mmc channel.
>> > + */
>> >  static void exynos_dwmci_clksel(struct dwmci_host *host)
>> >  {
>> > -       u32 val;
>> > -       val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
>> > -               DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
>> > DWMCI_SET_DIV_RATIO(0);
>> > +       dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
>> > +}
>> >
>> > -       dwmci_writel(host, DWMCI_CLKSEL, val);
>> > +unsigned int exynos_dwmci_get_clk(int dev_index)
>> > +{
>> > +       return get_mmc_clk(dev_index);
>> >  }
>> >
>> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index)
>> > +/*
>> > + * This function adds the mmc channel to be registered with mmc core.
>> > + * index -     mmc channel number.
>> > + * regbase -   register base address of mmc channel specified in
>> > 'index'.
>> > + * bus_width - operating bus width of mmc channel specified in 'index'.
>> > + * clksel -    value to be written into CLKSEL register in case of FDT.
>> > + *             NULL in case od non-FDT.
>> > + */
>> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
>> > clksel)
>> >  {
>> >         struct dwmci_host *host = NULL;
>> > +       unsigned int div;
>> > +       unsigned long freq, sclk;
>> >         host = malloc(sizeof(struct dwmci_host));
>> >         if (!host) {
>> >                 printf("dwmci_host malloc fail!\n");
>> >                 return 1;
>> >         }
>> > +       /* request mmc clock vlaue of 50MHz.  */
>> > +       freq = 50000000;
>>
>> Should this be 52MHz?
>
> Ok shall change it to 52MHz.
>>
>>
>> > +       sclk = get_mmc_clk(index);
>> > +       div = DIV_ROUND_UP(sclk, freq);
>> > +       /* set the clock divisor for mmc */
>> > +       set_mmc_clk(index, div);
>> >
>> > -       host->name = EXYNOS_NAME;
>> > +       host->name = "EXYNOS DWMMC";
>> >         host->ioaddr = (void *)regbase;
>> >         host->buswidth = bus_width;
>> > +
>> > +       if (clksel)
>>
>> {} around if() part since you have it for else.
>>
>
> Ok will put {} around if() part.
>
>>
>> > +               host->clksel_val = clksel;
>> > +       else {
>> > +               if (0 == index)
>> > +                       host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
>> > +               if (2 == index)
>> > +                       host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
>> > +       }
>> > +
>> >         host->clksel = exynos_dwmci_clksel;
>> >         host->dev_index = index;
>> > +       host->mmc_clk = exynos_dwmci_get_clk;
>> > +       /* Add the mmc channel to be registered with mmc core */
>> > +       if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
>> > +               printf("dwmmc%d registration failed\n", index);
>>
>> Should this be debug()?
>
> I think it should be printf(), because if the mmc channel registration
> fails, then this failure information needs to be conveyed to the user.
> This will happen if printf() is used.
> If debug() is used, then the statement will be printed only when the DEBUG
> is defined.

It's a tricky point, and it depends on what else you have in terns of
error reporting. This is not a user-facing function, but something
called by the internals of your code. I think as a general principle
errors should be reported by the commands that started the operation.

In this case exynos_dwmci_add_port() already has an error return code,
and its caller can presumable report the error, although it would not
include the 'dwmmc' text.

The problem with putting printf()s in drivers is that it can make it
impossible to silently do something. For example if I want to write a
command to check for a valid partition on an MMC device, and all of
the MMC, partition and filesystem code prints out errors along the
way, it can make for very confusing output, and can make it impossible
to write a higher-level function which silently checks for the valid
partition.

So in general I think we should try to keep printf() out of drivers.
We have talked on the list about some sort of dynamic verbosity, and
someone may look at that at some point, but at the moment we just have
printf() and debug().

Regards,
Simon

>
>> +               return -1;
>> +       }
>> +       return 0;
>> +}
>> +
>> +#ifdef CONFIG_OF_CONTROL
>> +int exynos_dwmmc_init(const void *blob)
>> +{
>> +       int index, bus_width;
>> +       int node_list[DWMMC_MAX_CH_NUM];
>> +       int err = 0, dev_id, flag, count, i;
>> +       u32 clksel_val, base, timing[3];
>> +
>> +       count = fdtdec_find_aliases_for_id(blob, "mmc",
>> +                               COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,
>> +                               DWMMC_MAX_CH_NUM);
>> +
>> +       for (i = 0; i < count; i++) {
>> +               int node = node_list[i];
>> +
>> +               if (node <= 0)
>> +                       continue;
>>
>> -       add_dwmci(host, 52000000, 400000);
>> +               /* Extract device id for each mmc channel */
>> +               dev_id = pinmux_decode_periph_id(blob, node);
>>
>> +               /* Get the bus width from the device node */
>> +               bus_width = fdtdec_get_int(blob, node,
>> "samsung,bus-width", 0);
>> +               if (bus_width <= 0) {
>> +                       debug("DWMMC: Can't get bus-width\n");
>> +                       return -1;
>> +               }
>> +               if (8 == bus_width)
>> +                       flag = PINMUX_FLAG_8BIT_MODE;
>> +               else
>> +                       flag = PINMUX_FLAG_NONE;
>> +
>> +               /* config pinmux for each mmc channel */
>> +               err = exynos_pinmux_config(dev_id, flag);
>> +               if (err) {
>> +                       debug("DWMMC not configured\n");
>> +                       return err;
>> +               }
>> +
>> +               index = dev_id - PERIPH_ID_SDMMC0;
>> +
>> +               /* Get the base address from the device node */
>> +               base = fdtdec_get_addr(blob, node, "reg");
>> +               if (!base) {
>> +                       debug("DWMMC: Can't get base address\n");
>> +                       return -1;
>> +               }
>> +               /* Extract the timing info from the node */
>> +               err = fdtdec_get_int_array(blob, node, "samsung,timing",
>> +                                       timing, 3);
>> +               if (err) {
>> +                       debug("Can't get sdr-timings for divider\n");
>> +                       return -1;
>> +               }
>> +
>> +               clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |
>> +                               DWMCI_SET_DRV_CLK(timing[1]) |
>> +                               DWMCI_SET_DIV_RATIO(timing[2]));
>> +               /* Initialise each mmc channel */
>> +               err = exynos_dwmci_add_port(index, base, bus_width,
>> clksel_val);
>> +               if (err)
>> +                       debug("dwmmc Channel-%d init failed\n", index);
>> +       }
>>         return 0;
>>  }
>> -
>> +#endif
>> diff --git a/include/dwmmc.h b/include/dwmmc.h
>> index c8b1d40..e142f3e 100644
>> --- a/include/dwmmc.h
>> +++ b/include/dwmmc.h
>> @@ -123,6 +123,8 @@
>>  #define MSIZE(x)               ((x) << 28)
>>  #define RX_WMARK(x)            ((x) << 16)
>>  #define TX_WMARK(x)            (x)
>> +#define RX_WMARK_SHIFT         16
>> +#define RX_WMARK_MASK          (0xfff << RX_WMARK_SHIFT)
>>
>>  #define DWMCI_IDMAC_OWN                (1 << 31)
>>  #define DWMCI_IDMAC_CH         (1 << 4)
>> @@ -144,6 +146,7 @@ struct dwmci_host {
>>         unsigned int bus_hz;
>>         int dev_index;
>>         int buswidth;
>> +       u32 clksel_val;
>>         u32 fifoth_val;
>>         struct mmc *mmc;
>>
>> --
>> 1.8.0
>>
>
>> Regards,
>> Simon
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>
>
Amarendra Reddy - Feb. 15, 2013, 6:18 a.m.
Hi Simon,
Thanks for the response.
Please find my response below.

Thanks & Regards
Amarendra

On 9 February 2013 22:24, Simon Glass <sjg@chromium.org> wrote:

> Hi Amarendra,
>
> On Mon, Jan 28, 2013 at 1:31 AM, Amarendra Reddy
> <amar.lavanuru@gmail.com> wrote:
> > Hi Simon,
> >
> > Please find the responses below.
> >
> > Thanks & Regards
> > Amarendra
> >
> > On 27 January 2013 01:38, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi,
> >>
> >> On Tue, Jan 22, 2013 at 12:43 AM, Amar <amarendra.xt@samsung.com>
> wrote:
> >> > This patch adds FDT support for DWMMC, by reading the DWMMC node data
> >> > from the device tree and initialising DWMMC channels as per data
> >> > obtained from the node.
> >> >
> >> > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> > Signed-off-by: Amar <amarendra.xt@samsung.com>
> >>
> >> Acked-by: Simon Glass <sjg@chromium.org>
> >>
> >> See below for nit/question.
> >>
> >> > ---
> >> > Changes since V1:
> >> >         1)Updated code to have same signature for the function
> >> >         exynos_dwmci_init() for both FDT and non-FDT versions.
> >> >         2)Updated code to pass device_id parameter to the function
> >> >         exynos5_mmc_set_clk_div() instead of index.
> >> >         3)Updated code to decode the value of "samsung,width" from
> FDT.
> >> >         4)Channel index is computed instead of getting from FDT.
> >> >
> >> > Changes since V2:
> >> >         1)Updation of commit message and resubmition of proper patch
> >> > set.
> >> >
> >> > Changes since V3:
> >> >         1)Replaced the new function exynos5_mmc_set_clk_div() with the
> >> >         existing function set_mmc_clk(). set_mmc_clk() will do the
> >> > purpose.
> >> >         2)Computation of FSYS block clock divisor (pre-ratio) is
> added.
> >> >
> >> > Changes since V4:
> >> >         1)Replaced "unsigned int exynos_dwmmc_init(int index, int
> >> > bus_width)" with
> >> >         int exynos_dwmci_add_port(int, u32, inth, u32)
> >> >                 i)exynos_dwmmc_add_port() will be used by non-FDT
> >> > boards.
> >> >                 ii)In FDT case, exynos_dwmmc_init(const void *blob)
> will
> >> > use
> >> >                 exynos_dwmmc_add_port() for every channel enabled in
> >> > device node.
> >> >         2)Changed the computation method of mmc clock divisor.
> >> >         3)Updated exynos_dwmmc_init() to compute the 'clksel_val'
> within
> >> > the function.
> >> >
> >> >  arch/arm/include/asm/arch-exynos/dwmmc.h |  11 +--
> >> >  drivers/mmc/exynos_dw_mmc.c              | 127
> >> > ++++++++++++++++++++++++++++---
> >> >  include/dwmmc.h                          |   3 +
> >> >  3 files changed, 124 insertions(+), 17 deletions(-)
> >> >
> >> > diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h
> >> > b/arch/arm/include/asm/arch-exynos/dwmmc.h
> >> > index 8acdf9b..3b147b8 100644
> >> > --- a/arch/arm/include/asm/arch-exynos/dwmmc.h
> >> > +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
> >> > @@ -27,10 +27,7 @@
> >> >  #define DWMCI_SET_DRV_CLK(x)   ((x) << 16)
> >> >  #define DWMCI_SET_DIV_RATIO(x) ((x) << 24)
> >> >
> >> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index);
> >> > -
> >> > -static inline unsigned int exynos_dwmmc_init(int index, int
> bus_width)
> >> > -{
> >> > -       unsigned int base = samsung_get_base_mmc() + (0x10000 *
> index);
> >> > -       return exynos_dwmci_init(base, bus_width, index);
> >> > -}
> >> > +#ifdef CONFIG_OF_CONTROL
> >> > +int exynos_dwmmc_init(const void *blob);
> >> > +#endif
> >> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
> >> > clksel);
> >> > diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> >> > index 72a31b7..1d09e56 100644
> >> > --- a/drivers/mmc/exynos_dw_mmc.c
> >> > +++ b/drivers/mmc/exynos_dw_mmc.c
> >> > @@ -19,39 +19,146 @@
> >> >   */
> >> >
> >> >  #include <common.h>
> >> > -#include <malloc.h>
> >> >  #include <dwmmc.h>
> >> > +#include <fdtdec.h>
> >> > +#include <libfdt.h>
> >> > +#include <malloc.h>
> >> >  #include <asm/arch/dwmmc.h>
> >> >  #include <asm/arch/clk.h>
> >> > +#include <asm/arch/pinmux.h>
> >> >
> >> > -static char *EXYNOS_NAME = "EXYNOS DWMMC";
> >> > +#define        DWMMC_MAX_CH_NUM                4
> >> > +#define        DWMMC_MAX_FREQ                  52000000
> >> > +#define        DWMMC_MIN_FREQ                  400000
> >> > +#define        DWMMC_MMC0_CLKSEL_VAL           0x03030001
> >> > +#define        DWMMC_MMC2_CLKSEL_VAL           0x03020001
> >> >
> >> > +/*
> >> > + * Function used as callback function to initialise the
> >> > + * CLKSEL register for every mmc channel.
> >> > + */
> >> >  static void exynos_dwmci_clksel(struct dwmci_host *host)
> >> >  {
> >> > -       u32 val;
> >> > -       val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> >> > -               DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
> >> > DWMCI_SET_DIV_RATIO(0);
> >> > +       dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
> >> > +}
> >> >
> >> > -       dwmci_writel(host, DWMCI_CLKSEL, val);
> >> > +unsigned int exynos_dwmci_get_clk(int dev_index)
> >> > +{
> >> > +       return get_mmc_clk(dev_index);
> >> >  }
> >> >
> >> > -int exynos_dwmci_init(u32 regbase, int bus_width, int index)
> >> > +/*
> >> > + * This function adds the mmc channel to be registered with mmc core.
> >> > + * index -     mmc channel number.
> >> > + * regbase -   register base address of mmc channel specified in
> >> > 'index'.
> >> > + * bus_width - operating bus width of mmc channel specified in
> 'index'.
> >> > + * clksel -    value to be written into CLKSEL register in case of
> FDT.
> >> > + *             NULL in case od non-FDT.
> >> > + */
> >> > +int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32
> >> > clksel)
> >> >  {
> >> >         struct dwmci_host *host = NULL;
> >> > +       unsigned int div;
> >> > +       unsigned long freq, sclk;
> >> >         host = malloc(sizeof(struct dwmci_host));
> >> >         if (!host) {
> >> >                 printf("dwmci_host malloc fail!\n");
> >> >                 return 1;
> >> >         }
> >> > +       /* request mmc clock vlaue of 50MHz.  */
> >> > +       freq = 50000000;
> >>
> >> Should this be 52MHz?
> >
> > Ok shall change it to 52MHz.
> >>
> >>
> >> > +       sclk = get_mmc_clk(index);
> >> > +       div = DIV_ROUND_UP(sclk, freq);
> >> > +       /* set the clock divisor for mmc */
> >> > +       set_mmc_clk(index, div);
> >> >
> >> > -       host->name = EXYNOS_NAME;
> >> > +       host->name = "EXYNOS DWMMC";
> >> >         host->ioaddr = (void *)regbase;
> >> >         host->buswidth = bus_width;
> >> > +
> >> > +       if (clksel)
> >>
> >> {} around if() part since you have it for else.
> >>
> >
> > Ok will put {} around if() part.
> >
> >>
> >> > +               host->clksel_val = clksel;
> >> > +       else {
> >> > +               if (0 == index)
> >> > +                       host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
> >> > +               if (2 == index)
> >> > +                       host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
> >> > +       }
> >> > +
> >> >         host->clksel = exynos_dwmci_clksel;
> >> >         host->dev_index = index;
> >> > +       host->mmc_clk = exynos_dwmci_get_clk;
> >> > +       /* Add the mmc channel to be registered with mmc core */
> >> > +       if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
> >> > +               printf("dwmmc%d registration failed\n", index);
> >>
> >> Should this be debug()?
> >
> > I think it should be printf(), because if the mmc channel registration
> > fails, then this failure information needs to be conveyed to the user.
> > This will happen if printf() is used.
> > If debug() is used, then the statement will be printed only when the
> DEBUG
> > is defined.
>
> It's a tricky point, and it depends on what else you have in terns of
> error reporting. This is not a user-facing function, but something
> called by the internals of your code. I think as a general principle
> errors should be reported by the commands that started the operation.
>
> In this case exynos_dwmci_add_port() already has an error return code,
> and its caller can presumable report the error, although it would not
> include the 'dwmmc' text.
>
> The problem with putting printf()s in drivers is that it can make it
> impossible to silently do something. For example if I want to write a
> command to check for a valid partition on an MMC device, and all of
> the MMC, partition and filesystem code prints out errors along the
> way, it can make for very confusing output, and can make it impossible
> to write a higher-level function which silently checks for the valid
> partition.
>
> So in general I think we should try to keep printf() out of drivers.
> We have talked on the list about some sort of dynamic verbosity, and
> someone may look at that at some point, but at the moment we just have
> printf() and debug().
>
> Regards,
> Simon
>
>

Ok, will replace the printf() with debug() in the driver file
drivers/mmc/exynos_dw_mmc.c.

Patch

diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h
index 8acdf9b..3b147b8 100644
--- a/arch/arm/include/asm/arch-exynos/dwmmc.h
+++ b/arch/arm/include/asm/arch-exynos/dwmmc.h
@@ -27,10 +27,7 @@ 
 #define DWMCI_SET_DRV_CLK(x)	((x) << 16)
 #define DWMCI_SET_DIV_RATIO(x)	((x) << 24)
 
-int exynos_dwmci_init(u32 regbase, int bus_width, int index);
-
-static inline unsigned int exynos_dwmmc_init(int index, int bus_width)
-{
-	unsigned int base = samsung_get_base_mmc() + (0x10000 * index);
-	return exynos_dwmci_init(base, bus_width, index);
-}
+#ifdef CONFIG_OF_CONTROL
+int exynos_dwmmc_init(const void *blob);
+#endif
+int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel);
diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
index 72a31b7..1d09e56 100644
--- a/drivers/mmc/exynos_dw_mmc.c
+++ b/drivers/mmc/exynos_dw_mmc.c
@@ -19,39 +19,146 @@ 
  */
 
 #include <common.h>
-#include <malloc.h>
 #include <dwmmc.h>
+#include <fdtdec.h>
+#include <libfdt.h>
+#include <malloc.h>
 #include <asm/arch/dwmmc.h>
 #include <asm/arch/clk.h>
+#include <asm/arch/pinmux.h>
 
-static char *EXYNOS_NAME = "EXYNOS DWMMC";
+#define	DWMMC_MAX_CH_NUM		4
+#define	DWMMC_MAX_FREQ			52000000
+#define	DWMMC_MIN_FREQ			400000
+#define	DWMMC_MMC0_CLKSEL_VAL		0x03030001
+#define	DWMMC_MMC2_CLKSEL_VAL		0x03020001
 
+/*
+ * Function used as callback function to initialise the
+ * CLKSEL register for every mmc channel.
+ */
 static void exynos_dwmci_clksel(struct dwmci_host *host)
 {
-	u32 val;
-	val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
-		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
+	dwmci_writel(host, DWMCI_CLKSEL, host->clksel_val);
+}
 
-	dwmci_writel(host, DWMCI_CLKSEL, val);
+unsigned int exynos_dwmci_get_clk(int dev_index)
+{
+	return get_mmc_clk(dev_index);
 }
 
-int exynos_dwmci_init(u32 regbase, int bus_width, int index)
+/*
+ * This function adds the mmc channel to be registered with mmc core.
+ * index -	mmc channel number.
+ * regbase -	register base address of mmc channel specified in 'index'.
+ * bus_width -	operating bus width of mmc channel specified in 'index'.
+ * clksel -	value to be written into CLKSEL register in case of FDT.
+ *		NULL in case od non-FDT.
+ */
+int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel)
 {
 	struct dwmci_host *host = NULL;
+	unsigned int div;
+	unsigned long freq, sclk;
 	host = malloc(sizeof(struct dwmci_host));
 	if (!host) {
 		printf("dwmci_host malloc fail!\n");
 		return 1;
 	}
+	/* request mmc clock vlaue of 50MHz.  */
+	freq = 50000000;
+	sclk = get_mmc_clk(index);
+	div = DIV_ROUND_UP(sclk, freq);
+	/* set the clock divisor for mmc */
+	set_mmc_clk(index, div);
 
-	host->name = EXYNOS_NAME;
+	host->name = "EXYNOS DWMMC";
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
+
+	if (clksel)
+		host->clksel_val = clksel;
+	else {
+		if (0 == index)
+			host->clksel_val = DWMMC_MMC0_CLKSEL_VAL;
+		if (2 == index)
+			host->clksel_val = DWMMC_MMC2_CLKSEL_VAL;
+	}
+
 	host->clksel = exynos_dwmci_clksel;
 	host->dev_index = index;
+	host->mmc_clk = exynos_dwmci_get_clk;
+	/* Add the mmc channel to be registered with mmc core */
+	if (add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ)) {
+		printf("dwmmc%d registration failed\n", index);
+		return -1;
+	}
+	return 0;
+}
+
+#ifdef CONFIG_OF_CONTROL
+int exynos_dwmmc_init(const void *blob)
+{
+	int index, bus_width;
+	int node_list[DWMMC_MAX_CH_NUM];
+	int err = 0, dev_id, flag, count, i;
+	u32 clksel_val, base, timing[3];
+
+	count = fdtdec_find_aliases_for_id(blob, "mmc",
+				COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list,
+				DWMMC_MAX_CH_NUM);
+
+	for (i = 0; i < count; i++) {
+		int node = node_list[i];
+
+		if (node <= 0)
+			continue;
 
-	add_dwmci(host, 52000000, 400000);
+		/* Extract device id for each mmc channel */
+		dev_id = pinmux_decode_periph_id(blob, node);
 
+		/* Get the bus width from the device node */
+		bus_width = fdtdec_get_int(blob, node, "samsung,bus-width", 0);
+		if (bus_width <= 0) {
+			debug("DWMMC: Can't get bus-width\n");
+			return -1;
+		}
+		if (8 == bus_width)
+			flag = PINMUX_FLAG_8BIT_MODE;
+		else
+			flag = PINMUX_FLAG_NONE;
+
+		/* config pinmux for each mmc channel */
+		err = exynos_pinmux_config(dev_id, flag);
+		if (err) {
+			debug("DWMMC not configured\n");
+			return err;
+		}
+
+		index = dev_id - PERIPH_ID_SDMMC0;
+
+		/* Get the base address from the device node */
+		base = fdtdec_get_addr(blob, node, "reg");
+		if (!base) {
+			debug("DWMMC: Can't get base address\n");
+			return -1;
+		}
+		/* Extract the timing info from the node */
+		err = fdtdec_get_int_array(blob, node, "samsung,timing",
+					timing, 3);
+		if (err) {
+			debug("Can't get sdr-timings for divider\n");
+			return -1;
+		}
+
+		clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) |
+				DWMCI_SET_DRV_CLK(timing[1]) |
+				DWMCI_SET_DIV_RATIO(timing[2]));
+		/* Initialise each mmc channel */
+		err = exynos_dwmci_add_port(index, base, bus_width, clksel_val);
+		if (err)
+			debug("dwmmc Channel-%d init failed\n", index);
+	}
 	return 0;
 }
-
+#endif
diff --git a/include/dwmmc.h b/include/dwmmc.h
index c8b1d40..e142f3e 100644
--- a/include/dwmmc.h
+++ b/include/dwmmc.h
@@ -123,6 +123,8 @@ 
 #define MSIZE(x)		((x) << 28)
 #define RX_WMARK(x)		((x) << 16)
 #define TX_WMARK(x)		(x)
+#define RX_WMARK_SHIFT		16
+#define RX_WMARK_MASK		(0xfff << RX_WMARK_SHIFT)
 
 #define DWMCI_IDMAC_OWN		(1 << 31)
 #define DWMCI_IDMAC_CH		(1 << 4)
@@ -144,6 +146,7 @@  struct dwmci_host {
 	unsigned int bus_hz;
 	int dev_index;
 	int buswidth;
+	u32 clksel_val;
 	u32 fifoth_val;
 	struct mmc *mmc;