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 <beomho....@samsung.com>
>>> Signed-off-by: Jaehoon Chung <jh80.ch...@samsung.com>
>>> Tested-by: Piotr Wilczek <p.wilc...@samsung.com>
>>> Cc: Lukasz Majewski <l.majew...@samsung.com>
>>> Cc: Piotr Wilczek <p.wilc...@samsung.com>
>>> Cc: Minkyu Kang <mk7.k...@samsung.com>
>>> ---
>>> 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 <asm/arch/dwmmc.h>
>>> #include <asm/arch/clk.h>
>>> #include <asm/arch/pinmux.h>
>>> +#include <asm/gpio.h>
>>>
>>> #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.
>>
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
