On Tue, 26 Mar 2019 at 08:04, Daniel Drake <drake@xxxxxxxxxxxx> wrote:
>
> The alcor driver is setting up data transfer and submitting the associated
> MMC command at the same time. While this works most of the time, it
> occasionally causes problems upon write.
>
> In the working case, after setting up the data transfer and submitting
> the MMC command, an interrupt comes in a moment later with CMD_END and
> WRITE_BUF_RDY bits set. The data transfer then happens without problem.
>
> However, on occasion, the interrupt that arrives at that point only
> has WRITE_BUF_RDY set. The hardware notifies that it's ready to write
> data, but the associated MMC command is still running. Regardless, the
> driver was proceeding to write data immediately, and that would then cause
> another interrupt indicating data CRC error, and the write would fail.
>
> Additionally, the transfer setup function alcor_trigger_data_transfer()
> was being called 3 times for each write operation, which was confusing
> and may be contributing to this issue.
>
> Solve this by tweaking the driver behaviour to follow the sequence observed
> in the original ampe_stor vendor driver:
> 1. When starting request handling, write 0 to DATA_XFER_CTRL
> 2. Submit the command
> 3. Wait for CMD_END interrupt and then trigger data transfer
> 4. For the PIO case, trigger the next step of the data transfer only
> upon the following DATA_END interrupt, which occurs after the block has
> been written.
>
> I confirmed that the read path still works (DMA & PIO) and also now
> presents more consistency with the operations performed by ampe_stor.
>
> Signed-off-by: Daniel Drake <drake@xxxxxxxxxxxx>
Applied for fixes, by adding a fixes and a stable tag, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/alcor.c | 34 +++++++++++++---------------------
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/mmc/host/alcor.c b/drivers/mmc/host/alcor.c
> index 82a97866e0cf4..7c8f203f9a24d 100644
> --- a/drivers/mmc/host/alcor.c
> +++ b/drivers/mmc/host/alcor.c
> @@ -48,7 +48,6 @@ struct alcor_sdmmc_host {
> struct mmc_command *cmd;
> struct mmc_data *data;
> unsigned int dma_on:1;
> - unsigned int early_data:1;
>
> struct mutex cmd_mutex;
>
> @@ -144,8 +143,7 @@ static void alcor_data_set_dma(struct alcor_sdmmc_host *host)
> host->sg_count--;
> }
>
> -static void alcor_trigger_data_transfer(struct alcor_sdmmc_host *host,
> - bool early)
> +static void alcor_trigger_data_transfer(struct alcor_sdmmc_host *host)
> {
> struct alcor_pci_priv *priv = host->alcor_pci;
> struct mmc_data *data = host->data;
> @@ -155,13 +153,6 @@ static void alcor_trigger_data_transfer(struct alcor_sdmmc_host *host,
> ctrl |= AU6601_DATA_WRITE;
>
> if (data->host_cookie == COOKIE_MAPPED) {
> - if (host->early_data) {
> - host->early_data = false;
> - return;
> - }
> -
> - host->early_data = early;
> -
> alcor_data_set_dma(host);
> ctrl |= AU6601_DATA_DMA_MODE;
> host->dma_on = 1;
> @@ -231,6 +222,7 @@ static void alcor_prepare_sg_miter(struct alcor_sdmmc_host *host)
> static void alcor_prepare_data(struct alcor_sdmmc_host *host,
> struct mmc_command *cmd)
> {
> + struct alcor_pci_priv *priv = host->alcor_pci;
> struct mmc_data *data = cmd->data;
>
> if (!data)
> @@ -248,7 +240,7 @@ static void alcor_prepare_data(struct alcor_sdmmc_host *host,
> if (data->host_cookie != COOKIE_MAPPED)
> alcor_prepare_sg_miter(host);
>
> - alcor_trigger_data_transfer(host, true);
> + alcor_write8(priv, 0, AU6601_DATA_XFER_CTRL);
> }
>
> static void alcor_send_cmd(struct alcor_sdmmc_host *host,
> @@ -435,7 +427,7 @@ static int alcor_cmd_irq_done(struct alcor_sdmmc_host *host, u32 intmask)
> if (!host->data)
> return false;
>
> - alcor_trigger_data_transfer(host, false);
> + alcor_trigger_data_transfer(host);
> host->cmd = NULL;
> return true;
> }
> @@ -456,7 +448,7 @@ static void alcor_cmd_irq_thread(struct alcor_sdmmc_host *host, u32 intmask)
> if (!host->data)
> alcor_request_complete(host, 1);
> else
> - alcor_trigger_data_transfer(host, false);
> + alcor_trigger_data_transfer(host);
> host->cmd = NULL;
> }
>
> @@ -487,15 +479,9 @@ static int alcor_data_irq_done(struct alcor_sdmmc_host *host, u32 intmask)
> break;
> case AU6601_INT_READ_BUF_RDY:
> alcor_trf_block_pio(host, true);
> - if (!host->blocks)
> - break;
> - alcor_trigger_data_transfer(host, false);
> return 1;
> case AU6601_INT_WRITE_BUF_RDY:
> alcor_trf_block_pio(host, false);
> - if (!host->blocks)
> - break;
> - alcor_trigger_data_transfer(host, false);
> return 1;
> case AU6601_INT_DMA_END:
> if (!host->sg_count)
> @@ -508,8 +494,14 @@ static int alcor_data_irq_done(struct alcor_sdmmc_host *host, u32 intmask)
> break;
> }
>
> - if (intmask & AU6601_INT_DATA_END)
> - return 0;
> + if (intmask & AU6601_INT_DATA_END) {
> + if (!host->dma_on && host->blocks) {
> + alcor_trigger_data_transfer(host);
> + return 1;
> + } else {
> + return 0;
> + }
> + }
>
> return 1;
> }
> --
> 2.19.1
>
