[U-Boot] [PATCH v3 1/4]drivers: mmc: dwmmc: enable support for DT

[Beomho Seo]

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.seo at samsung.com>
>> Signed-off-by: Jaehoon Chung <jh80.chung at samsung.com>
>> Tested-by: Piotr Wilczek <p.wilczek at samsung.com>
>> Cc: Lukasz Majewski <l.majewski at samsung.com>
>> Cc: Piotr Wilczek <p.wilczek at samsung.com>
>> Cc: Minkyu Kang <mk7.kang at 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.

>>  	/* 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.
>