Before reading date / time from the CMOS RTC, we wait for the UIP
(Update in progress) bit to clear --- so that the values are correct and
consistent. To avoid a hang, there is a time limit after which we give
up waiting.
Increase the time limit from 10 to 20ms in case there are RTCs out there
that are much slower than expected.
Note: This may cause problems with hpet_rtc_interrupt() if the CMOS RTC
breaks down while the system is running and RTC update interrupt / RTC
alarm interrupt happens to be enabled (which should be rare).
hpet_rtc_interrupt() is executed usually 64 times per second and after
this patch it may take up to 20ms to complete (always hitting RTC read
timeout) - which may constantly occupy one CPU. This looks very
unlikely, though.
Also fix "then" -> "than" in a comment.
Signed-off-by: Mateusz Jończyk <[email protected]>
Cc: Alessandro Zummo <[email protected]>
Cc: Alexandre Belloni <[email protected]>
Fixes: ec5895c0f2d8 ("rtc: mc146818-lib: extract mc146818_avoid_UIP")
---
This comment from Mr Alexandre Belloni got me thinking and is why I am
submitting this patch:
> We'll probably get some breakage later on because many RTCs using this
> driver are not adhering to the spec.
(See: https://lore.kernel.org/linux-rtc/[email protected]/T/ )
Googling for dmesg messages that indicate problems with reading from RTC
(such as "unable to read current time" - using quotation marks to force
exact phrase search) produces no results apart from kernel code and
patches. Any problems would happen rarely on affected systems, though.
drivers/rtc/rtc-mc146818-lib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
index ae9f131b43c0..29ceec9875f4 100644
--- a/drivers/rtc/rtc-mc146818-lib.c
+++ b/drivers/rtc/rtc-mc146818-lib.c
@@ -21,7 +21,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
unsigned long flags;
unsigned char seconds;
- for (i = 0; i < 10; i++) {
+ for (i = 0; i < 20; i++) {
spin_lock_irqsave(&rtc_lock, flags);
/*
@@ -79,8 +79,8 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
EXPORT_SYMBOL_GPL(mc146818_avoid_UIP);
/*
- * If the UIP (Update-in-progress) bit of the RTC is set for more then
- * 10ms, the RTC is apparently broken or not present.
+ * If the UIP (Update-in-progress) bit of the RTC is set for more than
+ * 20ms, the RTC is apparently broken or not present.
*/
bool mc146818_does_rtc_work(void)
{
base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
--
2.25.1
Hello,
On 12/02/2022 23:04:54+0100, Mateusz Jończyk wrote:
> Before reading date / time from the CMOS RTC, we wait for the UIP
> (Update in progress) bit to clear --- so that the values are correct and
> consistent. To avoid a hang, there is a time limit after which we give
> up waiting.
>
> Increase the time limit from 10 to 20ms in case there are RTCs out there
> that are much slower than expected.
>
I'm not going to apply that until we actually have reports that there
are issues because now, we can't know what is the benefit risk balance.
> Note: This may cause problems with hpet_rtc_interrupt() if the CMOS RTC
> breaks down while the system is running and RTC update interrupt / RTC
> alarm interrupt happens to be enabled (which should be rare).
> hpet_rtc_interrupt() is executed usually 64 times per second and after
> this patch it may take up to 20ms to complete (always hitting RTC read
> timeout) - which may constantly occupy one CPU. This looks very
> unlikely, though.
>
> Also fix "then" -> "than" in a comment.
>
> Signed-off-by: Mateusz Jończyk <[email protected]>
> Cc: Alessandro Zummo <[email protected]>
> Cc: Alexandre Belloni <[email protected]>
> Fixes: ec5895c0f2d8 ("rtc: mc146818-lib: extract mc146818_avoid_UIP")
> ---
>
> This comment from Mr Alexandre Belloni got me thinking and is why I am
> submitting this patch:
> > We'll probably get some breakage later on because many RTCs using this
> > driver are not adhering to the spec.
> (See: https://lore.kernel.org/linux-rtc/[email protected]/T/ )
>
> Googling for dmesg messages that indicate problems with reading from RTC
> (such as "unable to read current time" - using quotation marks to force
> exact phrase search) produces no results apart from kernel code and
> patches. Any problems would happen rarely on affected systems, though.
>
> drivers/rtc/rtc-mc146818-lib.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
> index ae9f131b43c0..29ceec9875f4 100644
> --- a/drivers/rtc/rtc-mc146818-lib.c
> +++ b/drivers/rtc/rtc-mc146818-lib.c
> @@ -21,7 +21,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
> unsigned long flags;
> unsigned char seconds;
>
> - for (i = 0; i < 10; i++) {
> + for (i = 0; i < 20; i++) {
> spin_lock_irqsave(&rtc_lock, flags);
>
> /*
> @@ -79,8 +79,8 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
> EXPORT_SYMBOL_GPL(mc146818_avoid_UIP);
>
> /*
> - * If the UIP (Update-in-progress) bit of the RTC is set for more then
> - * 10ms, the RTC is apparently broken or not present.
> + * If the UIP (Update-in-progress) bit of the RTC is set for more than
> + * 20ms, the RTC is apparently broken or not present.
> */
> bool mc146818_does_rtc_work(void)
> {
>
> base-commit: dfd42facf1e4ada021b939b4e19c935dcdd55566
> --
> 2.25.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com