From mboxrd@z Thu Jan  1 00:00:00 1970
From: Minkyu Kang <mk7.kang@samsung.com>
Date: Tue, 08 Apr 2014 11:53:48 +0900
Subject: [U-Boot] [PATCH v3 1/4]drivers: mmc: dwmmc: enable support for
	DT
In-Reply-To: <532A6B25.40000@samsung.com>
References: <532A6B25.40000@samsung.com>
Message-ID: <534364BC.5020600@samsung.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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.

> +
>  	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?

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.