Hi Jaehoon, Please find my response below
Thanks & Regards Amarendra On 23 April 2013 09:27, Jaehoon Chung <jh80.ch...@samsung.com> wrote: > On 04/16/2013 05:44 PM, Amarendra Reddy wrote: > > 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. > Well...if you want to set the CLKSEL,i will modify for exynos4 in future. > How about this? (I think you're only working for the exynos5) > > Ok fine, that will be really good. Hence right now I will not set CLKSEL for exynos4 in the file 'drivers/mmc/exynos_dw_mmc.c'. Best Regards, > Jaehoon Chung > > > > 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 > > > >
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot