Re: [PATCH v3 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Using 0 as a sentinel for `ll_recheck_at` leads to missed DMA rechecks when `jiffies + delay` evaluates to exactly 0.
--

--- Patch [15]: [PATCH v3 15/24] dmaengine: dw-edma: Recheck stopped LL channels before restart ---
commit 204ac5e83f10ca496840d5ab003b724253b46bfe
Author: Koichiro Den <[email protected]>

dmaengine: dw-edma: Recheck stopped LL channels before restart

This commit introduces a mechanism to recheck a stopped linked-list
channel before restarting it. It samples DMA_LLP again to reclaim any
newly visible progress for interfaces without a dedicated STOP event. It
also schedules a delayed recheck if the channel reports it is running.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index d28e3633a8665..dbccd11aa92fa 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -148,6 +149,26 @@ static void dw_edma_ll_event_discard_locked(struct dw_edma_chan *chan)
>  	dw_edma_ll_snapshot_discard_locked(chan);
>  }
>  
> +/* Must be called with vc.lock held. */
> +static void dw_edma_ll_recheck_cancel(struct dw_edma_chan *chan)
> +{
> +	chan->ll_recheck_at = 0;
> +	cancel_delayed_work(&chan->ll_recheck_work);
> +}
> +
> +/* Must be called with vc.lock held. */
> +static void dw_edma_ll_recheck_schedule(struct dw_edma_chan *chan)
> +{
> +	unsigned long delay =
> +		msecs_to_jiffies(DW_EDMA_LL_RECHECK_DELAY_MS);
> +
> +	if (unlikely(READ_ONCE(chan->dw->teardown)))
> +		return;
> +
> +	chan->ll_recheck_at = jiffies + delay;

[Severity: Medium]
If jiffies + delay evaluates to exactly 0 when it wraps around, will the
delayed work function mistake this for a cancelled timer?

> +	mod_delayed_work(chan->dw->wq, &chan->ll_recheck_work, delay);
> +}
[ ... ]
> +static void dw_edma_ll_recheck_work(struct work_struct *work)
> +{
> +	struct dw_edma_chan *chan =
> +		container_of(to_delayed_work(work), struct dw_edma_chan,
> +			     ll_recheck_work);
> +	unsigned long delay;
> +
> +	guard(spinlock_irqsave)(&chan->vc.lock);
> +
> +	if (unlikely(READ_ONCE(chan->dw->teardown))) {
> +		chan->ll_recheck_at = 0;
> +		return;
> +	}
> +
> +	if (!chan->ll_recheck_at)
> +		return;

[Severity: Medium]
If the scheduled time wrapped to 0, does this check incorrectly abort
the function and miss the necessary recheck of the stopped boundary?

> +
> +	if (time_before(jiffies, chan->ll_recheck_at)) {
> +		delay = chan->ll_recheck_at - jiffies;
> +		mod_delayed_work(chan->dw->wq, &chan->ll_recheck_work, delay);
> +		return;
> +	}
> +
> +	chan->ll_recheck_at = 0;
> +	if (chan->request == EDMA_REQ_NONE)
> +		dw_edma_core_ch_maybe_doorbell(chan);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15