[PATCH V7 3/3] dma: add Qualcomm Technologies HIDMA channel driver

On 12/1/2015 4:16 PM, Sinan Kaya wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+static enum dma_status hidma_tx_status(struct dma_chan *dmach,
+			dma_cookie_t cookie, struct dma_tx_state *txstate)
+{
+	struct hidma_chan *mchan = to_hidma_chan(dmach);
+	enum dma_status ret;
+
+	if (mchan->paused)
+		ret = DMA_PAUSED;

This is not quite right. The status query is for a descriptor and NOT for
channel. Here if the descriptor queried was running then it would be paused
for the rest pending txn, it would be DMA_IN_PROGRESS.

The channel can be paused by the hypervisor. If it is paused, then no
other transaction will go through including the pending requests. That's
why, I'm checking the state first.

You are missing the point. Channel can be paused, yes but the descriptor
is in queue and is not paused. The descriptor running is paused, yes.
There is subtle difference between these

I'll follow your recommendation. PAUSE for the currently active
descriptor and DMA_IN_PROGRESS for the rest.

I'm now confused.

I looked at several DMA driver implementations.

1. They call dma_cookie_status function to see if the job is done.
2. If done, they return right ahead.
3. Otherwise, dma_cookie_status returns DMA_IN_PROGRESS.
4. Next the code checks if the channel is paused and return value is
DMA_IN_PROGRESS. The code changes return code to DMA_PAUSED.

Whereas, I was returning paused first before even checking if the
descriptor is done. Are you OK with the sequence 1..4 above?

-- 
Sinan Kaya
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project