The commit cd57d07b1e4e ("sh: don't allow non-coherent DMA for NOMMU") made
CONFIG_NO_DMA to be set for some platforms, for good reasons.
Consequentially, CONFIG_HAS_DMA doesn't get set, which makes the DMA
mapping interface to be built as stub functions.
For weird reasons this causes the mmc_spi driver to fail to ->probe(), as
it relies on the dma mappings APIs, no matter of CONFIG_HAS_DMA is set or
not. This is wrong, so let's fix this.
Fixes: cd57d07b1e4e ("sh: don't allow non-coherent DMA for NOMMU")
Reported-by: Rich Felker <[email protected]>
Suggested-by: Christoph Hellwig <[email protected]>
Signed-off-by: Ulf Hansson <[email protected]>
---
drivers/mmc/host/mmc_spi.c | 86 +++++++++++++++++++++++---------------
1 file changed, 52 insertions(+), 34 deletions(-)
diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 39bb1e30c2d7..5055a7eb134a 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -1278,6 +1278,52 @@ mmc_spi_detect_irq(int irq, void *mmc)
return IRQ_HANDLED;
}
+#ifdef CONFIG_HAS_DMA
+static int mmc_spi_dma_alloc(struct mmc_spi_host *host)
+{
+ struct spi_device *spi = host->spi;
+ struct device *dev;
+
+ if (!spi->master->dev.parent->dma_mask)
+ return 0;
+
+ dev = spi->master->dev.parent;
+
+ host->ones_dma = dma_map_single(dev, host->ones, MMC_SPI_BLOCKSIZE,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(dev, host->ones_dma))
+ return -ENOMEM;
+
+ host->data_dma = dma_map_single(dev, host->data, sizeof(*host->data),
+ DMA_BIDIRECTIONAL);
+ if (dma_mapping_error(dev, host->data_dma)) {
+ dma_unmap_single(dev, host->ones_dma, MMC_SPI_BLOCKSIZE,
+ DMA_TO_DEVICE);
+ return -ENOMEM;
+ }
+
+ dma_sync_single_for_cpu(dev, host->data_dma, sizeof(*host->data),
+ DMA_BIDIRECTIONAL);
+
+ host->dma_dev = dev;
+ return 0;
+}
+
+static void mmc_spi_dma_free(struct mmc_spi_host *host)
+{
+ if (!host->dma_dev)
+ return;
+
+ dma_unmap_single(host->dma_dev, host->ones_dma, MMC_SPI_BLOCKSIZE,
+ DMA_TO_DEVICE);
+ dma_unmap_single(host->dma_dev, host->data_dma, sizeof(*host->data),
+ DMA_BIDIRECTIONAL);
+}
+#else
+static inline mmc_spi_dma_alloc(struct mmc_spi_host *host) { return 0; }
+static inline void mmc_spi_dma_free(struct mmc_spi_host *host) {}
+#endif
+
static int mmc_spi_probe(struct spi_device *spi)
{
void *ones;
@@ -1374,23 +1420,9 @@ static int mmc_spi_probe(struct spi_device *spi)
if (!host->data)
goto fail_nobuf1;
- if (spi->master->dev.parent->dma_mask) {
- struct device *dev = spi->master->dev.parent;
-
- host->dma_dev = dev;
- host->ones_dma = dma_map_single(dev, ones,
- MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
- if (dma_mapping_error(dev, host->ones_dma))
- goto fail_ones_dma;
- host->data_dma = dma_map_single(dev, host->data,
- sizeof(*host->data), DMA_BIDIRECTIONAL);
- if (dma_mapping_error(dev, host->data_dma))
- goto fail_data_dma;
-
- dma_sync_single_for_cpu(host->dma_dev,
- host->data_dma, sizeof(*host->data),
- DMA_BIDIRECTIONAL);
- }
+ status = mmc_spi_dma_alloc(host);
+ if (status)
+ goto fail_dma;
/* setup message for status/busy readback */
spi_message_init(&host->readback);
@@ -1458,20 +1490,12 @@ static int mmc_spi_probe(struct spi_device *spi)
fail_add_host:
mmc_remove_host(mmc);
fail_glue_init:
- if (host->dma_dev)
- dma_unmap_single(host->dma_dev, host->data_dma,
- sizeof(*host->data), DMA_BIDIRECTIONAL);
-fail_data_dma:
- if (host->dma_dev)
- dma_unmap_single(host->dma_dev, host->ones_dma,
- MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
-fail_ones_dma:
+ mmc_spi_dma_free(host);
+fail_dma:
kfree(host->data);
-
fail_nobuf1:
mmc_free_host(mmc);
mmc_spi_put_pdata(spi);
-
nomem:
kfree(ones);
return status;
@@ -1489,13 +1513,7 @@ static int mmc_spi_remove(struct spi_device *spi)
mmc_remove_host(mmc);
- if (host->dma_dev) {
- dma_unmap_single(host->dma_dev, host->ones_dma,
- MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
- dma_unmap_single(host->dma_dev, host->data_dma,
- sizeof(*host->data), DMA_BIDIRECTIONAL);
- }
-
+ mmc_spi_dma_free(host);
kfree(host->data);
kfree(host->ones);
--
2.25.1
On Mon, Aug 31, 2020 at 03:16:36PM +0200, Ulf Hansson wrote:
> The commit cd57d07b1e4e ("sh: don't allow non-coherent DMA for NOMMU") made
> CONFIG_NO_DMA to be set for some platforms, for good reasons.
> Consequentially, CONFIG_HAS_DMA doesn't get set, which makes the DMA
> mapping interface to be built as stub functions.
>
> For weird reasons this causes the mmc_spi driver to fail to ->probe(), as
> it relies on the dma mappings APIs, no matter of CONFIG_HAS_DMA is set or
> not. This is wrong, so let's fix this.
>
> Fixes: cd57d07b1e4e ("sh: don't allow non-coherent DMA for NOMMU")
> Reported-by: Rich Felker <[email protected]>
> Suggested-by: Christoph Hellwig <[email protected]>
> Signed-off-by: Ulf Hansson <[email protected]>
> ---
> drivers/mmc/host/mmc_spi.c | 86 +++++++++++++++++++++++---------------
> 1 file changed, 52 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> index 39bb1e30c2d7..5055a7eb134a 100644
> --- a/drivers/mmc/host/mmc_spi.c
> +++ b/drivers/mmc/host/mmc_spi.c
> @@ -1278,6 +1278,52 @@ mmc_spi_detect_irq(int irq, void *mmc)
> return IRQ_HANDLED;
> }
>
> +#ifdef CONFIG_HAS_DMA
> +static int mmc_spi_dma_alloc(struct mmc_spi_host *host)
> +{
> + struct spi_device *spi = host->spi;
> + struct device *dev;
> +
> + if (!spi->master->dev.parent->dma_mask)
> + return 0;
> +
> + dev = spi->master->dev.parent;
> +
> + host->ones_dma = dma_map_single(dev, host->ones, MMC_SPI_BLOCKSIZE,
> + DMA_TO_DEVICE);
> + if (dma_mapping_error(dev, host->ones_dma))
> + return -ENOMEM;
> +
> + host->data_dma = dma_map_single(dev, host->data, sizeof(*host->data),
> + DMA_BIDIRECTIONAL);
> + if (dma_mapping_error(dev, host->data_dma)) {
> + dma_unmap_single(dev, host->ones_dma, MMC_SPI_BLOCKSIZE,
> + DMA_TO_DEVICE);
> + return -ENOMEM;
> + }
> +
> + dma_sync_single_for_cpu(dev, host->data_dma, sizeof(*host->data),
> + DMA_BIDIRECTIONAL);
> +
> + host->dma_dev = dev;
> + return 0;
> +}
> +
> +static void mmc_spi_dma_free(struct mmc_spi_host *host)
> +{
> + if (!host->dma_dev)
> + return;
> +
> + dma_unmap_single(host->dma_dev, host->ones_dma, MMC_SPI_BLOCKSIZE,
> + DMA_TO_DEVICE);
> + dma_unmap_single(host->dma_dev, host->data_dma, sizeof(*host->data),
> + DMA_BIDIRECTIONAL);
> +}
> +#else
> +static inline mmc_spi_dma_alloc(struct mmc_spi_host *host) { return 0; }
> +static inline void mmc_spi_dma_free(struct mmc_spi_host *host) {}
> +#endif
> +
> static int mmc_spi_probe(struct spi_device *spi)
> {
> void *ones;
> @@ -1374,23 +1420,9 @@ static int mmc_spi_probe(struct spi_device *spi)
> if (!host->data)
> goto fail_nobuf1;
>
> - if (spi->master->dev.parent->dma_mask) {
> - struct device *dev = spi->master->dev.parent;
> -
> - host->dma_dev = dev;
> - host->ones_dma = dma_map_single(dev, ones,
> - MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
> - if (dma_mapping_error(dev, host->ones_dma))
> - goto fail_ones_dma;
> - host->data_dma = dma_map_single(dev, host->data,
> - sizeof(*host->data), DMA_BIDIRECTIONAL);
> - if (dma_mapping_error(dev, host->data_dma))
> - goto fail_data_dma;
> -
> - dma_sync_single_for_cpu(host->dma_dev,
> - host->data_dma, sizeof(*host->data),
> - DMA_BIDIRECTIONAL);
> - }
> + status = mmc_spi_dma_alloc(host);
> + if (status)
> + goto fail_dma;
>
> /* setup message for status/busy readback */
> spi_message_init(&host->readback);
> @@ -1458,20 +1490,12 @@ static int mmc_spi_probe(struct spi_device *spi)
> fail_add_host:
> mmc_remove_host(mmc);
> fail_glue_init:
> - if (host->dma_dev)
> - dma_unmap_single(host->dma_dev, host->data_dma,
> - sizeof(*host->data), DMA_BIDIRECTIONAL);
> -fail_data_dma:
> - if (host->dma_dev)
> - dma_unmap_single(host->dma_dev, host->ones_dma,
> - MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
> -fail_ones_dma:
> + mmc_spi_dma_free(host);
> +fail_dma:
> kfree(host->data);
> -
> fail_nobuf1:
> mmc_free_host(mmc);
> mmc_spi_put_pdata(spi);
> -
> nomem:
> kfree(ones);
> return status;
> @@ -1489,13 +1513,7 @@ static int mmc_spi_remove(struct spi_device *spi)
>
> mmc_remove_host(mmc);
>
> - if (host->dma_dev) {
> - dma_unmap_single(host->dma_dev, host->ones_dma,
> - MMC_SPI_BLOCKSIZE, DMA_TO_DEVICE);
> - dma_unmap_single(host->dma_dev, host->data_dma,
> - sizeof(*host->data), DMA_BIDIRECTIONAL);
> - }
> -
> + mmc_spi_dma_free(host);
> kfree(host->data);
> kfree(host->ones);
>
> --
> 2.25.1
A change to Kconfig is also needed to remove the HAS_DMA dependency.
Rich