Re: [PATCH v8 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks

[Frank Li]

On Tue, Jan 20, 2026 at 03:33:30PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Add support for device_{pause, resume}() callbacks. These are required by
> the RZ/G2L SCIFA driver.

"These are required by the RZ/G2L SCIFA driver", is not good enough. Can
you descript why RZ/G2L SCIFA require it?

Frank

>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v8:
> - reported residue for paused channels as well
>
> Changes in v7:
> - use guard() instead of scoped_guard()
> - in rz_dmac_device_pause() checked the channel is enabled
>   before suspending it to avoid read poll timeouts
> - added a comment in rz_dmac_device_resume()
>
> Changes in v6:
> - set CHCTRL_SETSUS for pause and CHCTRL_CLRSUS for resume
> - dropped read-modify-update approach for CHCTRL updates as the
>   HW returns zero when reading CHCTRL
> - moved the read_poll_timeout_atomic() under spin lock to
>   ensure avoid any races b/w pause and resume functionalities
>
> Changes in v5:
> - used suspend capability of the controller to pause/resume
>   the transfers
>
>  drivers/dma/sh/rz-dmac.c | 49 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 27c963083e29..caa3335bf95d 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -141,10 +141,12 @@ struct rz_dmac {
>  #define CHANNEL_8_15_COMMON_BASE	0x0700
>
>  #define CHSTAT_ER			BIT(4)
> +#define CHSTAT_SUS			BIT(3)
>  #define CHSTAT_EN			BIT(0)
>
>  #define CHCTRL_CLRINTMSK		BIT(17)
>  #define CHCTRL_CLRSUS			BIT(9)
> +#define CHCTRL_SETSUS			BIT(8)
>  #define CHCTRL_CLRTC			BIT(6)
>  #define CHCTRL_CLREND			BIT(5)
>  #define CHCTRL_CLRRQ			BIT(4)
> @@ -814,11 +816,18 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
>  	if (status == DMA_COMPLETE || !txstate)
>  		return status;
>
> -	scoped_guard(spinlock_irqsave, &channel->vc.lock)
> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
> +		u32 val;
> +
>  		residue = rz_dmac_chan_get_residue(channel, cookie);
>
> -	/* if there's no residue, the cookie is complete */
> -	if (!residue)
> +		val = rz_dmac_ch_readl(channel, CHSTAT, 1);
> +		if (val & CHSTAT_SUS)
> +			status = DMA_PAUSED;
> +	}
> +
> +	/* if there's no residue and no paused, the cookie is complete */
> +	if (!residue && status != DMA_PAUSED)
>  		return DMA_COMPLETE;
>
>  	dma_set_residue(txstate, residue);
> @@ -826,6 +835,38 @@ static enum dma_status rz_dmac_tx_status(struct dma_chan *chan,
>  	return status;
>  }
>
> +static int rz_dmac_device_pause(struct dma_chan *chan)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	u32 val;
> +
> +	guard(spinlock_irqsave)(&channel->vc.lock);
> +
> +	val = rz_dmac_ch_readl(channel, CHSTAT, 1);
> +	if (!(val & CHSTAT_EN))
> +		return 0;
> +
> +	rz_dmac_ch_writel(channel, CHCTRL_SETSUS, CHCTRL, 1);
> +	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					(val & CHSTAT_SUS), 1, 1024,
> +					false, channel, CHSTAT, 1);
> +}
> +
> +static int rz_dmac_device_resume(struct dma_chan *chan)
> +{
> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	u32 val;
> +
> +	guard(spinlock_irqsave)(&channel->vc.lock);
> +
> +	/* Do not check CHSTAT_SUS but rely on HW capabilities. */
> +
> +	rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> +	return read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> +					!(val & CHSTAT_SUS), 1, 1024,
> +					false, channel, CHSTAT, 1);
> +}
> +
>  /*
>   * -----------------------------------------------------------------------------
>   * IRQ handling
> @@ -1164,6 +1205,8 @@ static int rz_dmac_probe(struct platform_device *pdev)
>  	engine->device_terminate_all = rz_dmac_terminate_all;
>  	engine->device_issue_pending = rz_dmac_issue_pending;
>  	engine->device_synchronize = rz_dmac_device_synchronize;
> +	engine->device_pause = rz_dmac_device_pause;
> +	engine->device_resume = rz_dmac_device_resume;
>
>  	engine->copy_align = DMAENGINE_ALIGN_1_BYTE;
>  	dma_set_max_seg_size(engine->dev, U32_MAX);
> --
> 2.43.0
>