Re: [PATCH v5 10/17] dmaengine: sh: rz-dmac: Refactor pause/resume code

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware is prematurely resumed when multiple pause sources are active.
- [High] Software status bitmask is incorrectly updated even if the hardware resume operation times out.
--

commit 6c32a478012b8f47e162dfe05b689182e89ba6c2
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

dmaengine: sh: rz-dmac: Refactor pause/resume code

This commit refactors the pause/resume functionality to introduce _set
variants using a channel->status bitmask. This prepares the driver for
suspend/resume and cyclic DMA support where multiple pause sources might
exist.

> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
[ ... ]
> @@ -795,35 +811,70 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
[ ... ]
> +static int rz_dmac_device_resume_set(struct rz_dmac_chan *channel,
> +				     unsigned long clear_bitmask)
> +{
> +	int ret = 0;
> +	u32 val;
> +
> +	lockdep_assert_held(&channel->vc.lock);
> +
> +	/* Do not check CHSTAT_SUS but rely on HW capabilities. */
> +
> +	rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);

If there are multiple active pause sources (for example, paused by the DMA
client and by system suspend), will writing CHCTRL_CLRSUS here resume the
hardware prematurely?

Since this function is designed to clear a specific bitmask from the channel
status, should the hardware resume operation only be performed if there are
no other pause bits remaining in channel->status?

> +	ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +				       !(val & CHSTAT_SUS), 1, 1024, false,
> +				       channel, CHSTAT, 1);
> +
> +	channel->status &= ~clear_bitmask;

If the hardware resume operation times out and returns an error, is it
expected to clear the status bitmask anyway?

If a caller receives an error and attempts to retry the resume operation,
the channel->status bit will already be cleared. Does this prevent the retry
from succeeding, leaving the hardware stuck in a paused state?

> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512121219.216159-1-claudiu.beznea.uj@bp.renesas.com?part=10