Re: [PATCH v3 09/12] rtc: rzn1: Use temporary variable for struct device

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Asynchronous pm_runtime_put() is used immediately before pm_runtime_disable() during devres cleanup, creating a race condition that leaves the device active.
--

commit 5ee7d58e34962ad10d6a886e40f4b4757bc5611e
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

rtc: rzn1: Use temporary variable for struct device

This commit introduces a temporary variable to store the device pointer in
the probe function. This replaces multiple occurrences to avoid repeated
dereferencing.

> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 12bbbc18f7eab..4917bd223efd7 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c

[ ... ]

> @@ -421,19 +422,19 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>  	rtc->rtcdev->range_max = RTC_TIMESTAMP_END_2099;
>  	rtc->rtcdev->alarm_offset_max = 7 * 86400;
>  
> -	ret = devm_pm_runtime_enable(&pdev->dev);
> +	ret = devm_pm_runtime_enable(dev);
>  	if (ret < 0)
>  		return ret;
> -	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0)
>  		return ret;
>  
> -	ret = devm_add_action_or_reset(&pdev->dev, rzn1_rtc_disable_hardware, &pdev->dev);
> +	ret = devm_add_action_or_reset(dev, rzn1_rtc_disable_hardware, dev);

[Severity: High]
This is a pre-existing issue, but does the async suspend queued by
rzn1_rtc_disable_hardware() race with the subsequent runtime PM disable?

During devres cleanup (such as probe failure or driver removal), the cleanup
actions execute in reverse order (LIFO). First, the custom hardware disable
action runs:

static void rzn1_rtc_disable_hardware(void *data)
{
        ...
        pm_runtime_put(dev);
}

Because this is an asynchronous put, the suspend work is only queued.

Immediately after, devres unwinds devm_pm_runtime_enable() which calls
pm_runtime_disable(). This cleanup triggers __pm_runtime_barrier(), canceling
any pending async idle or suspend work and leaving the hardware powered on.

Should rzn1_rtc_disable_hardware() use pm_runtime_put_sync() instead to
ensure the suspend completes before runtime PM is disabled?

>  	if (ret)
>  		return ret;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706175138.12587-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=9