[PATCH v4 2/6] x86/time: move CMOS edge detection into read helper

Roger Pau Monne posted 6 patches 3 months ago
There is a newer version of this series
[PATCH v4 2/6] x86/time: move CMOS edge detection into read helper
Posted by Roger Pau Monne 3 months ago
Move the logic that ensures the CMOS RTC data is read just after it's been
updated into the __get_cmos_time() function that does the register reads.  This
requires returning a boolean from __get_cmos_time() to signal whether the read
has been successfully performed after an update.

Note that while __get_cmos_time() can be used without waiting for the update
edge, so far the only caller does wait for it, hence move the code inside of
the function.

The goal, albeit not accomplished by this patch, is to be able to split the
probing and the reading of the CMOS RTC data into two separate functions.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v3:
 - Reduce a bit the locked section.

Changes since v2:
 - New in this version.
---
 xen/arch/x86/time.c | 50 +++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 1959cc4a4f2b..571e23431ccd 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1246,8 +1246,26 @@ struct rtc_time {
     unsigned int year, mon, day, hour, min, sec;
 };
 
-static void __get_cmos_time(struct rtc_time *rtc)
+static bool __get_cmos_time(struct rtc_time *rtc)
 {
+    s_time_t start, t1, t2;
+    unsigned long flags;
+
+    spin_lock_irqsave(&rtc_lock, flags);
+
+    /* read RTC exactly on falling edge of update flag */
+    start = NOW();
+    do { /* may take up to 1 second... */
+        t1 = NOW() - start;
+    } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+              t1 <= SECONDS(1) );
+
+    start = NOW();
+    do { /* must try at least 2.228 ms */
+        t2 = NOW() - start;
+    } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
+              t2 < MILLISECS(3) );
+
     rtc->sec  = CMOS_READ(RTC_SECONDS);
     rtc->min  = CMOS_READ(RTC_MINUTES);
     rtc->hour = CMOS_READ(RTC_HOURS);
@@ -1265,13 +1283,17 @@ static void __get_cmos_time(struct rtc_time *rtc)
         BCD_TO_BIN(rtc->year);
     }
 
+    spin_unlock_irqrestore(&rtc_lock, flags);
+
     if ( (rtc->year += 1900) < 1970 )
         rtc->year += 100;
+
+    return t1 <= SECONDS(1) && t2 < MILLISECS(3);
 }
 
 static unsigned long get_cmos_time(void)
 {
-    unsigned long res, flags;
+    unsigned long res;
     struct rtc_time rtc;
     unsigned int seconds = 60;
     static bool __read_mostly cmos_rtc_probe;
@@ -1292,29 +1314,9 @@ static unsigned long get_cmos_time(void)
 
     for ( ; ; )
     {
-        s_time_t start, t1, t2;
-
-        spin_lock_irqsave(&rtc_lock, flags);
-
-        /* read RTC exactly on falling edge of update flag */
-        start = NOW();
-        do { /* may take up to 1 second... */
-            t1 = NOW() - start;
-        } while ( !(CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
-                  t1 <= SECONDS(1) );
-
-        start = NOW();
-        do { /* must try at least 2.228 ms */
-            t2 = NOW() - start;
-        } while ( (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) &&
-                  t2 < MILLISECS(3) );
-
-        __get_cmos_time(&rtc);
-
-        spin_unlock_irqrestore(&rtc_lock, flags);
+        bool success = __get_cmos_time(&rtc);
 
-        if ( likely(!cmos_rtc_probe) ||
-             t1 > SECONDS(1) || t2 >= MILLISECS(3) ||
+        if ( likely(!cmos_rtc_probe) || !success ||
              rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
              !rtc.day || rtc.day > 31 ||
              !rtc.mon || rtc.mon > 12 )
-- 
2.46.0


Re: [PATCH v4 2/6] x86/time: move CMOS edge detection into read helper
Posted by Jan Beulich 2 months, 4 weeks ago
On 04.09.2024 17:31, Roger Pau Monne wrote:
> Move the logic that ensures the CMOS RTC data is read just after it's been
> updated into the __get_cmos_time() function that does the register reads.  This
> requires returning a boolean from __get_cmos_time() to signal whether the read
> has been successfully performed after an update.
> 
> Note that while __get_cmos_time() can be used without waiting for the update
> edge, so far the only caller does wait for it, hence move the code inside of
> the function.
> 
> The goal, albeit not accomplished by this patch, is to be able to split the
> probing and the reading of the CMOS RTC data into two separate functions.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>