diff mbox

[v2] mmc: dw_mmc: fix bug that cause mmc_test failture

Message ID 1424400925-3867-1-git-send-email-addy.ke@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

addy ke Feb. 20, 2015, 2:55 a.m. UTC
The STOP command can terminate a data transfer between a memory card and
mmc controller.

As show in Synopsys DesignWare Cores Mobile Stroage Host Databook:
Data timeout and Data end-bit error will terminate further data transfer
by mmc controller. So we should not send abort command to terminate a
data transfer again if we got DRTO and EBE interrupt.

After this patch, all mmc_test cases can pass on RK3288-Pink2 board.

Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
---
Changes in v2:
- DRTO and EBE are both set, should not send abort command too,
  suggested by Doug Anderson. 

 drivers/mmc/host/dw_mmc.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Doug Anderson Feb. 20, 2015, 9:43 p.m. UTC | #1
Addy,

On Thu, Feb 19, 2015 at 6:55 PM, Addy Ke <addy.ke@rock-chips.com> wrote:
> The STOP command can terminate a data transfer between a memory card and
> mmc controller.
>
> As show in Synopsys DesignWare Cores Mobile Stroage Host Databook:
> Data timeout and Data end-bit error will terminate further data transfer
> by mmc controller. So we should not send abort command to terminate a
> data transfer again if we got DRTO and EBE interrupt.
>
> After this patch, all mmc_test cases can pass on RK3288-Pink2 board.
>
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> ---
> Changes in v2:
> - DRTO and EBE are both set, should not send abort command too,
>   suggested by Doug Anderson.
>
>  drivers/mmc/host/dw_mmc.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

I also suggested that you change the subject to "mmc: dw_mmc: fix
mmc_test by not sending abort for DRTO / EBE errors" which you didn't
do.  ...I guess that's not critical.  Perhaps Jaehoon (who is
collecting patches for Ulf now I think) can adjust the subject when he
applies.

In any case, v2 fixes my problems and seems OK to me.

Reviewed-by: Doug Anderson <dianders@chromium.org>

On rk3288 on a 3.14 kernel w/ lotsa backports:
Tested-by: Doug Anderson <dianders@chromium.org>
Javier Martinez Canillas Feb. 25, 2015, 6:17 p.m. UTC | #2
Hello Addy,

On 02/20/2015 03:55 AM, Addy Ke wrote:
> The STOP command can terminate a data transfer between a memory card and
> mmc controller.
> 
> As show in Synopsys DesignWare Cores Mobile Stroage Host Databook:

s/Stroage/Storage but maybe Ulf can amend the typo when applying?

> Data timeout and Data end-bit error will terminate further data transfer
> by mmc controller. So we should not send abort command to terminate a
> data transfer again if we got DRTO and EBE interrupt.
> 
> After this patch, all mmc_test cases can pass on RK3288-Pink2 board.
> 
> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>

I did not run all the tests listed in /sys/kernel/debug/mmc*/*/testlist
because some of them will overwrite the data in my card but at least
the read ones completed successfully and $subject does not cause a
regression in the Exynos5250 Snow, Exynos5420 Peach Pit and Exynos5800
Peach Pi Chromebooks where I tested it.

Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>

Best regards,
Javier
Jaehoon Chung Feb. 27, 2015, 7:11 a.m. UTC | #3
Hi,

I will apply into my-tree after change the subject and fix typo.

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

Best Regards,
Jaehoon Chung

On 02/26/2015 03:17 AM, Javier Martinez Canillas wrote:
> Hello Addy,
> 
> On 02/20/2015 03:55 AM, Addy Ke wrote:
>> The STOP command can terminate a data transfer between a memory card and
>> mmc controller.
>>
>> As show in Synopsys DesignWare Cores Mobile Stroage Host Databook:
> 
> s/Stroage/Storage but maybe Ulf can amend the typo when applying?
> 
>> Data timeout and Data end-bit error will terminate further data transfer
>> by mmc controller. So we should not send abort command to terminate a
>> data transfer again if we got DRTO and EBE interrupt.
>>
>> After this patch, all mmc_test cases can pass on RK3288-Pink2 board.
>>
>> Signed-off-by: Addy Ke <addy.ke@rock-chips.com>
> 
> I did not run all the tests listed in /sys/kernel/debug/mmc*/*/testlist
> because some of them will overwrite the data in my card but at least
> the read ones completed successfully and $subject does not cause a
> regression in the Exynos5250 Snow, Exynos5420 Peach Pit and Exynos5800
> Peach Pi Chromebooks where I tested it.
> 
> Tested-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> 
> Best regards,
> Javier
>
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 4d2e3c2..a27048a 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1520,7 +1520,10 @@  static void dw_mci_tasklet_func(unsigned long priv)
 			if (test_and_clear_bit(EVENT_DATA_ERROR,
 					       &host->pending_events)) {
 				dw_mci_stop_dma(host);
-				send_stop_abort(host, data);
+				if (data->stop ||
+				    !(host->data_status & (SDMMC_INT_DRTO |
+							   SDMMC_INT_EBE)))
+					send_stop_abort(host, data);
 				state = STATE_DATA_ERROR;
 				break;
 			}
@@ -1547,7 +1550,10 @@  static void dw_mci_tasklet_func(unsigned long priv)
 			if (test_and_clear_bit(EVENT_DATA_ERROR,
 					       &host->pending_events)) {
 				dw_mci_stop_dma(host);
-				send_stop_abort(host, data);
+				if (data->stop ||
+				    !(host->data_status & (SDMMC_INT_DRTO |
+							   SDMMC_INT_EBE)))
+					send_stop_abort(host, data);
 				state = STATE_DATA_ERROR;
 				break;
 			}