| Message ID | 30da53ebdf43b712da790fd2ae0f0040f71762b8.1706948717.git.andrea.porta@suse.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for BCM2712 DMA engine | expand |
On 2024-02-04 6:59 am, Andrea della Porta wrote: > From: Phil Elwell <phil@raspberrypi.com> > > Slave addresses for DMA are meant to be supplied as physical addresses > (contrary to what struct snd_dmaengine_dai_dma_data does). It is up to > the DMA controller driver to perform the translation based on its own > view of the world, as described in Device Tree. > > Now that the Pi Device Trees have the correct peripheral mappings, > replace the hacky address munging with phys_to_dma(). > > Signed-off-by: Phil Elwell <phil@raspberrypi.com> > --- > drivers/dma/bcm2835-dma.c | 23 +++++------------------ > 1 file changed, 5 insertions(+), 18 deletions(-) > > diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c > index 237dcdb8d726..077812eda609 100644 > --- a/drivers/dma/bcm2835-dma.c > +++ b/drivers/dma/bcm2835-dma.c > @@ -18,6 +18,7 @@ > * Copyright 2012 Marvell International Ltd. > */ > #include <linux/dmaengine.h> > +#include <linux/dma-direct.h> Please read the comment at the top of that file; this driver is definitely not a DMA API implementation, and should not be including it. > #include <linux/dma-mapping.h> > #include <linux/dmapool.h> > #include <linux/err.h> > @@ -980,22 +981,12 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_slave_sg( > if (direction == DMA_DEV_TO_MEM) { > if (c->cfg.src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > return NULL; > - src = c->cfg.src_addr; > - /* > - * One would think it ought to be possible to get the physical > - * to dma address mapping information from the dma-ranges DT > - * property, but I've not found a way yet that doesn't involve > - * open-coding the whole thing. > - */ > - if (c->is_40bit_channel) > - src |= 0x400000000ull; > + src = phys_to_dma(chan->device->dev, c->cfg.src_addr); FWIW I'd argue that abusing DMA API internals like this is even more hacky than bypassing it entirely. The appropriate public API for setting up the device end of a transfer is dma_map_resource(). Now, it *is* the case currently that the dma-direct implementation of that does not take dma_range_map into account, but that's already an open question: https://lore.kernel.org/linux-iommu/20220610080802.11147-1-Sergey.Semin@baikalelectronics.ru/ Thanks, Robin. > info |= BCM2835_DMA_S_DREQ | BCM2835_DMA_D_INC; > } else { > if (c->cfg.dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > return NULL; > - dst = c->cfg.dst_addr; > - if (c->is_40bit_channel) > - dst |= 0x400000000ull; > + dst = phys_to_dma(chan->device->dev, c->cfg.dst_addr); > info |= BCM2835_DMA_D_DREQ | BCM2835_DMA_S_INC; > } > > @@ -1064,17 +1055,13 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic( > if (direction == DMA_DEV_TO_MEM) { > if (c->cfg.src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > return NULL; > - src = c->cfg.src_addr; > - if (c->is_40bit_channel) > - src |= 0x400000000ull; > + src = phys_to_dma(chan->device->dev, c->cfg.src_addr); > dst = buf_addr; > info |= BCM2835_DMA_S_DREQ | BCM2835_DMA_D_INC; > } else { > if (c->cfg.dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) > return NULL; > - dst = c->cfg.dst_addr; > - if (c->is_40bit_channel) > - dst |= 0x400000000ull; > + dst = phys_to_dma(chan->device->dev, c->cfg.dst_addr); > src = buf_addr; > info |= BCM2835_DMA_D_DREQ | BCM2835_DMA_S_INC; >
diff --git a/drivers/dma/bcm2835-dma.c b/drivers/dma/bcm2835-dma.c index 237dcdb8d726..077812eda609 100644 --- a/drivers/dma/bcm2835-dma.c +++ b/drivers/dma/bcm2835-dma.c @@ -18,6 +18,7 @@ * Copyright 2012 Marvell International Ltd. */ #include <linux/dmaengine.h> +#include <linux/dma-direct.h> #include <linux/dma-mapping.h> #include <linux/dmapool.h> #include <linux/err.h> @@ -980,22 +981,12 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_slave_sg( if (direction == DMA_DEV_TO_MEM) { if (c->cfg.src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return NULL; - src = c->cfg.src_addr; - /* - * One would think it ought to be possible to get the physical - * to dma address mapping information from the dma-ranges DT - * property, but I've not found a way yet that doesn't involve - * open-coding the whole thing. - */ - if (c->is_40bit_channel) - src |= 0x400000000ull; + src = phys_to_dma(chan->device->dev, c->cfg.src_addr); info |= BCM2835_DMA_S_DREQ | BCM2835_DMA_D_INC; } else { if (c->cfg.dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return NULL; - dst = c->cfg.dst_addr; - if (c->is_40bit_channel) - dst |= 0x400000000ull; + dst = phys_to_dma(chan->device->dev, c->cfg.dst_addr); info |= BCM2835_DMA_D_DREQ | BCM2835_DMA_S_INC; } @@ -1064,17 +1055,13 @@ static struct dma_async_tx_descriptor *bcm2835_dma_prep_dma_cyclic( if (direction == DMA_DEV_TO_MEM) { if (c->cfg.src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return NULL; - src = c->cfg.src_addr; - if (c->is_40bit_channel) - src |= 0x400000000ull; + src = phys_to_dma(chan->device->dev, c->cfg.src_addr); dst = buf_addr; info |= BCM2835_DMA_S_DREQ | BCM2835_DMA_D_INC; } else { if (c->cfg.dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES) return NULL; - dst = c->cfg.dst_addr; - if (c->is_40bit_channel) - dst |= 0x400000000ull; + dst = phys_to_dma(chan->device->dev, c->cfg.dst_addr); src = buf_addr; info |= BCM2835_DMA_D_DREQ | BCM2835_DMA_S_INC;