From mboxrd@z Thu Jan 1 00:00:00 1970 From: Laurent Pinchart Date: Wed, 13 Jan 2016 23:15:37 +0000 Subject: Re: [PATCH 1/2] dmaengine: rcar-dmac: add iommu support for slave transfers Message-Id: <1647514.ouZGeHZKGU@avalon> List-Id: References: <1452478667-30966-2-git-send-email-niklas.soderlund+renesas@ragnatech.se> In-Reply-To: <1452478667-30966-2-git-send-email-niklas.soderlund+renesas@ragnatech.se> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: linux-sh@vger.kernel.org (Again with Linus' e-mail address fixed, sorry for the noise) On Thursday 14 January 2016 01:13:20 Laurent Pinchart wrote: > Hi Vinod, >=20 > (CC'ing Linus as he's mentioned) >=20 > On Wednesday 13 January 2016 14:55:50 Niklas S=F6derlund wrote: > > * Vinod Koul [2016-01-13 19:06:01 +0530]: > >> On Mon, Jan 11, 2016 at 03:17:46AM +0100, Niklas S=F6derlund wrote: > >>> Enable slave transfers to devices behind IPMMU:s by mapping the slave > >>> addresses using the dma-mapping API. > >>>=20 > >>> Signed-off-by: Niklas S=F6derlund > >>> --- > >>>=20 > >>> drivers/dma/sh/rcar-dmac.c | 64 +++++++++++++++++++++++++++++++++++-= -- > >>> 1 file changed, 60 insertions(+), 4 deletions(-) > >>>=20 > >>> 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 > >>> #include > >>> #include > >>> +#include > >>> #include > >>> #include > >>> #include > >>> @@ -1101,6 +1102,24 @@ rcar_dmac_prep_dma_cyclic(struct dma_chan *cha= n, > >>> dma_addr_t buf_addr, > >>> return desc; > >>> } > >>>=20 > >>> +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 =3D to_rcar_dmac_chan(chan); > >>> + struct page *page =3D phys_to_page(addr); > >>> + size_t offset =3D addr - page_to_phys(page); > >>> + dma_addr_t map =3D dma_map_page(chan->device->dev, page, offset, si= ze, > >>> + dir); > >>=20 > >> Hmmmm, dmaengine APIs for slave cases expect that client has already > >> ammped and provided an address which the dmaengine understands. So doi= ng > >> this in driver here does not sound good to me > >=20 > > 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'? >=20 > 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. >=20 > 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. >=20 > 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. >=20 > >>> 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 --=20 Regards, Laurent Pinchart