| Message ID | 1456068046-12500-1-git-send-email-vinod.koul@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Vinod, On Sun, Feb 21, 2016 at 4:20 PM, Vinod Koul <vinod.koul@intel.com> wrote: > The slave dmaengine semantics required the client to map dma > addresses and pass DMA address to dmaengine drivers. While this > was a convenient notion coming from generic dma offload cases > where dmaengines are interchangeable and client is not aware of > which engine to map to. > > But in case of slave, we know the dmaengine and always use a > specific one. Further the IOMMU cases can lead to failure of this > notion, so make this as physical address and now dmaengine driver > will do the required mapping. Thanks a lot! > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> You've dropped a few ;-) Original-patch-acked-by: Lee Jones <lee.jones@linaro.org> Original-patch-acked-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > include/linux/dmaengine.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 16a1cad30c33..d85ecd20af50 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > */ > struct dma_slave_config { > enum dma_transfer_direction direction; > - dma_addr_t src_addr; > - dma_addr_t dst_addr; > + phys_addr_t src_addr; > + phys_addr_t dst_addr; > enum dma_slave_buswidth src_addr_width; > enum dma_slave_buswidth dst_addr_width; > u32 src_maxburst; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Vinod, On 2016-02-21 20:50:46 +0530, Vinod Koul wrote: > The slave dmaengine semantics required the client to map dma > addresses and pass DMA address to dmaengine drivers. While this > was a convenient notion coming from generic dma offload cases > where dmaengines are interchangeable and client is not aware of > which engine to map to. > > But in case of slave, we know the dmaengine and always use a > specific one. Further the IOMMU cases can lead to failure of this > notion, so make this as physical address and now dmaengine driver > will do the required mapping. Thanks! I have tested this patch with my IOMMU series and it works nicely. > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > include/linux/dmaengine.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 16a1cad30c33..d85ecd20af50 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > */ > struct dma_slave_config { > enum dma_transfer_direction direction; > - dma_addr_t src_addr; > - dma_addr_t dst_addr; > + phys_addr_t src_addr; > + phys_addr_t dst_addr; > enum dma_slave_buswidth src_addr_width; > enum dma_slave_buswidth dst_addr_width; > u32 src_maxburst; -- Regards, Niklas Söderlund
On Mon, Feb 22, 2016 at 09:37:10AM +0100, Geert Uytterhoeven wrote: > Hi Vinod, > > On Sun, Feb 21, 2016 at 4:20 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > The slave dmaengine semantics required the client to map dma > > addresses and pass DMA address to dmaengine drivers. While this > > was a convenient notion coming from generic dma offload cases > > where dmaengines are interchangeable and client is not aware of > > which engine to map to. > > > > But in case of slave, we know the dmaengine and always use a > > specific one. Further the IOMMU cases can lead to failure of this > > notion, so make this as physical address and now dmaengine driver > > will do the required mapping. > > Thanks a lot! Yes, thanks! > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > You've dropped a few ;-) > > Original-patch-acked-by: Lee Jones <lee.jones@linaro.org> > Original-patch-acked-by: Arnd Bergmann <arnd@arndb.de> I'd vote for dropping the "Original-patch-" prefix and keep the original SoB and Acks because the content of the patch is still the same. And while the new commit message is a lot more precise, it is also in the same spirit as the old one. That being said: Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> I tested it on my Lager board on top of my sdhi-uhs testing branch and used DMA with SD cards and for I2C transfers. No regressions seen. Also no build warnings. Regards, Wolfram
Hi Vinod, On Sunday 21 February 2016 20:50:46 Vinod Koul wrote: > The slave dmaengine semantics required the client to map dma > addresses and pass DMA address to dmaengine drivers. While this s/While this/This/ ? > was a convenient notion coming from generic dma offload cases > where dmaengines are interchangeable and client is not aware of > which engine to map to. > > But in case of slave, we know the dmaengine and always use a > specific one. Further the IOMMU cases can lead to failure of this > notion, so make this as physical address and now dmaengine driver > will do the required mapping. You could also add "This finally bring the code in sync with the documentation.". > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Vinod Koul <vinod.koul@intel.com> It took a long time but we finally got there :-) Thank you. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/linux/dmaengine.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index 16a1cad30c33..d85ecd20af50 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > */ > struct dma_slave_config { > enum dma_transfer_direction direction; > - dma_addr_t src_addr; > - dma_addr_t dst_addr; > + phys_addr_t src_addr; > + phys_addr_t dst_addr; > enum dma_slave_buswidth src_addr_width; > enum dma_slave_buswidth dst_addr_width; > u32 src_maxburst;
> > But in case of slave, we know the dmaengine and always use a > > specific one. Further the IOMMU cases can lead to failure of this > > notion, so make this as physical address and now dmaengine driver > > will do the required mapping. > > You could also add "This finally bring the code in sync with the > documentation.". s/finally bring/also brings/ ?
On Monday 22 February 2016 13:31:04 Wolfram Sang wrote: > > > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > > > You've dropped a few > > > > Original-patch-acked-by: Lee Jones <lee.jones@linaro.org> > > Original-patch-acked-by: Arnd Bergmann <arnd@arndb.de> > > I'd vote for dropping the "Original-patch-" prefix and keep the original > SoB and Acks because the content of the patch is still the same. And > while the new commit message is a lot more precise, it is also in the > same spirit as the old one. > > That being said: > > Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > I tested it on my Lager board on top of my sdhi-uhs testing branch and > used DMA with SD cards and for I2C transfers. No regressions seen. Also > no build warnings. > Acked-by: Arnd Bergmann <arnd@arndb.de>
On Mon, Feb 22, 2016 at 05:02:07PM +0200, Laurent Pinchart wrote: > Hi Vinod, > > On Sunday 21 February 2016 20:50:46 Vinod Koul wrote: > > The slave dmaengine semantics required the client to map dma > > addresses and pass DMA address to dmaengine drivers. While this > > s/While this/This/ ? > > > was a convenient notion coming from generic dma offload cases > > where dmaengines are interchangeable and client is not aware of > > which engine to map to. > > > > But in case of slave, we know the dmaengine and always use a > > specific one. Further the IOMMU cases can lead to failure of this > > notion, so make this as physical address and now dmaengine driver > > will do the required mapping. > > You could also add "This finally bring the code in sync with the > documentation.". > > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > It took a long time but we finally got there :-) Thank you. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Updated thanks. > > > --- > > include/linux/dmaengine.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index 16a1cad30c33..d85ecd20af50 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > > */ > > struct dma_slave_config { > > enum dma_transfer_direction direction; > > - dma_addr_t src_addr; > > - dma_addr_t dst_addr; > > + phys_addr_t src_addr; > > + phys_addr_t dst_addr; > > enum dma_slave_buswidth src_addr_width; > > enum dma_slave_buswidth dst_addr_width; > > u32 src_maxburst; > > -- > Regards, > > Laurent Pinchart >
On Mon, Feb 22, 2016 at 09:37:10AM +0100, Geert Uytterhoeven wrote: > Hi Vinod, > > On Sun, Feb 21, 2016 at 4:20 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > The slave dmaengine semantics required the client to map dma > > addresses and pass DMA address to dmaengine drivers. While this > > was a convenient notion coming from generic dma offload cases > > where dmaengines are interchangeable and client is not aware of > > which engine to map to. > > > > But in case of slave, we know the dmaengine and always use a > > specific one. Further the IOMMU cases can lead to failure of this > > notion, so make this as physical address and now dmaengine driver > > will do the required mapping. > > Thanks a lot! > > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > You've dropped a few ;-) > > Original-patch-acked-by: Lee Jones <lee.jones@linaro.org> > Original-patch-acked-by: Arnd Bergmann <arnd@arndb.de> I never collected those :) Added now, But will use Arnd latest ack... > > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks, any chance you could test on your boards? > > > --- > > include/linux/dmaengine.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index 16a1cad30c33..d85ecd20af50 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > > */ > > struct dma_slave_config { > > enum dma_transfer_direction direction; > > - dma_addr_t src_addr; > > - dma_addr_t dst_addr; > > + phys_addr_t src_addr; > > + phys_addr_t dst_addr; > > enum dma_slave_buswidth src_addr_width; > > enum dma_slave_buswidth dst_addr_width; > > u32 src_maxburst; > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Mon, Feb 22, 2016 at 10:49:08AM +0100, Niklas Söderlund wrote: > Hi Vinod, > > On 2016-02-21 20:50:46 +0530, Vinod Koul wrote: > > The slave dmaengine semantics required the client to map dma > > addresses and pass DMA address to dmaengine drivers. While this > > was a convenient notion coming from generic dma offload cases > > where dmaengines are interchangeable and client is not aware of > > which engine to map to. > > > > But in case of slave, we know the dmaengine and always use a > > specific one. Further the IOMMU cases can lead to failure of this > > notion, so make this as physical address and now dmaengine driver > > will do the required mapping. > > Thanks! I have tested this patch with my IOMMU series and it works > nicely. > > > > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com> > > Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Thanks :) > > > --- > > include/linux/dmaengine.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > > index 16a1cad30c33..d85ecd20af50 100644 > > --- a/include/linux/dmaengine.h > > +++ b/include/linux/dmaengine.h > > @@ -357,8 +357,8 @@ enum dma_slave_buswidth { > > */ > > struct dma_slave_config { > > enum dma_transfer_direction direction; > > - dma_addr_t src_addr; > > - dma_addr_t dst_addr; > > + phys_addr_t src_addr; > > + phys_addr_t dst_addr; > > enum dma_slave_buswidth src_addr_width; > > enum dma_slave_buswidth dst_addr_width; > > u32 src_maxburst; > > -- > Regards, > Niklas Söderlund > -- > To unsubscribe from this list: send the line "unsubscribe dmaengine" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Feb 22, 2016 at 01:31:04PM +0100, Wolfram Sang wrote: > On Mon, Feb 22, 2016 at 09:37:10AM +0100, Geert Uytterhoeven wrote: > > Hi Vinod, > > > > On Sun, Feb 21, 2016 at 4:20 PM, Vinod Koul <vinod.koul@intel.com> wrote: > > > The slave dmaengine semantics required the client to map dma > > > addresses and pass DMA address to dmaengine drivers. While this > > > was a convenient notion coming from generic dma offload cases > > > where dmaengines are interchangeable and client is not aware of > > > which engine to map to. > > > > > > But in case of slave, we know the dmaengine and always use a > > > specific one. Further the IOMMU cases can lead to failure of this > > > notion, so make this as physical address and now dmaengine driver > > > will do the required mapping. > > > > Thanks a lot! > > Yes, thanks! > > > > Original-patch-by: Linus Walleij <linus.walleij@linaro.org> > > > > You've dropped a few ;-) > > > > Original-patch-acked-by: Lee Jones <lee.jones@linaro.org> > > Original-patch-acked-by: Arnd Bergmann <arnd@arndb.de> > > I'd vote for dropping the "Original-patch-" prefix and keep the original > SoB and Acks because the content of the patch is still the same. And > while the new commit message is a lot more precise, it is also in the > same spirit as the old one. > > That being said: > > Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > I tested it on my Lager board on top of my sdhi-uhs testing branch and > used DMA with SD cards and for I2C transfers. No regressions seen. Also > no build warnings. I did check build warnings and pushed this out, Feng's bot didn't complain so was more concerned about testing, so thanks for getting that done.
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index 16a1cad30c33..d85ecd20af50 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -357,8 +357,8 @@ enum dma_slave_buswidth { */ struct dma_slave_config { enum dma_transfer_direction direction; - dma_addr_t src_addr; - dma_addr_t dst_addr; + phys_addr_t src_addr; + phys_addr_t dst_addr; enum dma_slave_buswidth src_addr_width; enum dma_slave_buswidth dst_addr_width; u32 src_maxburst;
The slave dmaengine semantics required the client to map dma addresses and pass DMA address to dmaengine drivers. While this was a convenient notion coming from generic dma offload cases where dmaengines are interchangeable and client is not aware of which engine to map to. But in case of slave, we know the dmaengine and always use a specific one. Further the IOMMU cases can lead to failure of this notion, so make this as physical address and now dmaengine driver will do the required mapping. Original-patch-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Vinod Koul <vinod.koul@intel.com> --- include/linux/dmaengine.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)