On 01/11/2013 12:33 AM, Simon Glass wrote: > Hi Amar, > > On Fri, Jan 4, 2013 at 1:34 AM, Amar <amarendra...@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. >> >> Changes from 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 from V2: >> 1)Updation of commit message and resubmition of proper patch set. >> >> Changes from 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. >> >> Signed-off-by: Vivek Gautam <gautam.vi...@samsung.com> >> Signed-off-by: Amar <amarendra...@samsung.com> >> --- >> arch/arm/include/asm/arch-exynos/dwmmc.h | 4 + >> drivers/mmc/exynos_dw_mmc.c | 129 >> +++++++++++++++++++++++++++++-- >> include/dwmmc.h | 4 + >> 3 files changed, 130 insertions(+), 7 deletions(-) >> >> diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h >> b/arch/arm/include/asm/arch-exynos/dwmmc.h >> index 8acdf9b..40dcc7b 100644 >> --- a/arch/arm/include/asm/arch-exynos/dwmmc.h >> +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h >> @@ -29,8 +29,12 @@ >> >> int exynos_dwmci_init(u32 regbase, int bus_width, int index); >> >> +#ifdef CONFIG_OF_CONTROL >> +unsigned int exynos_dwmmc_init(const void *blob); >> +#else >> static inline unsigned int exynos_dwmmc_init(int index, int bus_width) > > Why unsigned? > > I'm really not that keen on functions which change their signature > based on an #ifdef. Can we perhaps have > > int exynos_dwmmc_init(const void *blob); > > which will pass NULL when there is no FDT, and > > int exynos_dwmmc_add_port(int index, int bus_width) > > for use by non-FDT boards? > >> { >> unsigned int base = samsung_get_base_mmc() + (0x10000 * index); >> return exynos_dwmci_init(base, bus_width, index); >> } >> +#endif >> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c >> index 72a31b7..d7ca7d0 100644 >> --- a/drivers/mmc/exynos_dw_mmc.c >> +++ b/drivers/mmc/exynos_dw_mmc.c >> @@ -19,39 +19,154 @@ >> */ >> >> #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> >> + >> +#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 >> +#define ONE_MEGA_HZ 1000000 >> +#define SCALED_VAL_FOUR_HUNDRED 400 > > I don't think you need these last two - you can just write the number > in the code Why didn't add into the dwmmc.h? > >> >> static char *EXYNOS_NAME = "EXYNOS DWMMC"; > > Same with this I think Sorry..What means? Also need not? > >> +u32 timing[3]; >> >> +/* >> + * 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) >> { >> struct dwmci_host *host = NULL; >> + unsigned int clock, div; >> host = malloc(sizeof(struct dwmci_host)); >> if (!host) { >> printf("dwmci_host malloc fail!\n"); >> return 1; >> } >> >> + /* >> + * The max operating freq of FSYS block is 400MHz. >> + * Scale down the 400MHz to number 400. >> + * Scale down the MPLL clock by dividing MPLL_CLK with ONE_MEGA_HZ. >> + * Arrive at the divisor value taking 400 as the reference. >> + */ >> + >> + /* get mpll clock and divide it by ONE_MEGA_HZ */ >> + clock = get_pll_clk(MPLL) / ONE_MEGA_HZ; >> + >> + /* Arrive at the divisor value. */ >> + for (div = 0; div <= 0xf; div++) { >> + if ((clock / (div + 1)) <= SCALED_VAL_FOUR_HUNDRED) >> + break; >> + } > > What if you don't find the right divisor? i want to use like this.
sclk = mmc_get_clk(); -> then we can get the FSYS1 clock value div = DIV_ROUND_UP(sclk, freq); => freq is request clock value. mmc_set_clk(index, div); Then we can set to div value at clock register. It didn't need to add this code... >> + >> + /* set the clock divisor for mmc */ >> + set_mmc_clk(index, div); >> + >> host->name = EXYNOS_NAME; >> host->ioaddr = (void *)regbase; >> host->buswidth = bus_width; >> +#ifdef CONFIG_OF_CONTROL >> + host->clksel_val = (DWMCI_SET_SAMPLE_CLK(timing[0]) | >> + DWMCI_SET_DRV_CLK(timing[1]) | >> + DWMCI_SET_DIV_RATIO(timing[2])); > > timing should be a parameter, not a global. For the non-FDT case > perhaps you can accept NULL, meaning default? Then: > > if (timing) > do the code above > else > do the code below > >> +#else >> + if (0 == index) >> + host->clksel_val = DWMMC_MMC0_CLKSEL_VAL; >> + if (2 == index) >> + host->clksel_val = DWMMC_MMC2_CLKSEL_VAL; >> +#endif >> host->clksel = exynos_dwmci_clksel; >> host->dev_index = index; >> - >> - add_dwmci(host, 52000000, 400000); >> + host->mmc_clk = exynos_dwmci_get_clk; >> + /* Add the mmc chennel to be registered with mmc core */ >> + add_dwmci(host, DWMMC_MAX_FREQ, DWMMC_MIN_FREQ); > > Is error checking needed here? > >> >> return 0; >> } >> >> +#ifdef CONFIG_OF_CONTROL >> +unsigned int exynos_dwmmc_init(const void *blob) >> +{ >> + u32 base; >> + int index, bus_width; >> + int node_list[DWMMC_MAX_CH_NUM]; >> + int err = 0; >> + int dev_id, flag; >> + int count, i; >> + >> + count = fdtdec_find_aliases_for_id(blob, "dwmmc", >> + 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; >> + >> + /* 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) { > > <= 0? The function will return 0 if there is no property. > >> + 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; >> + } >> + /* Initialise each mmc channel */ >> + err = exynos_dwmci_init(base, bus_width, index); >> + if (err) { >> + debug("Can't do dwmci init\n"); >> + return -1; >> + } >> + } >> + >> + return 0; >> +} >> +#endif >> diff --git a/include/dwmmc.h b/include/dwmmc.h >> index c8b1d40..4a42849 100644 >> --- a/include/dwmmc.h >> +++ b/include/dwmmc.h >> @@ -123,6 +123,9 @@ >> #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) >> + > > Remove this extra line? > >> >> #define DWMCI_IDMAC_OWN (1 << 31) >> #define DWMCI_IDMAC_CH (1 << 4) >> @@ -144,6 +147,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