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

[Claudiu Beznea <claudiu.beznea@tuxon.dev>]

On 1/7/26 15:48, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>> Sent: 07 January 2026 13:18
>> Subject: Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add device_{pause,resume}() callbacks
>>
>> Hi, Biju,
>>
>> On 12/23/25 16:43, Biju Das wrote:
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: Claudiu <claudiu.beznea@tuxon.dev>
>>>> Sent: 23 December 2025 13:50
>>>> Subject: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add
>>>> device_{pause,resume}() callbacks
>>>>
>>>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>>
>>>> Add support for device_{pause, resume}() callbacks. These are required by the RZ/G2L SCIFA driver.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> ---
>>>>

[...]

>>>> +
>>>> +static int rz_dmac_device_resume(struct dma_chan *chan) {
>>>> +	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
>>>> +	u32 val;
>>>> +	int ret;
>>>> +
>>>> +	scoped_guard(spinlock_irqsave, &channel->vc.lock) {
>>>
>>>
>>>> +		rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
>>>
>>>
>>> Similarly, first you need to check  CHSTAT_SUS bit first and then clear suspend state.
>>>
>>>
>>> Clears the suspend status. Setting this bit to 1 when 1 is set in SUS
>>> of the CHSTAT_n/nS register can clear the suspend status.
>>
>> I'll update this one as follows, to keep the code simple:
>>
>> 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);
>> }
>>
>> With this:
>>
>> 1/ in case the channel is not suspended and the CHCTRL_CLRSUS is set, the read_poll_timeout_atomic()
>> will not timeout, as the default value of the CHSTAT_SUS is zero.
> 
> Just a question as we are not following the hardware manual.
> 
> At hardware level does it have any implications?

The documentation of CHCTRL_CLRSUS states:

Setting this bit to 1 *when 1 is set in SUS of the CHSTAT_n/nS register* 
can clear the suspend status.

So, it takes effect only when CHSTAT.SUS=1

> 
> Eg: we set this write only register without the device being suspended
>      
> 
> The next suspend operation, immediately clears the suspend operation

The next suspend operation will switch the DMA channel to suspend if the 
channel is enabled. Nothing should take place if the device is not enabled.

>      
>      Or
> 
> does it work normally.

Yes, it works normally.

I set the CLRSUS bit in a loop for a DMA channel where the audio was 
playing, with the following command:

while :; do devmem2 0x118200a8 w 0x200 > /dev/null; done

There were no issues with the audio stream, as expected.

If I set the SETSUS bit for the same audio channel after the CLRSUS was 
set in a loop, the audio is stopped as expected.

Thank you,
Claudiu