Skip to contentSkip to conversation

clk: ti: clkctrl: Fix failed to enable error with double udelay timeout

PATCH Applied Tony Lindgren 4 participants 5 messages Sep 30, 2019 – Nov 4, 2019

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 clk: ti: clkctrl: Fix failed to enable error with double udelay timeout

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") added handling for cases when timekeeping is suspended. But looks like we can still get occasional "failed to enable" errors on the PM runtime resume path with udelay() returning faster than expected.

With ti-sysc interconnect target module driver this leads into device failure with PM runtime failing with "failed to enable" clkctrl error.

Let's fix the issue with a delay of two times the desired delay as in often done for udelay() to account for the inaccuracy.

Fixes 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc Keerthy <[email protected]>
Cc Tero Kristo <[email protected]>
---
 drivers/clk/ti/clkctrl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/clk/ti/clkctrl.c +3 -2
@@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
 	 * can be from a timer that requires pm_runtime access, which
 	 * will eventually bring us here with timekeeping_suspended,
 	 * during both suspend entry and resume paths. This happens
-	 * at least on am43xx platform.
+	 * at least on am43xx platform. Account for flakeyness
+	 * with udelay() by multiplying the timeout value by 2.
 	 */
 	if (unlikely(_early_timeout || timekeeping_suspended)) {
 		if (time->cycles++ < timeout) {
-			udelay(1);
+			udelay(1 * 2);
 			return false;
 		}
 	} else {
Signature · 1 line
2.23.0
Tested-by on v1 1/1
#1 lore raw
Quoted 11 lines · Tony Lindgren

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") added handling for cases when timekeeping is suspended. But looks like we can still get occasional "failed to enable" errors on the PM runtime resume path with udelay() returning faster than expected.

With ti-sysc interconnect target module driver this leads into device failure with PM runtime failing with "failed to enable" clkctrl error.

Let's fix the issue with a delay of two times the desired delay as in often done for udelay() to account for the inaccuracy.

Tested for DS0 and rtc+ddr modes on am43 and ds0 on am33.

Quoted 28 lines
Fixes 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc Keerthy <[email protected]>
Cc Tero Kristo <[email protected]>
---
  drivers/clk/ti/clkctrl.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
drivers/clk/ti/clkctrl.c +3 -2
@@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
  	 * can be from a timer that requires pm_runtime access, which
  	 * will eventually bring us here with timekeeping_suspended,
  	 * during both suspend entry and resume paths. This happens
-	 * at least on am43xx platform.
+	 * at least on am43xx platform. Account for flakeyness
+	 * with udelay() by multiplying the timeout value by 2.
  	 */
  	if (unlikely(_early_timeout || timekeeping_suspended)) {
  		if (time->cycles++ < timeout) {
-			udelay(1);
+			udelay(1 * 2);
  			return false;
  		}
  	} else {
#2 lore raw
Quoted 12 lines · Keerthy
Quoted 9 lines · Tony Lindgren

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") added handling for cases when timekeeping is suspended. But looks like we can still get occasional "failed to enable" errors on the PM runtime resume path with udelay() returning faster than expected.

With ti-sysc interconnect target module driver this leads into device failure with PM runtime failing with "failed to enable" clkctrl error.

Quoted 6 lines
Quoted 2 lines

Let's fix the issue with a delay of two times the desired delay as in often done for udelay() to account for the inaccuracy.

Tested for DS0 and rtc+ddr modes on am43 and ds0 on am33.

Thanks for testing. This one should be applied into v5.4-rc series please if no more comments.

Regards,

Tony

Quoted 27 lines
Quoted 27 lines
Fixes 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc Keerthy <[email protected]>
Cc Tero Kristo <[email protected]>
---
  drivers/clk/ti/clkctrl.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
drivers/clk/ti/clkctrl.c +3 -2
@@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
  	 * can be from a timer that requires pm_runtime access, which
  	 * will eventually bring us here with timekeeping_suspended,
  	 * during both suspend entry and resume paths. This happens
-	 * at least on am43xx platform.
+	 * at least on am43xx platform. Account for flakeyness
+	 * with udelay() by multiplying the timeout value by 2.
  	 */
  	if (unlikely(_early_timeout || timekeeping_suspended)) {
  		if (time->cycles++ < timeout) {
-			udelay(1);
+			udelay(1 * 2);
  			return false;
  		}
  	} else {
Applied
#3 lore raw
Quoted 16 lines · Tony Lindgren
Quoted 15 lines · Keerthy
Quoted 11 lines · Tony Lindgren

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") added handling for cases when timekeeping is suspended. But looks like we can still get occasional "failed to enable" errors on the PM runtime resume path with udelay() returning faster than expected.

With ti-sysc interconnect target module driver this leads into device failure with PM runtime failing with "failed to enable" clkctrl error.

Let's fix the issue with a delay of two times the desired delay as in often done for udelay() to account for the inaccuracy.

Quoted 6 lines
Quoted 3 lines

Tested for DS0 and rtc+ddr modes on am43 and ds0 on am33.

Thanks for testing. This one should be applied into v5.4-rc series please if no more comments.

Queued up for 5.4 fixes, thanks.

-Tero

Quoted 24 lines

Regards,

Tony

Quoted 19 lines
Quoted 19 lines
Fixes 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc Keerthy <[email protected]>
Cc Tero Kristo <[email protected]>
---
   drivers/clk/ti/clkctrl.c | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)
drivers/clk/ti/clkctrl.c +2 -1
@@ -100,11 +100,12 @@ static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout)
   	 * can be from a timer that requires pm_runtime access, which
   	 * will eventually bring us here with timekeeping_suspended,
   	 * during both suspend entry and resume paths. This happens
-	 * at least on am43xx platform.
+	 * at least on am43xx platform. Account for flakeyness
+	 * with udelay() by multiplying the timeout value by 2.
   	 */
Quoted 8 lines
Quoted 8 lines

drivers/clk/ti/clkctrl.c static bool _omap4_is_timeout(union omap4_timeout *time, u32 timeout) · line 111

Quoted 8 lines
drivers/clk/ti/clkctrl.c +1 -1
   	if (unlikely(_early_timeout || timekeeping_suspended)) {
   		if (time->cycles++ < timeout) {
-			udelay(1);
+			udelay(1 * 2);
   			return false;
   		}
   	} else {

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

maintainer Applied
#4 lore raw
In reply to Tony Lindgren

Quoting Tony Lindgren (2019-09-30 08:40:01)

Quoted 9 lines

Commit 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended") added handling for cases when timekeeping is suspended. But looks like we can still get occasional "failed to enable" errors on the PM runtime resume path with udelay() returning faster than expected.

With ti-sysc interconnect target module driver this leads into device failure with PM runtime failing with "failed to enable" clkctrl error.

Quoted 8 lines

Let's fix the issue with a delay of two times the desired delay as in often done for udelay() to account for the inaccuracy.

Fixes 3d8598fb9c5a ("clk: ti: clkctrl: use fallback udelay approach if timekeeping is suspended")
Cc Keerthy <[email protected]>
Cc Tero Kristo <[email protected]>

---

Applied to clk-fixes