diff mbox

[v2] mmc: dw_mmc: Reset DMA before enabling IDMAC

Message ID 1413478686-6857-1-git-send-email-sonnyrao@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sonny Rao Oct. 16, 2014, 4:58 p.m. UTC
We've already got a reset of DMA after it's done.  Add one before we
start DMA too.  This fixes a data corruption on Rockchip SoCs which
will get bad data when doing a DMA transfer after doing a PIO transfer.

We tested this on an Exynos 5800 with HS200 and didn't notice any
difference in sequential read throughput.

Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
Signed-off-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
---
 drivers/mmc/host/dw_mmc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Jaehoon Chung Oct. 17, 2014, 8:26 a.m. UTC | #1
Hi, Sonny.

On 10/17/2014 01:58 AM, Sonny Rao wrote:
> We've already got a reset of DMA after it's done.  Add one before we
> start DMA too.  This fixes a data corruption on Rockchip SoCs which
> will get bad data when doing a DMA transfer after doing a PIO transfer.
> 
> We tested this on an Exynos 5800 with HS200 and didn't notice any
> difference in sequential read throughput.

Didn't affect the write throughput?
I tested this on exynos3/4 with DDR50 and HS200.

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

> 
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Doug Anderson <dianders@chromium.org>
> ---
>  drivers/mmc/host/dw_mmc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..ca67f69 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -83,6 +83,7 @@ struct idmac_desc {
>  #endif /* CONFIG_MMC_DW_IDMAC */
>  
>  static bool dw_mci_reset(struct dw_mci *host);
> +static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>  
>  #if defined(CONFIG_DEBUG_FS)
>  static int dw_mci_req_show(struct seq_file *s, void *v)
> @@ -448,6 +449,10 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>  
>  	dw_mci_translate_sglist(host, host->data, sg_len);
>  
> +	/* Make sure to reset DMA in case we did PIO before this */
> +	dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET);
> +	dw_mci_idmac_reset(host);
> +
>  	/* Select IDMAC interface */
>  	temp = mci_readl(host, CTRL);
>  	temp |= SDMMC_CTRL_USE_IDMAC;
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alim Akhtar Oct. 17, 2014, 12:28 p.m. UTC | #2
Hi Sonny,

On Thu, Oct 16, 2014 at 10:28 PM, Sonny Rao <sonnyrao@chromium.org> wrote:
> We've already got a reset of DMA after it's done.  Add one before we
> start DMA too.  This fixes a data corruption on Rockchip SoCs which
> will get bad data when doing a DMA transfer after doing a PIO transfer.
>
> We tested this on an Exynos 5800 with HS200 and didn't notice any
> difference in sequential read throughput.
>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Doug Anderson <dianders@chromium.org>

This looks fine.

Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
> ---
>  drivers/mmc/host/dw_mmc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..ca67f69 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -83,6 +83,7 @@ struct idmac_desc {
>  #endif /* CONFIG_MMC_DW_IDMAC */
>
>  static bool dw_mci_reset(struct dw_mci *host);
> +static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>
>  #if defined(CONFIG_DEBUG_FS)
>  static int dw_mci_req_show(struct seq_file *s, void *v)
> @@ -448,6 +449,10 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>
>         dw_mci_translate_sglist(host, host->data, sg_len);
>
> +       /* Make sure to reset DMA in case we did PIO before this */
> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET);
> +       dw_mci_idmac_reset(host);
> +
>         /* Select IDMAC interface */
>         temp = mci_readl(host, CTRL);
>         temp |= SDMMC_CTRL_USE_IDMAC;
> --
> 2.1.2
>
Sonny Rao Oct. 18, 2014, 9:14 p.m. UTC | #3
On Fri, Oct 17, 2014 at 1:26 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi, Sonny.
>
> On 10/17/2014 01:58 AM, Sonny Rao wrote:
>> We've already got a reset of DMA after it's done.  Add one before we
>> start DMA too.  This fixes a data corruption on Rockchip SoCs which
>> will get bad data when doing a DMA transfer after doing a PIO transfer.
>>
>> We tested this on an Exynos 5800 with HS200 and didn't notice any
>> difference in sequential read throughput.
>
> Didn't affect the write throughput?

Write is usually much slower than read, but I went ahead and re-tested
and saw no difference on writes.

> I tested this on exynos3/4 with DDR50 and HS200.
>
> Acked-by: Jaehoon Chung <jh80.chung@samsung.com>
> Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
>
>>
>> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
>> Signed-off-by: Doug Anderson <dianders@chromium.org>
>> Tested-by: Doug Anderson <dianders@chromium.org>
>> ---
>>  drivers/mmc/host/dw_mmc.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 69f0cc6..ca67f69 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -83,6 +83,7 @@ struct idmac_desc {
>>  #endif /* CONFIG_MMC_DW_IDMAC */
>>
>>  static bool dw_mci_reset(struct dw_mci *host);
>> +static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>>
>>  #if defined(CONFIG_DEBUG_FS)
>>  static int dw_mci_req_show(struct seq_file *s, void *v)
>> @@ -448,6 +449,10 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>>
>>       dw_mci_translate_sglist(host, host->data, sg_len);
>>
>> +     /* Make sure to reset DMA in case we did PIO before this */
>> +     dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET);
>> +     dw_mci_idmac_reset(host);
>> +
>>       /* Select IDMAC interface */
>>       temp = mci_readl(host, CTRL);
>>       temp |= SDMMC_CTRL_USE_IDMAC;
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ulf Hansson Oct. 27, 2014, 2:13 p.m. UTC | #4
On 16 October 2014 18:58, Sonny Rao <sonnyrao@chromium.org> wrote:
> We've already got a reset of DMA after it's done.  Add one before we
> start DMA too.  This fixes a data corruption on Rockchip SoCs which
> will get bad data when doing a DMA transfer after doing a PIO transfer.
>
> We tested this on an Exynos 5800 with HS200 and didn't notice any
> difference in sequential read throughput.
>
> Signed-off-by: Sonny Rao <sonnyrao@chromium.org>
> Signed-off-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Doug Anderson <dianders@chromium.org>

Thanks! Applied for next!

Kind regards
Uffe

> ---
>  drivers/mmc/host/dw_mmc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> index 69f0cc6..ca67f69 100644
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -83,6 +83,7 @@ struct idmac_desc {
>  #endif /* CONFIG_MMC_DW_IDMAC */
>
>  static bool dw_mci_reset(struct dw_mci *host);
> +static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
>
>  #if defined(CONFIG_DEBUG_FS)
>  static int dw_mci_req_show(struct seq_file *s, void *v)
> @@ -448,6 +449,10 @@ static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
>
>         dw_mci_translate_sglist(host, host->data, sg_len);
>
> +       /* Make sure to reset DMA in case we did PIO before this */
> +       dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET);
> +       dw_mci_idmac_reset(host);
> +
>         /* Select IDMAC interface */
>         temp = mci_readl(host, CTRL);
>         temp |= SDMMC_CTRL_USE_IDMAC;
> --
> 2.1.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 69f0cc6..ca67f69 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -83,6 +83,7 @@  struct idmac_desc {
 #endif /* CONFIG_MMC_DW_IDMAC */
 
 static bool dw_mci_reset(struct dw_mci *host);
+static bool dw_mci_ctrl_reset(struct dw_mci *host, u32 reset);
 
 #if defined(CONFIG_DEBUG_FS)
 static int dw_mci_req_show(struct seq_file *s, void *v)
@@ -448,6 +449,10 @@  static void dw_mci_idmac_start_dma(struct dw_mci *host, unsigned int sg_len)
 
 	dw_mci_translate_sglist(host, host->data, sg_len);
 
+	/* Make sure to reset DMA in case we did PIO before this */
+	dw_mci_ctrl_reset(host, SDMMC_CTRL_DMA_RESET);
+	dw_mci_idmac_reset(host);
+
 	/* Select IDMAC interface */
 	temp = mci_readl(host, CTRL);
 	temp |= SDMMC_CTRL_USE_IDMAC;