Re: [PATCH v2 06/10] rtc: rzn1: Dynamically calculate synchronization delay based on clock rate

[sashiko-bot@kernel.org]

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

New issues:
- [Low] Incorrect time conversion macro NSEC_PER_MSEC used instead of USEC_PER_SEC for microsecond calculation.

Pre-existing issues:
- [High] Race condition between PM runtime teardown and devres device unregistration can lead to accessing powered-down hardware.
--

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

rtc: rzn1: Dynamically calculate synchronization delay based on clock rate

This patch replaces hardcoded hardware synchronization delays with a
calculated time window derived from the runtime clock rate. It stores this
synchronization time in the driver private structure to dynamically adjust
wait boundaries.

> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index 9969bcefede72..583d174954043 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
[ ... ]
> @@ -432,12 +433,20 @@ static int rzn1_rtc_probe(struct platform_device *pdev)
>  			scmp_val = RZN1_RTC_CTL0_SLSB_SCMP;
>  	}
>  
> +	/*
> +	 * The internal clock counter operates in synchronization with the
> +	 * RTC_PCLK clock. Calculate the duration of two RTC_PCLK clock
> +	 * cycles in microseconds required for operations to complete.
> +	 */
> +	rtc->sync_time = DIV_ROUND_UP(2 * NSEC_PER_MSEC, rate);

[Severity: Low]
This isn't a bug, but should this macro be USEC_PER_SEC instead?

The comment mentions calculating the duration in microseconds. While
NSEC_PER_MSEC evaluates to 1,000,000 which produces the correct runtime
result for converting cycles per second into microseconds, using
USEC_PER_SEC would more accurately reflect the intended units and improve
readability.

[Severity: High]
This is a pre-existing issue, but is there a race condition between the
PM runtime teardown and devres cleanup?

In drivers/rtc/rtc-rzn1.c, the RTC device is registered via devres
during rzn1_rtc_probe():

    ret = devm_rtc_register_device(rtc->rtcdev);

Devres runs its cleanup (which calls rtc_unregister_device()) strictly
after the driver's remove callback returns. However, in rzn1_rtc_remove(),
the driver manually drops the PM runtime reference:

    pm_runtime_put(&pdev->dev);

This powers down the hardware while the RTC character device is still
registered and exposed to userspace. If a userspace process calls an ioctl,
such as RTC_RD_TIME, during this window, the driver's rzn1_rtc_read_time()
callback executes and attempts to access unclocked registers, which could
lead to a synchronous external abort or bus fault.

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