Re: [PATCH v2] mmc: host: tmio: ensure end of DMA and SD access are in sync

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 17 February 2017 at 19:22, Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> The current code assumes that DMA is finished before SD access end is
> flagged. Thus, it schedules the 'dma_complete' tasklet in the SD card
> interrupt routine when DATAEND is set. The assumption is not safe,
> though. Even by mounting an SD card, it can be seen that sometimes DMA
> complete is first, sometimes DATAEND. It seems they are usually close
> enough timewise to not cause problems. However, a customer reported that
> with CMD53 sometimes things really break apart. As a result, the BSP has
> a patch which introduces flags for both events and makes sure both flags
> are set before scheduling the tasklet. The customer accepted the patch,
> yet it doesn't seem a proper upstream solution to me.
>
> This patch refactors the code to replace the tasklet with already
> existing and more lightweight mechanisms. First of all, we set the
> callback in a DMA descriptor to automatically get notified when DMA is
> done. In the callback, we then use a completion to make sure the SD
> access has already ended. Then, we proceed as before.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>

Thanks, applied for next!

Kind regards
Uffe

> ---
>
> Changes since V1/RFC:
>
> * removed #ifdef DEBUG
> * rebased to latest mmc/next
> * did some more performance testing. Couldn't spot any difference
>
>  drivers/mmc/host/tmio_mmc.h     |  2 +-
>  drivers/mmc/host/tmio_mmc_dma.c | 58 ++++++++++++++++++++++++-----------------
>  drivers/mmc/host/tmio_mmc_pio.c |  4 +--
>  3 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 2b349d48fb9a8a..891d400d2a7cf2 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -137,7 +137,7 @@ struct tmio_mmc_host {
>         bool                    force_pio;
>         struct dma_chan         *chan_rx;
>         struct dma_chan         *chan_tx;
> -       struct tasklet_struct   dma_complete;
> +       struct completion       dma_dataend;
>         struct tasklet_struct   dma_issue;
>         struct scatterlist      bounce_sg;
>         u8                      *bounce_buf;
> diff --git a/drivers/mmc/host/tmio_mmc_dma.c b/drivers/mmc/host/tmio_mmc_dma.c
> index fa8a936a3d9ba1..c7684fa91f1f9c 100644
> --- a/drivers/mmc/host/tmio_mmc_dma.c
> +++ b/drivers/mmc/host/tmio_mmc_dma.c
> @@ -43,6 +43,31 @@ void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
>         tmio_mmc_enable_dma(host, true);
>  }
>
> +static void tmio_mmc_dma_callback(void *arg)
> +{
> +       struct tmio_mmc_host *host = arg;
> +
> +       wait_for_completion(&host->dma_dataend);
> +
> +       spin_lock_irq(&host->lock);
> +
> +       if (!host->data)
> +               goto out;
> +
> +       if (host->data->flags & MMC_DATA_READ)
> +               dma_unmap_sg(host->chan_rx->device->dev,
> +                            host->sg_ptr, host->sg_len,
> +                            DMA_FROM_DEVICE);
> +       else
> +               dma_unmap_sg(host->chan_tx->device->dev,
> +                            host->sg_ptr, host->sg_len,
> +                            DMA_TO_DEVICE);
> +
> +       tmio_mmc_do_data_irq(host);
> +out:
> +       spin_unlock_irq(&host->lock);
> +}
> +
>  static void tmio_mmc_start_dma_rx(struct tmio_mmc_host *host)
>  {
>         struct scatterlist *sg = host->sg_ptr, *sg_tmp;
> @@ -88,6 +113,10 @@ static void tmio_mmc_start_dma_rx(struct tmio_mmc_host *host)
>                         DMA_DEV_TO_MEM, DMA_CTRL_ACK);
>
>         if (desc) {
> +               reinit_completion(&host->dma_dataend);
> +               desc->callback = tmio_mmc_dma_callback;
> +               desc->callback_param = host;
> +
>                 cookie = dmaengine_submit(desc);
>                 if (cookie < 0) {
>                         desc = NULL;
> @@ -162,6 +191,10 @@ static void tmio_mmc_start_dma_tx(struct tmio_mmc_host *host)
>                         DMA_MEM_TO_DEV, DMA_CTRL_ACK);
>
>         if (desc) {
> +               reinit_completion(&host->dma_dataend);
> +               desc->callback = tmio_mmc_dma_callback;
> +               desc->callback_param = host;
> +
>                 cookie = dmaengine_submit(desc);
>                 if (cookie < 0) {
>                         desc = NULL;
> @@ -221,29 +254,6 @@ static void tmio_mmc_issue_tasklet_fn(unsigned long priv)
>                 dma_async_issue_pending(chan);
>  }
>
> -static void tmio_mmc_tasklet_fn(unsigned long arg)
> -{
> -       struct tmio_mmc_host *host = (struct tmio_mmc_host *)arg;
> -
> -       spin_lock_irq(&host->lock);
> -
> -       if (!host->data)
> -               goto out;
> -
> -       if (host->data->flags & MMC_DATA_READ)
> -               dma_unmap_sg(host->chan_rx->device->dev,
> -                            host->sg_ptr, host->sg_len,
> -                            DMA_FROM_DEVICE);
> -       else
> -               dma_unmap_sg(host->chan_tx->device->dev,
> -                            host->sg_ptr, host->sg_len,
> -                            DMA_TO_DEVICE);
> -
> -       tmio_mmc_do_data_irq(host);
> -out:
> -       spin_unlock_irq(&host->lock);
> -}
> -
>  void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdata)
>  {
>         /* We can only either use DMA for both Tx and Rx or not use it at all */
> @@ -306,7 +316,7 @@ void tmio_mmc_request_dma(struct tmio_mmc_host *host, struct tmio_mmc_data *pdat
>                 if (!host->bounce_buf)
>                         goto ebouncebuf;
>
> -               tasklet_init(&host->dma_complete, tmio_mmc_tasklet_fn, (unsigned long)host);
> +               init_completion(&host->dma_dataend);
>                 tasklet_init(&host->dma_issue, tmio_mmc_issue_tasklet_fn, (unsigned long)host);
>         }
>
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 6b789a739d4dfe..c41f2252945eb7 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -596,11 +596,11 @@ static void tmio_mmc_data_irq(struct tmio_mmc_host *host, unsigned int stat)
>
>                 if (done) {
>                         tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND);
> -                       tasklet_schedule(&host->dma_complete);
> +                       complete(&host->dma_dataend);
>                 }
>         } else if (host->chan_rx && (data->flags & MMC_DATA_READ) && !host->force_pio) {
>                 tmio_mmc_disable_mmc_irqs(host, TMIO_STAT_DATAEND);
> -               tasklet_schedule(&host->dma_complete);
> +               complete(&host->dma_dataend);
>         } else {
>                 tmio_mmc_do_data_irq(host);
>                 tmio_mmc_disable_mmc_irqs(host, TMIO_MASK_READOP | TMIO_MASK_WRITEOP);
> --
> 2.11.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux