Re: [PATCH v6 3/5] dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dmaengine: dw-edma: non-LL watermark unmasked in dw_hdma_v0_core_int_setup()
--

commit 5e1e3ed55b7d4e9a83bc0d3c9ec3bbccd4a0395c
Author: Koichiro Den <den@valinux.co.jp>

dmaengine: dw-edma: Use HDMA watermark interrupts as LL progress

This commit enables HDMA watermark interrupts on linked-list entries that
request an interrupt, configures local or remote interrupt delivery, and
routes watermark events to the linked-list progress handler.

> diff --git a/drivers/dma/dw-edma/dw-hdma-v0-core.c b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> index aa430219d0af2..7ca5afa181fe5 100644
> --- a/drivers/dma/dw-edma/dw-hdma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-hdma-v0-core.c
> @@ -58,7 +58,8 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
>  {
>  	val &= ~(HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_REMOTE_ABORT_INT_EN |
>  		 HDMA_V0_LOCAL_STOP_INT_EN | HDMA_V0_REMOTE_STOP_INT_EN |
> -		 HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK);
> +		 HDMA_V0_ABORT_INT_MASK | HDMA_V0_WATERMARK_INT_MASK |
> +		 HDMA_V0_STOP_INT_MASK);
>  
>  	/*
>  	 * HDMA_INT_STATUS.STOP and .ABORT are latched only when LSIE and
> @@ -68,10 +69,14 @@ static u32 dw_hdma_v0_core_int_setup(struct dw_edma_chan *chan, u32 val)
>  	 */
>  	val |= HDMA_V0_LOCAL_ABORT_INT_EN | HDMA_V0_LOCAL_STOP_INT_EN;
>  
> -	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE)
> +	if (chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE) {
> +		if (!chan->non_ll)
> +			val |= HDMA_V0_WATERMARK_INT_MASK;
> +
>  		val |= HDMA_V0_REMOTE_ABORT_INT_EN |
>  		       HDMA_V0_REMOTE_STOP_INT_EN |
>  		       HDMA_V0_ABORT_INT_MASK | HDMA_V0_STOP_INT_MASK;
> +	}
>  
>  	return val;
>  }

[Severity: Medium]
In dw_hdma_v0_core_int_setup(), HDMA_V0_WATERMARK_INT_MASK is unconditionally
cleared at the start of the function, unmasking the local watermark interrupt
pin. Later, it is only set back to 1 (masked) when chan->irq_mode is
DW_EDMA_CH_IRQ_REMOTE and !chan->non_ll.

When a channel is started in non-LL mode via:

  dw_edma_device_issue_pending()
    -> dw_edma_start_transfer()
      -> dw_edma_core_start()
        -> dw_hdma_v0_core_non_ll_start()
          -> dw_hdma_v0_core_int_setup()

chan->non_ll is true, which leaves HDMA_V0_WATERMARK_INT_MASK cleared (0,
unmasked) in both DW_EDMA_CH_IRQ_LOCAL and DW_EDMA_CH_IRQ_REMOTE modes.

In remote IRQ mode, all local interrupt pins are expected to be masked. In
addition, if a watermark interrupt fires on a non-LL channel (for instance, if
the hardware watermark_en register is non-zero), dw_hdma_v0_core_handle_int()
will report DW_EDMA_IRQ_PROGRESS, causing dw_edma_done_interrupt() to
prematurely complete the active non-LL burst while the channel is still
running.

Should HDMA_V0_WATERMARK_INT_MASK be set whenever chan->non_ll is true or
chan->irq_mode == DW_EDMA_CH_IRQ_REMOTE, so that local watermark interrupts
are only unmasked for LL channels in local IRQ mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150742.2586776-1-den@valinux.co.jp?part=3