Re: [PATCH v8 3/8] dmaengine: sh: rz-dmac: Drop read of CHCTRL register

[Frank Li]

On Tue, Jan 20, 2026 at 03:33:25PM +0200, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The CHCTRL register has 11 bits that can be updated by software. The
> documentation for all these bits states the following:
> - A read operation results in 0 being read
> - Writing zero does not affect the operation
>
> All bits in the CHCTRL register accessible by software are set and clear
> bits.
>
> The documentation for the CLREND bit of CHCTRL states:
> Setting this bit to 1 can clear the END bit of the CHSTAT_n/nS register.
> Also, the DMA transfer end interrupt is cleared. An attempt to read this
> bit results in 0 being read.
> 1: Clears the END bit.
> 0: Does not affect the operation.
>
> Since writing zero to any bit in this register does not affect controller
> operation and reads always return zero, there is no need to perform
> read-modify-write accesses to set the CLREND bit. Drop the read of the
> CHCTRL register.
>
> Also, since setting the CLREND bit does not interact with other
> functionalities exposed through this register and only clears the END
> interrupt, there is no need to lock around this operation. Add a comment
> to document this.
>
> Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
>
> Changes in v8:
> - none
>
> Changes in v7:
> - collected tags
>
> Changes in v6:
> - none, this patch is new
>
>  drivers/dma/sh/rz-dmac.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index c0f1e77996bd..bb9ca19cf784 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -697,7 +697,7 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  {
>  	struct dma_chan *chan = &channel->vc.chan;
>  	struct rz_dmac *dmac = to_rz_dmac(chan->device);
> -	u32 chstat, chctrl;
> +	u32 chstat;
>
>  	chstat = rz_dmac_ch_readl(channel, CHSTAT, 1);
>  	if (chstat & CHSTAT_ER) {
> @@ -709,8 +709,11 @@ static void rz_dmac_irq_handle_channel(struct rz_dmac_chan *channel)
>  		goto done;
>  	}
>
> -	chctrl = rz_dmac_ch_readl(channel, CHCTRL, 1);
> -	rz_dmac_ch_writel(channel, chctrl | CHCTRL_CLREND, CHCTRL, 1);
> +	/*
> +	 * No need to lock. This just clears the END interrupt. Writing
> +	 * zeros to CHCTRL is just ignored by HW.
> +	 */
> +	rz_dmac_ch_writel(channel, CHCTRL_CLREND, CHCTRL, 1);
>  done:
>  	return;
>  }
> --
> 2.43.0
>