[U-Boot] [PATCH] MMC: DWMMC: Fix FIFO_DEPTH calculation

Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com --- drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH); - if (host->fifoth_val) + if (host->fifoth_val) { fifoth_val = host->fifoth_val; - else + } else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2); + host->fifoth_val = fifoth_val; + } dwmci_writel(host, DWMCI_FIFOTH, fifoth_val);
dwmci_writel(host, DWMCI_CLKENA, 0);

Hi Rajeshwari
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde <rajeshwari.s@samsung.com
wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) { fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
}
Looks Good to me. Reviewed-by: Alim Akhtar alim.akhtar@samsung.com
dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.

On 05/24/2013 03:27 AM, Jagan Teki wrote:
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
It should be set into board-specific file(dw-mmc_exynos.c) or others.
Best Regards, Jaehoon Chung
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.

On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung jh80.chung@samsung.com wrote:
On 05/24/2013 03:27 AM, Jagan Teki wrote:
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
It should be set into board-specific file(dw-mmc_exynos.c) or others.
I am unable to find dw-mmc_exynos.c on the master, and also I haven't see fifoth_val used other than dw_mmc.c Could you please help me out.
Thanks, Jagan.
Best Regards, Jaehoon Chung
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.

Hi Jagan,
Please find my comments below. On Fri, May 24, 2013 at 10:31 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung jh80.chung@samsung.com wrote:
On 05/24/2013 03:27 AM, Jagan Teki wrote:
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
It should be set into board-specific file(dw-mmc_exynos.c) or others.
I am unable to find dw-mmc_exynos.c on the master, and also I haven't see fifoth_val used other than dw_mmc.c Could you please help me out.
dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c
Thanks, Jagan.
Best Regards, Jaehoon Chung
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Hi Jagan,
On 05/24/2013 03:15 PM, Rajeshwari Birje wrote:
Hi Jagan,
Please find my comments below. On Fri, May 24, 2013 at 10:31 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung jh80.chung@samsung.com wrote:
On 05/24/2013 03:27 AM, Jagan Teki wrote:
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
It should be set into board-specific file(dw-mmc_exynos.c) or others.
I am unable to find dw-mmc_exynos.c on the master, and also I haven't see fifoth_val used other than dw_mmc.c Could you please help me out.
dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c
host->fifoth_val should be set to hard-coding by developer. In general case, it's right that fifoth_Val is calculated with register value. But sometime, it needs to set to other value, not register value. Then we can use the host->fifoth_val in board-specific file. Maybe it didn't use the host->fifoth_val anywhere. In future, we can use this.
Best Regards, Jaehoon Chung
Thanks, Jagan.
Best Regards, Jaehoon Chung
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Fri, May 24, 2013 at 12:16 PM, Jaehoon Chung jh80.chung@samsung.com wrote:
Hi Jagan,
On 05/24/2013 03:15 PM, Rajeshwari Birje wrote:
Hi Jagan,
Please find my comments below. On Fri, May 24, 2013 at 10:31 AM, Jagan Teki jagannadh.teki@gmail.com wrote:
On Fri, May 24, 2013 at 7:12 AM, Jaehoon Chung jh80.chung@samsung.com wrote:
On 05/24/2013 03:27 AM, Jagan Teki wrote:
On Thu, May 23, 2013 at 6:45 PM, Rajeshwari Shinde rajeshwari.s@samsung.com wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
if (host->fifoth_val)
if (host->fifoth_val) {
What is the inital value for host->fifoth_val, for the first time call.?
It should be set into board-specific file(dw-mmc_exynos.c) or others.
I am unable to find dw-mmc_exynos.c on the master, and also I haven't see fifoth_val used other than dw_mmc.c Could you please help me out.
dw-mmc_exynos.c is merged in mmc tree. Yes FIFO_DEPTH is set only in dw_mmc.c
host->fifoth_val should be set to hard-coding by developer. In general case, it's right that fifoth_Val is calculated with register value. But sometime, it needs to set to other value, not register value. Then we can use the host->fifoth_val in board-specific file. Maybe it didn't use the host->fifoth_val anywhere. In future, we can use this.
Ok, thanks for your response. If that is the case may be we can initialize host->fifoth_val on add_dwmci() for as of now. Once the proper init of this is done in board specific file in future may be we can remove this.
Thanks, Jagan.
Best Regards, Jaehoon Chung
Thanks, Jagan.
Best Regards, Jaehoon Chung
fifoth_val = host->fifoth_val;
else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val); dwmci_writel(host, DWMCI_CLKENA, 0);
-- 1.7.4.4
Thanks, Jagan.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

Acked-by: Jaehoon Chung jh80.chung@samsung.com
On 05/23/2013 10:15 PM, Rajeshwari Shinde wrote:
Current DWMMC driver used to give FIFO underrun/overrun error every 3rd time for mmc rescan command. In current code FIFO_DEPTH is getting calculated after reading the FIFOTH register and extracting the RX_WMARK bits from it i.e (RX_WMARK = FIFO_DEPTH/2 -1). Instead of storing the correct value, we were recalculating the FIFO_DEPT each time which is not correct.
Signed-off-by: Hatim Ali hatim.rv@samsung.com Signed-off-by: Rajeshwari Shinde rajeshwari.s@samsung.com
drivers/mmc/dw_mmc.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 4070d4e..be590a4 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -332,11 +332,13 @@ static int dwmci_init(struct mmc *mmc) dwmci_writel(host, DWMCI_BMOD, 1);
fifo_size = dwmci_readl(host, DWMCI_FIFOTH);
- if (host->fifoth_val)
- if (host->fifoth_val) { fifoth_val = host->fifoth_val;
- else
} else { fifoth_val = MSIZE(0x2) | RX_WMARK(fifo_size/2 -1) | TX_WMARK(fifo_size/2);
host->fifoth_val = fifoth_val;
} dwmci_writel(host, DWMCI_FIFOTH, fifoth_val);
dwmci_writel(host, DWMCI_CLKENA, 0);
participants (5)
-
Alim Akhtar
-
Jaehoon Chung
-
Jagan Teki
-
Rajeshwari Birje
-
Rajeshwari Shinde