Re: [PATCH v2 3/4] pxa_camera: Redesign DMA handling

[Robert Jarzmik <robert.jarzmik@free.fr>]

Guennadi Liakhovetski <g.liakhovetski@gmx.de> writes:

> What is QIF? Do you mean Quick Capture Interface - QCI? I also see CIF 
> used in the datasheet, probably, for "Capture InterFace," but I don't see 
> QIF anywhere. Also, please explain the first time you use the 
> abbreviation. Also fix it in the commit message to patch 1/4.
OK, will replace with QCI, my bad.

And OK for all the other typos in pxa_camera.txt

>> +static void pxa_dma_update_sg_tail(struct pxa_camera_dev *pcdev,
>> +				   struct pxa_buffer *buf)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < pcdev->channels; i++)
>> +		pcdev->sg_tail[i] = buf->dmas[i].sg_cpu + buf->dmas[i].sglen;
>> +}
>> +
>> +static void pxa_dma_add_tail_buf(struct pxa_camera_dev *pcdev,
>> +				 struct pxa_buffer *buf)
>> +{
>> +	int i;
>> +	struct pxa_dma_desc *buf_last_desc;
>> +
>> +	for (i = 0; i < pcdev->channels; i++) {
>> +		buf_last_desc = buf->dmas[i].sg_cpu + buf->dmas[i].sglen;
>> +		buf_last_desc->ddadr = DDADR_STOP;
>> +
>> +		if (!pcdev->sg_tail[i])
>> +			continue;
>> +		pcdev->sg_tail[i]->ddadr = buf->dmas[i].sg_dma;
>> +	}
>> +
>> +	pxa_dma_update_sg_tail(pcdev, buf);
>
> pxa_dma_update_sg_tail is called only here, why not inline it and also put 
> inside one loop?
As for the inline, I'm pretty sure you know it is automatically done by gcc.

As for moving it inside the loop, that would certainly improve performance. Yet
I find it more readable/maintainable that way, and will leave it. But I won't be
bothered at all if you retransform it back to your view, that's up to you.

Cheers.

--
Robert