Re: [PATCH v5 17/17] i3c: renesas: Add runtime PM support

On 7/15/26 19:54, Frank Li wrote:
> On Mon, Jul 13, 2026 at 04:05:45PM +0300, Claudiu Beznea wrote:
>> From: Claudiu Beznea <[email protected]>
>>
>> On the SoCs where the Renesas I3C driver is enabled (RZ/G3S and RZ/G3E),
>> the clocks of the IP are managed through a clock PM domain. To keep the
>> I3C code simpler, the explicit clock handling was dropped along with the
>> addition of runtime PM support, in favor of the runtime PM APIs. Only the
>> code for getting tclk was preserved, as it is necessary to compute the
>> I3C clock rate.
>>
>> All the APIs provided to the I3C subsystem through struct
>> i3c_master_controller_ops are guarded with runtime PM APIs to
>> enable/disable the controller at runtime.
>>
>> As the Renesas I3C driver implements an asynchronous transmit model by
>> preparing a transfer and waiting for its completion through the ISR,
>> renesas_i3c_abort_xfer() was added to disable interrupts and clear any
>> pending IRQ status bits when there is no completion in the defined
>> timeout. Along with this, renesas_i3c_wait_xfer() return type was changed
>> to unsigned long.
>>
>> Add runtime PM support for the Renesas I3C driver.
>>
>> Signed-off-by: Claudiu Beznea <[email protected]>
>> ---
>>
>> Changes in v5:
>> - updated renesas_i3c_irqs_mask_and_clear() to use the
>>    renesas_i3c_irqs_mask_and_clear_locked() introduced in patch 1 from
>>    this series
>>
>> Changes in v4:
>> - renamed renesas_i3c_abort_xfer() from v3 to
>>    renesas_i3c_irqs_mask_and_clear(); along with it renamed local varibles
>>    abort_xfer to xfer_failed
>>
>> Changes in v3:
>> - dropped the RPM resume/suspend in renesas_i3c_suspend() to read the
>>    DATBASn registers as the DATBASn register are not used on suspend/resume
>>    path anymore in this version
>>
>> Changes in v2:
>> - dropped the runtime suspend/resume functions as for now, there will be
>>    no pinctrl sleep state described in DT
>> - do not synchronize the IRQs in renesas_i3c_abort_xfer() as some handlers
>>    may re-enable interrupts; instead just disable the interrupts and clean
>>    any status bits that the IRQ handlers are touching; with this the
>>    struct renesas_i3c::{irqs, num_irqs} and the associated code was removed
>> - dropped the renesas_i3c_dont_use_autosuspend() along with the
>>    devm_add_action_or_reset() call to set it since the same operation is
>>    done by the devres cleanup helper of devm_pm_runtime_enable()
>> - adjusted the renesas_i3c_suspend() with RPM calls to save the DATBAS
>>    registers
>> - use pm_runtime_resume_and_get() in renesas_i3c_resume() to avoid
>>    mixing gotos with cleanup helpers
>> - adjusted the patch description to reflect these changes
>>
> ...
>>
>> @@ -1384,12 +1458,16 @@ static int renesas_i3c_probe(struct platform_device *pdev)
>>   	if (IS_ERR(i3c->regs))
>>   		return PTR_ERR(i3c->regs);
>>
>> -	ret = devm_clk_bulk_get_all_enabled(&pdev->dev, &i3c->clks);
>> -	if (ret <= RENESAS_I3C_TCLK_IDX)
>> -		return dev_err_probe(&pdev->dev, ret < 0 ? ret : -EINVAL,
>> -				     "Failed to get clocks (need > %d, got %d)\n",
>> -				     RENESAS_I3C_TCLK_IDX, ret);
>> -	i3c->num_clks = ret;
>> +	i3c->tclk = devm_clk_get(&pdev->dev, "tclk");
> 
> only one tclk? you remove devm_clk_bulk_get_all_enabled()
On all the SoCs where this driver is used the clocks are managed through clock 
PM domains. The clock PM domain registration is done though the drivers at [1] 
or [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].

Thus, we can make use of runtime PM APIs to request, enable, disable the clocks 
using generic runtime PM APIs.

Every time this driver calls runtime suspend/resume APIs this goes down to the 
IP clocks and clk_disable()/clk_enable() them.

Since we are relying on clock PM domains there is no need to implement runtime 
suspend/resume ops for this driver that does just only 
clock_disable()/clk_enable(), thus, the final code is simpler. We only request 
the tclk as we need to compute the bus rate based on it.

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