Skip to contentSkip to conversation

EXYNOS5: Enable dwmmc

PATCH New Amar 5 participants 19 messages Dec 5, 2012 – Mar 27, 2013

Subsystems (3)
lore
Share & tools
lore

Amar

author
#0 lore raw

cover EXYNOS5: Enable dwmmc

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(-)
author
#1 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +2 -0
index 4070d4e..62dc152 100644
@@ -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
Signature · 1 line
1.7.0.4
#2 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

It looks good to me.
Added minor comment.

Quoted 13 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 4 lines
drivers/mmc/dw_mmc.c +1 -0
@@ -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?

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
 	if (host->fifoth_val)
 		fifoth_val = host->fifoth_val;
 	else
#3 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

Hi Jaehoon,

Quoted 21 lines · Jaehoon Chung

It looks good to me.
Added minor comment.

Quoted 15 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644
@@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
      dwmci_writel(host, DWMCI_BMOD, 1);
 
Quoted 3 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 2 lines
drivers/mmc/dw_mmc.c +1 -0
      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;

Quoted 10 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 339

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
      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

#4 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

Hi Simon,

Quoted 28 lines · Simon Glass

Hi Jaehoon,

Quoted 21 lines · Jaehoon Chung

It looks good to me.
Added minor comment.

Quoted 15 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644
@@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
      dwmci_writel(host, DWMCI_BMOD, 1);
 
Quoted 3 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 2 lines
drivers/mmc/dw_mmc.c +1 -0
      fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
+     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;

How about using like FIFO_SIZE_MASK?

Quoted 5 lines

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

Quoted 24 lines

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

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

Quoted 10 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 339

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
      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

#5 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

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

Quoted 73 lines · Jaehoon Chung

Hi Simon,

Quoted 3 lines · Simon Glass

Hi Jaehoon,

On Wed, Dec 5, 2012 at 7:59 PM, Jaehoon Chung <[email protected]>

Quoted 25 lines · wrote:
Quoted 21 lines

It looks good to me.
Added minor comment.

Quoted 15 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644
@@ -332,6 +332,8 @@ static int dwmci_init(struct mmc *mmc)
      dwmci_writel(host, DWMCI_BMOD, 1);
 
Quoted 3 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 2 lines
drivers/mmc/dw_mmc.c +1 -0
      fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
+     fifo_size = ((fifo_size & RX_WMARK(0xFFF)) >> 16) + 1;

How about using like FIFO_SIZE_MASK?

Quoted 5 lines

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

Quoted 17 lines

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

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

Quoted 10 lines

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 339

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
      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

Quoted 7 lines

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

#6 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

In reply to Amar

Dear Amar,

Quoted 13 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 4 lines
drivers/mmc/dw_mmc.c +1 -0
@@ -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.

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 339

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
 	if (host->fifoth_val)
 		fifoth_val = host->fifoth_val;
 	else

Thanks,
Minkyu Kang.

#7 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

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

Quoted 36 lines · Minkyu Kang

Dear Amar,

Quoted 13 lines · Amar

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

---
 drivers/mmc/dw_mmc.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/mmc/dw_mmc.c +0 -0
index 4070d4e..62dc152 100644

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 335

Quoted 4 lines
drivers/mmc/dw_mmc.c +1 -0
@@ -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.

drivers/mmc/dw_mmc.c static int dwmci_init(struct mmc *mmc) · line 339

Quoted 5 lines
drivers/mmc/dw_mmc.c +1 -0
+
      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

#8 lore raw

PATCH 1/4 MMC: DWMMC: Modified fifo size computation

Dear Amar,

Quoted 2 lines · Amarendra Reddy

Dear Minkyu,

please don't top posting.

Quoted 3 lines

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.

Quoted 33 lines

Thanks & Regards
Amarendra Reddy

On 27 March 2013 10:33, Minkyu Kang <mk7.kang at samsung.com <mailto:[email protected]>> 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:[email protected]>>
    > ---
    >  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
Quoted 10 lines
    >

    Thanks,
    Minkyu Kang.
    _______________________________________________
    U-Boot mailing list
    U-Boot at lists.denx.de <mailto:[email protected]>
    http://lists.denx.de/mailman/listinfo/u-boot

Thanks,
Minkyu Kang.

author
#9 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

In reply to 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.

---
 drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
drivers/mmc/exynos_dw_mmc.c +13 -1
index 72a31b7..7cc8aba 100644
@@ -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);
 
Signature · 1 line
1.7.0.4
#10 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

Quoted 22 lines · 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.

---
 drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
drivers/mmc/exynos_dw_mmc.c +2 -1
index 72a31b7..7cc8aba 100644
@@ -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);
 }

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 47

Quoted 10 lines
drivers/mmc/exynos_dw_mmc.c +6 -0
 
+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.

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 56

Quoted 9 lines
drivers/mmc/exynos_dw_mmc.c +3 -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);

Then if div set to 1, what value is set?
Can this code adjust to every board?

Quoted 11 lines
drivers/mmc/exynos_dw_mmc.c +2 -0
+
 	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);
 
#11 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

In reply to Amar

Dear Amar,

Quoted 22 lines · 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.

---
 drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
drivers/mmc/exynos_dw_mmc.c +2 -1
index 72a31b7..7cc8aba 100644
@@ -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);
 }

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 47

Quoted 10 lines
drivers/mmc/exynos_dw_mmc.c +6 -0
 
+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.

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 54

Quoted 7 lines
drivers/mmc/exynos_dw_mmc.c +1 -0
 	host = malloc(sizeof(struct dwmci_host));
 	if (!host) {
 		printf("dwmci_host malloc fail!\n");
 		return 1;
 	}
 
+	div = 1;

ditto.

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 56

Quoted 2 lines
drivers/mmc/exynos_dw_mmc.c +2 -0
+	/* 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);

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 66

Quoted 11 lines
drivers/mmc/exynos_dw_mmc.c +2 -0
+
 	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.

#12 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

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

Quoted 76 lines · Minkyu Kang

Dear Amar,

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host

Quoted 13 lines · 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.

---
 drivers/mmc/exynos_dw_mmc.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)
drivers/mmc/exynos_dw_mmc.c +0 -0
index 72a31b7..7cc8aba 100644
@@ -30,25 +30,37 @@ static void exynos_dwmci_clksel(struct dwmci_host

*host)

Quoted 9 lines
drivers/mmc/exynos_dw_mmc.c +2 -1
 {
      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);
 }

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 47

Quoted 10 lines
drivers/mmc/exynos_dw_mmc.c +6 -0
 
+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.

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 54

Quoted 7 lines
drivers/mmc/exynos_dw_mmc.c +1 -0
      host = malloc(sizeof(struct dwmci_host));
      if (!host) {
              printf("dwmci_host malloc fail!\n");
              return 1;
      }
 
+     div = 1;

ditto.

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 56

Quoted 2 lines
drivers/mmc/exynos_dw_mmc.c +2 -0
+     /* 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);

drivers/mmc/exynos_dw_mmc.c static void exynos_dwmci_clksel(struct dwmci_host *host) · line 66

Quoted 11 lines
drivers/mmc/exynos_dw_mmc.c +2 -0
+
      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

#13 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

Dear Amar,

Quoted 5 lines · 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.

It means.. should I ignore this patchset?

Quoted 7 lines

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.

#14 lore raw

PATCH 2/4 MMC: EXYNOS: Added call back function for clock get

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

Quoted 25 lines · Minkyu Kang

Dear Amar,

Quoted 3 lines · Amarendra Reddy

Dear Minkyu,

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

patchset.

Quoted 2 lines

The function definition 'exynos_dwmci_init()' has been changed when

compared to V1 patchset.

It means.. should I ignore this patchset?

Quoted 7 lines

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.

author
#15 lore raw

PATCH 3/4 EXYNOS: CLOCK: Initialised the local variable

In reply to 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.

---
 arch/arm/cpu/armv7/exynos/clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/cpu/armv7/exynos/clock.c +2 -2
index 731bbff..0bcf05f 100644
@@ -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;
Signature · 1 line
1.7.0.4
#16 lore raw

PATCH 3/4 EXYNOS: CLOCK: Initialised the local variable

Quoted 32 lines · 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.

---
 arch/arm/cpu/armv7/exynos/clock.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/cpu/armv7/exynos/clock.c +2 -2
index 731bbff..0bcf05f 100644
@@ -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;
author
#17 lore raw

PATCH 4/4 SMDK5250: Initialise and enable dwmmc channels

In reply to Amar

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

---
 board/samsung/smdk5250/smdk5250.c |   22 +++++++++++++++++++++-
 include/configs/smdk5250.h        |    4 ++--
 2 files changed, 23 insertions(+), 3 deletions(-)
board/samsung/smdk5250/smdk5250.c +21 -1
index 4c50342..9503510 100644
@@ -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
include/configs/smdk5250.h +2 -2
index e412da8..7dc2d96 100644
@@ -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
 
Signature · 1 line
1.7.0.4
#18 lore raw

PATCH 4/4 SMDK5250: Initialise and enable dwmmc channels

Quoted 22 lines · Amar

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

---
 board/samsung/smdk5250/smdk5250.c |   22 +++++++++++++++++++++-
 include/configs/smdk5250.h        |    4 ++--
 2 files changed, 23 insertions(+), 3 deletions(-)
board/samsung/smdk5250/smdk5250.c +1 -0
index 4c50342..9503510 100644
@@ -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>

board/samsung/smdk5250/smdk5250.c int board_mmc_init(bd_t *bis) · line 147

Quoted 8 lines
board/samsung/smdk5250/smdk5250.c +5 -0
@@ -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.

Quoted 41 lines
board/samsung/smdk5250/smdk5250.c +15 -1
+
+	/*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
include/configs/smdk5250.h +2 -2
index e412da8..7dc2d96 100644
@@ -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