Re: [PATCH v6 10/18] 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] Software status tracking becomes inconsistent if hardware pause/resume operations time out.
- [High] Unconditional hardware resume ignores other active pause requests.
--

commit 9f2ce07d8501ca3cb4ac77194345402583e5ba4b
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

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

This refactors the pause/resume support in the rz-dmac driver.
It introduces a bitmask-based status field to allow reusing the pause
and resume code for system suspend/resume and cyclic DMA operations.

> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 1f884ec101f80..557364443a5fd 100644
> --- 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);

[Severity: High]
Does this unconditional hardware resume ignore other active pause requests?

The bitmask-based status tracking appears to be designed to handle multiple
independent pause conditions. However, writing CHCTRL_CLRSUS happens
regardless of the remaining state.

If a channel is paused for multiple reasons, clearing one condition will
immediately unpause the hardware while other constraints are still active.
Could this regression lead to unexpected DMA execution and data corruption?

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

[Severity: High]
Should channel->status be cleared if the hardware resume operation times out?

If read_poll_timeout_atomic returns an error like -ETIMEDOUT, the status
bitmask is still unconditionally cleared.

If a caller later attempts to retry the resume operation, it will exit early
because the status bit is already gone, leaving the hardware permanently
suspended while the software stack thinks it is active. Does this create a
regression where the channel can no longer be resumed?

> +
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526084710.3491480-1-claudiu.beznea@kernel.org?part=10