[PATCH v2] staging: ion: Add a default struct device for cma heap

[PATCH v2] staging: ion: Add a default struct device for cma heap

[Feng Tang]

When trying to use several cma heaps on our platforms,
we met a memory issue due to that the several cma_heaps
are sharing the same "struct device *".

As in current code base, the normal cma heap creating
process is, one platform device is created during boot,
and it will sequentially create cma heaps (usually passing
its own struct device * as a parameter)

For the multiple cma heaps case, there will be one "struct
cma" created for each cma heap, and this "struct cma *" is
saved in dev->cma_area. So the single platform device can't
meet the requirement here.

This patch adds one default device for each cma heap to avoid
sharing the same "struct device", thus fix the issue. And it
doesn't break existing code by only using that default device
when no "struct device *" is passed in.

Also, since the cma framework has been cleaned up recently,
this patch also adds a platform data member to pass the
"struct cma*" to ion_cma_heap_create().

Signed-off-by: Feng Tang <feng.tang@intel.com>
[From CMA’s point of view: ]
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/staging/android/ion/ion.h          |    4 ++++
 drivers/staging/android/ion/ion_cma_heap.c |   20 +++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index 443db84..11336df 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -44,6 +44,9 @@ struct ion_buffer;
  * @base:	base address of heap in physical memory if applicable
  * @size:	size of the heap in bytes if applicable
  * @align:	required alignment in physical memory if applicable
+ * @cma:	when creating CMA heap, platform device should better also
+ *		pass the "struct cma *" info, so that the cma buffer request
+ *		know where to go for the buffer
  * @priv:	private info passed from the board file
  *
  * Provided by the board file.
@@ -55,6 +58,7 @@ struct ion_platform_heap {
 	ion_phys_addr_t base;
 	size_t size;
 	ion_phys_addr_t align;
+	struct cma *cma;
 	void *priv;
 };
 
diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
index f4211f1..27f218a 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -29,6 +29,7 @@
 struct ion_cma_heap {
 	struct ion_heap heap;
 	struct device *dev;
+	struct device default_dma_dev;
 };
 
 #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
@@ -180,9 +181,22 @@ struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data)
 		return ERR_PTR(-ENOMEM);
 
 	cma_heap->heap.ops = &ion_cma_ops;
-	/* get device from private heaps data, later it will be
-	 * used to make the link with reserved CMA memory */
-	cma_heap->dev = data->priv;
+
+	/*
+	 * data->priv for cma heap is currently supposed to point
+	 * to a "struct device *"
+	 */
+	if (data->priv) {
+		cma_heap->dev = data->priv;
+	} else {
+		cma_heap->dev = &cma_heap->default_dma_dev;
+		cma_heap->dev->coherent_dma_mask = DMA_BIT_MASK(32);
+		cma_heap->dev->dma_mask = &cma_heap->dev->coherent_dma_mask;
+	}
+
+	if (data->cma)
+		dev_set_cma_area(cma_heap->dev, data->cma);
+
 	cma_heap->heap.type = ION_HEAP_TYPE_DMA;
 	return &cma_heap->heap;
 }
-- 
1.7.9.5

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Greg Kroah-Hartman]

On Fri, Aug 07, 2015 at 11:50:13AM +0800, Feng Tang wrote:
> When trying to use several cma heaps on our platforms,
> we met a memory issue due to that the several cma_heaps
> are sharing the same "struct device *".
> 
> As in current code base, the normal cma heap creating
> process is, one platform device is created during boot,
> and it will sequentially create cma heaps (usually passing
> its own struct device * as a parameter)
> 
> For the multiple cma heaps case, there will be one "struct
> cma" created for each cma heap, and this "struct cma *" is
> saved in dev->cma_area. So the single platform device can't
> meet the requirement here.
> 
> This patch adds one default device for each cma heap to avoid
> sharing the same "struct device", thus fix the issue. And it
> doesn't break existing code by only using that default device
> when no "struct device *" is passed in.
> 
> Also, since the cma framework has been cleaned up recently,
> this patch also adds a platform data member to pass the
> "struct cma*" to ion_cma_heap_create().
> 
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> [From CMA’s point of view: ]
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
>  drivers/staging/android/ion/ion.h          |    4 ++++
>  drivers/staging/android/ion/ion_cma_heap.c |   20 +++++++++++++++++---
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> index 443db84..11336df 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -44,6 +44,9 @@ struct ion_buffer;
>   * @base:	base address of heap in physical memory if applicable
>   * @size:	size of the heap in bytes if applicable
>   * @align:	required alignment in physical memory if applicable
> + * @cma:	when creating CMA heap, platform device should better also
> + *		pass the "struct cma *" info, so that the cma buffer request
> + *		know where to go for the buffer
>   * @priv:	private info passed from the board file
>   *
>   * Provided by the board file.
> @@ -55,6 +58,7 @@ struct ion_platform_heap {
>  	ion_phys_addr_t base;
>  	size_t size;
>  	ion_phys_addr_t align;
> +	struct cma *cma;
>  	void *priv;
>  };
>  
> diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
> index f4211f1..27f218a 100644
> --- a/drivers/staging/android/ion/ion_cma_heap.c
> +++ b/drivers/staging/android/ion/ion_cma_heap.c
> @@ -29,6 +29,7 @@
>  struct ion_cma_heap {
>  	struct ion_heap heap;
>  	struct device *dev;
> +	struct device default_dma_dev;

Why do we have 2 struct devices here?

>  };
>  
>  #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
> @@ -180,9 +181,22 @@ struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data)
>  		return ERR_PTR(-ENOMEM);
>  
>  	cma_heap->heap.ops = &ion_cma_ops;
> -	/* get device from private heaps data, later it will be
> -	 * used to make the link with reserved CMA memory */
> -	cma_heap->dev = data->priv;
> +
> +	/*
> +	 * data->priv for cma heap is currently supposed to point
> +	 * to a "struct device *"
> +	 */
> +	if (data->priv) {
> +		cma_heap->dev = data->priv;
> +	} else {
> +		cma_heap->dev = &cma_heap->default_dma_dev;

That's not good, it's not initialized, or if it is, what's the lifetime
rules for it now?

Why are you needing an extra struct device now?

this seems odd.

greg k-h

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Feng Tang]

Hi Greg,

Thanks for the review!

On Thu, Aug 06, 2015 at 09:54:04PM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 07, 2015 at 11:50:13AM +0800, Feng Tang wrote:
> > When trying to use several cma heaps on our platforms,
> > we met a memory issue due to that the several cma_heaps
> > are sharing the same "struct device *".
> > 
> > As in current code base, the normal cma heap creating
> > process is, one platform device is created during boot,
> > and it will sequentially create cma heaps (usually passing
> > its own struct device * as a parameter)
> > 
> > For the multiple cma heaps case, there will be one "struct
> > cma" created for each cma heap, and this "struct cma *" is
> > saved in dev->cma_area. So the single platform device can't
> > meet the requirement here.
> > 
> > This patch adds one default device for each cma heap to avoid
> > sharing the same "struct device", thus fix the issue. And it
> > doesn't break existing code by only using that default device
> > when no "struct device *" is passed in.
> > 
> > Also, since the cma framework has been cleaned up recently,
> > this patch also adds a platform data member to pass the
> > "struct cma*" to ion_cma_heap_create().
> > 
> > Signed-off-by: Feng Tang <feng.tang@intel.com>
> > [From CMA’s point of view: ]
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > ---
> >  drivers/staging/android/ion/ion.h          |    4 ++++
> >  drivers/staging/android/ion/ion_cma_heap.c |   20 +++++++++++++++++---
> >  2 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
> > index 443db84..11336df 100644
> > --- a/drivers/staging/android/ion/ion.h
> > +++ b/drivers/staging/android/ion/ion.h
> > @@ -44,6 +44,9 @@ struct ion_buffer;
> >   * @base:	base address of heap in physical memory if applicable
> >   * @size:	size of the heap in bytes if applicable
> >   * @align:	required alignment in physical memory if applicable
> > + * @cma:	when creating CMA heap, platform device should better also
> > + *		pass the "struct cma *" info, so that the cma buffer request
> > + *		know where to go for the buffer
> >   * @priv:	private info passed from the board file
> >   *
> >   * Provided by the board file.
> > @@ -55,6 +58,7 @@ struct ion_platform_heap {
> >  	ion_phys_addr_t base;
> >  	size_t size;
> >  	ion_phys_addr_t align;
> > +	struct cma *cma;
> >  	void *priv;
> >  };
> >  
> > diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c
> > index f4211f1..27f218a 100644
> > --- a/drivers/staging/android/ion/ion_cma_heap.c
> > +++ b/drivers/staging/android/ion/ion_cma_heap.c
> > @@ -29,6 +29,7 @@
> >  struct ion_cma_heap {
> >  	struct ion_heap heap;
> >  	struct device *dev;
> > +	struct device default_dma_dev;
> 
> Why do we have 2 struct devices here?

My bad, I haven't make it clear in git log

For cma heap, the ion_buffer creation will actually go to dma allocation
subsystem, for example
	dma_generic_alloc_coherent	(arch/x86/kernel/pci-dma.c)
		dma_alloc_from_contiguous(dev, count, get_order(size))
			cma_alloc(dev_get_cma_area(dev), count, align)
	
while the dev_get_cma_area(dev) == dev->cma_area in most of cma scenario.

so each struct cma_heap needs one "struct dev" to use it here, and its only
use is to simply proivde one dev->cma_area.

As in current code base, the normal cma heap creating process is, one
ion platform device is created during boot, and it will sequentially create
cma heaps (usually passing its own struct device * as a parameter)

On our customer's platform, they wanted to use multiple cma heaps, so
the only one ion device should not be shared to these cma heaps, as their
"dev->cma_area" is different.

My solution is to embed one by-default cma device inside the cma_heap for
this use case. Keeping the "struct device *dev" is simply to be compatible
with current existing ion platform driver, otherwise 

	struct ion_cma_heap {
		struct ion_heap heap;
		struct device default_dma_dev;
	}
removing the "dev" can also meet our needs.

> 
> >  };
> >  
> >  #define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
> > @@ -180,9 +181,22 @@ struct ion_heap *ion_cma_heap_create(struct ion_platform_heap *data)
> >  		return ERR_PTR(-ENOMEM);
> >  
> >  	cma_heap->heap.ops = &ion_cma_ops;
> > -	/* get device from private heaps data, later it will be
> > -	 * used to make the link with reserved CMA memory */
> > -	cma_heap->dev = data->priv;
> > +
> > +	/*
> > +	 * data->priv for cma heap is currently supposed to point
> > +	 * to a "struct device *"
> > +	 */
> > +	if (data->priv) {
> > +		cma_heap->dev = data->priv;
> > +	} else {
> > +		cma_heap->dev = &cma_heap->default_dma_dev;
> 
> That's not good, it's not initialized, or if it is, what's the lifetime
> rules for it now?

As I described above, the dummy struct device is only needed for
dma request, its lifetime is align with the cma_heap itself. 

I created it follow one example in arch/x86/kernel/pci-dma.c

	/* Dummy device used for NULL arguments (normally ISA). */
	struct device x86_dma_fallback_dev = {
		.init_name = "fallback device",
		.coherent_dma_mask = ISA_DMA_BIT_MASK,
		.dma_mask = &x86_dma_fallback_dev.coherent_dma_mask,
	};
	EXPORT_SYMBOL(x86_dma_fallback_dev);

Please let me know if you have any suggestions or concerns, thanks

- Feng

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Michal Nazarewicz]

On Fri, Aug 07 2015, Feng Tang wrote:
> As I described above, the dummy struct device is only needed for
> dma request, its lifetime is align with the cma_heap itself. 

Again, this is from perspective of someone who is unfamiliar with ION,
but perhaps a viable solution is to bypass DMA API and just call
cma_alloc directly?

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Feng Tang]

On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
> On Fri, Aug 07 2015, Feng Tang wrote:
> > As I described above, the dummy struct device is only needed for
> > dma request, its lifetime is align with the cma_heap itself. 
> 
> Again, this is from perspective of someone who is unfamiliar with ION,
> but perhaps a viable solution is to bypass DMA API and just call
> cma_alloc directly?

For ion cma heap, the buffer allocation func ion_cma_allocate() will
call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
implemented by each architeture(arm/m68k/x86 etc), and many Arch's
implementation doesn't use cma, but use alloc_pages() like APIs.
So I'm afraid we can't direcly call cma_alloc directly here.

Thanks,
Feng

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Greg Kroah-Hartman]

On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote:
> On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
> > On Fri, Aug 07 2015, Feng Tang wrote:
> > > As I described above, the dummy struct device is only needed for
> > > dma request, its lifetime is align with the cma_heap itself. 
> > 
> > Again, this is from perspective of someone who is unfamiliar with ION,
> > but perhaps a viable solution is to bypass DMA API and just call
> > cma_alloc directly?
> 
> For ion cma heap, the buffer allocation func ion_cma_allocate() will
> call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
> implemented by each architeture(arm/m68k/x86 etc), and many Arch's
> implementation doesn't use cma, but use alloc_pages() like APIs.
> So I'm afraid we can't direcly call cma_alloc directly here.

Ick.  But using a "fake" struct device here, for no real reason,
makes me very nervous that you are going to hit a codepath somewhere
that assumes this is a "real" struct device and tries to do something
with it (dev_printk(), look up what bus it is on, change the name of it,
etc.)  Trying to fake out the subsystem in this manner is a sign that
something is really wrong here.

Please either make this a real device, or fix up the api to not need
this type of thing.

thanks,

greg k-h

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Laura Abbott]

On 08/07/2015 11:05 AM, Greg Kroah-Hartman wrote:
> On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote:
>> On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
>>> On Fri, Aug 07 2015, Feng Tang wrote:
>>>> As I described above, the dummy struct device is only needed for
>>>> dma request, its lifetime is align with the cma_heap itself.
>>>
>>> Again, this is from perspective of someone who is unfamiliar with ION,
>>> but perhaps a viable solution is to bypass DMA API and just call
>>> cma_alloc directly?
>>
>> For ion cma heap, the buffer allocation func ion_cma_allocate() will
>> call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
>> implemented by each architeture(arm/m68k/x86 etc), and many Arch's
>> implementation doesn't use cma, but use alloc_pages() like APIs.
>> So I'm afraid we can't direcly call cma_alloc directly here.
>
> Ick.  But using a "fake" struct device here, for no real reason,
> makes me very nervous that you are going to hit a codepath somewhere
> that assumes this is a "real" struct device and tries to do something
> with it (dev_printk(), look up what bus it is on, change the name of it,
> etc.)  Trying to fake out the subsystem in this manner is a sign that
> something is really wrong here.
>
> Please either make this a real device, or fix up the api to not need
> this type of thing.
>

I think this issue represents one of the many current issues with Ion.
When the void * == struct dev was added, everything was working off of
board files. We now have devicetree which makes the device association
even more awkward to pull off. Every vendor out there is doing something
different right now so the assertion in the commit text about 'normal'
is not true; existing code has managed to work with the (not super great)
API.

There is going to be an Ion session at Plumbers in a few weeks. I'd like
to propose holding off on merging anything until after plumbers when
there can be some more discussion about what would be a reasonable API,
taking into consideration the points brought up in this patch series.

Thanks,
Laura

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Greg Kroah-Hartman]

On Fri, Aug 07, 2015 at 04:09:12PM -0700, Laura Abbott wrote:
> On 08/07/2015 11:05 AM, Greg Kroah-Hartman wrote:
> >On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote:
> >>On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
> >>>On Fri, Aug 07 2015, Feng Tang wrote:
> >>>>As I described above, the dummy struct device is only needed for
> >>>>dma request, its lifetime is align with the cma_heap itself.
> >>>
> >>>Again, this is from perspective of someone who is unfamiliar with ION,
> >>>but perhaps a viable solution is to bypass DMA API and just call
> >>>cma_alloc directly?
> >>
> >>For ion cma heap, the buffer allocation func ion_cma_allocate() will
> >>call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
> >>implemented by each architeture(arm/m68k/x86 etc), and many Arch's
> >>implementation doesn't use cma, but use alloc_pages() like APIs.
> >>So I'm afraid we can't direcly call cma_alloc directly here.
> >
> >Ick.  But using a "fake" struct device here, for no real reason,
> >makes me very nervous that you are going to hit a codepath somewhere
> >that assumes this is a "real" struct device and tries to do something
> >with it (dev_printk(), look up what bus it is on, change the name of it,
> >etc.)  Trying to fake out the subsystem in this manner is a sign that
> >something is really wrong here.
> >
> >Please either make this a real device, or fix up the api to not need
> >this type of thing.
> >
> 
> I think this issue represents one of the many current issues with Ion.
> When the void * == struct dev was added, everything was working off of
> board files. We now have devicetree which makes the device association
> even more awkward to pull off. Every vendor out there is doing something
> different right now so the assertion in the commit text about 'normal'
> is not true; existing code has managed to work with the (not super great)
> API.
> 
> There is going to be an Ion session at Plumbers in a few weeks. I'd like
> to propose holding off on merging anything until after plumbers when
> there can be some more discussion about what would be a reasonable API,
> taking into consideration the points brought up in this patch series.

Sounds like a good idea.  I'll be at that talk as well.

thanks,

greg k-h

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Feng Tang]

On Sat, Aug 08, 2015 at 03:18:59PM -0700, Greg Kroah-Hartman wrote:
> On Fri, Aug 07, 2015 at 04:09:12PM -0700, Laura Abbott wrote:
> > On 08/07/2015 11:05 AM, Greg Kroah-Hartman wrote:
> > >On Fri, Aug 07, 2015 at 11:50:04PM +0800, Feng Tang wrote:
> > >>On Fri, Aug 07, 2015 at 04:48:28PM +0200, Michal Nazarewicz wrote:
> > >>>On Fri, Aug 07 2015, Feng Tang wrote:
> > >>>>As I described above, the dummy struct device is only needed for
> > >>>>dma request, its lifetime is align with the cma_heap itself.
> > >>>
> > >>>Again, this is from perspective of someone who is unfamiliar with ION,
> > >>>but perhaps a viable solution is to bypass DMA API and just call
> > >>>cma_alloc directly?
> > >>
> > >>For ion cma heap, the buffer allocation func ion_cma_allocate() will
> > >>call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
> > >>implemented by each architeture(arm/m68k/x86 etc), and many Arch's
> > >>implementation doesn't use cma, but use alloc_pages() like APIs.
> > >>So I'm afraid we can't direcly call cma_alloc directly here.
> > >
> > >Ick.  But using a "fake" struct device here, for no real reason,
> > >makes me very nervous that you are going to hit a codepath somewhere
> > >that assumes this is a "real" struct device and tries to do something
> > >with it (dev_printk(), look up what bus it is on, change the name of it,
> > >etc.)  Trying to fake out the subsystem in this manner is a sign that
> > >something is really wrong here.
> > >
> > >Please either make this a real device, or fix up the api to not need
> > >this type of thing.
> > >
> > 
> > I think this issue represents one of the many current issues with Ion.
> > When the void * == struct dev was added, everything was working off of
> > board files. We now have devicetree which makes the device association
> > even more awkward to pull off. Every vendor out there is doing something
> > different right now so the assertion in the commit text about 'normal'
> > is not true; existing code has managed to work with the (not super great)
> > API.
> > 
> > There is going to be an Ion session at Plumbers in a few weeks. I'd like
> > to propose holding off on merging anything until after plumbers when
> > there can be some more discussion about what would be a reasonable API,
> > taking into consideration the points brought up in this patch series.
> 
> Sounds like a good idea.  I'll be at that talk as well.

Great! Hopefully this multiple cma heap support and other ion issue could
be addressed there.

Thanks,
Feng

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Michal Nazarewicz]

On Fri, Aug 07 2015, Feng Tang wrote:
> For ion cma heap, the buffer allocation func ion_cma_allocate() will
> call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
> implemented by each architeture(arm/m68k/x86 etc), and many Arch's
> implementation doesn't use cma, but use alloc_pages() like APIs.

So what you’re saying is that ‘ION CMA heap’ is a misnomer since it has
to do with CMA only if given architecture implements dma_alloc_coherent
using CMA.  It should rather be called ‘ION DMA coherent heap’.

This leads to realisation that the code should be oblivious to CMA areas
and never operate on them directly (e.g. never accept struct cma * or
use dev_set_cma_area).  In the current form, if architecture does not
use CMA, the whole dev_set_cma_area shenanigans are pointless.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

Re: [PATCH v2] staging: ion: Add a default struct device for cma heap

[Feng Tang]

On Sat, Aug 08, 2015 at 12:39:48PM +0200, Michal Nazarewicz wrote:
> On Fri, Aug 07 2015, Feng Tang wrote:
> > For ion cma heap, the buffer allocation func ion_cma_allocate() will
> > call dma_alloc_coherent(dev, ...). And dma_alloc_coherent() is
> > implemented by each architeture(arm/m68k/x86 etc), and many Arch's
> > implementation doesn't use cma, but use alloc_pages() like APIs.
> 
> So what you’re saying is that ‘ION CMA heap’ is a misnomer since it has
> to do with CMA only if given architecture implements dma_alloc_coherent
> using CMA.  It should rather be called ‘ION DMA coherent heap’.

> This leads to realisation that the code should be oblivious to CMA areas
> and never operate on them directly (e.g. never accept struct cma * or
> use dev_set_cma_area).  In the current form, if architecture does not
> use CMA, the whole dev_set_cma_area shenanigans are pointless.

I'm not very familiar with the history of ion cma code, my guess is
there are quite a few Arch's are using ion/cma actually, but their
drivers are off mainline tree. 

Like currently, many android devices are using ion (checking the 
products on current market or reading code in Android AOSP code),
but the only ion platform driver sits in mainline is tegra_ion.c

And if a architecture doesn't use cma, then their ion platform
driver just simply does't set the "struct cma *"

Thanks,
Feng