From: "Mateusz Jończyk" <mat.jonczyk@o2.pl>
To: linux-kernel@vger.kernel.org, linux-rtc@vger.kernel.org
Cc: "Mateusz Jończyk" <mat.jonczyk@o2.pl>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Alessandro Zummo" <a.zummo@towertech.it>,
"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Subject: [PATCH RESEND v2 3/6] rtc-mc146818-lib: fix RTC presence check
Date: Sun, 17 Oct 2021 21:39:24 +0200 [thread overview]
Message-ID: <20211017193927.277409-4-mat.jonczyk@o2.pl> (raw)
In-Reply-To: <20211017193927.277409-1-mat.jonczyk@o2.pl>
To prevent an infinite loop in mc146818_get_time(),
commit 211e5db19d15 ("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].
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.
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.
Fixes: 211e5db19d15 ("rtc: mc146818: Detect and handle broken RTCs")
Fixes: ebb22a059436 ("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>
---
drivers/rtc/rtc-cmos.c | 10 ++++------
drivers/rtc/rtc-mc146818-lib.c | 34 ++++++++++++++++++++++++++++++----
include/linux/mc146818rtc.h | 1 +
3 files changed, 35 insertions(+), 10 deletions(-)
v2: Tweak commit description, remove "Cc: stable" (I'll send it manually
after more regression testing).
diff --git a/drivers/rtc/rtc-cmos.c b/drivers/rtc/rtc-cmos.c
index 643433d984ab..f353a41dfe8c 100644
--- a/drivers/rtc/rtc-cmos.c
+++ b/drivers/rtc/rtc-cmos.c
@@ -833,16 +833,14 @@ cmos_do_probe(struct device *dev, struct resource *ports, int rtc_irq)
rename_region(ports, dev_name(&cmos_rtc.rtc->dev));
- spin_lock_irq(&rtc_lock);
-
- /* Ensure that the RTC is accessible. Bit 6 must be 0! */
- if ((CMOS_READ(RTC_VALID) & 0x40) != 0) {
- spin_unlock_irq(&rtc_lock);
- dev_warn(dev, "not accessible\n");
+ if (!mc146818_does_rtc_work()) {
+ dev_warn(dev, "broken or not accessible\n");
retval = -ENXIO;
goto cleanup1;
}
+ spin_lock_irq(&rtc_lock);
+
if (!(flags & CMOS_RTC_FLAGS_NOFREQ)) {
/* force periodic irq to CMOS reset default of 1024Hz;
*
diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
index dcfaf09946ee..9175e11baf26 100644
--- a/drivers/rtc/rtc-mc146818-lib.c
+++ b/drivers/rtc/rtc-mc146818-lib.c
@@ -8,10 +8,35 @@
#include <linux/acpi.h>
#endif
+/*
+ * If the UIP (Update-in-progress) bit of the RTC is set for more then
+ * 10ms, the RTC apparently is broken or not present.
+ */
+unsigned int mc146818_does_rtc_work(void)
+{
+ int i;
+ unsigned char val;
+ unsigned long flags;
+
+ for (i = 0; i < 20; i++) {
+ spin_lock_irqsave(&rtc_lock, flags);
+ val = CMOS_READ(RTC_FREQ_SELECT);
+ spin_unlock_irqrestore(&rtc_lock, flags);
+
+ if ((val & RTC_UIP) == 0)
+ return 1;
+
+ udelay(500);
+ }
+
+ return 0;
+}
+
unsigned int mc146818_get_time(struct rtc_time *time)
{
unsigned char ctrl;
unsigned long flags;
+ unsigned int iter_count = 0;
unsigned char century = 0;
bool retry;
@@ -20,13 +45,14 @@ unsigned int mc146818_get_time(struct rtc_time *time)
#endif
again:
- spin_lock_irqsave(&rtc_lock, flags);
- /* Ensure that the RTC is accessible. Bit 6 must be 0! */
- if (WARN_ON_ONCE((CMOS_READ(RTC_VALID) & 0x40) != 0)) {
- spin_unlock_irqrestore(&rtc_lock, flags);
+ if (iter_count > 10) {
+ pr_err_ratelimited("Unable to read current time from RTC\n");
memset(time, 0xff, sizeof(*time));
return 0;
}
+ iter_count++;
+
+ spin_lock_irqsave(&rtc_lock, flags);
/*
* Check whether there is an update in progress during which the
diff --git a/include/linux/mc146818rtc.h b/include/linux/mc146818rtc.h
index 0661af17a758..046a03df1e56 100644
--- a/include/linux/mc146818rtc.h
+++ b/include/linux/mc146818rtc.h
@@ -123,6 +123,7 @@ struct cmos_rtc_board_info {
#define RTC_IO_EXTENT_USED RTC_IO_EXTENT
#endif /* ARCH_RTC_LOCATION */
+unsigned int mc146818_does_rtc_work(void);
unsigned int mc146818_get_time(struct rtc_time *time);
int mc146818_set_time(struct rtc_time *time);
--
2.25.1
next prev parent reply other threads:[~2021-10-17 19:40 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-17 19:39 [PATCH RESEND v2 0/6] rtc,x86: CMOS RTC fixes Mateusz Jończyk
2021-10-17 19:39 ` [PATCH RESEND 1/6] rtc-cmos: take rtc_lock while reading from CMOS Mateusz Jończyk
2021-10-17 19:39 ` [PATCH RESEND 2/6] rtc-cmos: dont touch alarm registers during update Mateusz Jończyk
2021-10-17 19:41 ` [TEST PATCH] rtc-cmos: cmos_read_alarm bug demonstration Mateusz Jończyk
2021-10-18 5:58 ` [PATCH RESEND 2/6] rtc-cmos: dont touch alarm registers during update Greg KH
2021-10-17 19:39 ` Mateusz Jończyk [this message]
2021-10-17 19:39 ` [PATCH RESEND 4/6] rtc-mc146818-lib: reduce RTC_UIP polling period Mateusz Jończyk
2021-10-17 19:39 ` [PATCH RESEND 5/6] x86/rtc: mach_get_cmos_time - rm duplicated code Mateusz Jończyk
2021-10-17 19:39 ` [PATCH RESEND 6/6] x86/rtc: rename mach_set_rtc_mmss Mateusz Jończyk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211017193927.277409-4-mat.jonczyk@o2.pl \
--to=mat.jonczyk@o2.pl \
--cc=a.zummo@towertech.it \
--cc=alexandre.belloni@bootlin.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox