Hi Jaehoon,
Please find my response below.
Thanks & Regards
Amarendra Reddy
On 12 April 2013 16:50, Jaehoon Chung <jh80.ch...@samsung.com> wrote:
> On 04/10/2013 03:13 PM, Amarendra Reddy wrote:
> > Hi Jaehoon,
> >
> > Please find my responses below.
> >
> > Thanks & Regards
> > Amarendra Reddy
> >
> > On 9 April 2013 16:23, Jaehoon Chung <jh80.ch...@samsung.com> wrote:
> >
> >> On 04/03/2013 11:08 PM, Amar 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.vi...@samsung.com>
> >>> Signed-off-by: Amar <amarendra...@samsung.com>
> >>> Acked-by: Simon Glass <s...@chromium.org>
> >>> ---
> >>> 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.
> >>>
> >>> Changes since V5:
> >>> 1)Updated in response to review comments and changed the mmc
> clock
> >> value
> >>> from 50MHz to 52MHz.
> >>>
> >>> Changes since V6:
> >>> No change.
> >>>
> >>> Changes since V7:
> >>> No change.
> >>>
> >>> 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..4238dd9 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
> >> I known that CLKSEL's value is SoC specific.
> >> All exynos series are working fine? I didn't think so.
> >>
> >> Best Regards,
> >> Jaehoon Chung
> >>
> >
> > The above values of CLKSEL work fine for Exynos5 (5250 & 5410) and is
> > tested.
> >
> > The above CLKSEL values donot hold good for Exynos4 series. Exynos4 has
> > "Mobile Storage Host" with only one port.
> > But this file 'drivers/mmc/exynos_dw_mmc.c' is specific to Exynos5 series
> > and gets compiled only when CONFIG_EXYNOS_DWMMC is defined.
> No, driver/mmc/exynos_dw_mmc.c isn't specific to Exynos5 series.
> We can use this controller at the exynos4.
> Then we need to consider how get the clksel value according to SoC.
>
> Best Regards,
> Jaehoon Chung
>
OK. Then I shall add the macro for supporting exynos4 also as shown below
#define EXYNOS4_DWMMC_MMC0_CLKSEL_VAL 0x00000000 (reset value)
#define EXYNOS5_DWMMC_MMC0_CLKSEL_VAL 0x03030001
#define EXYNOS5_DWMMC_MMC2_CLKSEL_VAL 0x03020001
In Exynos4, Mobile Storage Host has only one channel.
The the below function will be updated as below
int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel)
{
......
......
if (clksel) {
host->clksel_val = clksel;
} else {
if (0 == index) {
if(cpu_is_exynos5())
host->clksel_val =
EXYNOS5_DWMMC_MMC0_CLKSEL_VAL;
else
host->clksel_val =
EXYNOS4_DWMMC_MMC0_CLKSEL_VAL; /* Exynos4 MSH has only one channel */
}
if (2 == index)
host->clksel_val = EXYNOS5_DWMMC_MMC2_CLKSEL_VAL;
}
........
........
}
Is it OK. Please comment on the above.
Thanks & Regards
Amarendra Reddy
> >
> >>
> >>> +/*
> >>> + * 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 52MHz. */
> >>> + freq = 52000000;
> >>> + 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)) {
> >>> + debug("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;
> >>>
> >>>
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot@lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
> >>
> >
> >
> >
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot@lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
