[PATCH v8 04/16] DMA: PL330: Remove the start operation for handling DMA_TERMINATE_ALL command

Jassi Brar wrote:

Subject: Re: [PATCH v8 04/16] DMA: PL330: Remove the start operation
for handling DMA_TERMINATE_ALL command

On 7 September 2011 12:53, Kukjin Kim [off-list ref] wrote:

quoted

Jassi Brar wrote:

quoted

On 6 September 2011 17:57, Russell King - ARM Linux
[off-list ref] wrote:

quoted

On Tue, Sep 06, 2011 at 05:52:19PM +0530, Jassi Brar wrote:

quoted

On Fri, Sep 2, 2011 at 6:14 AM, Boojin Kim

[off-list ref] wrote:

quoted

quoted

quoted

quoted

quoted

Origianl code carries out the start operation after flush

operation.

quoted

quoted

quoted

quoted

quoted

But start operation is not required for DMA_TERMINATE_ALL

command.

quoted

quoted

quoted

quoted

quoted

So, This patch removes the unnecessary start operation and

only carries out

quoted

quoted

quoted

quoted

quoted

the flush oeration for handling DMA_TERMINATE_ALL command.

Not exactly. The 'start' is impotent when called from this path

because there

quoted

quoted

quoted

quoted

is nothing left queued after the call to 'flush'.
pl330_tasklet() is called because that is a common function that

does the

quoted

quoted

quoted

quoted

house-keeping acc to what's presently in 'work' and 'done' lists.

Though as a side-effect, your patch does avoid doing callbacks

on submitted

quoted

quoted

quoted

quoted

xfers during DMA_TERMINATE_ALL - which may or may not be the

right thing

quoted

quoted

quoted

quoted

to do.

It is defined that DMA_TERMINATE_ALL does not call the callbacks:

1. int dmaengine_terminate_all(struct dma_chan *chan)

? This causes all activity for the DMA channel to be stopped, and

may

quoted

quoted

quoted

? discard data in the DMA FIFO which hasn't been fully transferred.
? No callback functions will be called for any incomplete

transfers.

quoted

quoted

quoted

Ok, thanks for the info.

Boojin Kim, please re-write the changelog to state only preventing
callbacks during DMA_TERMINATE_ALL as the reason.

As above, we don't need callback as well start operation in

DMA_TERMIANTE_ALL command and her patch just removed them for
DMA_TERMINATE_ALL here. So current git message seems to have proper
behavior of patch.

quoted

Nopes.

No modification to current code is needed if only the following is to
be the reason.
{
Origianl code carries out the start operation after flush operation.
But start operation is not required for DMA_TERMINATE_ALL command.
So, This patch removes the unnecessary start operation and only
carries out
the flush oeration for handling DMA_TERMINATE_ALL command.
}

The patch, however, has unintended good effect of "preventing
callbacks
from DMA_TERMINATE_ALL".
The only valid reason, and which is no way implied by the changelog.

According to dmaengine document, dmaengine_terminate_all() is used to pause
or stop the channel. So, dmaengine_terminate_all() doesn?t need to have the
start operation and callback calling.
This patch is originally aimed at removing unnecessary operations including
the start operation and callback call for dmaengine_terminate_all(). And I
think my message pretty expresses the purpose of this patch.

Thanks,
Boojin kim