[PATCH 00/13] DSPBRIDGE: Patchset with various fixes

[PATCH 00/13] DSPBRIDGE: Patchset with various fixes

[Ameya Palande]

Most of them have already got an ack, but can you please verify them again?
Following patches need an ack:

[PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else
[PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig
[PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode
[PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

Thanks!

Cheers,
Ameya.

[PATCH 01/13] DSPBRIDGE: Heuristic fixes of strlen/malloc out by one

[Ameya Palande]

From: Phil Carmody <ext-phil.2.carmody@nokia.com>

I say 'heuristic', as I can't prove they're wrong, they just look
wrong, and for that reason should be given extra close scrutiny.
These are basically just the old malloc-one-more-than-strlen.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 drivers/dsp/bridge/pmgr/wcd.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 86812c6..b8adb41 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -528,8 +528,9 @@ u32 MGRWRAP_RegisterObject(union Trapped_Args *args)
 	cp_fm_usr(&pUuid, args->ARGS_MGR_REGISTEROBJECT.pUuid, status, 1);
 	if (DSP_FAILED(status))
 		goto func_end;
+	/* pathSize is increased by 1 to accommodate NULL */
 	pathSize = strlen_user((char *)
-				args->ARGS_MGR_REGISTEROBJECT.pszPathName);
+			args->ARGS_MGR_REGISTEROBJECT.pszPathName) + 1;
 	pszPathName = MEM_Alloc(pathSize, MEM_NONPAGED);
 	if (!pszPathName)
 		goto func_end;
@@ -540,7 +541,6 @@ u32 MGRWRAP_RegisterObject(union Trapped_Args *args)
 		status = DSP_EPOINTER;
 		goto func_end;
 	}
-	pszPathName[pathSize] = '\0';
 
 	GT_1trace(WCD_debugMask, GT_ENTER,
 		 "MGRWRAP_RegisterObject: entered pg2hMsg "
@@ -900,7 +900,8 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 		if (argv[i] != NULL) {
                         /* User space pointer to argument */
                        temp = (char *) argv[i];
-                       len = strlen_user((char *)temp);
+			/* len is increased by 1 to accommodate NULL */
+			len = strlen_user((char *)temp) + 1;
 			/* Kernel space pointer to argument */
 			argv[i] = MEM_Alloc(len, MEM_NONPAGED);
 			if (argv[i] == NULL) {
@@ -910,7 +911,6 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 			cp_fm_usr(argv[i], temp, status, len);
 			if (DSP_FAILED(status))
 				goto func_cont;
-
 		}
 	}
 	/* TODO: validate this */
@@ -933,7 +933,8 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 		for (i = 0; DSP_SUCCEEDED(status) && (envp[i] != NULL); i++) {
                         /* User space pointer to argument */
                        temp = (char *)envp[i];
-                       len = strlen_user((char *)temp);
+			/* len is increased by 1 to accommodate NULL */
+			len = strlen_user((char *)temp) + 1;
 			/* Kernel space pointer to argument */
 			envp[i] = MEM_Alloc(len, MEM_NONPAGED);
 			if (envp[i] == NULL) {
-- 
1.6.2.4

[PATCH 02/13] DSPBRIDGE: PROCWRAP_Load function cleanup in a complete mess

[Ameya Palande]

If you followed some failure paths, it was entirely possible that
you'd attempt to MEM_Free a user-space pointer, because it wouldn't
have been replaced with a kernel-space copy yet. Now ensure there's
a NULL pointer to stop the cleanup at the position of the first error.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 drivers/dsp/bridge/pmgr/wcd.c |  114 +++++++++++++++++++++++-----------------
 1 files changed, 65 insertions(+), 49 deletions(-)

diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index b8adb41..8111b60 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -880,99 +880,115 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 {
 	s32 i, len;
 	DSP_STATUS status = DSP_SOK;
-       char *temp;
-	s32 argc = args->ARGS_PROC_LOAD.iArgc;
+	char *temp;
+	s32 count = args->ARGS_PROC_LOAD.iArgc;
 	u8 **argv, **envp = NULL;
 
+	DBC_Require(count > 0);
+	DBC_Require(count <= MAX_LOADARGS);
 
-	DBC_Require(argc > 0);
-	DBC_Require(argc <= MAX_LOADARGS);
-
-	argv = MEM_Alloc(argc * sizeof(u8 *), MEM_NONPAGED);
-	if (argv == NULL)
+	argv = MEM_Alloc(count * sizeof(u8 *), MEM_NONPAGED);
+	if (!argv) {
 		status = DSP_EMEMORY;
+		goto func_cont;
+	}
 
-	cp_fm_usr(argv, args->ARGS_PROC_LOAD.aArgv, status, argc);
-	if (DSP_FAILED(status))
+	cp_fm_usr(argv, args->ARGS_PROC_LOAD.aArgv, status, count);
+	if (DSP_FAILED(status)) {
+		MEM_Free(argv);
+		argv = NULL;
 		goto func_cont;
+	}
 
-	for (i = 0; DSP_SUCCEEDED(status) && (i < argc); i++) {
-		if (argv[i] != NULL) {
-                        /* User space pointer to argument */
-                       temp = (char *) argv[i];
+	for (i = 0; DSP_SUCCEEDED(status) && (i < count); i++) {
+		if (argv[i]) {
+			/* User space pointer to argument */
+			temp = (char *) argv[i];
 			/* len is increased by 1 to accommodate NULL */
 			len = strlen_user((char *)temp) + 1;
 			/* Kernel space pointer to argument */
 			argv[i] = MEM_Alloc(len, MEM_NONPAGED);
-			if (argv[i] == NULL) {
+			if (argv[i]) {
+				cp_fm_usr(argv[i], temp, status, len);
+				if (DSP_FAILED(status)) {
+					MEM_Free(argv[i]);
+					argv[i] = NULL;
+					goto func_cont;
+				}
+			} else {
 				status = DSP_EMEMORY;
-				break;
-			}
-			cp_fm_usr(argv[i], temp, status, len);
-			if (DSP_FAILED(status))
 				goto func_cont;
+			}
 		}
 	}
 	/* TODO: validate this */
-	if (args->ARGS_PROC_LOAD.aEnvp != NULL) {
+	if (args->ARGS_PROC_LOAD.aEnvp) {
 		/* number of elements in the envp array including NULL */
-		len = 0;
+		count = 0;
 		do {
-			len++;
-                       get_user(temp, args->ARGS_PROC_LOAD.aEnvp);
-               } while (temp);
-		envp = MEM_Alloc(len * sizeof(u8 *), MEM_NONPAGED);
-		if (envp == NULL) {
+			get_user(temp, args->ARGS_PROC_LOAD.aEnvp + count);
+			count++;
+		} while (temp);
+		envp = MEM_Alloc(count * sizeof(u8 *), MEM_NONPAGED);
+		if (!envp) {
 			status = DSP_EMEMORY;
 			goto func_cont;
 		}
 
-		cp_fm_usr(envp, args->ARGS_PROC_LOAD.aEnvp, status, len);
-		if (DSP_FAILED(status))
+		cp_fm_usr(envp, args->ARGS_PROC_LOAD.aEnvp, status, count);
+		if (DSP_FAILED(status)) {
+			MEM_Free(envp);
+			envp = NULL;
 			goto func_cont;
-		for (i = 0; DSP_SUCCEEDED(status) && (envp[i] != NULL); i++) {
-                        /* User space pointer to argument */
-                       temp = (char *)envp[i];
+		}
+		for (i = 0; DSP_SUCCEEDED(status) && envp[i]; i++) {
+			/* User space pointer to argument */
+			temp = (char *)envp[i];
 			/* len is increased by 1 to accommodate NULL */
 			len = strlen_user((char *)temp) + 1;
 			/* Kernel space pointer to argument */
 			envp[i] = MEM_Alloc(len, MEM_NONPAGED);
-			if (envp[i] == NULL) {
+			if (envp[i]) {
+				cp_fm_usr(envp[i], temp, status, len);
+				if (DSP_FAILED(status)) {
+					MEM_Free(envp[i]);
+					envp[i] = NULL;
+					goto func_cont;
+				}
+			} else {
 				status = DSP_EMEMORY;
-				break;
-			}
-			cp_fm_usr(envp[i], temp, status, len);
-			if (DSP_FAILED(status))
 				goto func_cont;
+			}
 		}
 	}
 	GT_5trace(WCD_debugMask, GT_ENTER,
-		 "PROCWRAP_Load, hProcessor: 0x%x\n\tiArgc:"
-		 "0x%x\n\taArgv: 0x%x\n\taArgv[0]: %s\n\taEnvp: 0x%0x\n",
-		 args->ARGS_PROC_LOAD.hProcessor,
-		 args->ARGS_PROC_LOAD.iArgc, args->ARGS_PROC_LOAD.aArgv,
-		 argv[0], args->ARGS_PROC_LOAD.aEnvp);
+		"PROCWRAP_Load, hProcessor: 0x%x\n\tiArgc:"
+		"0x%x\n\taArgv: 0x%x\n\taArgv[0]: %s\n\taEnvp: 0x%0x\n",
+		args->ARGS_PROC_LOAD.hProcessor,
+		args->ARGS_PROC_LOAD.iArgc, args->ARGS_PROC_LOAD.aArgv,
+		argv[0], args->ARGS_PROC_LOAD.aEnvp);
 	if (DSP_SUCCEEDED(status)) {
 		status = PROC_Load(args->ARGS_PROC_LOAD.hProcessor,
-				  args->ARGS_PROC_LOAD.iArgc,
-				  (CONST char **)argv, (CONST char **)envp);
+				args->ARGS_PROC_LOAD.iArgc,
+				(CONST char **)argv, (CONST char **)envp);
 	}
 func_cont:
-	if (envp != NULL) {
+	if (envp) {
 		i = 0;
-		while (envp[i] != NULL)
+		while (envp[i])
 			MEM_Free(envp[i++]);
 
 		MEM_Free(envp);
 	}
-	if (argv != NULL) {
-		for (i = 0; i < argc; i++) {
-			if (argv[i] != NULL)
-				MEM_Free(argv[i]);
 
-		}
+	if (argv) {
+		count = args->ARGS_PROC_LOAD.iArgc;
+		for (i = 0; (i < count) && argv[i]; i++)
+			MEM_Free(argv[i]);
+
 		MEM_Free(argv);
 	}
+
 	return status;
 }
 
-- 
1.6.2.4

[PATCH 03/13] DSPBRIDGE: Remove unnecessary conditions from some for loops

[Ameya Palande]

The status cannot be a failure value at the start of the loops,
and the only changes to a failure state are accompanied by a
break or goto, so these conditions are always true.

Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
---
 drivers/dsp/bridge/pmgr/wcd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 8111b60..441130c 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -900,7 +900,7 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 		goto func_cont;
 	}
 
-	for (i = 0; DSP_SUCCEEDED(status) && (i < count); i++) {
+	for (i = 0; i < count; i++) {
 		if (argv[i]) {
 			/* User space pointer to argument */
 			temp = (char *) argv[i];
@@ -941,7 +941,7 @@ u32 PROCWRAP_Load(union Trapped_Args *args)
 			envp = NULL;
 			goto func_cont;
 		}
-		for (i = 0; DSP_SUCCEEDED(status) && envp[i]; i++) {
+		for (i = 0; envp[i]; i++) {
 			/* User space pointer to argument */
 			temp = (char *)envp[i];
 			/* len is increased by 1 to accommodate NULL */
-- 
1.6.2.4

[PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else

[Ameya Palande]

From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>

Based on the following discussion:
      http://marc.info/?l=linux-omap&m=124697893724881&w=2

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/pmgr/wcd.c |   60 ++++++++++++++++++++++++++--------------
 1 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
index 441130c..8708c78 100644
--- a/drivers/dsp/bridge/pmgr/wcd.c
+++ b/drivers/dsp/bridge/pmgr/wcd.c
@@ -145,27 +145,45 @@
 #define MAX_STREAMS     16
 #define MAX_BUFS	64
 
-/* Following two macros should ideally have do{}while(0) */
-
-#define cp_fm_usr(dest, src, status, elements)    \
-    if (DSP_SUCCEEDED(status)) {\
-	    if (unlikely(src == NULL) ||				\
-		unlikely(copy_from_user(dest, src, elements * sizeof(*(dest))))) { \
-		GT_1trace(WCD_debugMask, GT_7CLASS, \
-		"copy_from_user failed, src=0x%x\n", src);  \
-		status = DSP_EPOINTER ; \
-	} \
-    }
-
-#define cp_to_usr(dest, src, status, elements)    \
-    if (DSP_SUCCEEDED(status)) {\
-	    if (unlikely(dest == NULL) ||				\
-		unlikely(copy_to_user(dest, src, elements * sizeof(*(src))))) { \
-		GT_1trace(WCD_debugMask, GT_7CLASS, \
-		"copy_to_user failed, dest=0x%x\n", dest); \
-		status = DSP_EPOINTER ;\
-	} \
-    }
+static inline void __cp_fm_usr(void *to, const void __user *from,
+			       DSP_STATUS *err, unsigned long bytes)
+{
+	if (DSP_FAILED(*err))
+		return;
+
+	if (unlikely(!from)) {
+		*err = DSP_EPOINTER;
+		return;
+	}
+
+	if (unlikely(copy_from_user(to, from, bytes))) {
+		GT_2trace(WCD_debugMask, GT_7CLASS,
+			  "%s failed, from=0x%08x\n", __func__, from);
+		*err = DSP_EPOINTER;
+	}
+}
+#define cp_fm_usr(to, from, err, n)				\
+	__cp_fm_usr(to, from, &(err), (n) * sizeof(*(to)))
+
+static inline void __cp_to_usr(void __user *to, const void *from,
+			       DSP_STATUS *err, unsigned long bytes)
+{
+	if (DSP_FAILED(*err))
+		return;
+
+	if (unlikely(!to)) {
+		*err = DSP_EPOINTER;
+		return;
+	}
+
+	if (unlikely(copy_to_user(to, from, bytes))) {
+		GT_2trace(WCD_debugMask, GT_7CLASS,
+			  "%s failed, to=0x%08x\n", __func__, to);
+		*err = DSP_EPOINTER;
+	}
+}
+#define cp_to_usr(to, from, err, n)				\
+	__cp_to_usr(to, from, &(err), (n) * sizeof(*(from)))
 
 /* Device IOCtl function pointer */
 struct WCD_Cmd {
-- 
1.6.2.4

[PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig

[Ameya Palande]

From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>

Reported-by: Tero Kristo <tero.kristo@nokia.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/hw/hw_mbox.c |    4 +++-
 drivers/dsp/bridge/hw/hw_mbox.h |    5 +++++
 drivers/dsp/bridge/wmd/io_sm.c  |    1 +
 3 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c
index ee79032..5a87597 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.c
+++ b/drivers/dsp/bridge/hw/hw_mbox.c
@@ -33,7 +33,9 @@
 /* width in bits of MBOX Id */
 #define HW_MBOX_ID_WIDTH	   2
 
-struct MAILBOX_CONTEXT mboxsetting = {0x4, 0x1, 0x1};
+static struct MAILBOX_CONTEXT mboxsetting = {
+	.sysconfig = 2 << 3 | 1, /* SMART/AUTO-IDLE */
+};
 
 /* Saves the mailbox context */
 HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddress)
diff --git a/drivers/dsp/bridge/hw/hw_mbox.h b/drivers/dsp/bridge/hw/hw_mbox.h
index ad1a89c..8a5f6bd 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.h
+++ b/drivers/dsp/bridge/hw/hw_mbox.h
@@ -320,4 +320,9 @@ extern HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddres);
 */
 extern HW_STATUS HW_MBOX_restoreSettings(void __iomem *baseAddres);
 
+static inline void HW_MBOX_initSettings(void __iomem *baseAddres)
+{
+	HW_MBOX_restoreSettings(baseAddres);
+}
+
 #endif  /* __MBOX_H */
diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index 39c34f7..d8ae1f1 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -282,6 +282,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
 		pIOMgr->fSharedIRQ = pMgrAttrs->fShared;
 		IO_DisableInterrupt(hWmdContext);
 		if (devType == DSP_UNIT) {
+			HW_MBOX_initSettings(hostRes.dwMboxBase);
 			/* Plug the channel ISR:. */
 			if ((request_irq(INT_MAIL_MPU_IRQ, IO_ISR, 0,
 				"DspBridge\tmailbox", (void *)pIOMgr)) == 0)
-- 
1.6.2.4

[PATCH 06/13] DSPBRIDGE: avoid to dump unnecessary registers without debug

[Ameya Palande]

Make "GetHWRegs()" available only with CONFIG_BRIDGE_DEBUG.

[Hiroshi DOYU: split the original to logical ones]

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/wmd/tiomap3430.c |   57 +++++++++++++++++++----------------
 1 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c
index 204ddd5..fd4c11c 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430.c
@@ -141,7 +141,37 @@ static DSP_STATUS PteSet(struct PgTableAttrs *pt, u32 pa, u32 va,
 static DSP_STATUS MemMapVmalloc(struct WMD_DEV_CONTEXT *hDevContext,
 			u32 ulMpuAddr, u32 ulVirtAddr,
 			u32 ulNumBytes, struct HW_MMUMapAttrs_t *hwAttrs);
-static void GetHWRegs(void __iomem *prcm_base, void __iomem *cm_base);
+
+#ifdef CONFIG_BRIDGE_DEBUG
+static void GetHWRegs(void __iomem *prm_base, void __iomem *cm_base)
+{
+	u32 temp;
+	temp = __raw_readl((cm_base) + 0x00);
+	DBG_Trace(DBG_LEVEL6, "CM_FCLKEN_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((cm_base) + 0x10);
+	DBG_Trace(DBG_LEVEL6, "CM_ICLKEN1_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((cm_base) + 0x20);
+	DBG_Trace(DBG_LEVEL6, "CM_IDLEST_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((cm_base) + 0x48);
+	DBG_Trace(DBG_LEVEL6, "CM_CLKSTCTRL_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((cm_base) + 0x4c);
+	DBG_Trace(DBG_LEVEL6, "CM_CLKSTST_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((prm_base) + 0x50);
+	DBG_Trace(DBG_LEVEL6, "RM_RSTCTRL_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((prm_base) + 0x58);
+	DBG_Trace(DBG_LEVEL6, "RM_RSTST_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((prm_base) + 0xE0);
+	DBG_Trace(DBG_LEVEL6, "PM_PWSTCTRL_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((prm_base) + 0xE4);
+	DBG_Trace(DBG_LEVEL6, "PM_PWSTST_IVA2 = 0x%x \n", temp);
+	temp = __raw_readl((cm_base) + 0xA10);
+	DBG_Trace(DBG_LEVEL6, "CM_ICLKEN1_CORE = 0x%x \n", temp);
+}
+#else
+static inline void GetHWRegs(void __iomem *prm_base, void __iomem *cm_base)
+{
+}
+#endif
 
 /*  ----------------------------------- Globals */
 
@@ -2021,31 +2051,6 @@ static DSP_STATUS MemMapVmalloc(struct WMD_DEV_CONTEXT *pDevContext,
 	return status;
 }
 
-static void GetHWRegs(void __iomem *prm_base, void __iomem *cm_base)
-{
-	u32 temp;
-	temp = __raw_readl((cm_base) + 0x00);
-	DBG_Trace(DBG_LEVEL6, "CM_FCLKEN_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((cm_base) + 0x10);
-	DBG_Trace(DBG_LEVEL6, "CM_ICLKEN1_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((cm_base) + 0x20);
-	DBG_Trace(DBG_LEVEL6, "CM_IDLEST_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((cm_base) + 0x48);
-	DBG_Trace(DBG_LEVEL6, "CM_CLKSTCTRL_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((cm_base) + 0x4c);
-	DBG_Trace(DBG_LEVEL6, "CM_CLKSTST_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((prm_base) + 0x50);
-	DBG_Trace(DBG_LEVEL6, "RM_RSTCTRL_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((prm_base) + 0x58);
-	DBG_Trace(DBG_LEVEL6, "RM_RSTST_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((prm_base) + 0xE0);
-	DBG_Trace(DBG_LEVEL6, "PM_PWSTCTRL_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((prm_base) + 0xE4);
-	DBG_Trace(DBG_LEVEL6, "PM_PWSTST_IVA2 = 0x%x \n", temp);
-	temp = __raw_readl((cm_base) + 0xA10);
-	DBG_Trace(DBG_LEVEL6, "CM_ICLKEN1_CORE = 0x%x \n", temp);
-}
-
 /*
  *  ======== configureDspMmu ========
  *      Make DSP MMU page table entries.
-- 
1.6.2.4

[PATCH 07/13] DSPBRIDGE: flush_all() bumps VDD1_OPP to 2 if it is at 1

[Ameya Palande]

[Hiroshi DOYU: split the original to logical ones]

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/wmd/tiomap3430.c |   14 +++++++++++++-
 1 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c
index fd4c11c..54c53b6 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430.c
@@ -291,8 +291,20 @@ static inline void flush_all(struct WMD_DEV_CONTEXT *pDevContext)
 		WakeDSP(pDevContext, NULL);
 		tlb_flush_all(pDevContext->dwDSPMmuBase);
 		CLK_Disable(SERVICESCLK_iva2_ck);
-	} else
+	} else {
+#ifdef CONFIG_BRIDGE_DVFS
+		struct dspbridge_platform_data *pdata =
+			omap_dspbridge_dev->dev.platform_data;
+		u32 opplevel = 0;
+		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
 		tlb_flush_all(pDevContext->dwDSPMmuBase);
+	}
 }
 
 static void bad_page_dump(u32 pa, struct page *pg)
-- 
1.6.2.4

[PATCH 08/13] DSPBRIDGE: Prevent memory access during hibernation

[Ameya Palande]

If DSP is in hibernated then WakeDSP() returns without waking it up, which
results in flush_all function access IVA MMU while IVA is hibernated.

[Hiroshi DOYU: split the original to logical ones]

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   65 +++++++++++--------------------
 drivers/dsp/bridge/wmd/tiomap_sm.c      |   32 +++++++--------
 2 files changed, 38 insertions(+), 59 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
index a725548..7cc29b7 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
@@ -295,59 +295,40 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
  */
 DSP_STATUS WakeDSP(struct WMD_DEV_CONTEXT *pDevContext, IN void *pArgs)
 {
-	DSP_STATUS status = DSP_SOK;
 #ifdef CONFIG_PM
-	struct CFG_HOSTRES resources;
+	DSP_STATUS status = DSP_SOK;
+#ifdef CONFIG_BRIDGE_DEBUG
 	enum HW_PwrState_t pwrState;
-	u32 temp;
+	struct CFG_HOSTRES resources;
 
 	status = CFG_GetHostResources(
 		 (struct CFG_DEVNODE *)DRV_GetFirstDevExtension(), &resources);
 	if (DSP_FAILED(status))
 		return status;
-	/* check the BRD/WMD state, if it is not 'SLEEP' then return failure */
+#endif /* CONFIG_BRIDGE_DEBUG */
+
+	/* Check the BRD/WMD state, if it is not 'SLEEP' then return failure */
 	if (pDevContext->dwBrdState == BRD_RUNNING ||
-		pDevContext->dwBrdState == BRD_STOPPED ||
-		pDevContext->dwBrdState == BRD_DSP_HIBERNATION) {
+	    pDevContext->dwBrdState == BRD_STOPPED) {
 		/* The Device is in 'RET' or 'OFF' state and WMD state is not
 		 * 'SLEEP', this means state inconsistency, so return  */
-		status = DSP_SOK;
-		return status;
-	}
-	/* Enable the DSP peripheral clocks and load monitor timer
-	 * before waking the DSP */
-	DBG_Trace(DBG_LEVEL6, "WakeDSP: enable DSP Peripheral Clks = 0x%x \n",
-		 pDevContext->uDspPerClks);
-	status = DSP_PeripheralClocks_Enable(pDevContext, NULL);
-
-	/* Enabling Dppll in lock mode */
-		temp = (u32) *((REG_UWORD32 *)
-			((u32) (resources.dwCmBase) + 0x34));
-		temp = (temp & 0xFFFFFFFE) | 0x1;
-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
-						(u32) temp;
-		temp = (u32) *((REG_UWORD32 *)
-			((u32) (resources.dwCmBase) + 0x4));
-		temp = (temp & 0xFFFFFC8) | 0x37;
-
-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
-						(u32) temp;
-
-	udelay(10);
-	if (DSP_SUCCEEDED(status)) {
-		/* Send a message to DSP to wake up */
-		CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP);
-		HW_PWR_IVA2StateGet(resources.dwPrmBase, HW_PWR_DOMAIN_DSP,
-				    &pwrState);
-		DBG_Trace(DBG_LEVEL7,
-			 "\nWakeDSP: Power State After sending Interrupt "
-			 "to DSP %x\n", pwrState);
-		/* set the device state to RUNNIG */
-		pDevContext->dwBrdState = BRD_RUNNING;
-	} else {
-		DBG_Trace(DBG_LEVEL6, "WakeDSP: FAILED\n");
+		return DSP_SOK;
 	}
-#endif
+
+	/* Send a wakeup message to DSP */
+	CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP);
+
+#ifdef CONFIG_BRIDGE_DEBUG
+	HW_PWR_IVA2StateGet(resources.dwPrmBase, HW_PWR_DOMAIN_DSP,
+			&pwrState);
+	DBG_Trace(DBG_LEVEL7,
+			"\nWakeDSP: Power State After sending Interrupt "
+			"to DSP %x\n", pwrState);
+#endif /* CONFIG_BRIDGE_DEBUG */
+
+	/* Set the device state to RUNNIG */
+	pDevContext->dwBrdState = BRD_RUNNING;
+#endif /* CONFIG_PM */
 	return status;
 }
 
diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index a6d5d62..b5d3d6d 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -126,24 +126,22 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 
 	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
 	    pDevContext->dwBrdState == BRD_HIBERNATION) {
+		/* Restart the peripheral clocks */
+		DSP_PeripheralClocks_Enable(pDevContext, NULL);
+
 		/* Restore mailbox settings */
-		/* Restart the peripheral clocks that were disabled only
-		 * in DSP initiated Hibernation case.*/
-		if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION) {
-			DSP_PeripheralClocks_Enable(pDevContext, NULL);
-			/* Enabling Dpll in lock mode*/
-			temp = (u32) *((REG_UWORD32 *)
-				       ((u32) (resources.dwCmBase) + 0x34));
-			temp = (temp & 0xFFFFFFFE) | 0x1;
-			*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
-				(u32) temp;
-			temp = (u32) *((REG_UWORD32 *)
-				       ((u32) (resources.dwCmBase) + 0x4));
-			temp = (temp & 0xFFFFFC8) | 0x37;
-
-			*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
-				(u32) temp;
-		}
+		/* Enabling Dpll in lock mode*/
+		temp = (u32) *((REG_UWORD32 *)
+				((u32) (resources.dwCmBase) + 0x34));
+		temp = (temp & 0xFFFFFFFE) | 0x1;
+		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
+			(u32) temp;
+		temp = (u32) *((REG_UWORD32 *)
+				((u32) (resources.dwCmBase) + 0x4));
+		temp = (temp & 0xFFFFFC8) | 0x37;
+
+		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
+			(u32) temp;
 		HW_MBOX_restoreSettings(resources.dwMboxBase);
 
 		/*  Access MMU SYS CONFIG register to generate a short wakeup */
-- 
1.6.2.4

[PATCH 09/13] DSPBRIDGE: fix incorrect mask of DPLL in CHNLSM_InterruptDSP2()

[Ameya Palande]

This patch also contains indentation fixes and cleanups for
CHNLSM_InterruptDSP2().

[Hiroshi DOYU: split the original to logical ones]

Reported-by: Roman Tereshonkov <roman.tereshonkov@nokia.com>
Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/wmd/tiomap_sm.c |   56 +++++++++++++++++------------------
 1 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index b5d3d6d..7d389e3 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -108,60 +108,58 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 	unsigned long timeout;
 	u32 temp;
 
-	status = CFG_GetHostResources((struct CFG_DEVNODE *)DRV_GetFirstDevExtension(),
-				      &resources);
+	status = CFG_GetHostResources((struct CFG_DEVNODE *)
+			DRV_GetFirstDevExtension(), &resources);
 	if (DSP_FAILED(status))
 		return DSP_EFAIL;
-#ifdef CONFIG_BRIDGE_DVFS
+
 	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
 	    pDevContext->dwBrdState == BRD_HIBERNATION) {
+#ifdef CONFIG_BRIDGE_DVFS
 		if (pdata->dsp_get_opp)
 			opplevel = (*pdata->dsp_get_opp)();
-		if (opplevel == 1) {
+		if (opplevel == VDD1_OPP1) {
 			if (pdata->dsp_set_min_opp)
-				(*pdata->dsp_set_min_opp)(opplevel+1);
+				(*pdata->dsp_set_min_opp)(VDD1_OPP2);
 		}
-	}
 #endif
-
-	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
-	    pDevContext->dwBrdState == BRD_HIBERNATION) {
 		/* Restart the peripheral clocks */
 		DSP_PeripheralClocks_Enable(pDevContext, NULL);
 
+		/*
+		 * 2:0 AUTO_IVA2_DPLL - Enabling IVA2 DPLL auto control
+		 *     in CM_AUTOIDLE_PLL_IVA2 register
+		 */
+		*(REG_UWORD32 *)(resources.dwCmBase + 0x34) = 0x1;
+
+		/*
+		 * 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
+		 */
+		temp = *(REG_UWORD32 *)(resources.dwCmBase + 0x4);
+		temp = (temp & 0xFFFFFF08) | 0x37;
+		*(REG_UWORD32 *)(resources.dwCmBase + 0x4) = temp;
+
 		/* Restore mailbox settings */
-		/* Enabling Dpll in lock mode*/
-		temp = (u32) *((REG_UWORD32 *)
-				((u32) (resources.dwCmBase) + 0x34));
-		temp = (temp & 0xFFFFFFFE) | 0x1;
-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
-			(u32) temp;
-		temp = (u32) *((REG_UWORD32 *)
-				((u32) (resources.dwCmBase) + 0x4));
-		temp = (temp & 0xFFFFFC8) | 0x37;
-
-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
-			(u32) temp;
 		HW_MBOX_restoreSettings(resources.dwMboxBase);
 
-		/*  Access MMU SYS CONFIG register to generate a short wakeup */
-		temp = (u32) *((REG_UWORD32 *) ((u32)
-						(resources.dwDmmuBase) + 0x10));
+		/* Access MMU SYS CONFIG register to generate a short wakeup */
+		temp = *(REG_UWORD32 *)(resources.dwDmmuBase + 0x10);
 
 		pDevContext->dwBrdState = BRD_RUNNING;
 	}
+
 	timeout = jiffies + msecs_to_jiffies(1);
 	while (fifo_full((void __iomem *) resources.dwMboxBase, 0)) {
 		if (time_after(jiffies, timeout)) {
-			printk(KERN_ERR "dspbridge: timed out waiting for mailbox\n");
+			pr_err("dspbridge: timed out waiting for mailbox\n");
 			return WMD_E_TIMEOUT;
 		}
 	}
-	DBG_Trace(DBG_LEVEL3, "writing %x to Mailbox\n",
-		  wMbVal);
 
-	HW_MBOX_MsgWrite(resources.dwMboxBase, MBOX_ARM2DSP,
-			 wMbVal);
+	DBG_Trace(DBG_LEVEL3, "writing %x to Mailbox\n", wMbVal);
+	HW_MBOX_MsgWrite(resources.dwMboxBase, MBOX_ARM2DSP, wMbVal);
 	return DSP_SOK;
 }
 
-- 
1.6.2.4

[PATCH 10/13] DSPBRIDGE: HACK: insert delay for DPLL stabilization

[Ameya Palande]

*HACK*: status register should be checked for this purpose.

[Hiroshi DOYU: split the original to logical ones]

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/wmd/tiomap_sm.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index 7d389e3..02e2675 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -141,6 +141,13 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 		temp = (temp & 0xFFFFFF08) | 0x37;
 		*(REG_UWORD32 *)(resources.dwCmBase + 0x4) = temp;
 
+		/*
+		 * This delay is needed to avoid mailbox timed out
+		 * issue experienced while SmartReflex is ON.
+		 * TODO: Instead of 1 ms calculate proper value.
+		 */
+		mdelay(1);
+
 		/* Restore mailbox settings */
 		HW_MBOX_restoreSettings(resources.dwMboxBase);
 
-- 
1.6.2.4

[PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode

[Ameya Palande]

Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
---
 drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
index 7cc29b7..7c7d9c4 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
@@ -167,17 +167,19 @@ DSP_STATUS handle_hibernation_fromDSP(struct WMD_DEV_CONTEXT *pDevContext)
 			if (DSP_FAILED(status))
 				return status;
 			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
-			/* Set the OPP to low level before moving to OFF mode */
 			if (opplevel != VDD1_OPP1) {
 				DBG_Trace(DBG_LEVEL5,
-					"Tiomap_pwr.c - DSP requested"
-					" OPP = %d, MPU requesting low"
-					" OPP %d instead\n", opplevel,
-					VDD1_OPP1);
-				if (pdata->dsp_set_min_opp)
-					(*pdata->dsp_set_min_opp)(VDD1_OPP1);
-				status = DSP_SOK;
+					" DSP requested OPP = %d, MPU"
+					" requesting low OPP %d instead\n",
+					opplevel, VDD1_OPP1);
 			}
+			/*
+			 * 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 = DSP_SOK;
 #endif /* CONFIG_BRIDGE_DVFS */
 		} else {
 			DBG_Trace(DBG_LEVEL7,
-- 
1.6.2.4

[PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

[Ameya Palande]

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

The constraints(opp >= 2) should be set whenever returning from DSP
hibernation.

This fixes the problem at the following steps:
1) wait DSP hibernate
2) echo mem > /sys/power/state
3) type key to wakeup

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/wmd/tiomap_sm.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index 02e2675..69f6fb3 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -98,11 +98,6 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct WMD_DEV_CONTEXT *pDevContext)
 DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 				u16 wMbVal)
 {
-#ifdef CONFIG_BRIDGE_DVFS
-	struct dspbridge_platform_data *pdata =
-		omap_dspbridge_dev->dev.platform_data;
-	u32 opplevel = 0;
-#endif
 	struct CFG_HOSTRES resources;
 	DSP_STATUS status = DSP_SOK;
 	unsigned long timeout;
@@ -116,12 +111,11 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
 	    pDevContext->dwBrdState == BRD_HIBERNATION) {
 #ifdef CONFIG_BRIDGE_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);
-		}
+		struct dspbridge_platform_data *pdata =
+			omap_dspbridge_dev->dev.platform_data;
+
+		if (pdata->dsp_set_min_opp)
+			(*pdata->dsp_set_min_opp)(VDD1_OPP2);
 #endif
 		/* Restart the peripheral clocks */
 		DSP_PeripheralClocks_Enable(pDevContext, NULL);
-- 
1.6.2.4

RE: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

[Guzman Lugo, Fernando]

Hi,

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Wednesday, July 15, 2009 9:57 AM
> To: linux-omap@vger.kernel.org
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> hiroshi.doyu@nokia.com
> Subject: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp
> hibernation
> 
> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> 
> The constraints(opp >= 2) should be set whenever returning from DSP
> hibernation.
> 
> This fixes the problem at the following steps:
> 1) wait DSP hibernate
> 2) echo mem > /sys/power/state
> 3) type key to wakeup
> 

The patch looks good, you are removing the check for OPP1 because it is suppose that if the DSP is in Hibernation we have a DSP constrain in OPP1, so I think the "if" is always true. However I don't see the issue the mention could you please tell me what was the problem you were seeing?


> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>  drivers/dsp/bridge/wmd/tiomap_sm.c |   16 +++++-----------
>  1 files changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c
> b/drivers/dsp/bridge/wmd/tiomap_sm.c
> index 02e2675..69f6fb3 100644
> --- a/drivers/dsp/bridge/wmd/tiomap_sm.c
> +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
> @@ -98,11 +98,6 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct
> WMD_DEV_CONTEXT *pDevContext)
>  DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
>  				u16 wMbVal)
>  {
> -#ifdef CONFIG_BRIDGE_DVFS
> -	struct dspbridge_platform_data *pdata =
> -		omap_dspbridge_dev->dev.platform_data;
> -	u32 opplevel = 0;
> -#endif
>  	struct CFG_HOSTRES resources;
>  	DSP_STATUS status = DSP_SOK;
>  	unsigned long timeout;
> @@ -116,12 +111,11 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct
> WMD_DEV_CONTEXT *pDevContext,
>  	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
>  	    pDevContext->dwBrdState == BRD_HIBERNATION) {
>  #ifdef CONFIG_BRIDGE_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);
> -		}
> +		struct dspbridge_platform_data *pdata =
> +			omap_dspbridge_dev->dev.platform_data;
> +
> +		if (pdata->dsp_set_min_opp)
> +			(*pdata->dsp_set_min_opp)(VDD1_OPP2);
>  #endif
>  		/* Restart the peripheral clocks */
>  		DSP_PeripheralClocks_Enable(pDevContext, NULL);
> --
> 1.6.2.4
> 

Regards,
Fernando.

Re: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

[Ameya Palande]

Hi Fernando,

ext Guzman Lugo, Fernando wrote:
> 
> Hi,
> 
>> -----Original Message-----
>> From: Ameya Palande [mailto:ameya.palande@nokia.com]
>> Sent: Wednesday, July 15, 2009 9:57 AM
>> To: linux-omap@vger.kernel.org
>> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
>> hiroshi.doyu@nokia.com
>> Subject: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp
>> hibernation
>>
>> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>>
>> The constraints(opp >= 2) should be set whenever returning from DSP
>> hibernation.
>>
>> This fixes the problem at the following steps:
>> 1) wait DSP hibernate
>> 2) echo mem > /sys/power/state
>> 3) type key to wakeup
>>
> 
> The patch looks good, you are removing the check for OPP1 because it is suppose
> that if the DSP is in Hibernation we have a DSP constrain in OPP1,
> so I think the "if" is always true. However I don't see the issue the mention
> could you please tell me what was the problem you were seeing?

While waking up DSP if VDD1 OPP is greater that 1 then without this patch,
constraint won't be set. This results in a case where PM layer later decides to
switch to OPP1 while DSP is active, which results in a crash. To avoid this we
are setting VDD1 to OPP2 unconditionally while waking up from hibernation :)

Cheers,
Ameya.

Re: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation

[Hiroshi DOYU]

From: "ext Guzman Lugo, Fernando" <x0095840@ti.com>
Subject: RE: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation
Date: Thu, 16 Jul 2009 01:10:58 +0200

> 
> 
> Hi,
> 
> > -----Original Message-----
> > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > Sent: Wednesday, July 15, 2009 9:57 AM
> > To: linux-omap@vger.kernel.org
> > Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> > hiroshi.doyu@nokia.com
> > Subject: [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp
> > hibernation
> > 
> > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > 
> > The constraints(opp >= 2) should be set whenever returning from DSP
> > hibernation.
> > 
> > This fixes the problem at the following steps:
> > 1) wait DSP hibernate
> > 2) echo mem > /sys/power/state
> > 3) type key to wakeup
> > 
> 
> The patch looks good, you are removing the check for OPP1 because it
> is suppose that if the DSP is in Hibernation we have a DSP constrain
> in OPP1, so I think the "if" is always true. However I don't see the
> issue the mention could you please tell me what was the problem you
> were seeing?

We haven't found the exact steps to reproduce this, but logically this
is necessary since it has to set *min* OPP2(constraints) in spite of
whatever the current OPP value is.

> 
> 
> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > ---
> >  drivers/dsp/bridge/wmd/tiomap_sm.c |   16 +++++-----------
> >  1 files changed, 5 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c
> > b/drivers/dsp/bridge/wmd/tiomap_sm.c
> > index 02e2675..69f6fb3 100644
> > --- a/drivers/dsp/bridge/wmd/tiomap_sm.c
> > +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
> > @@ -98,11 +98,6 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct
> > WMD_DEV_CONTEXT *pDevContext)
> >  DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
> >  				u16 wMbVal)
> >  {
> > -#ifdef CONFIG_BRIDGE_DVFS
> > -	struct dspbridge_platform_data *pdata =
> > -		omap_dspbridge_dev->dev.platform_data;
> > -	u32 opplevel = 0;
> > -#endif
> >  	struct CFG_HOSTRES resources;
> >  	DSP_STATUS status = DSP_SOK;
> >  	unsigned long timeout;
> > @@ -116,12 +111,11 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct
> > WMD_DEV_CONTEXT *pDevContext,
> >  	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
> >  	    pDevContext->dwBrdState == BRD_HIBERNATION) {
> >  #ifdef CONFIG_BRIDGE_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);
> > -		}
> > +		struct dspbridge_platform_data *pdata =
> > +			omap_dspbridge_dev->dev.platform_data;
> > +
> > +		if (pdata->dsp_set_min_opp)
> > +			(*pdata->dsp_set_min_opp)(VDD1_OPP2);
> >  #endif
> >  		/* Restart the peripheral clocks */
> >  		DSP_PeripheralClocks_Enable(pDevContext, NULL);
> > --
> > 1.6.2.4
> > 
> 
> Regards,
> Fernando.

RE: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode

[Guzman Lugo, Fernando]

Hi,

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Wednesday, July 15, 2009 9:57 AM
> To: linux-omap@vger.kernel.org
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> hiroshi.doyu@nokia.com
> Subject: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while
> moving to off mode
> 
> Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
> ---
>  drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   18 ++++++++++--------
>  1 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> index 7cc29b7..7c7d9c4 100644
> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> @@ -167,17 +167,19 @@ DSP_STATUS handle_hibernation_fromDSP(struct
> WMD_DEV_CONTEXT *pDevContext)
>  			if (DSP_FAILED(status))
>  				return status;
>  			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
> -			/* Set the OPP to low level before moving to OFF mode */
>  			if (opplevel != VDD1_OPP1) {
>  				DBG_Trace(DBG_LEVEL5,
> -					"Tiomap_pwr.c - DSP requested"
> -					" OPP = %d, MPU requesting low"
> -					" OPP %d instead\n", opplevel,
> -					VDD1_OPP1);
> -				if (pdata->dsp_set_min_opp)
> -					(*pdata->dsp_set_min_opp)(VDD1_OPP1);
> -				status = DSP_SOK;
> +					" DSP requested OPP = %d, MPU"
> +					" requesting low OPP %d instead\n",
> +					opplevel, VDD1_OPP1);
>  			}
> +			/*
> +			 * 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 = DSP_SOK;

I think that the "if statement" is only avoiding setting the OPP1 if the DSP is already in OPP1. Were you seeing an issue without this change?

>  #endif /* CONFIG_BRIDGE_DVFS */
>  		} else {
>  			DBG_Trace(DBG_LEVEL7,
> --
> 1.6.2.4
> 

Regards,
Fernando.

Re: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode

[Hiroshi DOYU]

From: "ext Guzman Lugo, Fernando" <x0095840@ti.com>
Subject: RE: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode
Date: Thu, 16 Jul 2009 00:53:24 +0200

> 
> Hi,
> 
> > -----Original Message-----
> > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > Sent: Wednesday, July 15, 2009 9:57 AM
> > To: linux-omap@vger.kernel.org
> > Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> > hiroshi.doyu@nokia.com
> > Subject: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while
> > moving to off mode
> > 
> > Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
> > ---
> >  drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   18 ++++++++++--------
> >  1 files changed, 10 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > index 7cc29b7..7c7d9c4 100644
> > --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > @@ -167,17 +167,19 @@ DSP_STATUS handle_hibernation_fromDSP(struct
> > WMD_DEV_CONTEXT *pDevContext)
> >  			if (DSP_FAILED(status))
> >  				return status;
> >  			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
> > -			/* Set the OPP to low level before moving to OFF mode */
> >  			if (opplevel != VDD1_OPP1) {
> >  				DBG_Trace(DBG_LEVEL5,
> > -					"Tiomap_pwr.c - DSP requested"
> > -					" OPP = %d, MPU requesting low"
> > -					" OPP %d instead\n", opplevel,
> > -					VDD1_OPP1);
> > -				if (pdata->dsp_set_min_opp)
> > -					(*pdata->dsp_set_min_opp)(VDD1_OPP1);
> > -				status = DSP_SOK;
> > +					" DSP requested OPP = %d, MPU"
> > +					" requesting low OPP %d instead\n",
> > +					opplevel, VDD1_OPP1);
> >  			}
> > +			/*
> > +			 * 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 = DSP_SOK;
> 
> I think that the "if statement" is only avoiding setting the OPP1 if
> the DSP is already in OPP1. Were you seeing an issue without this
> change?

We got the different opplevel with the following patch, which means
that, OPP value stored in SHM isn't equal to the real OPP since we've
introduce some codes to bump up OPP1 to OPP2, but never written back
to SHM.

	Modified drivers/dsp/bridge/wmd/tiomap3430_pwr.c
diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
index 5f7e855..9cde41f 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
@@ -167,6 +167,10 @@ DSP_STATUS handle_hibernation_fromDSP(struct WMD_DEV_CONTEXT *pDevContext)
 			if (DSP_FAILED(status))
 				return status;
 			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
+			pr_info("%s(%d) %d\n", __func__, __LINE__, opplevel);
+			if (pdata->dsp_get_opp)
+				opplevel = (*pdata->dsp_get_opp)();
+			pr_info("%s(%d) %d\n", __func__, __LINE__, opplevel);
 			/* Set the OPP to low level before moving to OFF mode */
 			if (opplevel != VDD1_OPP1) {
 				DBG_Trace(DBG_LEVEL5,

> 
> >  #endif /* CONFIG_BRIDGE_DVFS */
> >  		} else {
> >  			DBG_Trace(DBG_LEVEL7,
> > --
> > 1.6.2.4
> > 
> 
> Regards,
> Fernando.
> 

RE: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode

[Guzman Lugo, Fernando]

> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> Sent: Wednesday, July 15, 2009 6:10 PM
> To: Guzman Lugo, Fernando
> Cc: ameya.palande@nokia.com; linux-omap@vger.kernel.org; Ramirez Luna,
> Omar; Menon, Nishanth
> Subject: Re: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while
> moving to off mode
> 
> From: "ext Guzman Lugo, Fernando" <x0095840@ti.com>
> Subject: RE: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while
> moving to off mode
> Date: Thu, 16 Jul 2009 00:53:24 +0200
> 
> >
> > Hi,
> >
> > > -----Original Message-----
> > > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > > Sent: Wednesday, July 15, 2009 9:57 AM
> > > To: linux-omap@vger.kernel.org
> > > Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> > > hiroshi.doyu@nokia.com
> > > Subject: [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while
> > > moving to off mode
> > >
> > > Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
> > > ---
> > >  drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   18 ++++++++++--------
> > >  1 files changed, 10 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > > b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > > index 7cc29b7..7c7d9c4 100644
> > > --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > > +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> > > @@ -167,17 +167,19 @@ DSP_STATUS handle_hibernation_fromDSP(struct
> > > WMD_DEV_CONTEXT *pDevContext)
> > >  			if (DSP_FAILED(status))
> > >  				return status;
> > >  			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
> > > -			/* Set the OPP to low level before moving to OFF mode */
> > >  			if (opplevel != VDD1_OPP1) {
> > >  				DBG_Trace(DBG_LEVEL5,
> > > -					"Tiomap_pwr.c - DSP requested"
> > > -					" OPP = %d, MPU requesting low"
> > > -					" OPP %d instead\n", opplevel,
> > > -					VDD1_OPP1);
> > > -				if (pdata->dsp_set_min_opp)
> > > -					(*pdata->dsp_set_min_opp)(VDD1_OPP1);
> > > -				status = DSP_SOK;
> > > +					" DSP requested OPP = %d, MPU"
> > > +					" requesting low OPP %d instead\n",
> > > +					opplevel, VDD1_OPP1);
> > >  			}
> > > +			/*
> > > +			 * 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 = DSP_SOK;
> >
> > I think that the "if statement" is only avoiding setting the OPP1 if
> > the DSP is already in OPP1. Were you seeing an issue without this
> > change?
> 
> We got the different opplevel with the following patch, which means
> that, OPP value stored in SHM isn't equal to the real OPP since we've
> introduce some codes to bump up OPP1 to OPP2, but never written back
> to SHM.

Ok I see the problem, then the patch looks good for me it will resolve the problem.

> 
> 	Modified drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> index 5f7e855..9cde41f 100644
> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> @@ -167,6 +167,10 @@ DSP_STATUS handle_hibernation_fromDSP(struct
> WMD_DEV_CONTEXT *pDevContext)
>  			if (DSP_FAILED(status))
>  				return status;
>  			IO_SHMsetting(hIOMgr, SHM_GETOPP, &opplevel);
> +			pr_info("%s(%d) %d\n", __func__, __LINE__, opplevel);
> +			if (pdata->dsp_get_opp)
> +				opplevel = (*pdata->dsp_get_opp)();
> +			pr_info("%s(%d) %d\n", __func__, __LINE__, opplevel);
>  			/* Set the OPP to low level before moving to OFF mode */
>  			if (opplevel != VDD1_OPP1) {
>  				DBG_Trace(DBG_LEVEL5,
> 
> >
> > >  #endif /* CONFIG_BRIDGE_DVFS */
> > >  		} else {
> > >  			DBG_Trace(DBG_LEVEL7,
> > > --
> > > 1.6.2.4
> > >
> >
> > Regards,
> > Fernando.
> >

Acked-by Fernando Guzman Lugo <x0095840@ti.com>

Re: [PATCH 10/13] DSPBRIDGE: HACK: insert delay for DPLL stabilization

[Hiroshi DOYU]

Hi,

From: "Palande Ameya (Nokia-D/Helsinki)" <ameya.palande@nokia.com>
Subject: [PATCH 10/13] DSPBRIDGE: HACK: insert delay for DPLL stabilization
Date: Wed, 15 Jul 2009 16:56:33 +0200

> *HACK*: status register should be checked for this purpose.
> 
> [Hiroshi DOYU: split the original to logical ones]
> 
> Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
> Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
> ---
>  drivers/dsp/bridge/wmd/tiomap_sm.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
> index 7d389e3..02e2675 100644
> --- a/drivers/dsp/bridge/wmd/tiomap_sm.c
> +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
> @@ -141,6 +141,13 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
>  		temp = (temp & 0xFFFFFF08) | 0x37;
>  		*(REG_UWORD32 *)(resources.dwCmBase + 0x4) = temp;
>  
> +		/*
> +		 * This delay is needed to avoid mailbox timed out
> +		 * issue experienced while SmartReflex is ON.
> +		 * TODO: Instead of 1 ms calculate proper value.
> +		 */
> +		mdelay(1);
> +

I have tried the following patch and it seems that CM_IDLEST_PLL_IVA2
has never got locked.

Is any special sequence required?

>From af2ab46ad5bdcae57e7bf9ac735e3862ec2f45e6 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Thu, 16 Jul 2009 11:48:19 +0300
Subject: [PATCH 1/1] DSPBRIDGE: poll IDLEST_PLL register for DPLL locking

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/wmd/tiomap_sm.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index ca9202d..aaea896 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -137,12 +137,21 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
 		temp = (temp & 0xFFFFFF08) | 0x37;
 		*(REG_UWORD32 *)(resources.dwCmBase + 0x4) = temp;
 
-		/*
-		 * This delay is needed to avoid mailbox timed out
-		 * issue experienced while SmartReflex is ON.
-		 * TODO: Instead of 1 ms calculate proper value.
-		 */
-		mdelay(1);
+		/* Poll CM_IDLEST_PLL_IVA2 */
+		timeout = jiffies + msecs_to_jiffies(1000);
+		while (1) {
+			u32 l;
+
+			l = __raw_readl(resources.dwCmBase + 0x24);
+			if (l & 1)
+				break;
+
+			if (time_after(jiffies, timeout)) {
+				pr_err("%s(): dpll2 lock timeout\n", __func__);
+				return WMD_E_TIMEOUT;
+			}
+		}
+		pr_info("%s(): dpll2 locked\n", __func__);
 
 		/* Restore mailbox settings */
 		HW_MBOX_restoreSettings(resources.dwMboxBase);
-- 
1.6.0.4



>  		/* Restore mailbox settings */
>  		HW_MBOX_restoreSettings(resources.dwMboxBase);
>  
> -- 
> 1.6.2.4
> 

RE: [PATCH 09/13] DSPBRIDGE: fix incorrect mask of DPLL in CHNLSM_InterruptDSP2()

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 09/13] DSPBRIDGE: fix incorrect mask of DPLL in CHNLSM_InterruptDSP2()
>
>This patch also contains indentation fixes and cleanups for
>CHNLSM_InterruptDSP2().
>
>[Hiroshi DOYU: split the original to logical ones]
>
>Reported-by: Roman Tereshonkov <roman.tereshonkov@nokia.com>
>Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
>Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>---
> drivers/dsp/bridge/wmd/tiomap_sm.c |   56 +++++++++++++++++------------------
> 1 files changed, 27 insertions(+), 29 deletions(-)
>

Pushed to d.o-z

RE: [PATCH 08/13] DSPBRIDGE: Prevent memory access during hibernation

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 08/13] DSPBRIDGE: Prevent memory access during hibernation
>
>If DSP is in hibernated then WakeDSP() returns without waking it up, which
>results in flush_all function access IVA MMU while IVA is hibernated.
>
>[Hiroshi DOYU: split the original to logical ones]
>
>Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
>Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>---
> drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   65 +++++++++++--------------------
> drivers/dsp/bridge/wmd/tiomap_sm.c      |   32 +++++++--------
> 2 files changed, 38 insertions(+), 59 deletions(-)
>
>diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>index a725548..7cc29b7 100644
>--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>@@ -295,59 +295,40 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>  */
> DSP_STATUS WakeDSP(struct WMD_DEV_CONTEXT *pDevContext, IN void *pArgs)
> {
>-	DSP_STATUS status = DSP_SOK;
> #ifdef CONFIG_PM
>-	struct CFG_HOSTRES resources;
>+	DSP_STATUS status = DSP_SOK;
>+#ifdef CONFIG_BRIDGE_DEBUG
> 	enum HW_PwrState_t pwrState;
>-	u32 temp;
>+	struct CFG_HOSTRES resources;
>
> 	status = CFG_GetHostResources(
> 		 (struct CFG_DEVNODE *)DRV_GetFirstDevExtension(), &resources);
> 	if (DSP_FAILED(status))
> 		return status;
>-	/* check the BRD/WMD state, if it is not 'SLEEP' then return failure */
>+#endif /* CONFIG_BRIDGE_DEBUG */
>+
>+	/* Check the BRD/WMD state, if it is not 'SLEEP' then return failure */
> 	if (pDevContext->dwBrdState == BRD_RUNNING ||
>-		pDevContext->dwBrdState == BRD_STOPPED ||
>-		pDevContext->dwBrdState == BRD_DSP_HIBERNATION) {
>+	    pDevContext->dwBrdState == BRD_STOPPED) {
> 		/* The Device is in 'RET' or 'OFF' state and WMD state is not
> 		 * 'SLEEP', this means state inconsistency, so return  */
>-		status = DSP_SOK;
>-		return status;
>-	}
>-	/* Enable the DSP peripheral clocks and load monitor timer
>-	 * before waking the DSP */
>-	DBG_Trace(DBG_LEVEL6, "WakeDSP: enable DSP Peripheral Clks = 0x%x \n",
>-		 pDevContext->uDspPerClks);
>-	status = DSP_PeripheralClocks_Enable(pDevContext, NULL);
>-
>-	/* Enabling Dppll in lock mode */
>-		temp = (u32) *((REG_UWORD32 *)
>-			((u32) (resources.dwCmBase) + 0x34));
>-		temp = (temp & 0xFFFFFFFE) | 0x1;
>-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
>-						(u32) temp;
>-		temp = (u32) *((REG_UWORD32 *)
>-			((u32) (resources.dwCmBase) + 0x4));
>-		temp = (temp & 0xFFFFFC8) | 0x37;
>-
>-		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
>-						(u32) temp;
>-
>-	udelay(10);
>-	if (DSP_SUCCEEDED(status)) {
>-		/* Send a message to DSP to wake up */
>-		CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP);
>-		HW_PWR_IVA2StateGet(resources.dwPrmBase, HW_PWR_DOMAIN_DSP,
>-				    &pwrState);
>-		DBG_Trace(DBG_LEVEL7,
>-			 "\nWakeDSP: Power State After sending Interrupt "
>-			 "to DSP %x\n", pwrState);
>-		/* set the device state to RUNNIG */
>-		pDevContext->dwBrdState = BRD_RUNNING;
>-	} else {
>-		DBG_Trace(DBG_LEVEL6, "WakeDSP: FAILED\n");
>+		return DSP_SOK;
> 	}
>-#endif
>+
>+	/* Send a wakeup message to DSP */
>+	CHNLSM_InterruptDSP2(pDevContext, MBX_PM_DSPWAKEUP);
>+
>+#ifdef CONFIG_BRIDGE_DEBUG
>+	HW_PWR_IVA2StateGet(resources.dwPrmBase, HW_PWR_DOMAIN_DSP,
>+			&pwrState);
>+	DBG_Trace(DBG_LEVEL7,
>+			"\nWakeDSP: Power State After sending Interrupt "
>+			"to DSP %x\n", pwrState);
>+#endif /* CONFIG_BRIDGE_DEBUG */
>+
>+	/* Set the device state to RUNNIG */
>+	pDevContext->dwBrdState = BRD_RUNNING;
>+#endif /* CONFIG_PM */
> 	return status;
> }

Reworked compiler error ifndef CONFIG_PM: "undeclared status"

>
>diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
>index a6d5d62..b5d3d6d 100644
>--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
>+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
>@@ -126,24 +126,22 @@ DSP_STATUS CHNLSM_InterruptDSP2(struct WMD_DEV_CONTEXT *pDevContext,
>
> 	if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION ||
> 	    pDevContext->dwBrdState == BRD_HIBERNATION) {
>+		/* Restart the peripheral clocks */
>+		DSP_PeripheralClocks_Enable(pDevContext, NULL);
>+
> 		/* Restore mailbox settings */
>-		/* Restart the peripheral clocks that were disabled only
>-		 * in DSP initiated Hibernation case.*/
>-		if (pDevContext->dwBrdState == BRD_DSP_HIBERNATION) {
>-			DSP_PeripheralClocks_Enable(pDevContext, NULL);
>-			/* Enabling Dpll in lock mode*/
>-			temp = (u32) *((REG_UWORD32 *)
>-				       ((u32) (resources.dwCmBase) + 0x34));
>-			temp = (temp & 0xFFFFFFFE) | 0x1;
>-			*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
>-				(u32) temp;
>-			temp = (u32) *((REG_UWORD32 *)
>-				       ((u32) (resources.dwCmBase) + 0x4));
>-			temp = (temp & 0xFFFFFC8) | 0x37;
>-
>-			*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
>-				(u32) temp;
>-		}
>+		/* Enabling Dpll in lock mode*/
>+		temp = (u32) *((REG_UWORD32 *)
>+				((u32) (resources.dwCmBase) + 0x34));
>+		temp = (temp & 0xFFFFFFFE) | 0x1;
>+		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x34)) =
>+			(u32) temp;
>+		temp = (u32) *((REG_UWORD32 *)
>+				((u32) (resources.dwCmBase) + 0x4));
>+		temp = (temp & 0xFFFFFC8) | 0x37;
>+
>+		*((REG_UWORD32 *) ((u32) (resources.dwCmBase) + 0x4)) =
>+			(u32) temp;
> 		HW_MBOX_restoreSettings(resources.dwMboxBase);
>
> 		/*  Access MMU SYS CONFIG register to generate a short wakeup */
>--
>1.6.2.4
>

Pushed to d.o-z

- omar

RE: [PATCH 06/13] DSPBRIDGE: avoid to dump unnecessary registers without debug

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 06/13] DSPBRIDGE: avoid to dump unnecessary registers without debug
>
>Make "GetHWRegs()" available only with CONFIG_BRIDGE_DEBUG.
>
>[Hiroshi DOYU: split the original to logical ones]
>
>Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
>Acked-by: Omar Ramirez Luna <omar.ramirez@ti.com>
>---
> drivers/dsp/bridge/wmd/tiomap3430.c |   57 +++++++++++++++++++----------------
> 1 files changed, 31 insertions(+), 26 deletions(-)
>

Pushed to d.o-z

- omar

RE: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig

[Guzman Lugo, Fernando]

Hi,

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Wednesday, July 15, 2009 9:56 AM
> To: linux-omap@vger.kernel.org
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> hiroshi.doyu@nokia.com
> Subject: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox
> sysconfig
> 
> From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>
> 
> Reported-by: Tero Kristo <tero.kristo@nokia.com>
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>  drivers/dsp/bridge/hw/hw_mbox.c |    4 +++-
>  drivers/dsp/bridge/hw/hw_mbox.h |    5 +++++
>  drivers/dsp/bridge/wmd/io_sm.c  |    1 +
>  3 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/hw/hw_mbox.c
> b/drivers/dsp/bridge/hw/hw_mbox.c
> index ee79032..5a87597 100644
> --- a/drivers/dsp/bridge/hw/hw_mbox.c
> +++ b/drivers/dsp/bridge/hw/hw_mbox.c
> @@ -33,7 +33,9 @@
>  /* width in bits of MBOX Id */
>  #define HW_MBOX_ID_WIDTH	   2
> 
> -struct MAILBOX_CONTEXT mboxsetting = {0x4, 0x1, 0x1};
> +static struct MAILBOX_CONTEXT mboxsetting = {
> +	.sysconfig = 2 << 3 | 1, /* SMART/AUTO-IDLE */
> +};
> 
>  /* Saves the mailbox context */
>  HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddress)
> diff --git a/drivers/dsp/bridge/hw/hw_mbox.h
> b/drivers/dsp/bridge/hw/hw_mbox.h
> index ad1a89c..8a5f6bd 100644
> --- a/drivers/dsp/bridge/hw/hw_mbox.h
> +++ b/drivers/dsp/bridge/hw/hw_mbox.h
> @@ -320,4 +320,9 @@ extern HW_STATUS HW_MBOX_saveSettings(void __iomem
> *baseAddres);
>  */
>  extern HW_STATUS HW_MBOX_restoreSettings(void __iomem *baseAddres);
> 
> +static inline void HW_MBOX_initSettings(void __iomem *baseAddres)
> +{
> +	HW_MBOX_restoreSettings(baseAddres);
> +}
> +
>  #endif  /* __MBOX_H */
> diff --git a/drivers/dsp/bridge/wmd/io_sm.c
> b/drivers/dsp/bridge/wmd/io_sm.c
> index 39c34f7..d8ae1f1 100644
> --- a/drivers/dsp/bridge/wmd/io_sm.c
> +++ b/drivers/dsp/bridge/wmd/io_sm.c
> @@ -282,6 +282,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
>  		pIOMgr->fSharedIRQ = pMgrAttrs->fShared;
>  		IO_DisableInterrupt(hWmdContext);
>  		if (devType == DSP_UNIT) {
> +			HW_MBOX_initSettings(hostRes.dwMboxBase);

What do think about doing a call to __raw_writel to avoid a new function wrapper as Omar suggested? Something like that:
				
			/* Enabling mailbox SMART/AUTO-IDLE */
			__raw_writel(2 << 3 | 1, hostRes.dwMboxBase + 10);


>  			/* Plug the channel ISR:. */
>  			if ((request_irq(INT_MAIL_MPU_IRQ, IO_ISR, 0,
>  				"DspBridge\tmailbox", (void *)pIOMgr)) == 0)
> --
> 1.6.2.4
> 

Regards,
Fernando
Guzman
Lugo.

Re: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig

[Nishanth Menon]

Guzman Lugo, Fernando had written, on 07/15/2009 04:59 PM, the following:
>> diff --git a/drivers/dsp/bridge/hw/hw_mbox.c
>> b/drivers/dsp/bridge/hw/hw_mbox.c
>> index ee79032..5a87597 100644
>> --- a/drivers/dsp/bridge/hw/hw_mbox.c
>> +++ b/drivers/dsp/bridge/hw/hw_mbox.c
>> @@ -33,7 +33,9 @@
>>  /* width in bits of MBOX Id */
>>  #define HW_MBOX_ID_WIDTH	   2
>>
>> -struct MAILBOX_CONTEXT mboxsetting = {0x4, 0x1, 0x1};
>> +static struct MAILBOX_CONTEXT mboxsetting = {
>> +	.sysconfig = 2 << 3 | 1, /* SMART/AUTO-IDLE */
>> +};
>>
>>  /* Saves the mailbox context */
>>  HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddress)
>> diff --git a/drivers/dsp/bridge/hw/hw_mbox.h
>> b/drivers/dsp/bridge/hw/hw_mbox.h
>> index ad1a89c..8a5f6bd 100644
>> --- a/drivers/dsp/bridge/hw/hw_mbox.h
>> +++ b/drivers/dsp/bridge/hw/hw_mbox.h
>> @@ -320,4 +320,9 @@ extern HW_STATUS HW_MBOX_saveSettings(void __iomem
>> *baseAddres);
>>  */
>>  extern HW_STATUS HW_MBOX_restoreSettings(void __iomem *baseAddres);
>>
>> +static inline void HW_MBOX_initSettings(void __iomem *baseAddres)
>> +{
>> +	HW_MBOX_restoreSettings(baseAddres);
>> +}
>> +
>>  #endif  /* __MBOX_H */
>> diff --git a/drivers/dsp/bridge/wmd/io_sm.c
>> b/drivers/dsp/bridge/wmd/io_sm.c
>> index 39c34f7..d8ae1f1 100644
>> --- a/drivers/dsp/bridge/wmd/io_sm.c
>> +++ b/drivers/dsp/bridge/wmd/io_sm.c
>> @@ -282,6 +282,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
>>  		pIOMgr->fSharedIRQ = pMgrAttrs->fShared;
>>  		IO_DisableInterrupt(hWmdContext);
>>  		if (devType == DSP_UNIT) {
>> +			HW_MBOX_initSettings(hostRes.dwMboxBase);
> 
> What do think about doing a call to __raw_writel to avoid a new function wrapper as Omar suggested? Something like that:
> 				
> 			/* Enabling mailbox SMART/AUTO-IDLE */
> 			__raw_writel(2 << 3 | 1, hostRes.dwMboxBase + 10);
> 
> 
I think this is cleaner than piggybacking on the context save/restore 
structure. We should use macros though for 2<<3 | 1 etc.. just my 2Cents 
for readability.

Regards,
Nishanth Menon

Re: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig

[Hiroshi DOYU]

From: ext Nishanth Menon <nm@ti.com>
Subject: Re: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig
Date: Thu, 16 Jul 2009 00:01:11 +0200

> Guzman Lugo, Fernando had written, on 07/15/2009 04:59 PM, the following:
> >> diff --git a/drivers/dsp/bridge/hw/hw_mbox.c
> >> b/drivers/dsp/bridge/hw/hw_mbox.c
> >> index ee79032..5a87597 100644
> >> --- a/drivers/dsp/bridge/hw/hw_mbox.c
> >> +++ b/drivers/dsp/bridge/hw/hw_mbox.c
> >> @@ -33,7 +33,9 @@
> >>  /* width in bits of MBOX Id */
> >>  #define HW_MBOX_ID_WIDTH	   2
> >>
> >> -struct MAILBOX_CONTEXT mboxsetting = {0x4, 0x1, 0x1};
> >> +static struct MAILBOX_CONTEXT mboxsetting = {
> >> +	.sysconfig = 2 << 3 | 1, /* SMART/AUTO-IDLE */
> >> +};
> >>
> >>  /* Saves the mailbox context */
> >>  HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddress)
> >> diff --git a/drivers/dsp/bridge/hw/hw_mbox.h
> >> b/drivers/dsp/bridge/hw/hw_mbox.h
> >> index ad1a89c..8a5f6bd 100644
> >> --- a/drivers/dsp/bridge/hw/hw_mbox.h
> >> +++ b/drivers/dsp/bridge/hw/hw_mbox.h
> >> @@ -320,4 +320,9 @@ extern HW_STATUS HW_MBOX_saveSettings(void __iomem
> >> *baseAddres);
> >>  */
> >>  extern HW_STATUS HW_MBOX_restoreSettings(void __iomem *baseAddres);
> >>
> >> +static inline void HW_MBOX_initSettings(void __iomem *baseAddres)
> >> +{
> >> +	HW_MBOX_restoreSettings(baseAddres);
> >> +}
> >> +
> >>  #endif  /* __MBOX_H */
> >> diff --git a/drivers/dsp/bridge/wmd/io_sm.c
> >> b/drivers/dsp/bridge/wmd/io_sm.c
> >> index 39c34f7..d8ae1f1 100644
> >> --- a/drivers/dsp/bridge/wmd/io_sm.c
> >> +++ b/drivers/dsp/bridge/wmd/io_sm.c
> >> @@ -282,6 +282,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
> >>  		pIOMgr->fSharedIRQ = pMgrAttrs->fShared;
> >>  		IO_DisableInterrupt(hWmdContext);
> >>  		if (devType == DSP_UNIT) {
> >> +			HW_MBOX_initSettings(hostRes.dwMboxBase);
> > 
> > What do think about doing a call to __raw_writel to avoid a new function wrapper as Omar suggested? Something like that:
> > 				
> > 			/* Enabling mailbox SMART/AUTO-IDLE */
> > 			__raw_writel(2 << 3 | 1, hostRes.dwMboxBase + 10);
> > 
> > 
> I think this is cleaner than piggybacking on the context save/restore 
> structure.

Hm....let me know your comment on my explanation in another reply.

> We should use macros though for 2<<3 | 1 etc.. just my 2Cents 
> for readability.

Agree.

Considering the encapsulation of hw_mbox to conceal the direct h/w
manipulation, introducing those macros for register bit definition
would be located in "hw_mbox.c", I guess.....


> 
> Regards,
> Nishanth Menon
> --
> 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

Re: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig

[Hiroshi DOYU]

Hi Fernando, Thank you for reviewing.

From: "ext Guzman Lugo, Fernando" <x0095840@ti.com>
Subject: RE: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig
Date: Wed, 15 Jul 2009 23:59:22 +0200

> 
> Hi,
> 
> > -----Original Message-----
> > From: Ameya Palande [mailto:ameya.palande@nokia.com]
> > Sent: Wednesday, July 15, 2009 9:56 AM
> > To: linux-omap@vger.kernel.org
> > Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> > hiroshi.doyu@nokia.com
> > Subject: [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox
> > sysconfig
> > 
> > From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>
> > 
> > Reported-by: Tero Kristo <tero.kristo@nokia.com>
> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > ---
> >  drivers/dsp/bridge/hw/hw_mbox.c |    4 +++-
> >  drivers/dsp/bridge/hw/hw_mbox.h |    5 +++++
> >  drivers/dsp/bridge/wmd/io_sm.c  |    1 +
> >  3 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/dsp/bridge/hw/hw_mbox.c
> > b/drivers/dsp/bridge/hw/hw_mbox.c
> > index ee79032..5a87597 100644
> > --- a/drivers/dsp/bridge/hw/hw_mbox.c
> > +++ b/drivers/dsp/bridge/hw/hw_mbox.c
> > @@ -33,7 +33,9 @@
> >  /* width in bits of MBOX Id */
> >  #define HW_MBOX_ID_WIDTH	   2
> > 
> > -struct MAILBOX_CONTEXT mboxsetting = {0x4, 0x1, 0x1};
> > +static struct MAILBOX_CONTEXT mboxsetting = {
> > +	.sysconfig = 2 << 3 | 1, /* SMART/AUTO-IDLE */
> > +};
> > 
> >  /* Saves the mailbox context */
> >  HW_STATUS HW_MBOX_saveSettings(void __iomem *baseAddress)
> > diff --git a/drivers/dsp/bridge/hw/hw_mbox.h
> > b/drivers/dsp/bridge/hw/hw_mbox.h
> > index ad1a89c..8a5f6bd 100644
> > --- a/drivers/dsp/bridge/hw/hw_mbox.h
> > +++ b/drivers/dsp/bridge/hw/hw_mbox.h
> > @@ -320,4 +320,9 @@ extern HW_STATUS HW_MBOX_saveSettings(void __iomem
> > *baseAddres);
> >  */
> >  extern HW_STATUS HW_MBOX_restoreSettings(void __iomem *baseAddres);
> > 
> > +static inline void HW_MBOX_initSettings(void __iomem *baseAddres)
> > +{
> > +	HW_MBOX_restoreSettings(baseAddres);
> > +}
> > +
> >  #endif  /* __MBOX_H */
> > diff --git a/drivers/dsp/bridge/wmd/io_sm.c
> > b/drivers/dsp/bridge/wmd/io_sm.c
> > index 39c34f7..d8ae1f1 100644
> > --- a/drivers/dsp/bridge/wmd/io_sm.c
> > +++ b/drivers/dsp/bridge/wmd/io_sm.c
> > @@ -282,6 +282,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
> >  		pIOMgr->fSharedIRQ = pMgrAttrs->fShared;
> >  		IO_DisableInterrupt(hWmdContext);
> >  		if (devType == DSP_UNIT) {
> > +			HW_MBOX_initSettings(hostRes.dwMboxBase);
> 
> What do think about doing a call to __raw_writel to avoid a new function wrapper as Omar suggested? Something like that:
> 				
> 			/* Enabling mailbox SMART/AUTO-IDLE */
> 			__raw_writel(2 << 3 | 1, hostRes.dwMboxBase + 10);

I think that the basic concept of "hw_mbox.[ch]" is to abstruct
h/w mailbox feature, to provide lowlevel mailbox APIs(h/w accessors),
and to conceal the direct h/w manipulation from other
modules. Considering this concept, manipulating mailbox
registers with __raw_writel() looks to conflict this original design
purpose, IMHO.

> 
> 
> >  			/* Plug the channel ISR:. */
> >  			if ((request_irq(INT_MAIL_MPU_IRQ, IO_ISR, 0,
> >  				"DspBridge\tmailbox", (void *)pIOMgr)) == 0)
> > --
> > 1.6.2.4
> > 
> 
> Regards,
> Fernando
> Guzman
> Lugo.
> 

RE: [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else

[Guzman Lugo, Fernando]

The patch looks good for me.

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com]
> Sent: Wednesday, July 15, 2009 9:56 AM
> To: linux-omap@vger.kernel.org
> Cc: Ramirez Luna, Omar; Guzman Lugo, Fernando; Menon, Nishanth;
> hiroshi.doyu@nokia.com
> Subject: [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an
> if/else
> 
> From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>
> 
> Based on the following discussion:
>       http://marc.info/?l=linux-omap&m=124697893724881&w=2
> 
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>  drivers/dsp/bridge/pmgr/wcd.c |   60 ++++++++++++++++++++++++++----------
> ----
>  1 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/pmgr/wcd.c b/drivers/dsp/bridge/pmgr/wcd.c
> index 441130c..8708c78 100644
> --- a/drivers/dsp/bridge/pmgr/wcd.c
> +++ b/drivers/dsp/bridge/pmgr/wcd.c
> @@ -145,27 +145,45 @@
>  #define MAX_STREAMS     16
>  #define MAX_BUFS	64
> 
> -/* Following two macros should ideally have do{}while(0) */
> -
> -#define cp_fm_usr(dest, src, status, elements)    \
> -    if (DSP_SUCCEEDED(status)) {\
> -	    if (unlikely(src == NULL) ||				\
> -		unlikely(copy_from_user(dest, src, elements *
> sizeof(*(dest))))) { \
> -		GT_1trace(WCD_debugMask, GT_7CLASS, \
> -		"copy_from_user failed, src=0x%x\n", src);  \
> -		status = DSP_EPOINTER ; \
> -	} \
> -    }
> -
> -#define cp_to_usr(dest, src, status, elements)    \
> -    if (DSP_SUCCEEDED(status)) {\
> -	    if (unlikely(dest == NULL) ||				\
> -		unlikely(copy_to_user(dest, src, elements * sizeof(*(src)))))
> { \
> -		GT_1trace(WCD_debugMask, GT_7CLASS, \
> -		"copy_to_user failed, dest=0x%x\n", dest); \
> -		status = DSP_EPOINTER ;\
> -	} \
> -    }
> +static inline void __cp_fm_usr(void *to, const void __user *from,
> +			       DSP_STATUS *err, unsigned long bytes)
> +{
> +	if (DSP_FAILED(*err))
> +		return;
> +
> +	if (unlikely(!from)) {
> +		*err = DSP_EPOINTER;
> +		return;
> +	}
> +
> +	if (unlikely(copy_from_user(to, from, bytes))) {
> +		GT_2trace(WCD_debugMask, GT_7CLASS,
> +			  "%s failed, from=0x%08x\n", __func__, from);
> +		*err = DSP_EPOINTER;
> +	}
> +}
> +#define cp_fm_usr(to, from, err, n)				\
> +	__cp_fm_usr(to, from, &(err), (n) * sizeof(*(to)))
> +
> +static inline void __cp_to_usr(void __user *to, const void *from,
> +			       DSP_STATUS *err, unsigned long bytes)
> +{
> +	if (DSP_FAILED(*err))
> +		return;
> +
> +	if (unlikely(!to)) {
> +		*err = DSP_EPOINTER;
> +		return;
> +	}
> +
> +	if (unlikely(copy_to_user(to, from, bytes))) {
> +		GT_2trace(WCD_debugMask, GT_7CLASS,
> +			  "%s failed, to=0x%08x\n", __func__, to);
> +		*err = DSP_EPOINTER;
> +	}
> +}
> +#define cp_to_usr(to, from, err, n)				\
> +	__cp_to_usr(to, from, &(err), (n) * sizeof(*(from)))
> 
>  /* Device IOCtl function pointer */
>  struct WCD_Cmd {
> --
> 1.6.2.4
> 
Acked-by Fernando Guzman Lugo <x0095840@ti.com>

RE: [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else
>
>From: Doyu Hiroshi (Nokia-D/Helsinki) <hiroshi.doyu@nokia.com>
>
>Based on the following discussion:
>      http://marc.info/?l=linux-omap&m=124697893724881&w=2
>
>Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
>---
> drivers/dsp/bridge/pmgr/wcd.c |   60 ++++++++++++++++++++++++++--------------
> 1 files changed, 39 insertions(+), 21 deletions(-)
>

Fixed compiler error with debug enabled: "WCD_debugMask undeclared..."

Pushed to d.o-z

- omar

RE: [PATCH 03/13] DSPBRIDGE: Remove unnecessary conditions from some for loops

[Ramirez Luna, Omar]

>From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Ameya
>Palande
>Subject: [PATCH 03/13] DSPBRIDGE: Remove unnecessary conditions from some for loops
>
>The status cannot be a failure value at the start of the loops,
>and the only changes to a failure state are accompanied by a
>break or goto, so these conditions are always true.
>
>Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
>Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
>---
> drivers/dsp/bridge/pmgr/wcd.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>

Pushed to d.o-z

- omar

RE: [PATCH 02/13] DSPBRIDGE: PROCWRAP_Load function cleanup in a complete mess

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 02/13] DSPBRIDGE: PROCWRAP_Load function cleanup in a complete mess
>
>If you followed some failure paths, it was entirely possible that
>you'd attempt to MEM_Free a user-space pointer, because it wouldn't
>have been replaced with a kernel-space copy yet. Now ensure there's
>a NULL pointer to stop the cleanup at the position of the first error.
>
>Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
>Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
>Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
>---
> drivers/dsp/bridge/pmgr/wcd.c |  114 +++++++++++++++++++++++-----------------
> 1 files changed, 65 insertions(+), 49 deletions(-)
>

Small rework to apply, original file (on which the patch is based) had spaces instead of tabs.

Pushed to d.o-z

 - omar

RE: [PATCH 01/13] DSPBRIDGE: Heuristic fixes of strlen/malloc out by one

[Ramirez Luna, Omar]

>From: Ameya Palande [mailto:ameya.palande@nokia.com]
>Subject: [PATCH 01/13] DSPBRIDGE: Heuristic fixes of strlen/malloc out by one
>
>From: Phil Carmody <ext-phil.2.carmody@nokia.com>
>
>I say 'heuristic', as I can't prove they're wrong, they just look
>wrong, and for that reason should be given extra close scrutiny.
>These are basically just the old malloc-one-more-than-strlen.
>
>Signed-off-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
>Acked-by: Fernando Guzman Lugo <x0095840@ti.com>
>---
> drivers/dsp/bridge/pmgr/wcd.c |   11 ++++++-----
> 1 files changed, 6 insertions(+), 5 deletions(-)
>

Pushed to d.o-z

- omar

Re: [PATCH 00/13] DSPBRIDGE: Patchset with various fixes

[Nishanth Menon]

Ameya Palande had written, on 07/15/2009 09:56 AM, the following:
> Most of them have already got an ack, but can you please verify them again?
> Following patches need an ack:
> 
> [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else
> [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig
> [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode
> [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation
Is there a 13/13? [1] shows 12/13 max..

Regards,
Nishanth Menon

Ref:
[1] http://marc.info/?l=linux-omap&r=1&b=200907&w=2

Re: [PATCH 00/13] DSPBRIDGE: Patchset with various fixes

[Ameya Palande]

Hi Nishant,

ext Nishanth Menon wrote:
> Ameya Palande had written, on 07/15/2009 09:56 AM, the following:
>> Most of them have already got an ack, but can you please verify them again?
>> Following patches need an ack:
>>
>> [PATCH 04/13] DSPBRIDGE: fix macros that break when inside an if/else
>> [PATCH 05/13] DSPBRIDGE: enable smart/autoidle for mailbox sysconfig
>> [PATCH 11/13] DSPBRIDGE: Set VDD1 OPP1 unconditionally while moving to off mode
>> [PATCH 12/13] DSPBRIDGE: fix resuming problem with dsp hibernation
> Is there a 13/13? [1] shows 12/13 max..
> 
> Regards,
> Nishanth Menon
> 
> Ref:
> [1] http://marc.info/?l=linux-omap&r=1&b=200907&w=2

Sorry for the confusion!
13th Patch is: 0013-DSPBRIDGE-add-debug-prints-for-SR-DVFS-problem.patch
It is only helpful for debugging hence not included in final patchset.

Cheers,
Ameya.