(Again with Linus' e-mail address fixed, sorry for the noise)
On Thursday 14 January 2016 01:13:20 Laurent Pinchart wrote:
> Hi Vinod,
>
> (CC'ing Linus as he's mentioned)
>
> On Wednesday 13 January 2016 14:55:50 Niklas Söderlund wrote:
> > * Vinod Koul <vinod.koul@xxxxxxxxx> [2016-01-13 19:06:01 +0530]:
> >> On Mon, Jan 11, 2016 at 03:17:46AM +0100, Niklas Söderlund wrote:
> >>> Enable slave transfers to devices behind IPMMU:s by mapping the slave
> >>> addresses using the dma-mapping API.
> >>>
> >>> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> >>> ---
> >>>
> >>> drivers/dma/sh/rcar-dmac.c | 64 +++++++++++++++++++++++++++++++++++---
> >>> 1 file changed, 60 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> >>> index 7820d07..da94809 100644
> >>> --- a/drivers/dma/sh/rcar-dmac.c
> >>> +++ b/drivers/dma/sh/rcar-dmac.c
> >>> @@ -13,6 +13,7 @@
> >>> #include <linux/dma-mapping.h>
> >>> #include <linux/dmaengine.h>
> >>> #include <linux/interrupt.h>
> >>> +#include <linux/iommu.h>
> >>> #include <linux/list.h>
> >>> #include <linux/module.h>
> >>> #include <linux/mutex.h>
> >>> @@ -1101,6 +1102,24 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *chan,
> >>> dma_addr_t buf_addr,
> >>> return desc;
> >>> }
> >>>
> >>> +static dma_addr_t __rcar_dmac_dma_map(struct dma_chan *chan,
> >>> phys_addr_t addr,
> >>> + size_t size, enum dma_data_direction dir)
> >>> +{
> >>> + struct rcar_dmac_chan *rchan = to_rcar_dmac_chan(chan);
> >>> + struct page *page = phys_to_page(addr);
> >>> + size_t offset = addr - page_to_phys(page);
> >>> + dma_addr_t map = dma_map_page(chan->device->dev, page, offset, size,
> >>> + dir);
> >>
> >> Hmmmm, dmaengine APIs for slave cases expect that client has already
> >> ammped and provided an address which the dmaengine understands. So doing
> >> this in driver here does not sound good to me
> >
> > It was my understanding that clients do not do this mapping and in fact
> > are expected not to. Is this not what Linus Walleij is trying to address
> > in '[PATCH] dmaengine: use phys_addr_t for slave configuration'?
>
> There's a problem somewhere and we need to fix it. Clients currently pass
> physical addresses and the DMA engine API expects a DMA address. There's
> only two ways to fix that, either modify the API to expect a phys_addr_t,
> or modify the clients to provide a dma_addr_t.
>
> The struct device used to map buffer through the DMA mapping API needs to be
> the DMA engine struct device, not the client struct device. As the client
> is not expected to have access to the DMA engine device I would argue that
> DMA engines should perform the mapping and the API should take a
> phys_addr_t.
>
> Vinod, unless you have reasons to do it otherwise, can we get your ack on
> this approach and start hammering at the code ? The problem has remained
> known and unfixed for too long, we need to move on.
>
> >>> On Fri, Apr 26, 2013 at 11:06 AM, Linus Walleij wrote:
> >>>> The documentation already says these are physical addresses, and
> >>>> we have concluded that any translation into the DMA address space
> >>>> needs to reside in the dmaengine driver, so change the type of
> >>>> the passed arguments
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
