[PATCH] IRQ Pending signal's state leaved true once stopped

[PATCH] IRQ Pending signal's state leaved true once stopped

[Eric Debief]

Hi,

We've observed that the IRQ Pending signal's state stays TRUE once the
XDMA channel is stopped. This is due to the missg acknowledgement
(stats register read) on the last interrupt.
We simply move up the status register read.

Below my patch.

Hope this helps.
Eric.

=============================================
From 1f49f5e2537741949b6af90d09c8c22764333ff6 Mon Sep 17 00:00:00 2001
From: Eric DEBIEF <debief@digigram.com>
Date: Mon, 29 Apr 2024 16:16:45 +0200
Subject: FIX: IRQ Pending TRUE once stopped.

The last interrupt is not acknowledged so the IRQ Pending signal's
state is leaved TRUE. Move up the read of the status register to
acknowledge the IRQ lowering the IRQ Pending signal's state.
---
 drivers/dma/xilinx/xdma.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 306099c920bb..de23f75bc76f 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -923,16 +923,16 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)

     spin_lock(&xchan->vchan.lock);

-    /* get submitted request */
-    vd = vchan_next_desc(&xchan->vchan);
-    if (!vd)
-        goto out;
-
     /* Clear-on-read the status register */
     ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &st);
     if (ret)
         goto out;

+    /* get submitted request */
+    vd = vchan_next_desc(&xchan->vchan);
+    if (!vd)
+        goto out;
+
     desc = to_xdma_desc(vd);

     st &= XDMA_CHAN_STATUS_MASK;
-- 
2.34.1

-- 
 
<https://www.digigram.com/digigram-critical-audio-at-critical-communications-world/>

Re: [PATCH] IRQ Pending signal's state leaved true once stopped

[Vinod Koul]

Hello Eric,

On 29-04-24, 16:21, Eric Debief wrote:
> Hi,
> 
> We've observed that the IRQ Pending signal's state stays TRUE once the
> XDMA channel is stopped. This is due to the missg acknowledgement
> (stats register read) on the last interrupt.
> We simply move up the status register read.
> 
> Below my patch.

Thanks for the patch, you should cc the driver authors as well for the
comments on the patch.

checkpatch would tell you that these are:
Lizhi Hou <lizhi.hou@amd.com> (supporter:XILINX XDMA DRIVER)
Brian Xu <brian.xu@amd.com> (supporter:XILINX XDMA DRIVER)
Raj Kumar Rampelli <raj.kumar.rampelli@amd.com> (supporter:XILINX XDMA DRIVER)
Michal Simek <michal.simek@amd.com> (supporter:ARM/ZYNQ ARCHITECTURE)
Vinod Koul <vkoul@kernel.org> (maintainer:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM)


> 
> Hope this helps.
> Eric.
> 
> =============================================
> >From 1f49f5e2537741949b6af90d09c8c22764333ff6 Mon Sep 17 00:00:00 2001
> From: Eric DEBIEF <debief@digigram.com>
> Date: Mon, 29 Apr 2024 16:16:45 +0200
> Subject: FIX: IRQ Pending TRUE once stopped.

subject should have tags: dmaengine: xilinx: 

> 
> The last interrupt is not acknowledged so the IRQ Pending signal's
> state is leaved TRUE. Move up the read of the status register to
> acknowledge the IRQ lowering the IRQ Pending signal's state.
> ---
>  drivers/dma/xilinx/xdma.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
> index 306099c920bb..de23f75bc76f 100644
> --- a/drivers/dma/xilinx/xdma.c
> +++ b/drivers/dma/xilinx/xdma.c
> @@ -923,16 +923,16 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
> 
>      spin_lock(&xchan->vchan.lock);
> 
> -    /* get submitted request */
> -    vd = vchan_next_desc(&xchan->vchan);
> -    if (!vd)
> -        goto out;
> -
>      /* Clear-on-read the status register */
>      ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &st);
>      if (ret)
>          goto out;
> 
> +    /* get submitted request */
> +    vd = vchan_next_desc(&xchan->vchan);
> +    if (!vd)
> +        goto out;
> +
>      desc = to_xdma_desc(vd);
> 
>      st &= XDMA_CHAN_STATUS_MASK;
> -- 
> 2.34.1
> 
> -- 
>  
> <https://www.digigram.com/digigram-critical-audio-at-critical-communications-world/>

-- 
~Vinod

Re: [PATCH] IRQ Pending signal's state leaved true once stopped

[Lizhi Hou]

Thanks for adding me.

Reviewed-by: lizhi.hou@amd.com

On 5/4/24 05:26, Vinod Koul wrote:
> Hello Eric,
>
> On 29-04-24, 16:21, Eric Debief wrote:
>> Hi,
>>
>> We've observed that the IRQ Pending signal's state stays TRUE once the
>> XDMA channel is stopped. This is due to the missg acknowledgement
>> (stats register read) on the last interrupt.
>> We simply move up the status register read.
>>
>> Below my patch.
> Thanks for the patch, you should cc the driver authors as well for the
> comments on the patch.
>
> checkpatch would tell you that these are:
> Lizhi Hou <lizhi.hou@amd.com> (supporter:XILINX XDMA DRIVER)
> Brian Xu <brian.xu@amd.com> (supporter:XILINX XDMA DRIVER)
> Raj Kumar Rampelli <raj.kumar.rampelli@amd.com> (supporter:XILINX XDMA DRIVER)
> Michal Simek <michal.simek@amd.com> (supporter:ARM/ZYNQ ARCHITECTURE)
> Vinod Koul <vkoul@kernel.org> (maintainer:DMA GENERIC OFFLOAD ENGINE SUBSYSTEM)
>
>
>> Hope this helps.
>> Eric.
>>
>> =============================================
>> >From 1f49f5e2537741949b6af90d09c8c22764333ff6 Mon Sep 17 00:00:00 2001
>> From: Eric DEBIEF <debief@digigram.com>
>> Date: Mon, 29 Apr 2024 16:16:45 +0200
>> Subject: FIX: IRQ Pending TRUE once stopped.
> subject should have tags: dmaengine: xilinx:
>
>> The last interrupt is not acknowledged so the IRQ Pending signal's
>> state is leaved TRUE. Move up the read of the status register to
>> acknowledge the IRQ lowering the IRQ Pending signal's state.
>> ---
>>   drivers/dma/xilinx/xdma.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
>> index 306099c920bb..de23f75bc76f 100644
>> --- a/drivers/dma/xilinx/xdma.c
>> +++ b/drivers/dma/xilinx/xdma.c
>> @@ -923,16 +923,16 @@ static irqreturn_t xdma_channel_isr(int irq, void *dev_id)
>>
>>       spin_lock(&xchan->vchan.lock);
>>
>> -    /* get submitted request */
>> -    vd = vchan_next_desc(&xchan->vchan);
>> -    if (!vd)
>> -        goto out;
>> -
>>       /* Clear-on-read the status register */
>>       ret = regmap_read(xdev->rmap, xchan->base + XDMA_CHAN_STATUS_RC, &st);
>>       if (ret)
>>           goto out;
>>
>> +    /* get submitted request */
>> +    vd = vchan_next_desc(&xchan->vchan);
>> +    if (!vd)
>> +        goto out;
>> +
>>       desc = to_xdma_desc(vd);
>>
>>       st &= XDMA_CHAN_STATUS_MASK;
>> -- 
>> 2.34.1
>>
>> -- 
>>   
>> <https://www.digigram.com/digigram-critical-audio-at-critical-communications-world/>