| Message ID | 20200831131636.51502-1-ulf.hansson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: mmc_spi: Don't use dma mappings unless CONFIG_HAS_DMA is set | expand |
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 <dalias@libc.org> > Suggested-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> > --- > 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
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);
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 <dalias@libc.org> Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org> --- drivers/mmc/host/mmc_spi.c | 86 +++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 34 deletions(-)