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

[Jaehoon Chung <jh80.chung@samsung.com>]

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@samsung.com>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Tested-by: Piotr Wilczek <p.wilczek@samsung.com>
>> Cc: Lukasz Majewski <l.majewski@samsung.com>
>> Cc: Piotr Wilczek <p.wilczek@samsung.com>
>> Cc: Minkyu Kang <mk7.kang@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?
> 
>>  	/* 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.
In my patch, this is used  the below.
+
+	if (cpu_is_exynos4())
+		compat_id = COMPAT_SAMSUNG_EXYNOS4_DWMMC;
+	else if (cpu_is_exynos5())
+		compat_id = COMPAT_SAMSUNG_EXYNOS5_DWMMC;
+	else
+		compat_id = COMPAT_UNKNOWN;
+
> 
>> +
>>  	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.
> 
>>  		/* 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?
> 
>>
>>  		/* 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?
It didn't need to separate.

Best Regards,
Jaehoon Chung

> 
> 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>