Re: [PATCH v3 3/4] rtc: Add support for configuring the UIP timeout for RTC reads

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/27/2023 14:31, Mateusz Jończyk wrote:
W dniu 27.11.2023 o 20:25, Mario Limonciello pisze:
The UIP timeout is hardcoded to 10ms for all RTC reads, but in some
contexts this might not be enough time. Add a timeout parameter to
mc146818_get_time() and mc146818_get_time_callback().

If UIP timeout is configured by caller to be >=100 ms and a call
takes this long, log a warning.

Make all callers use 10ms to ensure no functional changes.

Cc: stable@xxxxxxxxxxxxxxx # 6.1.y
Fixes: ec5895c0f2d8 ("rtc: mc146818-lib: extract mc146818_avoid_UIP")
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v2->v3:
  * Logic adjustments
  * Clarify warning message
v1->v2:
  * Add a warning if 100ms or more
  * Add stable and fixes tags
[snip]
diff --git a/drivers/rtc/rtc-mc146818-lib.c b/drivers/rtc/rtc-mc146818-lib.c
index 43a28e82674e..ab077dde397b 100644
--- a/drivers/rtc/rtc-mc146818-lib.c
+++ b/drivers/rtc/rtc-mc146818-lib.c
@@ -8,26 +8,31 @@
  #include <linux/acpi.h>
  #endif
+#define UIP_RECHECK_DELAY 100 /* usec */
+#define UIP_RECHECK_DELAY_MS		(USEC_PER_MSEC / UIP_RECHECK_DELAY)
+#define UIP_RECHECK_TIMEOUT_MS(x)	(x / UIP_RECHECK_DELAY_MS)
+
  /*
   * Execute a function while the UIP (Update-in-progress) bit of the RTC is
- * unset.
+ * unset. The timeout is configurable by the caller in ms.
   *
   * Warning: callback may be executed more then once.
   */
  bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
+			int timeout,
  			void *param)
  {
  	int i;
  	unsigned long flags;
  	unsigned char seconds;
- for (i = 0; i < 100; i++) {
+	for (i = 0; i < UIP_RECHECK_TIMEOUT_MS(timeout); i++) {

Sorry, this will not work. UIP_RECHECK_DELAY_MS is 10, so
UIP_RECHECK_TIMEOUT_MS(timeout) will be 1 for timeout=10. Should be

       for (i = 0; UIP_RECHECK_TIMEOUT_MS(i) < timeout; i++) {

With this, for i == 99, UIP_RECHECK_TIMEOUT_MS(i) = 9
for i == 100, UIP_RECHECK_TIMEOUT_MS(i) = 10 and the loop correctly terminates.

The macro should probably be renamed UIP_RECHECK_LOOPS_MS as it converts
loop count to ms.

Got it; thanks for that.


  		spin_lock_irqsave(&rtc_lock, flags);
/*
  		 * Check whether there is an update in progress during which the
  		 * readout is unspecified. The maximum update time is ~2ms. Poll
-		 * every 100 usec for completion.
+		 * for completion.
  		 *
  		 * Store the second value before checking UIP so a long lasting
  		 * NMI which happens to hit after the UIP check cannot make
@@ -37,7 +42,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
  			spin_unlock_irqrestore(&rtc_lock, flags);
-			udelay(100);
+			udelay(UIP_RECHECK_DELAY);
  			continue;
  		}
@@ -56,7 +61,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
  		 */
  		if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
  			spin_unlock_irqrestore(&rtc_lock, flags);
-			udelay(100);
+			udelay(UIP_RECHECK_DELAY);
  			continue;
  		}
@@ -72,6 +77,10 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
  		}
  		spin_unlock_irqrestore(&rtc_lock, flags);
+ if (i >= UIP_RECHECK_TIMEOUT_MS(100))

Same, should be:

           if (UIP_RECHECK_TIMEOUT_MS(i) >= 100)

+			pr_warn("Reading current time from RTC took around %d ms\n",
+				UIP_RECHECK_TIMEOUT_MS(i));
+
  		return true;
  	}
  	return false;

[snip]

Greetings,

Mateusz







[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux