Re: [PATCH v3 4/5] dma: cppi41: only allocate descriptor memory once

[Daniel Mack <zonque@gmail.com>]

On 01.10.2013 18:22, Sebastian Andrzej Siewior wrote:
> On 10/01/2013 03:09 PM, Daniel Mack wrote:

>> A simplified version of the code as it stands is:
>>
>>   for (i = 0; i < DESCS_AREAS; i++)
>>     cdd->cd = dma_alloc_coherent(dev, ..., &cdd->descs_phys, GFP_KERNEL);
>>
>>   for (i = 0; i < DESCS_AREAS; i++)
>>     dma_free_coherent(dev, mem_decs, cdd->cd, cdd->descs_phys);

...

>> So, after all, my patch doesn't really change any of the runtime
>> behaviour. Consider it a cosmetic cleanup if you wish :)
> 
> But it looks strange (in my opinion at least).

I still disagree, but maybe that doesn't matter any more, because ...

> But now, that I look
> again at it yes you moved the alloc out of the loop and replaced
> mem_decs with the computation. So nothing changed but you moved it
> outside.
> 
> Right I think one desc area should be enough so I would just remove the
> for loop and DESCS_AREAS as well. How does this sound to you?

That sound's like a good idea. Everything that make the driver smaller
and easier to understand is certainly a good thing :)

So we can drop this patch favor of your cleanup. However, I appreciate
if you did it on top of the second round of patches I sent today please,
because my resume() implementation uses DESCS_AREAS as well.


Thanks,
Daniel