diff mbox

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

Message ID 532A6B25.40000@samsung.com
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Commit Message

Beomho Seo March 20, 2014, 4:14 a.m. UTC 
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(-)

Comments

Minkyu Kang April 8, 2014, 2:53 a.m. UTC | #1 
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.
Jaehoon Chung April 8, 2014, 5:03 a.m. UTC | #2 
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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Beomho Seo April 8, 2014, 5:08 a.m. UTC | #3 
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@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?
> 

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.
>
Jaehoon Chung April 10, 2014, 5:12 a.m. UTC | #4 
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.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?
>>
> 
> 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.
>>
> 
>
diff mbox

Patch

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;
 	/* 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
+
 	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);
 		/* 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;

 		/* 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 */
 	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"),