From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jaehoon Chung Date: Thu, 10 Apr 2014 14:12:44 +0900 Subject: [U-Boot] [PATCH v3 1/4]drivers: mmc: dwmmc: enable support for DT In-Reply-To: <53438450.7040900@samsung.com> References: <532A6B25.40000@samsung.com> <534364BC.5020600@samsung.com> <53438450.7040900@samsung.com> Message-ID: <5346284C.9030200@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, Minkyu. On 04/08/2014 02:08 PM, Beomho Seo wrote: > Thank you for your advice. > > On 04/08/2014 11:53 AM, Minkyu Kang wrote: >> Dear Beonho Seo, >> >> On 20/03/14 13:14, Beomho Seo wrote: >>> This patch enables for device tree for dw-mmc driver. >>> Driver have been fixed because it is used exynos5 and exynos4. >>> Add, fdt compat id of exynos4 dwmmc. >>> >>> Signed-off-by: Beomho Seo >>> Signed-off-by: Jaehoon Chung >>> Tested-by: Piotr Wilczek >>> Cc: Lukasz Majewski >>> Cc: Piotr Wilczek >>> Cc: Minkyu Kang >>> --- >>> Changes for v3: >>> - None. >>> >>> drivers/mmc/exynos_dw_mmc.c | 21 +++++++++++++++++---- >>> include/fdtdec.h | 1 + >>> lib/fdtdec.c | 1 + >>> 3 files changed, 19 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c >>> index de8cdcc..b9d41d4 100644 >>> --- a/drivers/mmc/exynos_dw_mmc.c >>> +++ b/drivers/mmc/exynos_dw_mmc.c >>> @@ -13,6 +13,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #define DWMMC_MAX_CH_NUM 4 >>> #define DWMMC_MAX_FREQ 52000000 >>> @@ -82,6 +83,7 @@ int exynos_dwmci_add_port(int index, u32 regbase, int bus_width, u32 clksel) >>> freq = 52000000; >>> sclk = get_mmc_clk(index); >>> div = DIV_ROUND_UP(sclk, freq); >>> + div -= 1; >> >> why? >> > > It need for set accurately required frequency. > Above code prevent to set below the required frequency. > For example above case, > If sclk is 100 MHz and required frequency 50 MHz, Where div = 2. > and then mmc clock set 33 MHz calculating. > So It need div minus one. It didn't consider how div value is set into CMU register. SCLK = (MOUT/(PRE_RATIO + 1))/(RATIO + 1). Finally, it didn't consider for plus one. Source clock is set to wrong value. It needs to change other approach than using just "div -= 1". If i understood something wrong, let me know, plz. Best Regards, Jaehoon Chung > >>> /* set the clock divisor for mmc */ >>> set_mmc_clk(index, div); >>> >>> @@ -118,15 +120,21 @@ 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; >>> + int err = 0, dev_id, flag, count, i, compat_id; >>> u32 clksel_val, base, timing[3]; >>> >>> +#ifdef CONFIG_EXYNOS4 >>> + compat_id = COMPAT_SAMSUNG_EXYNOS4_DWMMC; >>> +#else >>> + compat_id = COMPAT_SAMSUNG_EXYNOS5_DWMMC; >>> +#endif >> >> NAK. >> please don't use ifdef. >> > > OK. I will change it. > >>> + >>> count = fdtdec_find_aliases_for_id(blob, "mmc", >>> - COMPAT_SAMSUNG_EXYNOS5_DWMMC, node_list, >>> - DWMMC_MAX_CH_NUM); >>> + compat_id, node_list, DWMMC_MAX_CH_NUM); >>> >>> for (i = 0; i < count; i++) { >>> int node = node_list[i]; >>> + struct fdt_gpio_state pwr_gpio; >>> >>> if (node <= 0) >>> continue; >>> @@ -145,6 +153,9 @@ int exynos_dwmmc_init(const void *blob) >>> else >>> flag = PINMUX_FLAG_NONE; >>> >>> + fdtdec_decode_gpio(blob, node, "pwr-gpios", &pwr_gpio); >>> + if (fdt_gpio_isvalid(&pwr_gpio)) >>> + gpio_direction_output(pwr_gpio.gpio, 1); >> >> please add blank line. >> > > OK. I will add blank line. > >>> /* config pinmux for each mmc channel */ >>> err = exynos_pinmux_config(dev_id, flag); >>> if (err) { >>> @@ -152,7 +163,9 @@ int exynos_dwmmc_init(const void *blob) >>> return err; >>> } >>> >>> - index = dev_id - PERIPH_ID_SDMMC0; >>> + index = fdtdec_get_int(blob, node, "index", dev_id); >>> + if (index == dev_id) >>> + index = dev_id - PERIPH_ID_SDMMC0; >> >> I can't understand why this routine is needed. >> Could you please explain? >> > > SDMMC index(0, 1 ... 4) is need to get/set mmc clk. > It is decided the difference between dev_id and PERIPH_ID_SDMMC0(=75). > But If dev_id is PERIPH_ID_SDMMC4(=131), index is set inaccurately. > So I add index property at device tree. and then parse it. > If index property not exist, index is decided the difference between dev_id and PERIPH_ID_SDMMC0. > >>> >>> /* Get the base address from the device node */ >>> base = fdtdec_get_addr(blob, node, "reg"); >>> diff --git a/include/fdtdec.h b/include/fdtdec.h >>> index 63027bd..15f50fe 100644 >>> --- a/include/fdtdec.h >>> +++ b/include/fdtdec.h >>> @@ -83,6 +83,7 @@ enum fdt_compat_id { >>> COMPAT_SAMSUNG_EXYNOS5_DP, /* Exynos Display port controller */ >>> COMPAT_SAMSUNG_EXYNOS5_DWMMC, /* Exynos5 DWMMC controller */ >>> COMPAT_SAMSUNG_EXYNOS_MMC, /* Exynos MMC controller */ >>> + COMPAT_SAMSUNG_EXYNOS4_DWMMC, /* Exynos4 DWMMC controller */ >> >> Do we need to separate exynos4 dwmmc and exynos5 dwmmc? >> > > OK, I will change and test it. > >> Jaehoon, >> how you think? >> >>> COMPAT_SAMSUNG_EXYNOS_SERIAL, /* Exynos UART */ >>> COMPAT_MAXIM_MAX77686_PMIC, /* MAX77686 PMIC */ >>> COMPAT_GENERIC_SPI_FLASH, /* Generic SPI Flash chip */ >>> diff --git a/lib/fdtdec.c b/lib/fdtdec.c >>> index be04598..79179cf 100644 >>> --- a/lib/fdtdec.c >>> +++ b/lib/fdtdec.c >>> @@ -56,6 +56,7 @@ static const char * const compat_names[COMPAT_COUNT] = { >>> COMPAT(SAMSUNG_EXYNOS5_DP, "samsung,exynos5-dp"), >>> COMPAT(SAMSUNG_EXYNOS5_DWMMC, "samsung,exynos5250-dwmmc"), >>> COMPAT(SAMSUNG_EXYNOS_MMC, "samsung,exynos-mmc"), >>> + COMPAT(SAMSUNG_EXYNOS4_DWMMC, "samsung,exynos4412-dwmmc"), >>> COMPAT(SAMSUNG_EXYNOS_SERIAL, "samsung,exynos4210-uart"), >>> COMPAT(MAXIM_MAX77686_PMIC, "maxim,max77686_pmic"), >>> COMPAT(GENERIC_SPI_FLASH, "spi-flash"), >>> >> >> Thanks, >> Minkyu Kang. >> > >