Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <dfa515b1-2322-25b2-7b90-f7dd11b2ff47@huawei.com>
Date:   Tue, 26 Jul 2022 09:38:48 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH v2 2/7] dmaengine: hisilicon: Fix CQ head update
To:     Vinod Koul <vkoul@kernel.org>
CC:     <wangzhou1@hisilicon.com>, <dmaengine@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
References: <20220625074422.3479591-1-haijie1@huawei.com>
 <20220629035549.44181-1-haijie1@huawei.com>
 <20220629035549.44181-3-haijie1@huawei.com> <YtlULeGbn+z5AVP+@matsya>
From:   Jie Hai <haijie1@huawei.com>
In-Reply-To: <YtlULeGbn+z5AVP+@matsya>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
>