Re: [PATCH v5 04/17] rtc: rzn1: Handle unset alarm weekday in rzn1_rtc_read_alarm

[email protected]
Newsgroups org.kernel.vger.linux-rtc,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The RTC calibration offset cannot be reset to 0 once configured because `rzn1_rtc_set_offset()` returns early without updating the hardware.
--

--- Patch [4]: [PATCH v5 04/17] rtc: rzn1: Handle unset alarm weekday in rzn1_rtc_read_alarm ---
commit 03655d07aa4a099ff33de67c8c707b707a119002
Author: Lad Prabhakar <[email protected]>

rtc: rzn1: Handle unset alarm weekday in rzn1_rtc_read_alarm

This commit handles the case where the alarm weekday register is unset by
returning early to avoid fabricating an invalid alarm date. It also moves
the control register read before this check so that the enabled flag is
updated properly.

Cc: [email protected]

> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c

[ ... ]

This is a pre-existing issue, but I noticed a potential logic flaw in the
RTC offset configuration while reviewing this file:

drivers/rtc/rtc-rzn1.c:rzn1_rtc_set_offset() {
...
	if (stepsh >= -0x3E && stepsh <= 0x3E) {
		/* 1017 ppb per step */
		steps = stepsh;
		subu |= RZN1_RTC_SUBU_DEV;
	} else if (stepsl >= -0x3E && stepsl <= 0x3E) {
		/* 3051 ppb per step */
		steps = stepsl;
	} else {
		return -ERANGE;
	}

	if (!steps)
		return 0;
...
}

[Severity: Medium]
Does this early return prevent the hardware register from being updated to
clear the calibration offset when steps evaluates to zero?

If a process attempts to reset the RTC offset to 0 ppb, this early return
prevents the hardware register (RZN1_RTC_SUBU) from being written. This
seems to leave the old calibration value applied in hardware, which would
result in persistent clock drift since the offset cannot be cleared.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.