Re: [PATCH v2 04/16] PCI: epf-test: Fix DMA transfer completion detection

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 2023/03/16 0:20, Manivannan Sadhasivam wrote:
>> @@ -152,25 +160,35 @@ static int pci_epf_test_data_transfer(struct pci_epf_test *epf_test,
>>  	}
>>  
>>  	reinit_completion(&epf_test->transfer_complete);
>> +	epf_test->transfer_chan = chan;
>>  	tx->callback = pci_epf_test_dma_callback;
>>  	tx->callback_param = epf_test;
>> -	cookie = tx->tx_submit(tx);
>> +	epf_test->transfer_cookie = tx->tx_submit(tx);
>>  
>> -	ret = dma_submit_error(cookie);
>> +	ret = dma_submit_error(epf_test->transfer_cookie);
>>  	if (ret) {
>> -		dev_err(dev, "Failed to do DMA tx_submit %d\n", cookie);
>> -		return -EIO;
>> +		dev_err(dev, "Failed to do DMA tx_submit %d\n", ret);
>> +		goto terminate;
>>  	}
>>  
>>  	dma_async_issue_pending(chan);
>>  	ret = wait_for_completion_interruptible(&epf_test->transfer_complete);
>>  	if (ret < 0) {
>> -		dmaengine_terminate_sync(chan);
>> -		dev_err(dev, "DMA wait_for_completion_timeout\n");
>> -		return -ETIMEDOUT;
>> +		dev_err(dev, "DMA wait_for_completion interrupted\n");
>> +		goto terminate;
>>  	}
>>  
>> -	return 0;
>> +	if (epf_test->transfer_status == DMA_ERROR) {
>> +		dev_err(dev, "DMA transfer failed\n");
>> +		ret = -EIO;
>> +	}
>> +
>> +	WARN_ON(epf_test->transfer_status != DMA_COMPLETE);
> 
> Why do you need this check? Even if required, WARN_ON is superfluous here.

The check is needed to return -EIO if there was a problem with the transfer,
because wait_for_completion_interruptible() does not notify such errors (it only
notifies timeouts).

And yes, the WARN can go away. Will remove it.

-- 
Damien Le Moal
Western Digital Research