diff mbox series

[v2,2/7] dmaengine: hisilicon: Fix CQ head update

Message ID 20220629035549.44181-3-haijie1@huawei.com (mailing list archive)
State Superseded
Headers show
Series dmaengine: hisilicon: Add support for hisi dma driver | expand

Commit Message

Jie Hai June 29, 2022, 3:55 a.m. UTC
After completion of data transfer of one or multiple descriptors,
the completion status and the current head pointer to submission
queue are written into the CQ and interrupt can be generated to
inform the software. In interrupt process CQ is read and cq_head
is updated.

hisi_dma_irq updates cq_head only when the completion status is
success. When an abnormal interrupt reports, cq_head will not update
which will cause subsequent interrupt processes read the error CQ
and never report the correct status.

This patch updates cq_head whenever CQ is accessed.

Fixes: e9f08b65250d ("dmaengine: hisilicon: Add Kunpeng DMA engine support")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 drivers/dma/hisi_dma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Vinod Koul July 21, 2022, 1:27 p.m. UTC | #1
On 29-06-22, 11:55, Jie Hai wrote:
> After completion of data transfer of one or multiple descriptors,
> the completion status and the current head pointer to submission
> queue are written into the CQ and interrupt can be generated to
> inform the software. In interrupt process CQ is read and cq_head
> is updated.
> 
> hisi_dma_irq updates cq_head only when the completion status is
> success. When an abnormal interrupt reports, cq_head will not update
> which will cause subsequent interrupt processes read the error CQ
> and never report the correct status.
> 
> This patch updates cq_head whenever CQ is accessed.
> 
> Fixes: e9f08b65250d ("dmaengine: hisilicon: Add Kunpeng DMA engine support")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>  drivers/dma/hisi_dma.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/hisi_dma.c b/drivers/dma/hisi_dma.c
> index 98bc488893cc..7609e6e7eb37 100644
> --- a/drivers/dma/hisi_dma.c
> +++ b/drivers/dma/hisi_dma.c
> @@ -436,12 +436,12 @@ static irqreturn_t hisi_dma_irq(int irq, void *data)
>  	desc = chan->desc;
>  	cqe = chan->cq + chan->cq_head;
>  	if (desc) {
> +		chan->cq_head = (chan->cq_head + 1) %
> +				hdma_dev->chan_depth;

This can look better with single line

> +		hisi_dma_chan_write(hdma_dev->base,
> +				    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
> +				    chan->cq_head);

maybe two lines?

>  		if (FIELD_GET(STATUS_MASK, cqe->w0) == STATUS_SUCC) {
> -			chan->cq_head = (chan->cq_head + 1) %
> -					hdma_dev->chan_depth;
> -			hisi_dma_chan_write(hdma_dev->base,
> -					    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
> -					    chan->cq_head);
>  			vchan_cookie_complete(&desc->vd);
>  		} else {
>  			dev_err(&hdma_dev->pdev->dev, "task error!\n");
> -- 
> 2.33.0
Jie Hai July 26, 2022, 1:38 a.m. UTC | #2
On 2022/7/21 21:27, Vinod Koul wrote:
> On 29-06-22, 11:55, Jie Hai wrote:
>> After completion of data transfer of one or multiple descriptors,
>> the completion status and the current head pointer to submission
>> queue are written into the CQ and interrupt can be generated to
>> inform the software. In interrupt process CQ is read and cq_head
>> is updated.
>>
>> hisi_dma_irq updates cq_head only when the completion status is
>> success. When an abnormal interrupt reports, cq_head will not update
>> which will cause subsequent interrupt processes read the error CQ
>> and never report the correct status.
>>
>> This patch updates cq_head whenever CQ is accessed.
>>
>> Fixes: e9f08b65250d ("dmaengine: hisilicon: Add Kunpeng DMA engine support")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
>>   drivers/dma/hisi_dma.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/hisi_dma.c b/drivers/dma/hisi_dma.c
>> index 98bc488893cc..7609e6e7eb37 100644
>> --- a/drivers/dma/hisi_dma.c
>> +++ b/drivers/dma/hisi_dma.c
>> @@ -436,12 +436,12 @@ static irqreturn_t hisi_dma_irq(int irq, void *data)
>>   	desc = chan->desc;
>>   	cqe = chan->cq + chan->cq_head;
>>   	if (desc) {
>> +		chan->cq_head = (chan->cq_head + 1) %
>> +				hdma_dev->chan_depth;
> 
> This can look better with single line
> 
Thanks for your reviewing.
These issues have been fixed in V3.
>> +		hisi_dma_chan_write(hdma_dev->base,
>> +				    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
>> +				    chan->cq_head);
> 
> maybe two lines?
> 
>>   		if (FIELD_GET(STATUS_MASK, cqe->w0) == STATUS_SUCC) {
>> -			chan->cq_head = (chan->cq_head + 1) %
>> -					hdma_dev->chan_depth;
>> -			hisi_dma_chan_write(hdma_dev->base,
>> -					    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
>> -					    chan->cq_head);
>>   			vchan_cookie_complete(&desc->vd);
>>   		} else {
>>   			dev_err(&hdma_dev->pdev->dev, "task error!\n");
>> -- 
>> 2.33.0
>
diff mbox series

Patch

diff --git a/drivers/dma/hisi_dma.c b/drivers/dma/hisi_dma.c
index 98bc488893cc..7609e6e7eb37 100644
--- a/drivers/dma/hisi_dma.c
+++ b/drivers/dma/hisi_dma.c
@@ -436,12 +436,12 @@  static irqreturn_t hisi_dma_irq(int irq, void *data)
 	desc = chan->desc;
 	cqe = chan->cq + chan->cq_head;
 	if (desc) {
+		chan->cq_head = (chan->cq_head + 1) %
+				hdma_dev->chan_depth;
+		hisi_dma_chan_write(hdma_dev->base,
+				    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
+				    chan->cq_head);
 		if (FIELD_GET(STATUS_MASK, cqe->w0) == STATUS_SUCC) {
-			chan->cq_head = (chan->cq_head + 1) %
-					hdma_dev->chan_depth;
-			hisi_dma_chan_write(hdma_dev->base,
-					    HISI_DMA_CQ_HEAD_PTR, chan->qp_num,
-					    chan->cq_head);
 			vchan_cookie_complete(&desc->vd);
 		} else {
 			dev_err(&hdma_dev->pdev->dev, "task error!\n");