rtc: hibvt: judge the time by its counter, not the low-voltage flag by openipc-ai · Pull Request #81 · OpenIPC/linux

openipc-ai · 2026-10-13T10:59:36Z

Port of #79 (hisilicon-hi3516ev200) to hisilicon-hi3536dv100. Read #79 for the analysis and the hardware results.

This branch's copy differs from #79 in one way that matters. It kept the vendor's handling: read_time() logs low voltage detected, date/time is not reliable and returns -EINVAL whenever the backup supply low-voltage flag is set. The flag is "cleared" by writing 1 to RTC_INT_CLR, though, and that is the alarm bit. So once the flag is set it never clears, every read fails and logs an error, and the RTC stays unreadable.

The flag is not a reliable verdict either. On the hi3516ev300 in #79 it stays set, and returns about 0.5 s after each correct clear, with a fresh backup cell that keeps the time through a 5-minute power cut. A board like that would have a working RTC that this driver refuses to read.

Change (the same as #79)

Do not fail the read, or log, because of the flag. Clear it with LV_INT_MASK.
Judge the time by the counter: return -EINVAL for a value before 2000-01-01, which is what a counter that lost power reads.
set_time() refuses the same range, so every time it accepts can be read back (Qodo's first point).

Qodo's second point, that init could clear the flag before it was reported, no longer applies: nothing reports the flag.

Verification

Build: compiles with no warnings against hi3536dv100.generic.config (CONFIG_ARCH_HI3536DV100=y).
On hardware: the lab hi3536dv100 (hi3536dv100_lite, RTC at 120b0000.rtc). I built this branch's rtc-hibvt.c as an out-of-tree module with the driver renamed to hibvt_rtc_test, unbound the built-in driver, and loaded the module in its place:

hibvt_rtc_test 120b0000.rtc: rtc core: registered 120b0000.rtc as rtc0
hibvt_rtc_test 120b0000.rtc: RTC driver for hibvt enabled
== valid counter
Tue Oct 20 11:22:22 2026  0.000000 seconds
1792495342
== force counter to 1970 (load 0 via LR regs)
hwclock: RTC_RD_TIME: Invalid argument
cat: read error: Invalid argument          # /sys/class/rtc/rtc0/since_epoch
== set_time 1999-12-31 23:59:59
hwclock: RTC_SET_TIME: Invalid argument
== restore from system clock
Tue Oct 20 11:22:24 2026  0.000000 seconds

The built-in driver was bound again afterwards.

With the driver built in, on the same board. I built hi3536dv100_lite's kernel with Buildroot, changed only rtc-hibvt.c, and U-Boot loaded it over TFTP for the test. The NOR kernel partition was never written. Afterwards the board went back to its stock kernel and original environment.

Cold boot after an 8 s power cut through a switchable plug (the board stopped answering ping during the cut). This board has backup power, and the time was applied with no message:

Linux version 4.9.37 (ai@forge-hel1) (buildroot-gcc-13.3.0) #2 Tue Oct 20 11:33:20 UTC 2026
hibvt_rtc 120b0000.rtc: rtc core: registered 120b0000.rtc as rtc0
hibvt_rtc 120b0000.rtc: RTC driver for hibvt enabled
hibvt_rtc 120b0000.rtc: setting system clock to 2026-10-20 11:47:50 UTC (1792496870)

Counter zeroed, then reboot -f. The counter is refused at boot, and userspace restores the clock:

hibvt_rtc 120b0000.rtc: hctosys: unable to read the hardware clock
fake-hwclock: clock restored to 2026-10-20 11:48:12 UTC

set_time() and read_time() agree:

hwclock: RTC_SET_TIME: Invalid argument      # 1999-12-31 23:59:59
rc=0                                          # 2000-01-01 00:00:00
Sat Jan  1 00:00:00 2000  0.000000 seconds

Not reproducible here: this board's low-voltage flag is clear (RTC_INT_RAW reads 0), so the old driver's refusal to read while the flag is set couldn't be shown on it. The flag behaviour comes from the hi3516ev300 in #79.

Review fix: `set_time()` errors

hibvt_rtc_set_time() collected the SPI write results in an unsigned char, so a bridge timeout (-EIO) came back as 251, and RTC_SET_TIME reported success for a write that never reached the RTC (Qodo). It is now an int, like every other function in the file.

The SPI timeout can't be triggered on demand, so the error path itself is covered only by the type fix. The same file, with the same fix, was re-run on a hi3516cv300 in #125: setting the time returns 0, 1999 is refused, and the time reads back. The lab hi3536dv100 was off the network for this round, so it was not re-run there.

openipc-ai · 2026-10-13T11:01:10Z

/review

qodo-free-for-open-source-projects · 2026-10-13T11:01:16Z

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Valid clocks become unreadable after 2106 🐞 Bug ≡ Correctness

Description

hibvt_rtc_read_time() compares RTC_MIN_VALID_SEC against seconds after DATE_TO_SEC has
multiplied the unsigned 32-bit day count, allowing the conversion to wrap. Starting at day 49711
(2106-02-07), a running counter can therefore produce a value below the cutoff and return -EINVAL,
so clock restoration cannot use that valid RTC time.

Code

drivers/rtc/rtc-hibvt.c[R247-248]
+	if (seconds < RTC_MIN_VALID_SEC)
+		return -EINVAL;

Evidence

day is an unsigned int assembled from two day registers, and DATE_TO_SEC multiplies it without
a wider cast. The newly added check rejects the wrapped result before converting it to a calendar
time; the RTC core propagates a negative read result rather than using the time.

drivers/rtc/rtc-hibvt.c[110-110]
drivers/rtc/rtc-hibvt.c[239-250]
drivers/rtc/interface.c[25-49]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new minimum-time check classifies some valid RTC counters as unset because `DATE_TO_SEC` wraps in 32-bit arithmetic before the comparison.
## Fix Focus Areas
- drivers/rtc/rtc-hibvt.c[110-110]
- drivers/rtc/rtc-hibvt.c[239-250]
## Recommended Fix
Compute the counter time in 64-bit arithmetic before applying the minimum check, and use the 64-bit RTC time conversion when populating `rtc_time`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Users cannot read a time they just set~~ ✓ Resolved

Description

hibvt_rtc_read_time() rejects every counter value before 2000, while hibvt_rtc_set_time()
accepts and writes those dates. Setting a valid date such as 1999-12-31 therefore succeeds, but the
next read returns -EINVAL, including when the system tries to restore its clock from the RTC.

Code

drivers/rtc/rtc-hibvt.c[R255-256]
+	if (seconds < RTC_MIN_VALID_SEC)
+		return -EINVAL;

Evidence
The new comparison rejects seconds below the cutoff, but the set path writes dates without that
check. The RTC core accepts valid calendar dates and propagates read failures to its callers.
drivers/rtc/rtc-hibvt.c[248-259]
drivers/rtc/rtc-hibvt.c[263-278]
drivers/rtc/interface.c[58-79]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The driver accepts pre-2000 dates in set_time but rejects them in read_time, so a successful write cannot be read back.
## Fix Focus Areas
- drivers/rtc/rtc-hibvt.c[255-256]
- drivers/rtc/rtc-hibvt.c[263-278]
## Recommended Fix
Apply the same minimum-date policy to set_time, returning an error before writing a date that read_time will reject.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Initialization can hide a low-cell warning 🐞 Bug ◔ Observability

Description

hibvt_rtc_init() clears a low-voltage flag without warning, assuming registration already reported
it through hibvt_rtc_read_time(). If registration's initial status read fails, registration
continues and initialization can subsequently read and clear the flag; later time reads then have no
indication to report.

Code

drivers/rtc/rtc-hibvt.c[R529-533]

+	 * No warning here: devm_rtc_device_register() has already read the
+	 * time, so hibvt_rtc_read_time() reported the flag and cleared it.
+	 */
+	if (val & LV_INT_MASK)
+		hibvt_spi_write(rtc->regs, RTC_INT_CLR, LV_INT_MASK);

Evidence
Registration calls read_time first but continues if that call fails. The driver returns before
warning when its first status read fails; probe then runs initialization, whose changed path clears
a successfully read flag without checking whether it was reported.
drivers/rtc/rtc-hibvt.c[190-216]
drivers/rtc/interface.c[168-178]
drivers/rtc/class.c[221-244]
drivers/rtc/rtc-hibvt.c[525-533]
drivers/rtc/rtc-hibvt.c[564-574]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Initialization can clear the low-voltage flag when the registration-time read failed before reporting it.
## Fix Focus Areas
- drivers/rtc/rtc-hibvt.c[528-533]
- drivers/rtc/rtc-hibvt.c[210-216]
## Recommended Fix
When initialization sees the flag, emit the warning if lv_warned is false, set lv_warned, and then clear the flag.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can open the rule or relationship behind a finding in one click

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

openipc-ai · 2026-10-14T18:51:47Z

/review

qodo-free-for-open-source-projects · 2026-10-14T18:54:57Z

Code review by qodo was updated up to the latest commit afbd139

Port of the hisilicon-hi3516ev200 fix. This branch's copy kept the vendor's handling of the backup supply low-voltage flag: read_time() logs "low voltage detected" and returns -EINVAL whenever the flag is set. The flag is "cleared" by writing 1 to RTC_INT_CLR, though, and that is the alarm bit. So once the flag is set it never clears, every read fails and logs an error, and the RTC stays unreadable. The flag is not a reliable verdict anyway. On a hi3516ev300 it stays set, and returns about 0.5 s after each correct clear, with a fresh backup cell that keeps the time through a 5-minute power cut. - Do not fail the read, or log, because of the flag. Clear it with LV_INT_MASK. - Judge the time by the counter: return -EINVAL for a value before 2000-01-01, which is what a counter that lost power reads. set_time() refuses the same range, so every time it accepts can be read back.

set_time() collected the SPI write results in an unsigned char, so a bridge timeout (-EIO) came back as 251: RTC_SET_TIME reported success for a write that never reached the RTC. Every other function here already uses int.

openipc-ai · 2026-10-20T14:17:41Z

/review

qodo-free-for-open-source-projects · 2026-10-20T14:20:27Z

Code review by qodo was updated up to the latest commit 0e9e3dd

… counter (#125) arm: dts: hi3516cv300: let rtc-hibvt bind the RTC The RTC node said "hisilicon,hi_rtc", which no in-tree driver matches. rtc-hibvt matches "hisilicon,hi35xx-rtc" and drives the same block (SPI bridge at 0x12090000, the same register map as the SDK's hi3516cv300 RTC module), so with CONFIG_RTC_DRV_HIBVT the board still had no /dev/rtc0. That is OpenIPC/firmware#191: hwclock has no RTC to open, and the SDK module only offers private HI_RTC_* ioctls, so pointing /dev/rtc0 at it fails with EPERM. rtc: hibvt: judge the time by its counter, and leave hi3516cv300's CL… …K_CFG alone Port of the hisilicon-hi3536dv100 change (#81), which this branch's copy of the driver matches byte for byte: - read_time() no longer fails, or logs, because of the low-voltage flag. The flag was "cleared" by writing 1 to RTC_INT_CLR, which is the alarm bit, so once set it never cleared and the RTC stayed unreadable. It is not a verdict on the time anyway: on a hi3516ev300 it stays set with a backup cell that keeps the time. - A counter before 2000-01-01, which is what a counter that lost power reads, is reported as unset, so hctosys does not set the clock to 1970. set_time() refuses the same range. - The flag is cleared with LV_INT_MASK. On hi3516cv300, skip the RTC_CLK_CFG write. The SDK's own hi3516cv300 RTC driver never writes it, and it reads 0 on a running board; the 0x01 here is the hi3536cv100 value. rtc: hibvt: return set_time()'s SPI errors as errors set_time() collected the SPI write results in an unsigned char, so a bridge timeout (-EIO) came back as 251: RTC_SET_TIME reported success for a write that never reached the RTC. Every other function here already uses int.

openipc-ai mentioned this pull request Oct 13, 2026

rtc: hibvt: do not set the clock to 1970 from a counter that lost power #79

Merged

qodo-free-for-open-source-projects Bot reviewed Oct 14, 2026

View reviewed changes

Comment thread drivers/rtc/rtc-hibvt.c

Comment thread drivers/rtc/rtc-hibvt.c Outdated

openipc-ai force-pushed the rtc/hibvt-low-voltage-hisilicon-hi3536dv100 branch from afbd139 to 083acb3 Compare October 20, 2026 11:15

openipc-ai changed the title ~~rtc: hibvt: report a flat backup cell once, and judge the time by its counter~~ rtc: hibvt: judge the time by its counter, not the low-voltage flag Oct 20, 2026

This was referenced Oct 20, 2026

hi3516cv300: bind rtc-hibvt to the SoC RTC, and judge the time by its counter #125

Merged

hi3516cv300: give hwclock the SoC RTC as /dev/rtc0 OpenIPC/firmware#2643

Merged

rtc: hibvt: return set_time()'s SPI errors as errors

0e9e3dd

set_time() collected the SPI write results in an unsigned char, so a bridge timeout (-EIO) came back as 251: RTC_SET_TIME reported success for a write that never reached the RTC. Every other function here already uses int.

qodo-free-for-open-source-projects Bot reviewed Oct 20, 2026

View reviewed changes

Comment thread drivers/rtc/rtc-hibvt.c

openipc-ai merged commit 55ec0dd into hisilicon-hi3536dv100 Oct 20, 2026

Navigation Menu

Repository navigation

rtc: hibvt: judge the time by its counter, not the low-voltage flag - #81

rtc: hibvt: judge the time by its counter, not the low-voltage flag#81
openipc-ai merged 2 commits into
hisilicon-hi3536dv100from
rtc/hibvt-low-voltage-hisilicon-hi3536dv100

openipc-ai commented Oct 13, 2026 •

edited

Loading

Uh oh!

openipc-ai commented Oct 13, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 13, 2026 •

edited

Loading

Uh oh!

openipc-ai commented Oct 14, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 14, 2026

Uh oh!

openipc-ai commented Oct 20, 2026

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 20, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

openipc-ai commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change (the same as #79)

Verification

Review fix: set_time() errors

Uh oh!

openipc-ai commented Oct 13, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

openipc-ai commented Oct 14, 2026

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 14, 2026

Uh oh!

openipc-ai commented Oct 20, 2026

Uh oh!

Uh oh!

qodo-free-for-open-source-projects Bot commented Oct 20, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

openipc-ai commented Oct 13, 2026 •

edited

Loading

Review fix: `set_time()` errors

qodo-free-for-open-source-projects Bot commented Oct 13, 2026 •

edited

Loading