Re: [PATCH v5 13/17] dmaengine: sh: rz-dmac: Add runtime PM support

[Claudiu Beznea <claudiu.beznea@kernel.org>]

Hi, Frank,

On 5/13/26 22:56, Frank Li wrote:
> On Wed, May 13, 2026 at 04:39:12PM +0300, Claudiu Beznea wrote:
>> Hi, Frank,
>>
>> On 5/13/26 01:03, Frank Li wrote:
>>> On Tue, May 12, 2026 at 03:12:14PM +0300, Claudiu Beznea wrote:
>>>> Protect the driver exposed APIs with runtime PM suspend/resume calls
>>>> before accessing HW registers. As the current driver leaves runtime PM
>>>> enabled in probe, the purpose of the changes in this patch is to avoid
>>>> accessing HW registers after a failed system suspend leaves the runtime
>>>> PM state of the device improperly reinitialized.
>>>>
>>>> In that case, the driver remains bound to the device, the APIs are still
>>>> exposed, and any access to HW registers without runtime resuming the
>>>> device may lead to synchronous aborts.
>>>>
>>>> This patch prepares the driver for suspend-to-RAM support.
>>>>
>>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>>> ---
>>>>
>>>> Changes in v5:
>>>> - none, this patch is new
>>>>
>>>>    drivers/dma/sh/rz-dmac.c | 48 ++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 48 insertions(+)
>>>>
>>>> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
>>>> index d6ad070be705..df91657fd5e3 100644
>>>> --- a/drivers/dma/sh/rz-dmac.c
>>>> +++ b/drivers/dma/sh/rz-dmac.c
>>>> @@ -488,7 +488,15 @@ static void rz_dmac_prepare_descs_for_cyclic(struct rz_dmac_chan *channel)
>>>>
>>>>    static void rz_dmac_xfer_desc(struct rz_dmac_chan *chan)
>>>>    {
>>>> +	struct dma_chan *ch = &chan->vc.chan;
>>>> +	struct rz_dmac *dmac = to_rz_dmac(ch->device);
>>>>    	struct virt_dma_desc *vd;
>>>> +	int ret;
>>>> +
>>>> +	PM_RUNTIME_ACQUIRE_IF_ENABLED(dmac->dev, pm);
>>>> +	ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
>>>> +	if (ret)
>>>> +		return;
>>>
>>> According vnod comment *_prep() call may be called in atomic context
>>> (complete callback). but runtime_pm may sleep.
>>
>> That's why the pm_runtime_irq_safe() was called in probe, to allow it being
>> called in atomic context.
>>
>> The series was tested with CONFIG_LOCKDEP=y and CONFIG_DEBUG_ATOMIC_SLEEP=y
>> no issue was identified.
> 
> I am not sure how magic it makes pm_runtime_get_sync() work under atomic
> context, suppose runtime callback involve clk_(un)prep() and power domain,
> if you call pm_runtime_irq_safe() in probe, it may makes all parent resource
> on when probe. At least it should defer to alloc chan.

The rz-dmac driver is used on platforms using drivers/clk/renesas/rzg2l-cpg.c 
and drivers/clk/renesas/rzv2h-cpg.c clock drivers.

All the platforms that supports the rz-dmac driver register always-on clock 
power domains through the above mentioned clock drivers, see [1], [2].

The genpd registered by those drivers are passing GENPD_FLAG_PM_CLK flag to the 
pm_genpd_init(). In that case the start/stop APIs of the genpd are 
pm_clk_suspend/pm_clk_resume [3].

The clocks to the rz-dmac driver are module clocks, so they are handled by [4] 
or [5] which both have the enable and disable APIs implemented, thus the 
prepare/unprepare is not going to be called through the 
pm_clk_suspend()/pm_clk_resume() functions.

Also, there is no sleep in the enable/disable APIs of those clocks.

Since the registered clock power domain is always on and the rz-dmac is marked 
as IRQ safe the genpd_lock()/genpd_unlock() (and 
genpd_power_off()/genpd_power_on()) will not be called for rz-dmac driver. 
Anyhow, the rzg2l-cpg.c and rzv2h-cpg.c are not implementing any power on/off 
APIs (they are always on power domains).

The irq_safe_dev_in_sleep_domain() calls in 
genpd_runtime_suspend()/genpd_runtime_resume() is anyway blocking any access to 
the genpd_lock()/genpd_unlock() (which may call mutex operations thought he 
genpd_mtx_ops APIs) for the rz-dmac (due to the pm_domain_irq_safe() call in 
probe) .

Also, on pm_runtime_* APIs, documentation it is mentioned (e.g. [6]):

* This routine may be called in atomic context if the RPM_ASYNC flag is set, 

* or if pm_runtime_irq_safe() has been called.

Due to all these, I consider we are safe with this approach.

Thank you,
Claudiu

[1] 
https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/clk/renesas/rzg2l-cpg.c#L2013
[2] 
https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/clk/renesas/rzv2h-cpg.c#L1549
[3] https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/pmdomain/core.c#L2439
[4] 
https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/clk/renesas/rzg2l-cpg.c#L1560
[5] 
https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/clk/renesas/rzv2h-cpg.c#L1243

[6] 
https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/base/power/runtime.c#L1147