[v4,3/7,RFC] arm/arm64: introduce is_dma_coherent

Commit Message

Stefano Stabellini Oct. 25, 2014, 1:29 p.m. UTC

On Fri, 24 Oct 2014, Stefano Stabellini wrote:
> On Fri, 24 Oct 2014, Stefano Stabellini wrote:
> > On Fri, 24 Oct 2014, Catalin Marinas wrote:
> > > > However given the timing constraints I hope you would be OK with the
> > > > suboptimal solution for now and create a common is_dma_coherent function
> > > > in 3.19?
> > > 
> > > If you want to push something for 3.18, you could have a temporary
> > > solution but I would prefer a bool or something in the dev_archdata
> > > structure. Another untested patch:
> > > 
> > > diff --git a/arch/arm64/include/asm/device.h b/arch/arm64/include/asm/device.h
> > > index cf98b362094b..243ef256b8c9 100644
> > > --- a/arch/arm64/include/asm/device.h
> > > +++ b/arch/arm64/include/asm/device.h
> > > @@ -21,6 +21,7 @@ struct dev_archdata {
> > >  #ifdef CONFIG_IOMMU_API
> > >  	void *iommu;			/* private IOMMU data */
> > >  #endif
> > > +	bool dma_coherent;
> > >  };
> > >  
> > >  struct pdev_archdata {
> > > diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
> > > index adeae3f6f0fc..b6bc4c268878 100644
> > > --- a/arch/arm64/include/asm/dma-mapping.h
> > > +++ b/arch/arm64/include/asm/dma-mapping.h
> > > @@ -54,11 +54,17 @@ static inline void set_dma_ops(struct device *dev, struct dma_map_ops *ops)
> > >  
> > >  static inline int set_arch_dma_coherent_ops(struct device *dev)
> > >  {
> > > +	dev->dev_archdata.dma_coherent = true;
> > >  	set_dma_ops(dev, &coherent_swiotlb_dma_ops);
> > >  	return 0;
> > >  }
> > >  #define set_arch_dma_coherent_ops	set_arch_dma_coherent_ops
> > >  
> > > +static inline int is_device_dma_coherent(struct device *dev)
> > > +{
> > > +	return dev->dev_archdata.dma_coherent;
> > > +}
> > > +
> > >  #include <asm-generic/dma-mapping-common.h>
> > >  
> > >  static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr)
> > > 
> > > 
> > > This way you don't have to test for swiotlb vs iommu ops (we don't have
> > > the latter yet on arm64 but they are coming).
 
Your suggestions and looking more at the code gave me another idea, that
I think is clean and at the same time suitable for 3.18.
What do you think of the following? It is simple, self-contained and
doesn't need a new flag in struct device.

Comments

Ian Campbell Oct. 25, 2014, 3:40 p.m. UTC | #1

On Sat, 2014-10-25 at 14:29 +0100, Stefano Stabellini wrote:
>  
> Your suggestions and looking more at the code gave me another idea, that
> I think is clean and at the same time suitable for 3.18.
> What do you think of the following? It is simple, self-contained and
> doesn't need a new flag in struct device.

of_dma_is_coherent looks to be quite expensive though (walks up the
Device Tree doing strcmps on each property of each node until it finds
the one it is looking for.

Ian.

Stefano Stabellini Oct. 25, 2014, 4:15 p.m. UTC | #2

On Sat, 25 Oct 2014, Ian Campbell wrote:
> On Sat, 2014-10-25 at 14:29 +0100, Stefano Stabellini wrote:
> >  
> > Your suggestions and looking more at the code gave me another idea, that
> > I think is clean and at the same time suitable for 3.18.
> > What do you think of the following? It is simple, self-contained and
> > doesn't need a new flag in struct device.
> 
> of_dma_is_coherent looks to be quite expensive though (walks up the
> Device Tree doing strcmps on each property of each node until it finds
> the one it is looking for.

It takes spin_locks too!
Too bad, I think I'll have to ditch it. In that case I'l try the new
flag in struct device approach.

David Vrabel Oct. 27, 2014, 11:02 a.m. UTC | #3

On 25/10/14 17:15, Stefano Stabellini wrote:
> On Sat, 25 Oct 2014, Ian Campbell wrote:
>> On Sat, 2014-10-25 at 14:29 +0100, Stefano Stabellini wrote:
>>>  
>>> Your suggestions and looking more at the code gave me another idea, that
>>> I think is clean and at the same time suitable for 3.18.
>>> What do you think of the following? It is simple, self-contained and
>>> doesn't need a new flag in struct device.
>>
>> of_dma_is_coherent looks to be quite expensive though (walks up the
>> Device Tree doing strcmps on each property of each node until it finds
>> the one it is looking for.
> 
> It takes spin_locks too!
> Too bad, I think I'll have to ditch it. In that case I'l try the new
> flag in struct device approach.

If you're having to make changes to struct device, this is looking like
a series for 3.19 (not 3.18).

David

Stefano Stabellini Oct. 27, 2014, 3:22 p.m. UTC | #4

On Mon, 27 Oct 2014, David Vrabel wrote:
> On 25/10/14 17:15, Stefano Stabellini wrote:
> > On Sat, 25 Oct 2014, Ian Campbell wrote:
> >> On Sat, 2014-10-25 at 14:29 +0100, Stefano Stabellini wrote:
> >>>  
> >>> Your suggestions and looking more at the code gave me another idea, that
> >>> I think is clean and at the same time suitable for 3.18.
> >>> What do you think of the following? It is simple, self-contained and
> >>> doesn't need a new flag in struct device.
> >>
> >> of_dma_is_coherent looks to be quite expensive though (walks up the
> >> Device Tree doing strcmps on each property of each node until it finds
> >> the one it is looking for.
> > 
> > It takes spin_locks too!
> > Too bad, I think I'll have to ditch it. In that case I'l try the new
> > flag in struct device approach.
> 
> If you're having to make changes to struct device, this is looking like
> a series for 3.19 (not 3.18).

After looking more into it, I went for adding the flag to dev_archdata
under arm and arm64. Let's see what the maintainers say.

Patch

diff --git a/arch/arm/xen/mm32.c b/arch/arm/xen/mm32.c
index 6153d61..2b259f1 100644
--- a/arch/arm/xen/mm32.c
+++ b/arch/arm/xen/mm32.c
@@ -2,10 +2,16 @@ 
 #include <linux/dma-mapping.h>
 #include <linux/gfp.h>
 #include <linux/highmem.h>
+#include <linux/of_address.h>
 
 #include <xen/features.h>
 
 
+static inline bool is_dma_coherent(struct device *dev)
+{
+	return of_dma_is_coherent(dev->of_node);
+}
+
 /* functions called by SWIOTLB */
 
 static void dma_cache_maint(dma_addr_t handle, unsigned long offset,