Skip to content

Commit ea6fa49

Browse files
matjonalexandrebelloni
authored andcommitted
rtc: mc146818-lib: fix RTC presence check
To prevent an infinite loop in mc146818_get_time(), commit 211e5db ("rtc: mc146818: Detect and handle broken RTCs") added a check for RTC availability. Together with a later fix, it checked if bit 6 in register 0x0d is cleared. This, however, caused a false negative on a motherboard with an AMD SB710 southbridge; according to the specification [1], bit 6 of register 0x0d of this chipset is a scratchbit. This caused a regression in Linux 5.11 - the RTC was determined broken by the kernel and not used by rtc-cmos.c [3]. This problem was also reported in Fedora [4]. As a better alternative, check whether the UIP ("Update-in-progress") bit is set for longer then 10ms. If that is the case, then apparently the RTC is either absent (and all register reads return 0xff) or broken. Also limit the number of loop iterations in mc146818_get_time() to 10 to prevent an infinite loop there. The functions mc146818_get_time() and mc146818_does_rtc_work() will be refactored later in this patch series, in order to fix a separate problem with reading / setting the RTC alarm time. This is done so to avoid a confusion about what is being fixed when. In a previous approach to this problem, I implemented a check whether the RTC_HOURS register contains a value <= 24. This, however, sometimes did not work correctly on my Intel Kaby Lake laptop. According to Intel's documentation [2], "the time and date RAM locations (0-9) are disconnected from the external bus" during the update cycle so reading this register without checking the UIP bit is incorrect. [1] AMD SB700/710/750 Register Reference Guide, page 308, https://developer.amd.com/wordpress/media/2012/10/43009_sb7xx_rrg_pub_1.00.pdf [2] 7th Generation Intel ® Processor Family I/O for U/Y Platforms [...] Datasheet Volume 1 of 2, page 209 Intel's Document Number: 334658-006, https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/7th-and-8th-gen-core-family-mobile-u-y-processor-lines-i-o-datasheet-vol-1.pdf [3] Functions in arch/x86/kernel/rtc.c apparently were using it. [4] https://bugzilla.redhat.com/show_bug.cgi?id=1936688 Fixes: 211e5db ("rtc: mc146818: Detect and handle broken RTCs") Fixes: ebb22a0 ("rtc: mc146818: Dont test for bit 0-5 in Register D") Signed-off-by: Mateusz Jończyk <mat.jonczyk@o2.pl> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com> Link: https://lore.kernel.org/r/20211210200131.153887-5-mat.jonczyk@o2.pl
1 parent 0dd8d6c commit ea6fa49

File tree

3 files changed

+35
-10
lines changed

3 files changed

+35
-10
lines changed

drivers/rtc/rtc-cmos.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -800,16 +800,14 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
800800

801801
rename_region(ports, dev_name(&cmos_rtc.rtc->dev));
802802

803-
spin_lock_irq(&rtc_lock);
804-
805-
/* Ensure that the RTC is accessible. Bit 6 must be 0! */
806-
if ((CMOS_READ(RTC_VALID) & 0x40) != 0) {
807-
spin_unlock_irq(&rtc_lock);
808-
dev_warn(dev, "not accessible\n");
803+
if (!mc146818_does_rtc_work()) {
804+
dev_warn(dev, "broken or not accessible\n");
809805
retval = -ENXIO;
810806
goto cleanup1;
811807
}
812808

809+
spin_lock_irq(&rtc_lock);
810+
813811
if (!(flags & CMOS_RTC_FLAGS_NOFREQ)) {
814812
/* force periodic irq to CMOS reset default of 1024Hz;
815813
*

drivers/rtc/rtc-mc146818-lib.c

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,36 @@
88
#include <linux/acpi.h>
99
#endif
1010

11+
/*
12+
* If the UIP (Update-in-progress) bit of the RTC is set for more then
13+
* 10ms, the RTC is apparently broken or not present.
14+
*/
15+
bool mc146818_does_rtc_work(void)
16+
{
17+
int i;
18+
unsigned char val;
19+
unsigned long flags;
20+
21+
for (i = 0; i < 10; i++) {
22+
spin_lock_irqsave(&rtc_lock, flags);
23+
val = CMOS_READ(RTC_FREQ_SELECT);
24+
spin_unlock_irqrestore(&rtc_lock, flags);
25+
26+
if ((val & RTC_UIP) == 0)
27+
return true;
28+
29+
mdelay(1);
30+
}
31+
32+
return false;
33+
}
34+
EXPORT_SYMBOL_GPL(mc146818_does_rtc_work);
35+
1136
unsigned int mc146818_get_time(struct rtc_time *time)
1237
{
1338
unsigned char ctrl;
1439
unsigned long flags;
40+
unsigned int iter_count = 0;
1541
unsigned char century = 0;
1642
bool retry;
1743

@@ -20,13 +46,13 @@ unsigned int mc146818_get_time(struct rtc_time *time)
2046
#endif
2147

2248
again:
23-
spin_lock_irqsave(&rtc_lock, flags);
24-
/* Ensure that the RTC is accessible. Bit 6 must be 0! */
25-
if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x40) != 0)) {
26-
spin_unlock_irqrestore(&rtc_lock, flags);
49+
if (iter_count > 10) {
2750
memset(time, 0, sizeof(*time));
2851
return -EIO;
2952
}
53+
iter_count++;
54+
55+
spin_lock_irqsave(&rtc_lock, flags);
3056

3157
/*
3258
* Check whether there is an update in progress during which the

include/linux/mc146818rtc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,7 @@ struct cmos_rtc_board_info {
123123
#define RTC_IO_EXTENT_USED RTC_IO_EXTENT
124124
#endif /* ARCH_RTC_LOCATION */
125125

126+
bool mc146818_does_rtc_work(void);
126127
unsigned int mc146818_get_time(struct rtc_time *time);
127128
int mc146818_set_time(struct rtc_time *time);
128129

0 commit comments

Comments
 (0)