[SRU,Bionic,2/2] rtc: opal: Fix OPAL RTC driver OPAL_BUSY loops

Message ID bf40556d0150752c69acc266f00a96fa4cc00643.1527609708.git.joseph.salisbury@canonical.com
State New
Headers show
Series
  • Fixes for LP:1773964
Related show

Commit Message

Joseph Salisbury June 13, 2018, 3:10 p.m.
From: Nicholas Piggin <npiggin@gmail.com>

BugLink: http://bugs.launchpad.net/bugs/1773964

The OPAL RTC driver does not sleep in case it gets OPAL_BUSY or
OPAL_BUSY_EVENT from firmware, which causes large scheduling
latencies, up to 50 seconds have been observed here when RTC stops
responding (BMC reboot can do it).

Fix this by converting it to the standard form OPAL_BUSY loop that
sleeps.

Fixes: 628daa8d5abf ("powerpc/powernv: Add RTC and NVRAM support plus RTAS fallbacks")
Cc: stable@vger.kernel.org # v3.2+
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
(cherry picked from commit 682e6b4da5cbe8e9a53f979a58c2a9d7dc997175)
Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
---
 arch/powerpc/platforms/powernv/opal-rtc.c |  8 ++++---
 drivers/rtc/rtc-opal.c                    | 37 +++++++++++++++++++------------
 2 files changed, 28 insertions(+), 17 deletions(-)

Comments

Khaled Elmously June 18, 2018, 6:41 p.m. | #1
On 2018-06-13 11:10:08 , Joseph Salisbury wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1773964
> 
> The OPAL RTC driver does not sleep in case it gets OPAL_BUSY or
> OPAL_BUSY_EVENT from firmware, which causes large scheduling
> latencies, up to 50 seconds have been observed here when RTC stops
> responding (BMC reboot can do it).
> 
> Fix this by converting it to the standard form OPAL_BUSY loop that
> sleeps.
> 
> Fixes: 628daa8d5abf ("powerpc/powernv: Add RTC and NVRAM support plus RTAS fallbacks")
> Cc: stable@vger.kernel.org # v3.2+
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> (cherry picked from commit 682e6b4da5cbe8e9a53f979a58c2a9d7dc997175)
> Signed-off-by: Joseph Salisbury <joseph.salisbury@canonical.com>
> ---
>  arch/powerpc/platforms/powernv/opal-rtc.c |  8 ++++---
>  drivers/rtc/rtc-opal.c                    | 37 +++++++++++++++++++------------
>  2 files changed, 28 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal-rtc.c b/arch/powerpc/platforms/powernv/opal-rtc.c
> index f886886..aa2a513 100644
> --- a/arch/powerpc/platforms/powernv/opal-rtc.c
> +++ b/arch/powerpc/platforms/powernv/opal-rtc.c
> @@ -48,10 +48,12 @@ unsigned long __init opal_get_boot_time(void)
>  
>  	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>  		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
> -		if (rc == OPAL_BUSY_EVENT)
> +		if (rc == OPAL_BUSY_EVENT) {
> +			mdelay(OPAL_BUSY_DELAY_MS);
>  			opal_poll_events(NULL);
> -		else if (rc == OPAL_BUSY)
> -			mdelay(10);
> +		} else if (rc == OPAL_BUSY) {
> +			mdelay(OPAL_BUSY_DELAY_MS);
> +		}
>  	}
>  	if (rc != OPAL_SUCCESS)
>  		return 0;
> diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
> index 304e891..60f2250 100644
> --- a/drivers/rtc/rtc-opal.c
> +++ b/drivers/rtc/rtc-opal.c
> @@ -57,7 +57,7 @@ static void tm_to_opal(struct rtc_time *tm, u32 *y_m_d, u64 *h_m_s_ms)
>  
>  static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
>  {
> -	long rc = OPAL_BUSY;
> +	s64 rc = OPAL_BUSY;
>  	int retries = 10;
>  	u32 y_m_d;
>  	u64 h_m_s_ms;
> @@ -66,13 +66,17 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
>  
>  	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>  		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
> -		if (rc == OPAL_BUSY_EVENT)
> +		if (rc == OPAL_BUSY_EVENT) {
> +			msleep(OPAL_BUSY_DELAY_MS);
>  			opal_poll_events(NULL);
> -		else if (retries-- && (rc == OPAL_HARDWARE
> -				       || rc == OPAL_INTERNAL_ERROR))
> -			msleep(10);
> -		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
> -			break;
> +		} else if (rc == OPAL_BUSY) {
> +			msleep(OPAL_BUSY_DELAY_MS);
> +		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
> +			if (retries--) {
> +				msleep(10); /* Wait 10ms before retry */

I was expecting this msleep() to use the new OPAL_BUSY_DELAY_MS instead of a hardcoded 10 .. wasn't that the whole point?

> +				rc = OPAL_BUSY; /* go around again */
> +			}
> +		}
>  	}
>  
>  	if (rc != OPAL_SUCCESS)
> @@ -87,21 +91,26 @@ static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
>  
>  static int opal_set_rtc_time(struct device *dev, struct rtc_time *tm)
>  {
> -	long rc = OPAL_BUSY;
> +	s64 rc = OPAL_BUSY;
>  	int retries = 10;
>  	u32 y_m_d = 0;
>  	u64 h_m_s_ms = 0;
>  
>  	tm_to_opal(tm, &y_m_d, &h_m_s_ms);
> +
>  	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>  		rc = opal_rtc_write(y_m_d, h_m_s_ms);
> -		if (rc == OPAL_BUSY_EVENT)
> +		if (rc == OPAL_BUSY_EVENT) {
> +			msleep(OPAL_BUSY_DELAY_MS);
>  			opal_poll_events(NULL);
> -		else if (retries-- && (rc == OPAL_HARDWARE
> -				       || rc == OPAL_INTERNAL_ERROR))
> -			msleep(10);
> -		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
> -			break;
> +		} else if (rc == OPAL_BUSY) {
> +			msleep(OPAL_BUSY_DELAY_MS);
> +		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
> +			if (retries--) {
> +				msleep(10); /* Wait 10ms before retry */

Same as above.

> +				rc = OPAL_BUSY; /* go around again */
> +			}
> +		}
>  	}
>  
>  	return rc == OPAL_SUCCESS ? 0 : -EIO;
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

Patch

diff --git a/arch/powerpc/platforms/powernv/opal-rtc.c b/arch/powerpc/platforms/powernv/opal-rtc.c
index f886886..aa2a513 100644
--- a/arch/powerpc/platforms/powernv/opal-rtc.c
+++ b/arch/powerpc/platforms/powernv/opal-rtc.c
@@ -48,10 +48,12 @@  unsigned long __init opal_get_boot_time(void)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			mdelay(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (rc == OPAL_BUSY)
-			mdelay(10);
+		} else if (rc == OPAL_BUSY) {
+			mdelay(OPAL_BUSY_DELAY_MS);
+		}
 	}
 	if (rc != OPAL_SUCCESS)
 		return 0;
diff --git a/drivers/rtc/rtc-opal.c b/drivers/rtc/rtc-opal.c
index 304e891..60f2250 100644
--- a/drivers/rtc/rtc-opal.c
+++ b/drivers/rtc/rtc-opal.c
@@ -57,7 +57,7 @@  static void tm_to_opal(struct rtc_time *tm, u32 *y_m_d, u64 *h_m_s_ms)
 
 static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d;
 	u64 h_m_s_ms;
@@ -66,13 +66,17 @@  static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_read(&__y_m_d, &__h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
-			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	if (rc != OPAL_SUCCESS)
@@ -87,21 +91,26 @@  static int opal_get_rtc_time(struct device *dev, struct rtc_time *tm)
 
 static int opal_set_rtc_time(struct device *dev, struct rtc_time *tm)
 {
-	long rc = OPAL_BUSY;
+	s64 rc = OPAL_BUSY;
 	int retries = 10;
 	u32 y_m_d = 0;
 	u64 h_m_s_ms = 0;
 
 	tm_to_opal(tm, &y_m_d, &h_m_s_ms);
+
 	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
 		rc = opal_rtc_write(y_m_d, h_m_s_ms);
-		if (rc == OPAL_BUSY_EVENT)
+		if (rc == OPAL_BUSY_EVENT) {
+			msleep(OPAL_BUSY_DELAY_MS);
 			opal_poll_events(NULL);
-		else if (retries-- && (rc == OPAL_HARDWARE
-				       || rc == OPAL_INTERNAL_ERROR))
-			msleep(10);
-		else if (rc != OPAL_BUSY && rc != OPAL_BUSY_EVENT)
-			break;
+		} else if (rc == OPAL_BUSY) {
+			msleep(OPAL_BUSY_DELAY_MS);
+		} else if (rc == OPAL_HARDWARE || rc == OPAL_INTERNAL_ERROR) {
+			if (retries--) {
+				msleep(10); /* Wait 10ms before retry */
+				rc = OPAL_BUSY; /* go around again */
+			}
+		}
 	}
 
 	return rc == OPAL_SUCCESS ? 0 : -EIO;