[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <7161814f-5317-49f0-9240-c84d331737c4@tuxon.dev>
Date: Wed, 7 Jan 2026 16:56:58 +0200
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Biju Das <biju.das.jz@...renesas.com>, "vkoul@...nel.org"
<vkoul@...nel.org>, Fabrizio Castro <fabrizio.castro.jz@...esas.com>,
"geert+renesas@...der.be" <geert+renesas@...der.be>,
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Cc: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v6 8/8] dmaengine: sh: rz-dmac: Add
device_{pause,resume}() callbacks
On 1/7/26 15:48, Biju Das wrote:
> Hi Claudiu,
>
>> -----Original Message-----
>> From: Claudiu Beznea <claudiu.beznea@...on.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@...on.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@...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@...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
Powered by blists - more mailing lists