[U-Boot] [PATCH 0/4] EXYNOS5: Enable dwmmc

[U-Boot] [PATCH 0/4] EXYNOS5: Enable dwmmc

[Amar]

This patch set enables the dwmmc for Exynos5250 on SMDK5250.
Also does the required driver changes. 

This patch set is based on:
"EXYNOS: mmc: support DesignWare Controller for Samsung-SoC"
"Exynos: clock: support get_mmc_clk for exynos"

Amar (4):
  MMC: DWMMC: Modified fifo size computation
  MMC: EXYNOS: Added call back function for clock get
  EXYNOS: CLOCK: Initialised the local variable
  SMDK5250: Initialise and enable dwmmc channels

 arch/arm/cpu/armv7/exynos/clock.c |    4 ++--
 board/samsung/smdk5250/smdk5250.c |   32 ++++++++++++++++++++++++++------
 drivers/mmc/dw_mmc.c              |    2 ++
 drivers/mmc/exynos_dw_mmc.c       |   13 ++++++++++++-
 include/configs/smdk5250.h        |    4 ++--
 5 files changed, 44 insertions(+), 11 deletions(-)

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Amar]

The current implementation of fifo size computation was giving improper
values for eMMC channel. Modified the computation as per user manual.

Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
index 4070d4e..62dc152 100644
--- a/drivers/mmc/dw_mmc.c
+++ b/drivers/mmc/dw_mmc.c
@@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
 	dwmci_writel(host, DWMCI_BMOD, 1);
 
 	fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
+	fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
+
 	if (host->fifoth_val)
 		fifoth_val = host->fifoth_val;
 	else
-- 
1.7.0.4

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Amar]

This patch defines the call back required by dw mmc driver to get the
clock value. It also adds function to set the dw mmc clock divider ratio.

Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
---
 drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
index 72a31b7..7cc8aba 100644
--- a/drivers/mmc/exynos_dw_mmc.c
+++ b/drivers/mmc/exynos_dw_mmc.c
@@ -30,25 +30,37 @@ static void exynos_dwmci_clksel(struct dwmci_host *host)
 {
 	u32 val;
 	val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
-		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
+		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
+		DWMCI_SET_DIV_RATIO(DWMCI_SHIFT_0);
 
 	dwmci_writel(host, DWMCI_CLKSEL, val);
 }
 
+unsigned int exynos_dwmci_get_clk(int dev_index)
+{
+	return get_mmc_clk(dev_index);
+}
+
 int exynos_dwmci_init(u32 regbase, int bus_width, int index)
 {
 	struct dwmci_host *host = NULL;
+	int div = 0;
 	host = malloc(sizeof(struct dwmci_host));
 	if (!host) {
 		printf("dwmci_host malloc fail!\n");
 		return 1;
 	}
 
+	div = 1;
+	/* Set the mmc clock divider ratio & pre-ratio */
+	set_mmc_clk(index, div);
+
 	host->name = EXYNOS_NAME;
 	host->ioaddr = (void *)regbase;
 	host->buswidth = bus_width;
 	host->clksel = exynos_dwmci_clksel;
 	host->dev_index = index;
+	host->mmc_clk = exynos_dwmci_get_clk;
 
 	add_dwmci(host, 52000000, 400000);
 
-- 
1.7.0.4

[U-Boot] [PATCH 3/4] EXYNOS: CLOCK: Initialised the local variable

[Amar]

This patch initialises the local variable 'shift' to zero to avoid
improper extraction of ratio and pre-ratio divider values. Extraction of
improper values was happening due to garbage value present in local variable.

Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
---
 arch/arm/cpu/armv7/exynos/clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c
index 731bbff..0bcf05f 100644
--- a/arch/arm/cpu/armv7/exynos/clock.c
+++ b/arch/arm/cpu/armv7/exynos/clock.c
@@ -379,7 +379,7 @@ static unsigned long exynos4_get_mmc_clk(int dev_index)
 		(struct exynos4_clock *)samsung_get_base_clock();
 	unsigned long uclk, sclk;
 	unsigned int sel, ratio, pre_ratio;
-	int shift;
+	int shift = 0;
 
 	sel = readl(&clk->src_fsys);
 	sel = (sel >> (dev_index << 2)) & 0xf;
@@ -428,7 +428,7 @@ static unsigned long exynos5_get_mmc_clk(int dev_index)
 		(struct exynos5_clock *)samsung_get_base_clock();
 	unsigned long uclk, sclk;
 	unsigned int sel, ratio, pre_ratio;
-	int shift;
+	int shift = 0;
 
 	sel = readl(&clk->src_fsys);
 	sel = (sel >> (dev_index << 2)) & 0xf;
-- 
1.7.0.4

[U-Boot] [PATCH 4/4] SMDK5250: Initialise and enable dwmmc channels

[Amar]

This patch initialises and enables dwmmc channels 0 and 2 for SMDK5250.
It also initialises the pinmux for the same.

Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
---
 board/samsung/smdk5250/smdk5250.c |   22 +++++++++++++++++++++-
 include/configs/smdk5250.h        |    4 ++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/board/samsung/smdk5250/smdk5250.c b/board/samsung/smdk5250/smdk5250.c
index 4c50342..9503510 100644
--- a/board/samsung/smdk5250/smdk5250.c
+++ b/board/samsung/smdk5250/smdk5250.c
@@ -26,6 +26,7 @@
 #include <netdev.h>
 #include <spi.h>
 #include <asm/arch/cpu.h>
+#include <asm/arch/dwmmc.h>
 #include <asm/arch/gpio.h>
 #include <asm/arch/mmc.h>
 #include <asm/arch/pinmux.h>
@@ -139,13 +140,32 @@ int board_mmc_init(bd_t *bis)
 {
 	int err;
 
+	err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
+	if (err) {
+		debug("SDMMC2 not configured\n");
+		return err;
+	}
+
+	/*SD: dwmmc Channel-2 with 4 bit bus width */
+	err = exynos_dwmmc_init(2, 4);
+	if (err) {
+		debug("dwmmc Channel-2 init failed\n");
+		return err;
+	}
+
 	err = exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE);
 	if (err) {
 		debug("SDMMC0 not configured\n");
 		return err;
 	}
 
-	err = s5p_mmc_init(0, 8);
+	/*eMMC: dwmmc Channel-0 with 8 bit bus width */
+	err = exynos_dwmmc_init(0, 8);
+	if (err) {
+		debug("dwmmc Channel-0 init failed\n");
+		return err;
+	}
+
 	return err;
 }
 #endif
diff --git a/include/configs/smdk5250.h b/include/configs/smdk5250.h
index e412da8..7dc2d96 100644
--- a/include/configs/smdk5250.h
+++ b/include/configs/smdk5250.h
@@ -77,8 +77,8 @@
 /* SD/MMC configuration */
 #define CONFIG_GENERIC_MMC
 #define CONFIG_MMC
-#define CONFIG_SDHCI
-#define CONFIG_S5P_SDHCI
+#define CONFIG_DWMMC
+#define CONFIG_EXYNOS_DWMMC
 
 #define CONFIG_BOARD_EARLY_INIT_F
 
-- 
1.7.0.4

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Jaehoon Chung]

It looks good to me.
Added minor comment.

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 12/05/2012 10:31 PM, Amar wrote:
> The current implementation of fifo size computation was giving improper
> values for eMMC channel. Modified the computation as per user manual.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  drivers/mmc/dw_mmc.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 4070d4e..62dc152 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
>  	dwmci_writel(host, DWMCI_BMOD, 1);
>  
>  	fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
> +	fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
How about using like FIFO_SIZE_MASK?
> +
>  	if (host->fifoth_val)
>  		fifoth_val = host->fifoth_val;
>  	else
> 

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Jaehoon Chung]

On 12/05/2012 10:31 PM, Amar wrote:
> This patch defines the call back required by dw mmc driver to get the
> clock value. It also adds function to set the dw mmc clock divider ratio.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> index 72a31b7..7cc8aba 100644
> --- a/drivers/mmc/exynos_dw_mmc.c
> +++ b/drivers/mmc/exynos_dw_mmc.c
> @@ -30,25 +30,37 @@ static void exynos_dwmci_clksel(struct dwmci_host *host)
>  {
>  	u32 val;
>  	val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> -		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
> +		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
> +		DWMCI_SET_DIV_RATIO(DWMCI_SHIFT_0);
>  
>  	dwmci_writel(host, DWMCI_CLKSEL, val);
>  }
>  
> +unsigned int exynos_dwmci_get_clk(int dev_index)
> +{
> +	return get_mmc_clk(dev_index);
> +}
> +
>  int exynos_dwmci_init(u32 regbase, int bus_width, int index)
>  {
>  	struct dwmci_host *host = NULL;
> +	int div = 0;
Why didn't initialize to 1? div is assigned to 1 at the below.
>  	host = malloc(sizeof(struct dwmci_host));
>  	if (!host) {
>  		printf("dwmci_host malloc fail!\n");
>  		return 1;
>  	}
>  
> +	div = 1;
> +	/* Set the mmc clock divider ratio & pre-ratio */
> +	set_mmc_clk(index, div);
Then if div set to 1, what value is set?
Can this code adjust to every board?
> +
>  	host->name = EXYNOS_NAME;
>  	host->ioaddr = (void *)regbase;
>  	host->buswidth = bus_width;
>  	host->clksel = exynos_dwmci_clksel;
>  	host->dev_index = index;
> +	host->mmc_clk = exynos_dwmci_get_clk;
>  
>  	add_dwmci(host, 52000000, 400000);
>  
> 

[U-Boot] [PATCH 3/4] EXYNOS: CLOCK: Initialised the local variable

[Jaehoon Chung]

Acked-by: Jaehoon Chung <jh80.chung@samsung.com>

On 12/05/2012 10:31 PM, Amar wrote:
> This patch initialises the local variable 'shift' to zero to avoid
> improper extraction of ratio and pre-ratio divider values. Extraction of
> improper values was happening due to garbage value present in local variable.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  arch/arm/cpu/armv7/exynos/clock.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/exynos/clock.c b/arch/arm/cpu/armv7/exynos/clock.c
> index 731bbff..0bcf05f 100644
> --- a/arch/arm/cpu/armv7/exynos/clock.c
> +++ b/arch/arm/cpu/armv7/exynos/clock.c
> @@ -379,7 +379,7 @@ static unsigned long exynos4_get_mmc_clk(int dev_index)
>  		(struct exynos4_clock *)samsung_get_base_clock();
>  	unsigned long uclk, sclk;
>  	unsigned int sel, ratio, pre_ratio;
> -	int shift;
> +	int shift = 0;
>  
>  	sel = readl(&clk->src_fsys);
>  	sel = (sel >> (dev_index << 2)) & 0xf;
> @@ -428,7 +428,7 @@ static unsigned long exynos5_get_mmc_clk(int dev_index)
>  		(struct exynos5_clock *)samsung_get_base_clock();
>  	unsigned long uclk, sclk;
>  	unsigned int sel, ratio, pre_ratio;
> -	int shift;
> +	int shift = 0;
>  
>  	sel = readl(&clk->src_fsys);
>  	sel = (sel >> (dev_index << 2)) & 0xf;
> 

[U-Boot] [PATCH 4/4] SMDK5250: Initialise and enable dwmmc channels

[Jaehoon Chung]

On 12/05/2012 10:31 PM, Amar wrote:
> This patch initialises and enables dwmmc channels 0 and 2 for SMDK5250.
> It also initialises the pinmux for the same.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  board/samsung/smdk5250/smdk5250.c |   22 +++++++++++++++++++++-
>  include/configs/smdk5250.h        |    4 ++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/board/samsung/smdk5250/smdk5250.c b/board/samsung/smdk5250/smdk5250.c
> index 4c50342..9503510 100644
> --- a/board/samsung/smdk5250/smdk5250.c
> +++ b/board/samsung/smdk5250/smdk5250.c
> @@ -26,6 +26,7 @@
>  #include <netdev.h>
>  #include <spi.h>
>  #include <asm/arch/cpu.h>
> +#include <asm/arch/dwmmc.h>
>  #include <asm/arch/gpio.h>
>  #include <asm/arch/mmc.h>
>  #include <asm/arch/pinmux.h>
> @@ -139,13 +140,32 @@ int board_mmc_init(bd_t *bis)
>  {
>  	int err;
>  
> +	err = exynos_pinmux_config(PERIPH_ID_SDMMC2, PINMUX_FLAG_NONE);
> +	if (err) {
> +		debug("SDMMC2 not configured\n");
> +		return err;
> +	}
If return error, we didn't need to initialize the MMC0?
And why do you initialize the MMC2 before initialized MMC0?
I didn't test the this patch, but i think if you inserted SD-card, it didn't work well.
> +
> +	/*SD: dwmmc Channel-2 with 4 bit bus width */
> +	err = exynos_dwmmc_init(2, 4);
> +	if (err) {
> +		debug("dwmmc Channel-2 init failed\n");
> +		return err;
> +	}
> +
>  	err = exynos_pinmux_config(PERIPH_ID_SDMMC0, PINMUX_FLAG_8BIT_MODE);
>  	if (err) {
>  		debug("SDMMC0 not configured\n");
>  		return err;
>  	}
>  
> -	err = s5p_mmc_init(0, 8);
> +	/*eMMC: dwmmc Channel-0 with 8 bit bus width */
> +	err = exynos_dwmmc_init(0, 8);
> +	if (err) {
> +		debug("dwmmc Channel-0 init failed\n");
> +		return err;
> +	}
> +
>  	return err;
>  }
>  #endif
> diff --git a/include/configs/smdk5250.h b/include/configs/smdk5250.h
> index e412da8..7dc2d96 100644
> --- a/include/configs/smdk5250.h
> +++ b/include/configs/smdk5250.h
> @@ -77,8 +77,8 @@
>  /* SD/MMC configuration */
>  #define CONFIG_GENERIC_MMC
>  #define CONFIG_MMC
> -#define CONFIG_SDHCI
> -#define CONFIG_S5P_SDHCI
> +#define CONFIG_DWMMC
> +#define CONFIG_EXYNOS_DWMMC
>  
>  #define CONFIG_BOARD_EARLY_INIT_F
>  
> 

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Simon Glass]

Hi Jaehoon,

On Wed, Dec 5, 2012 at 7:59 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> It looks good to me.
> Added minor comment.
>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>
> On 12/05/2012 10:31 PM, Amar wrote:
>> The current implementation of fifo size computation was giving improper
>> values for eMMC channel. Modified the computation as per user manual.
>>
>> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
>> ---
>>  drivers/mmc/dw_mmc.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>> index 4070d4e..62dc152 100644
>> --- a/drivers/mmc/dw_mmc.c
>> +++ b/drivers/mmc/dw_mmc.c
>> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
>>       dwmci_writel(host, DWMCI_BMOD, 1);
>>
>>       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
>> +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
> How about using like FIFO_SIZE_MASK?

It might be better to avoid macros in header files which shift and
mask x, since they obscure the operation, and just define the amount
of shift and mask in the header file. Also if you have a #define for
the mask you should probably also have one for the shift, otherwise
you have the information in two places. So maybe:

#define RX_WMARK_SHIFT 16
#define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)

fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1;

>> +
>>       if (host->fifoth_val)
>>               fifoth_val = host->fifoth_val;
>>       else
>>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Jaehoon Chung]

Hi Simon,

On 12/09/2012 04:49 AM, Simon Glass wrote:
> Hi Jaehoon,
> 
> On Wed, Dec 5, 2012 at 7:59 PM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> It looks good to me.
>> Added minor comment.
>>
>> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
>>
>> On 12/05/2012 10:31 PM, Amar wrote:
>>> The current implementation of fifo size computation was giving improper
>>> values for eMMC channel. Modified the computation as per user manual.
>>>
>>> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
>>> ---
>>>  drivers/mmc/dw_mmc.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>>> index 4070d4e..62dc152 100644
>>> --- a/drivers/mmc/dw_mmc.c
>>> +++ b/drivers/mmc/dw_mmc.c
>>> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
>>>       dwmci_writel(host, DWMCI_BMOD, 1);
>>>
>>>       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
>>> +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
>> How about using like FIFO_SIZE_MASK?
> 
> It might be better to avoid macros in header files which shift and
> mask x, since they obscure the operation, and just define the amount
> of shift and mask in the header file. Also if you have a #define for
> the mask you should probably also have one for the shift, otherwise
> you have the information in two places. So maybe:
I want to add the macro into header file like your suggestion.
RX_WMARK() is used to set with user input or pdata.
If Amarendra will change this patch, it will looks great to me.

Best Regards,
Jaehoon Chung
> 
> #define RX_WMARK_SHIFT 16
> #define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
> 
> fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1;
> 
>>> +
>>>       if (host->fifoth_val)
>>>               fifoth_val = host->fifoth_val;
>>>       else
>>>
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
> 
> Regards,
> Simon
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
> 

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Amarendra Reddy]

Hi Jaehoon Chung,

I will update the patches as per your review comments.

As suggested by Mr.Simon, I will create macros similar to the below macros
in header file.

 #define RX_WMARK_SHIFT 16
 #define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)


Thanks & Regards
Amarendra

On 10 December 2012 07:51, Jaehoon Chung <jh80.chung@samsung.com> wrote:

> Hi Simon,
>
> On 12/09/2012 04:49 AM, Simon Glass wrote:
> > Hi Jaehoon,
> >
> > On Wed, Dec 5, 2012 at 7:59 PM, Jaehoon Chung <jh80.chung@samsung.com>
> wrote:
> >> It looks good to me.
> >> Added minor comment.
> >>
> >> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> >>
> >> On 12/05/2012 10:31 PM, Amar wrote:
> >>> The current implementation of fifo size computation was giving improper
> >>> values for eMMC channel. Modified the computation as per user manual.
> >>>
> >>> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> >>> ---
> >>>  drivers/mmc/dw_mmc.c |    2 ++
> >>>  1 files changed, 2 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> >>> index 4070d4e..62dc152 100644
> >>> --- a/drivers/mmc/dw_mmc.c
> >>> +++ b/drivers/mmc/dw_mmc.c
> >>> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
> >>>       dwmci_writel(host, DWMCI_BMOD, 1);
> >>>
> >>>       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
> >>> +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
> >> How about using like FIFO_SIZE_MASK?
> >
> > It might be better to avoid macros in header files which shift and
> > mask x, since they obscure the operation, and just define the amount
> > of shift and mask in the header file. Also if you have a #define for
> > the mask you should probably also have one for the shift, otherwise
> > you have the information in two places. So maybe:
> I want to add the macro into header file like your suggestion.
> RX_WMARK() is used to set with user input or pdata.
> If Amarendra will change this patch, it will looks great to me.
>
> Best Regards,
> Jaehoon Chung
>  >
> > #define RX_WMARK_SHIFT 16
> > #define RX_WMARK_MASK (0xfff << RX_WMARK_SHIFT)
> >
> > fifo_size = ((fifo_size & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1;
> >
> >>> +
> >>>       if (host->fifoth_val)
> >>>               fifoth_val = host->fifoth_val;
> >>>       else
> >>>
> >>
> >> _______________________________________________
> >> U-Boot mailing list
> >> U-Boot at lists.denx.de
> >> http://lists.denx.de/mailman/listinfo/u-boot
> >
> > Regards,
> > Simon
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > http://lists.denx.de/mailman/listinfo/u-boot
> >
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Minkyu Kang]

Dear Amar,

On 05/12/12 22:31, Amar wrote:
> The current implementation of fifo size computation was giving improper
> values for eMMC channel. Modified the computation as per user manual.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  drivers/mmc/dw_mmc.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> index 4070d4e..62dc152 100644
> --- a/drivers/mmc/dw_mmc.c
> +++ b/drivers/mmc/dw_mmc.c
> @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
>  	dwmci_writel(host, DWMCI_BMOD, 1);
>  
>  	fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
> +	fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;

The definition of RX_WMARK is missing.

> +
>  	if (host->fifoth_val)
>  		fifoth_val = host->fifoth_val;
>  	else
> 

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Minkyu Kang]

Dear Amar,

On 05/12/12 22:31, Amar wrote:
> This patch defines the call back required by dw mmc driver to get the
> clock value. It also adds function to set the dw mmc clock divider ratio.
> 
> Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> ---
>  drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> index 72a31b7..7cc8aba 100644
> --- a/drivers/mmc/exynos_dw_mmc.c
> +++ b/drivers/mmc/exynos_dw_mmc.c
> @@ -30,25 +30,37 @@ static void exynos_dwmci_clksel(struct dwmci_host *host)
>  {
>  	u32 val;
>  	val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> -		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
> +		DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
> +		DWMCI_SET_DIV_RATIO(DWMCI_SHIFT_0);
>  
>  	dwmci_writel(host, DWMCI_CLKSEL, val);
>  }
>  
> +unsigned int exynos_dwmci_get_clk(int dev_index)
> +{
> +	return get_mmc_clk(dev_index);
> +}
> +
>  int exynos_dwmci_init(u32 regbase, int bus_width, int index)
>  {
>  	struct dwmci_host *host = NULL;
> +	int div = 0;

please add a blank line here.

>  	host = malloc(sizeof(struct dwmci_host));
>  	if (!host) {
>  		printf("dwmci_host malloc fail!\n");
>  		return 1;
>  	}
>  
> +	div = 1;

ditto.

> +	/* Set the mmc clock divider ratio & pre-ratio */
> +	set_mmc_clk(index, div);

If the div is constant value, then just put 1.
I think.. it's better.
set_mmc_clk(index, 1);

> +
>  	host->name = EXYNOS_NAME;
>  	host->ioaddr = (void *)regbase;
>  	host->buswidth = bus_width;
>  	host->clksel = exynos_dwmci_clksel;
>  	host->dev_index = index;
> +	host->mmc_clk = exynos_dwmci_get_clk;
>  
>  	add_dwmci(host, 52000000, 400000);
>  
> 

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Amarendra Reddy]

Dear Minkyu,

The definition of "RX_WMARK" is present in "[PATCH V7 04/10] EXYNOS5:
DWMMC: Added FDT support for DWMMC".

Here is the URL,
http://www.mail-archive.com/u-boot at lists.denx.de/msg107515.html

Thanks & Regards
Amarendra Reddy

On 27 March 2013 10:33, Minkyu Kang <mk7.kang@samsung.com> wrote:

> Dear Amar,
>
> On 05/12/12 22:31, Amar wrote:
> > The current implementation of fifo size computation was giving improper
> > values for eMMC channel. Modified the computation as per user manual.
> >
> > Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> > ---
> >  drivers/mmc/dw_mmc.c |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
> > index 4070d4e..62dc152 100644
> > --- a/drivers/mmc/dw_mmc.c
> > +++ b/drivers/mmc/dw_mmc.c
> > @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
> >       dwmci_writel(host, DWMCI_BMOD, 1);
> >
> >       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
> > +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
>
> The definition of RX_WMARK is missing.
>
> > +
> >       if (host->fifoth_val)
> >               fifoth_val = host->fifoth_val;
> >       else
> >
>
> Thanks,
> Minkyu Kang.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Amarendra Reddy]

Dear Minkyu,

Please refer to the latest version of emmc patchset, which is V7 patchset.

The function definition 'exynos_dwmci_init()' has been changed when
compared to V1 patchset.

Here is the URL for latest version.
http://www.mail-archive.com/u-boot at lists.denx.de/msg107511.html

Thanks & Regards
Amarendra

On 27 March 2013 10:44, Minkyu Kang <mk7.kang@samsung.com> wrote:

> Dear Amar,
>
> On 05/12/12 22:31, Amar wrote:
> > This patch defines the call back required by dw mmc driver to get the
> > clock value. It also adds function to set the dw mmc clock divider ratio.
> >
> > Signed-off-by: Amarendra Reddy <amarendra.xt@samsung.com>
> > ---
> >  drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
> >  1 files changed, 13 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/exynos_dw_mmc.c b/drivers/mmc/exynos_dw_mmc.c
> > index 72a31b7..7cc8aba 100644
> > --- a/drivers/mmc/exynos_dw_mmc.c
> > +++ b/drivers/mmc/exynos_dw_mmc.c
> > @@ -30,25 +30,37 @@ static void exynos_dwmci_clksel(struct dwmci_host
> *host)
> >  {
> >       u32 val;
> >       val = DWMCI_SET_SAMPLE_CLK(DWMCI_SHIFT_0) |
> > -             DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) | DWMCI_SET_DIV_RATIO(0);
> > +             DWMCI_SET_DRV_CLK(DWMCI_SHIFT_0) |
> > +             DWMCI_SET_DIV_RATIO(DWMCI_SHIFT_0);
> >
> >       dwmci_writel(host, DWMCI_CLKSEL, val);
> >  }
> >
> > +unsigned int exynos_dwmci_get_clk(int dev_index)
> > +{
> > +     return get_mmc_clk(dev_index);
> > +}
> > +
> >  int exynos_dwmci_init(u32 regbase, int bus_width, int index)
> >  {
> >       struct dwmci_host *host = NULL;
> > +     int div = 0;
>
> please add a blank line here.
>
> >       host = malloc(sizeof(struct dwmci_host));
> >       if (!host) {
> >               printf("dwmci_host malloc fail!\n");
> >               return 1;
> >       }
> >
> > +     div = 1;
>
> ditto.
>
> > +     /* Set the mmc clock divider ratio & pre-ratio */
> > +     set_mmc_clk(index, div);
>
> If the div is constant value, then just put 1.
> I think.. it's better.
> set_mmc_clk(index, 1);
>
> > +
> >       host->name = EXYNOS_NAME;
> >       host->ioaddr = (void *)regbase;
> >       host->buswidth = bus_width;
> >       host->clksel = exynos_dwmci_clksel;
> >       host->dev_index = index;
> > +     host->mmc_clk = exynos_dwmci_get_clk;
> >
> >       add_dwmci(host, 52000000, 400000);
> >
> >
>
> Thanks,
> Minkyu Kang.
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH 1/4] MMC: DWMMC: Modified fifo size computation

[Minkyu Kang]

Dear Amar,

On 27/03/13 14:18, Amarendra Reddy wrote:
> Dear Minkyu,
> 

please don't top posting.

> The definition of "RX_WMARK" is present in "[PATCH V7 04/10] EXYNOS5: DWMMC: Added FDT support for DWMMC".
> 
> Here is the URL, http://www.mail-archive.com/u-boot at lists.denx.de/msg107515.html

Then please move it to this patch.

> 
> Thanks & Regards
> Amarendra Reddy
> 
> On 27 March 2013 10:33, Minkyu Kang <mk7.kang at samsung.com <mailto:mk7.kang@samsung.com>> wrote:
> 
>     Dear Amar,
> 
>     On 05/12/12 22:31, Amar wrote:
>     > The current implementation of fifo size computation was giving improper
>     > values for eMMC channel. Modified the computation as per user manual.
>     >
>     > Signed-off-by: Amarendra Reddy <amarendra.xt at samsung.com <mailto:amarendra.xt@samsung.com>>
>     > ---
>     >  drivers/mmc/dw_mmc.c |    2 ++
>     >  1 files changed, 2 insertions(+), 0 deletions(-)
>     >
>     > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c
>     > index 4070d4e..62dc152 100644
>     > --- a/drivers/mmc/dw_mmc.c
>     > +++ b/drivers/mmc/dw_mmc.c
>     > @@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
>     >       dwmci_writel(host, DWMCI_BMOD, 1);
>     >
>     >       fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
>     > +     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;
> 
>     The definition of RX_WMARK is missing.
> 
>     > +
>     >       if (host->fifoth_val)
>     >               fifoth_val = host->fifoth_val;
>     >       else
>     >
> 
>     Thanks,
>     Minkyu Kang.
>     _______________________________________________
>     U-Boot mailing list
>     U-Boot at lists.denx.de <mailto:U-Boot@lists.denx.de>
>     http://lists.denx.de/mailman/listinfo/u-boot
> 
> 

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Minkyu Kang]

Dear Amar,

On 27/03/13 14:38, Amarendra Reddy wrote:
> Dear Minkyu,
> 
> Please refer to the latest version of emmc patchset, which is V7 patchset.
> 
> The function definition 'exynos_dwmci_init()' has been changed when compared to V1 patchset.

It means.. should I ignore this patchset?

> 
> Here is the URL for latest version.
> http://www.mail-archive.com/u-boot at lists.denx.de/msg107511.html
> 
> Thanks & Regards
> Amarendra
> 

Thanks,
Minkyu Kang.

[U-Boot] [PATCH 2/4] MMC: EXYNOS: Added call back function for clock get

[Amarendra Reddy]

Dear Minkyu,

Yes, the patchset you referred needs to be ignored as it is V1 patchset.

You need to refer to V7 patchset present at the URL
http://www.mail-archive.com/u-boot at lists.denx.de/msg107511.html

Thanks & Regards
Amarendra

On 27 March 2013 13:10, Minkyu Kang <mk7.kang@samsung.com> wrote:

> Dear Amar,
>
> On 27/03/13 14:38, Amarendra Reddy wrote:
> > Dear Minkyu,
> >
> > Please refer to the latest version of emmc patchset, which is V7
> patchset.
> >
> > The function definition 'exynos_dwmci_init()' has been changed when
> compared to V1 patchset.
>
> It means.. should I ignore this patchset?
>
> >
> > Here is the URL for latest version.
> > http://www.mail-archive.com/u-boot at lists.denx.de/msg107511.html
> >
> > Thanks & Regards
> > Amarendra
> >
>
> Thanks,
> Minkyu Kang.
>
>