[1/2] clocksource: timer-ti-dm: Do not restore context on every timer enable
diff mbox series

Message ID 20200224050753.17784-2-lokeshvutla@ti.com
State New
Headers show
Series
  • clocksource: timer-ti-dm: Misc fixes for omap dm timer
Related show

Commit Message

Lokesh Vutla Feb. 24, 2020, 5:07 a.m. UTC
omap_dm_timer_enable() restores the entire context(including counter)
based on 2 conditions:
- If get_context_loss_count is populated and context is lost.
- If get_context_loss_count is not populated update unconditionally.

Case2 has a side effect of updating the counter register even though
context is not lost. When timer is configured in pwm mode, this is
causing undesired behaviour in the pwm period. So restore context only
if get_context_loss_count is populated and context is actually lost.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/clocksource/timer-ti-dm.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Tony Lindgren Feb. 26, 2020, 5:09 p.m. UTC | #1
* Lokesh Vutla <lokeshvutla@ti.com> [200224 05:09]:
> omap_dm_timer_enable() restores the entire context(including counter)
> based on 2 conditions:
> - If get_context_loss_count is populated and context is lost.
> - If get_context_loss_count is not populated update unconditionally.
> 
> Case2 has a side effect of updating the counter register even though
> context is not lost. When timer is configured in pwm mode, this is
> causing undesired behaviour in the pwm period. So restore context only
> if get_context_loss_count is populated and context is actually lost.

Sounds like this will break things. We have get_context_loss_count()
only on omap4 and later, and even that was only partially implemented
from what I recall.

To mee it seems the right thing to do here is to save and restore
context on CPU_CLUSTER_PM_ENTER and CPU_CLUSTER_PM_EXIT. And I'd
just get rid of the partially implemented custom calls to
get_context_loss_count().

See for example a recent patch on removing the legacy SDMA code
for CPU_CLUSTER_PM_ENTER in commit d2f924879e19 ("thermal:
ti-soc-thermal:  Enable addition power management").

Then if we really need to add checks for context lost at some point,
that can be implemented via reset_control_status() returning -ENOLINK
or something similar.

Regards,

Tony

Patch
diff mbox series

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/timer-ti-dm.c
index 269a994d6a99..40742715ed21 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -229,15 +229,12 @@  static void omap_dm_timer_enable(struct omap_dm_timer *timer)
 
 	pm_runtime_get_sync(&timer->pdev->dev);
 
-	if (!(timer->capability & OMAP_TIMER_ALWON)) {
-		if (timer->get_context_loss_count) {
-			c = timer->get_context_loss_count(&timer->pdev->dev);
-			if (c != timer->ctx_loss_count) {
-				omap_timer_restore_context(timer);
-				timer->ctx_loss_count = c;
-			}
-		} else {
+	if (!(timer->capability & OMAP_TIMER_ALWON) &&
+	    timer->get_context_loss_count) {
+		c = timer->get_context_loss_count(&timer->pdev->dev);
+		if (c != timer->ctx_loss_count) {
 			omap_timer_restore_context(timer);
+			timer->ctx_loss_count = c;
 		}
 	}
 }