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
