On Tue, Apr 10, 2018 at 11:38:27AM +0200, Wolfram Sang wrote:
> Early revisions of certain SoCs cannot do multiple DMA RX streams in
> parallel. To avoid data corruption, only allow one DMA RX channel and
> fall back to PIO, if needed.
>
> Signed-off-by: Wolfram Sang <[email protected]>
> Reviewed-by: Yoshihiro Shimoda <[email protected]>
> Tested-by: Nguyen Viet Dung <[email protected]>
> ---
> drivers/mmc/host/renesas_sdhi_internal_dmac.c | 35
> ++++++++++++++++++++++++---
> 1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> index 8e0acd197c43..9c50d64cd10c 100644
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -9,6 +9,7 @@
> * published by the Free Software Foundation.
> */
>
> +#include <linux/bitops.h>
> #include <linux/device.h>
> #include <linux/dma-mapping.h>
> #include <linux/io-64-nonatomic-hi-lo.h>
> @@ -62,6 +63,17 @@
> * need a custom accessor.
> */
>
> +static unsigned long global_flags;
Is the restriction on concurrent DMA RX streams global or per-device?
> +/*
> + * Workaround for avoiding to use RX DMAC by multiple channels.
> + * On R-Car H3 ES1.* and M3-W ES1.0, when multiple SDHI channels use
> + * RX DMAC simultaneously, sometimes hundreds of bytes data are not
> + * stored into the system memory even if the DMAC interrupt happened.
> + * So, this driver then uses one RX DMAC channel only.
> + */
> +#define SDHI_INTERNAL_DMAC_ONE_RX_ONLY 0
> +#define SDHI_INTERNAL_DMAC_RX_IN_USE 1
> +
> /* Definitions for sampling clocks */
> static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
> {
> @@ -126,6 +138,10 @@ renesas_sdhi_internal_dmac_abort_dma(struct
> tmio_mmc_host *host) {
> renesas_sdhi_internal_dmac_dm_write(host, DM_CM_RST,
> RST_RESERVED_BITS | val);
>
> +
> + if (host->data && host->data->flags & MMC_DATA_READ)
> + clear_bit(SDHI_INTERNAL_DMAC_RX_IN_USE, &global_flags);
> renesas_sdhi_internal_dmac_enable_dma(host, true);
> }
>
> @@ -155,6 +171,9 @@ renesas_sdhi_internal_dmac_start_dma(struct tmio_mmc_host
> *host,
> if (data->flags & MMC_DATA_READ) {
> dtran_mode |= DTRAN_MODE_CH_NUM_CH1;
> dir = DMA_FROM_DEVICE;
> + if (test_bit(SDHI_INTERNAL_DMAC_ONE_RX_ONLY, &global_flags) &&
> + test_and_set_bit(SDHI_INTERNAL_DMAC_RX_IN_USE,
> &global_flags))
> + goto force_pio;
> } else {
> dtran_mode |= DTRAN_MODE_CH_NUM_CH0;
> dir = DMA_TO_DEVICE;
> @@ -208,6 +227,9 @@ static void
> renesas_sdhi_internal_dmac_complete_tasklet_fn(unsigned long arg)
> renesas_sdhi_internal_dmac_enable_dma(host, false);
> dma_unmap_sg(&host->pdev->dev, host->sg_ptr, host->sg_len, dir);
>
> + if (dir == DMA_FROM_DEVICE)
> + clear_bit(SDHI_INTERNAL_DMAC_RX_IN_USE, &global_flags);
> +
Is clear_bit() expensive? If so it might be worth avoiding on SoCs that
don't have the restriction covered by this patch.
> +
> tmio_mmc_do_data_irq(host);
> out:
> spin_unlock_irq(&host->lock);
> @@ -251,18 +273,25 @@ static const struct tmio_mmc_dma_ops
> renesas_sdhi_internal_dmac_dma_ops = {
> * implementation as others may use a different implementation.
> */
> static const struct soc_device_attribute gen3_soc_whitelist[] = {
> - { .soc_id = "r8a7795", .revision = "ES1.*" },
> + { .soc_id = "r8a7795", .revision = "ES1.*",
> + .data = (void *)BIT(SDHI_INTERNAL_DMAC_ONE_RX_ONLY) },
> { .soc_id = "r8a7795", .revision = "ES2.0" },
> - { .soc_id = "r8a7796", .revision = "ES1.0" },
> + { .soc_id = "r8a7796", .revision = "ES1.0",
> + .data = (void *)BIT(SDHI_INTERNAL_DMAC_ONE_RX_ONLY) },
> { .soc_id = "r8a77995", .revision = "ES1.0" },
> { /* sentinel */ }
> };
>
> static int renesas_sdhi_internal_dmac_probe(struct platform_device *pdev)
> {
> - if (!soc_device_match(gen3_soc_whitelist))
> + const struct soc_device_attribute *soc =
> soc_device_match(gen3_soc_whitelist);
> +
> + if (!soc)
> return -ENODEV;
>
> + if (soc->data)
> + global_flags |= (unsigned long)soc->data;
> +
> return renesas_sdhi_probe(pdev, &renesas_sdhi_internal_dmac_dma_ops);
> }
>
> --
> 2.11.0
>