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

[Laura Abbott <labbott@redhat.com>]

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