[PATCH 0/8] staging: tidspbridge: PM routines cleanup

[PATCH 0/8] staging: tidspbridge: PM routines cleanup

[Omar Ramirez Luna]

Cleanup and fixes for tidspbridge's PM routines.

The most significant ones are:
- Protection of critical sections when disabling clocks which
  could cause issues during PM transitions.
- Fix of resume path from RET power state.

Other patches are cleanups and preparations for the usage of
spinlock.

Omar Ramirez Luna (8):
  staging: tidspbridge: make wake_dsp to handle PM code
  staging: tidspbridge: fix resume path from retention
  staging: tidspbridge: remove msleep for dsp transition wait
  staging: tidspbridge: send mbox PM command directly
  staging: tidspbridge: send wake message even if dsp is running
  staging: tidspbridge: remove redundant code from PM routines
  staging: tidspbridge: remove redundant indentation in PM routines
  staging: tidspbridge: protect critical sections on PM routines

 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |  248 ++++++++++++++-------
 drivers/staging/tidspbridge/core/tiomap_io.c      |   58 +-----
 2 files changed, 168 insertions(+), 138 deletions(-)

[PATCH 1/8] staging: tidspbridge: make wake_dsp to handle PM code

[Omar Ramirez Luna]

Instead of being a wrapper around sm_interrupt_dsp, make wake_dsp
to be the one actually waking up the dsp and handling clock
configurations. No impact for sm_interrupt_dsp since it now calls
wake_dsp and that function decides what to do based on internal
states.

This should help in encapsulating code capable of sleep/wake the
dsp into a single place.

While at it, change 'if' blocks to a 'switch'; and use
omap_mbox_msg_send to send the wake up command to the dsp.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   72 ++++++++++++++++++--
 drivers/staging/tidspbridge/core/tiomap_io.c      |   58 +---------------
 2 files changed, 68 insertions(+), 62 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index 02dd439..d5245a3 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -256,20 +256,76 @@ int wake_dsp(struct bridge_dev_context *dev_context, void *pargs)
 {
 	int status = 0;
 #ifdef CONFIG_PM
+#ifdef CONFIG_TIDSPBRIDGE_DVFS
+	u32 opplevel = 0;
+#endif
+	struct omap_dsp_platform_data *pdata =
+		omap_dspbridge_dev->dev.platform_data;
+	struct cfg_hostres *resources = dev_context->resources;
 
-	/* Check the board state, if it is not 'SLEEP' then return */
-	if (dev_context->brd_state == BRD_RUNNING ||
-	    dev_context->brd_state == BRD_STOPPED) {
-		/* The Device is in 'RET' or 'OFF' state and Bridge state is not
-		 * 'SLEEP', this means state inconsistency, so return */
+	if (!dev_context->mbox || !resources)
+		return -EPERM;
+
+	switch (dev_context->brd_state) {
+	case BRD_STOPPED:
+	case BRD_RUNNING:
 		return 0;
+	case BRD_RETENTION:
+		/* Restart the peripheral clocks */
+		dsp_clock_enable_all(dev_context->dsp_per_clks);
+
+		break;
+	case BRD_HIBERNATION:
+	case BRD_DSP_HIBERNATION:
+#ifdef CONFIG_TIDSPBRIDGE_DVFS
+		if (pdata->dsp_get_opp)
+			opplevel = (*pdata->dsp_get_opp) ();
+		if (opplevel == VDD1_OPP1) {
+			if (pdata->dsp_set_min_opp)
+				(*pdata->dsp_set_min_opp) (VDD1_OPP2);
+		}
+#endif
+		/* Restart the peripheral clocks */
+		dsp_clock_enable_all(dev_context->dsp_per_clks);
+
+		dsp_wdt_enable(true);
+
+		/*
+		 * 2:0 AUTO_IVA2_DPLL - Enabling IVA2 DPLL auto control
+		 *     in CM_AUTOIDLE_PLL_IVA2 register
+		 */
+		(*pdata->dsp_cm_write)(1 << OMAP3430_AUTO_IVA2_DPLL_SHIFT,
+				OMAP3430_IVA2_MOD, OMAP3430_CM_AUTOIDLE_PLL);
+
+		/*
+		 * 7:4 IVA2_DPLL_FREQSEL - IVA2 internal frq set to
+		 *     0.75 MHz - 1.0 MHz
+		 * 2:0 EN_IVA2_DPLL - Enable IVA2 DPLL in lock mode
+		 */
+		(*pdata->dsp_cm_rmw_bits)(OMAP3430_IVA2_DPLL_FREQSEL_MASK |
+				OMAP3430_EN_IVA2_DPLL_MASK,
+				0x3 << OMAP3430_IVA2_DPLL_FREQSEL_SHIFT |
+				0x7 << OMAP3430_EN_IVA2_DPLL_SHIFT,
+				OMAP3430_IVA2_MOD, OMAP3430_CM_CLKEN_PLL);
+
+		/* Restore mailbox settings */
+		omap_mbox_restore_ctx(dev_context->mbox);
+
+		/* Access MMU SYS CONFIG register to generate a short wakeup */
+		readl(resources->dmmu_base + 0x10);
+
+		dev_context->brd_state = BRD_RUNNING;
+
+		break;
+	default:
+		pr_err("%s: unexpected state %x\n", __func__,
+						dev_context->brd_state);
+		return -EINVAL;
 	}
 
 	/* Send a wakeup message to DSP */
-	sm_interrupt_dsp(dev_context, MBX_PM_DSPWAKEUP);
+	status = omap_mbox_msg_send(dev_context->mbox, MBX_PM_DSPWAKEUP);
 
-	/* Set the device state to RUNNIG */
-	dev_context->brd_state = BRD_RUNNING;
 #endif /* CONFIG_PM */
 	return status;
 }
diff --git a/drivers/staging/tidspbridge/core/tiomap_io.c b/drivers/staging/tidspbridge/core/tiomap_io.c
index dfb356e..ff350e5 100644
--- a/drivers/staging/tidspbridge/core/tiomap_io.c
+++ b/drivers/staging/tidspbridge/core/tiomap_io.c
@@ -386,64 +386,14 @@ int write_ext_dsp_data(struct bridge_dev_context *dev_context,
 
 int sm_interrupt_dsp(struct bridge_dev_context *dev_context, u16 mb_val)
 {
-#ifdef CONFIG_TIDSPBRIDGE_DVFS
-	u32 opplevel = 0;
-#endif
-	struct omap_dsp_platform_data *pdata =
-		omap_dspbridge_dev->dev.platform_data;
-	struct cfg_hostres *resources = dev_context->resources;
-	int status = 0;
-	u32 temp;
+	int status;
 
 	if (!dev_context->mbox)
 		return 0;
 
-	if (!resources)
-		return -EPERM;
-
-	if (dev_context->brd_state == BRD_DSP_HIBERNATION ||
-	    dev_context->brd_state == BRD_HIBERNATION) {
-#ifdef CONFIG_TIDSPBRIDGE_DVFS
-		if (pdata->dsp_get_opp)
-			opplevel = (*pdata->dsp_get_opp) ();
-		if (opplevel == VDD1_OPP1) {
-			if (pdata->dsp_set_min_opp)
-				(*pdata->dsp_set_min_opp) (VDD1_OPP2);
-		}
-#endif
-		/* Restart the peripheral clocks */
-		dsp_clock_enable_all(dev_context->dsp_per_clks);
-		dsp_wdt_enable(true);
-
-		/*
-		 * 2:0 AUTO_IVA2_DPLL - Enabling IVA2 DPLL auto control
-		 *     in CM_AUTOIDLE_PLL_IVA2 register
-		 */
-		(*pdata->dsp_cm_write)(1 << OMAP3430_AUTO_IVA2_DPLL_SHIFT,
-				OMAP3430_IVA2_MOD, OMAP3430_CM_AUTOIDLE_PLL);
-
-		/*
-		 * 7:4 IVA2_DPLL_FREQSEL - IVA2 internal frq set to
-		 *     0.75 MHz - 1.0 MHz
-		 * 2:0 EN_IVA2_DPLL - Enable IVA2 DPLL in lock mode
-		 */
-		(*pdata->dsp_cm_rmw_bits)(OMAP3430_IVA2_DPLL_FREQSEL_MASK |
-				OMAP3430_EN_IVA2_DPLL_MASK,
-				0x3 << OMAP3430_IVA2_DPLL_FREQSEL_SHIFT |
-				0x7 << OMAP3430_EN_IVA2_DPLL_SHIFT,
-				OMAP3430_IVA2_MOD, OMAP3430_CM_CLKEN_PLL);
-
-		/* Restore mailbox settings */
-		omap_mbox_restore_ctx(dev_context->mbox);
-
-		/* Access MMU SYS CONFIG register to generate a short wakeup */
-		temp = readl(resources->dmmu_base + 0x10);
-
-		dev_context->brd_state = BRD_RUNNING;
-	} else if (dev_context->brd_state == BRD_RETENTION) {
-		/* Restart the peripheral clocks */
-		dsp_clock_enable_all(dev_context->dsp_per_clks);
-	}
+	status = wake_dsp(dev_context, NULL);
+	if (status)
+		return status;
 
 	status = omap_mbox_msg_send(dev_context->mbox, mb_val);
 
-- 
1.7.1

[PATCH 2/8] staging: tidspbridge: fix resume path from retention

[Omar Ramirez Luna]

If the target state is RET for suspend, the driver
disables wdt3; this is never enabled again on resume,
nor the state updated to running, fixing that.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index d5245a3..69cc2c4 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -274,6 +274,10 @@ int wake_dsp(struct bridge_dev_context *dev_context, void *pargs)
 		/* Restart the peripheral clocks */
 		dsp_clock_enable_all(dev_context->dsp_per_clks);
 
+		dsp_wdt_enable(true);
+
+		dev_context->brd_state = BRD_RUNNING;
+
 		break;
 	case BRD_HIBERNATION:
 	case BRD_DSP_HIBERNATION:
-- 
1.7.1

[PATCH 3/8] staging: tidspbridge: remove msleep for dsp transition wait

[Omar Ramirez Luna]

Remove msleep from the waiting loop used to the dsp
transition into a lower power state. Instead use jiffies
as the timeout count and do not sleep, otherwise when
protecting this functions through spin_lock_bh it will
cause a nasty BUG message to appear.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   41 ++++++++++-----------
 1 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index 69cc2c4..9595abc 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -81,7 +81,8 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 {
 	int status = 0;
 #ifdef CONFIG_PM
-	u16 timeout = PWRSTST_TIMEOUT / 10;
+	u8 t;
+	unsigned long v;
 	u32 pwr_state;
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
 	u32 opplevel;
@@ -90,18 +91,17 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 	struct omap_dsp_platform_data *pdata =
 		omap_dspbridge_dev->dev.platform_data;
 
-	pwr_state = (*pdata->dsp_prm_read)(OMAP3430_IVA2_MOD, OMAP2_PM_PWSTST) &
-						OMAP_POWERSTATEST_MASK;
 	/* Wait for DSP to move into OFF state */
-	while ((pwr_state != PWRDM_POWER_OFF) && --timeout) {
-		if (msleep_interruptible(10)) {
-			pr_err("Waiting for DSP OFF mode interrupted\n");
-			return -EPERM;
-		}
+	v = msecs_to_jiffies(PWRSTST_TIMEOUT) + jiffies;
+	do {
+		t = time_is_after_jiffies(v);
 		pwr_state = (*pdata->dsp_prm_read)(OMAP3430_IVA2_MOD,
 					OMAP2_PM_PWSTST) & OMAP_POWERSTATEST_MASK;
-	}
-	if (timeout == 0) {
+		if (pwr_state == PWRDM_POWER_OFF)
+			break;
+	} while (t);
+
+	if (!t) {
 		pr_err("%s: Timed out waiting for DSP off mode\n", __func__);
 		status = -ETIMEDOUT;
 		return status;
@@ -154,7 +154,8 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 #ifdef CONFIG_TIDSPBRIDGE_NTFY_PWRERR
 	struct deh_mgr *hdeh_mgr;
 #endif /* CONFIG_TIDSPBRIDGE_NTFY_PWRERR */
-	u16 timeout = PWRSTST_TIMEOUT / 10;
+	u8 t;
+	unsigned long v;
 	u32 pwr_state, target_pwr_state;
 	struct omap_dsp_platform_data *pdata =
 		omap_dspbridge_dev->dev.platform_data;
@@ -198,21 +199,17 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 		return -EPERM;
 	}
 
-	/* Get the PRCM DSP power domain status */
-	pwr_state = (*pdata->dsp_prm_read)(OMAP3430_IVA2_MOD, OMAP2_PM_PWSTST) &
-						OMAP_POWERSTATEST_MASK;
-
 	/* Wait for DSP to move into target power state */
-	while ((pwr_state != target_pwr_state) && --timeout) {
-		if (msleep_interruptible(10)) {
-			pr_err("Waiting for DSP to Suspend interrupted\n");
-			return -EPERM;
-		}
+	v = msecs_to_jiffies(PWRSTST_TIMEOUT) + jiffies;
+	do {
+		t = time_is_after_jiffies(v);
 		pwr_state = (*pdata->dsp_prm_read)(OMAP3430_IVA2_MOD,
 					OMAP2_PM_PWSTST) & OMAP_POWERSTATEST_MASK;
-	}
+		if (pwr_state == target_pwr_state)
+			break;
+	} while (t);
 
-	if (!timeout) {
+	if (!t) {
 		pr_err("%s: Timed out waiting for DSP off mode, state %x\n",
 		       __func__, pwr_state);
 #ifdef CONFIG_TIDSPBRIDGE_NTFY_PWRERR
-- 
1.7.1

[PATCH 4/8] staging: tidspbridge: send mbox PM command directly

[Omar Ramirez Luna]

Instead of using sm_interrupt_dsp and checking for the power states,
send the sleep command directly since we are transitioning to sleep
anyway.

Also remove redundant calls to save_ctx in these cases.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index 9595abc..dd035af 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -156,6 +156,7 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 #endif /* CONFIG_TIDSPBRIDGE_NTFY_PWRERR */
 	u8 t;
 	unsigned long v;
+	u32 mbx_msg;
 	u32 pwr_state, target_pwr_state;
 	struct omap_dsp_platform_data *pdata =
 		omap_dspbridge_dev->dev.platform_data;
@@ -166,21 +167,19 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 
 	switch (dev_context->brd_state) {
 	case BRD_RUNNING:
-		omap_mbox_save_ctx(dev_context->mbox);
 		if (dsp_test_sleepstate == PWRDM_POWER_OFF) {
-			sm_interrupt_dsp(dev_context, MBX_PM_DSPHIBERNATE);
+			mbx_msg = MBX_PM_DSPHIBERNATE;
 			dev_dbg(bridge, "PM: %s - sent hibernate cmd to DSP\n",
 				__func__);
 			target_pwr_state = PWRDM_POWER_OFF;
 		} else {
-			sm_interrupt_dsp(dev_context, MBX_PM_DSPRETENTION);
+			mbx_msg = MBX_PM_DSPRETENTION;
 			target_pwr_state = PWRDM_POWER_RET;
 		}
 		break;
 	case BRD_RETENTION:
-		omap_mbox_save_ctx(dev_context->mbox);
 		if (dsp_test_sleepstate == PWRDM_POWER_OFF) {
-			sm_interrupt_dsp(dev_context, MBX_PM_DSPHIBERNATE);
+			mbx_msg = MBX_PM_DSPHIBERNATE;
 			target_pwr_state = PWRDM_POWER_OFF;
 		} else
 			return 0;
@@ -199,6 +198,12 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 		return -EPERM;
 	}
 
+	omap_mbox_save_ctx(dev_context->mbox);
+
+	status = omap_mbox_msg_send(dev_context->mbox, mbx_msg);
+	if (status)
+		return status;
+
 	/* Wait for DSP to move into target power state */
 	v = msecs_to_jiffies(PWRSTST_TIMEOUT) + jiffies;
 	do {
-- 
1.7.1

[PATCH 5/8] staging: tidspbridge: send wake message even if dsp is running

[Omar Ramirez Luna]

Even if the board state is 'running', send a wake up message,
which should refresh self hibernation timeout (on dsp side) and
prevent it from going into sleep by itself.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index dd035af..93c441c 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -270,8 +270,9 @@ int wake_dsp(struct bridge_dev_context *dev_context, void *pargs)
 
 	switch (dev_context->brd_state) {
 	case BRD_STOPPED:
-	case BRD_RUNNING:
 		return 0;
+	case BRD_RUNNING:
+		break;
 	case BRD_RETENTION:
 		/* Restart the peripheral clocks */
 		dsp_clock_enable_all(dev_context->dsp_per_clks);
-- 
1.7.1

[PATCH 6/8] staging: tidspbridge: remove redundant code from PM routines

[Omar Ramirez Luna]

Trivial cleanup to remove redundant code.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index 93c441c..cb2a873 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -103,8 +103,7 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 
 	if (!t) {
 		pr_err("%s: Timed out waiting for DSP off mode\n", __func__);
-		status = -ETIMEDOUT;
-		return status;
+		return -ETIMEDOUT;
 	} else {
 
 		/* Save mailbox settings */
@@ -122,10 +121,8 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
 			status =
 			    dev_get_io_mgr(dev_context->dev_obj, &hio_mgr);
-			if (!hio_mgr) {
-				status = DSP_EHANDLE;
-				return status;
-			}
+			if (!hio_mgr)
+				return -EINVAL;
 			io_sh_msetting(hio_mgr, SHM_GETOPP, &opplevel);
 
 			/*
@@ -134,7 +131,6 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 			 */
 			if (pdata->dsp_set_min_opp)
 				(*pdata->dsp_set_min_opp) (VDD1_OPP1);
-			status = 0;
 #endif /* CONFIG_TIDSPBRIDGE_DVFS */
 		}
 	}
-- 
1.7.1

[PATCH 7/8] staging: tidspbridge: remove redundant indentation in PM routines

[Omar Ramirez Luna]

Trivial cleaunp to remove redundant indentation.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   87 ++++++++++-----------
 1 files changed, 41 insertions(+), 46 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index cb2a873..95089ef 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -104,36 +104,31 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 	if (!t) {
 		pr_err("%s: Timed out waiting for DSP off mode\n", __func__);
 		return -ETIMEDOUT;
-	} else {
+	}
 
-		/* Save mailbox settings */
-		omap_mbox_save_ctx(dev_context->mbox);
+	/* Save mailbox settings */
+	omap_mbox_save_ctx(dev_context->mbox);
 
-		/* Turn off DSP Peripheral clocks and DSP Load monitor timer */
-		status = dsp_clock_disable_all(dev_context->dsp_per_clks);
+	/* Turn off DSP Peripheral clocks and DSP Load monitor timer */
+	status = dsp_clock_disable_all(dev_context->dsp_per_clks);
+	if (status)
+		return status;
 
-		/* Disable wdt on hibernation. */
-		dsp_wdt_enable(false);
+	/* Disable wdt on hibernation. */
+	dsp_wdt_enable(false);
 
-		if (!status) {
-			/* Update the Bridger Driver state */
-			dev_context->brd_state = BRD_DSP_HIBERNATION;
+	/* Update the Bridger Driver state */
+	dev_context->brd_state = BRD_DSP_HIBERNATION;
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
-			status =
-			    dev_get_io_mgr(dev_context->dev_obj, &hio_mgr);
-			if (!hio_mgr)
-				return -EINVAL;
-			io_sh_msetting(hio_mgr, SHM_GETOPP, &opplevel);
-
-			/*
-			 * Set the OPP to low level before moving to OFF
-			 * mode
-			 */
-			if (pdata->dsp_set_min_opp)
-				(*pdata->dsp_set_min_opp) (VDD1_OPP1);
+	status = dev_get_io_mgr(dev_context->dev_obj, &hio_mgr);
+	if (!hio_mgr)
+		return -EINVAL;
+	io_sh_msetting(hio_mgr, SHM_GETOPP, &opplevel);
+
+	/* Set the OPP to low level before moving to OFF mode */
+	if (pdata->dsp_set_min_opp)
+		(*pdata->dsp_set_min_opp) (VDD1_OPP1);
 #endif /* CONFIG_TIDSPBRIDGE_DVFS */
-		}
-	}
 #endif
 	return status;
 }
@@ -218,30 +213,30 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 		bridge_deh_notify(hdeh_mgr, DSP_PWRERROR, 0);
 #endif /* CONFIG_TIDSPBRIDGE_NTFY_PWRERR */
 		return -ETIMEDOUT;
-	} else {
-		/* Update the Bridger Driver state */
-		if (dsp_test_sleepstate == PWRDM_POWER_OFF)
-			dev_context->brd_state = BRD_HIBERNATION;
-		else
-			dev_context->brd_state = BRD_RETENTION;
-
-		/* Disable wdt on hibernation. */
-		dsp_wdt_enable(false);
-
-		/* Turn off DSP Peripheral clocks */
-		status = dsp_clock_disable_all(dev_context->dsp_per_clks);
-		if (status)
-			return status;
+	}
+
+	/* Update the Bridger Driver state */
+	if (dsp_test_sleepstate == PWRDM_POWER_OFF)
+		dev_context->brd_state = BRD_HIBERNATION;
+	else
+		dev_context->brd_state = BRD_RETENTION;
+
+	/* Disable wdt on hibernation. */
+	dsp_wdt_enable(false);
+
+	/* Turn off DSP Peripheral clocks */
+	status = dsp_clock_disable_all(dev_context->dsp_per_clks);
+	if (status)
+		return status;
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
-		else if (target_pwr_state == PWRDM_POWER_OFF) {
-			/*
-			 * Set the OPP to low level before moving to OFF mode
-			 */
-			if (pdata->dsp_set_min_opp)
-				(*pdata->dsp_set_min_opp) (VDD1_OPP1);
-		}
-#endif /* CONFIG_TIDSPBRIDGE_DVFS */
+	if (target_pwr_state == PWRDM_POWER_OFF) {
+		/*
+		 * Set the OPP to low level before moving to OFF mode
+		 */
+		if (pdata->dsp_set_min_opp)
+			(*pdata->dsp_set_min_opp) (VDD1_OPP1);
 	}
+#endif /* CONFIG_TIDSPBRIDGE_DVFS */
 #endif /* CONFIG_PM */
 	return status;
 }
-- 
1.7.1

[PATCH 8/8] staging: tidspbridge: protect critical sections on PM routines

[Omar Ramirez Luna]

To fix some race conditions which:

- Wake the dsp, through the GFLUSH register, while tidspbridge has
  initiated a power off transition but hasn't disabled the peripheral
  clocks nor set the brd_state to HIB or self HIB. This cancels dsp
  transition but cuts off the clocks.

- Wake the dsp, through sm_interrupt_dsp, same as above.

- Wake the domain only, if the peripheral clocks have been cut, the
  IVA domain is woken but no response recieved until the peripheral
  clocks are reenabled. This triggers another race; when waking the
  dsp, clocks are reenabled and dsp starts its resume sequence,
  however dmtimer framework also resets the GPT clocks as part of
  its 'request' routine, which leaves the dsp without tick and unable
  to transition between power states.

It fixes the following future races, if IVA clock is enabled/disabled
in PM routines:

- Depending on timing it can cause an abort while accessing MMU
  registers from mpu side.

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/staging/tidspbridge/core/tiomap3430_pwr.c |   32 +++++++++++++++++++--
 1 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
index 95089ef..fdbf4eb 100644
--- a/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
+++ b/drivers/staging/tidspbridge/core/tiomap3430_pwr.c
@@ -49,6 +49,8 @@
 
 #define PWRSTST_TIMEOUT          200
 
+DEFINE_SPINLOCK(pwr_lock);
+
 /*
  *  ======== handle_constraints_set ========
  *  	Sets new DSP constraint
@@ -91,6 +93,8 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 	struct omap_dsp_platform_data *pdata =
 		omap_dspbridge_dev->dev.platform_data;
 
+	spin_lock_bh(&pwr_lock);
+
 	/* Wait for DSP to move into OFF state */
 	v = msecs_to_jiffies(PWRSTST_TIMEOUT) + jiffies;
 	do {
@@ -103,6 +107,7 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 
 	if (!t) {
 		pr_err("%s: Timed out waiting for DSP off mode\n", __func__);
+		spin_unlock_bh(&pwr_lock);
 		return -ETIMEDOUT;
 	}
 
@@ -111,14 +116,19 @@ int handle_hibernation_from_dsp(struct bridge_dev_context *dev_context)
 
 	/* Turn off DSP Peripheral clocks and DSP Load monitor timer */
 	status = dsp_clock_disable_all(dev_context->dsp_per_clks);
-	if (status)
+	if (status) {
+		spin_unlock_bh(&pwr_lock);
 		return status;
+	}
 
 	/* Disable wdt on hibernation. */
 	dsp_wdt_enable(false);
 
 	/* Update the Bridger Driver state */
 	dev_context->brd_state = BRD_DSP_HIBERNATION;
+
+	spin_unlock_bh(&pwr_lock);
+
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
 	status = dev_get_io_mgr(dev_context->dev_obj, &hio_mgr);
 	if (!hio_mgr)
@@ -189,11 +199,15 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 		return -EPERM;
 	}
 
+	spin_lock_bh(&pwr_lock);
+
 	omap_mbox_save_ctx(dev_context->mbox);
 
 	status = omap_mbox_msg_send(dev_context->mbox, mbx_msg);
-	if (status)
+	if (status) {
+		spin_unlock_bh(&pwr_lock);
 		return status;
+	}
 
 	/* Wait for DSP to move into target power state */
 	v = msecs_to_jiffies(PWRSTST_TIMEOUT) + jiffies;
@@ -212,6 +226,7 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 		dev_get_deh_mgr(dev_context->dev_obj, &hdeh_mgr);
 		bridge_deh_notify(hdeh_mgr, DSP_PWRERROR, 0);
 #endif /* CONFIG_TIDSPBRIDGE_NTFY_PWRERR */
+		spin_unlock_bh(&pwr_lock);
 		return -ETIMEDOUT;
 	}
 
@@ -226,8 +241,13 @@ int sleep_dsp(struct bridge_dev_context *dev_context, u32 dw_cmd,
 
 	/* Turn off DSP Peripheral clocks */
 	status = dsp_clock_disable_all(dev_context->dsp_per_clks);
-	if (status)
+	if (status) {
+		spin_unlock_bh(&pwr_lock);
 		return status;
+	}
+
+	spin_unlock_bh(&pwr_lock);
+
 #ifdef CONFIG_TIDSPBRIDGE_DVFS
 	if (target_pwr_state == PWRDM_POWER_OFF) {
 		/*
@@ -259,8 +279,11 @@ int wake_dsp(struct bridge_dev_context *dev_context, void *pargs)
 	if (!dev_context->mbox || !resources)
 		return -EPERM;
 
+	spin_lock_bh(&pwr_lock);
+
 	switch (dev_context->brd_state) {
 	case BRD_STOPPED:
+		spin_unlock_bh(&pwr_lock);
 		return 0;
 	case BRD_RUNNING:
 		break;
@@ -318,12 +341,15 @@ int wake_dsp(struct bridge_dev_context *dev_context, void *pargs)
 	default:
 		pr_err("%s: unexpected state %x\n", __func__,
 						dev_context->brd_state);
+		spin_unlock_bh(&pwr_lock);
 		return -EINVAL;
 	}
 
 	/* Send a wakeup message to DSP */
 	status = omap_mbox_msg_send(dev_context->mbox, MBX_PM_DSPWAKEUP);
 
+	spin_unlock_bh(&pwr_lock);
+
 #endif /* CONFIG_PM */
 	return status;
 }
-- 
1.7.1

Re: [PATCH 0/8] staging: tidspbridge: PM routines cleanup

[Ramirez Luna, Omar]

On Wed, Mar 23, 2011 at 1:49 PM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Cleanup and fixes for tidspbridge's PM routines.
>
> The most significant ones are:
> - Protection of critical sections when disabling clocks which
>  could cause issues during PM transitions.
> - Fix of resume path from RET power state.
>
> Other patches are cleanups and preparations for the usage of
> spinlock.
>
> Omar Ramirez Luna (8):
>  staging: tidspbridge: make wake_dsp to handle PM code
>  staging: tidspbridge: fix resume path from retention
>  staging: tidspbridge: remove msleep for dsp transition wait
>  staging: tidspbridge: send mbox PM command directly
>  staging: tidspbridge: send wake message even if dsp is running
>  staging: tidspbridge: remove redundant code from PM routines
>  staging: tidspbridge: remove redundant indentation in PM routines
>  staging: tidspbridge: protect critical sections on PM routines
>
>  drivers/staging/tidspbridge/core/tiomap3430_pwr.c |  248 ++++++++++++++-------
>  drivers/staging/tidspbridge/core/tiomap_io.c      |   58 +-----
>  2 files changed, 168 insertions(+), 138 deletions(-)

Pushed to dspbridge.

Regards,

Omar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html