[PATCH 0/2] dspbridge: maemo lingering patches

[PATCH 0/2] dspbridge: maemo lingering patches

[Felipe Contreras]

Hi,

These are some patches that were lingering in the maemo branch since a long
time ago.

Jouni Hogander (1):
  DSPBRIDGE: Use enable_off_mode variable instead of
    dsp_test_sleepstate

Mika Kukkonen (1):
  DSPBRIDGE: Various compile warning fixes

 arch/arm/plat-omap/include/dspbridge/dbc.h |    6 +++---
 arch/arm/plat-omap/include/dspbridge/dbg.h |    4 ++--
 arch/arm/plat-omap/include/dspbridge/gt.h  |   16 +++++++++-------
 arch/arm/plat-omap/include/dspbridge/mem.h |    2 +-
 drivers/dsp/bridge/rmgr/drv_interface.c    |    5 -----
 drivers/dsp/bridge/wmd/io_sm.c             |    5 ++---
 drivers/dsp/bridge/wmd/tiomap3430_pwr.c    |    9 ++++-----
 drivers/dsp/bridge/wmd/ue_deh.c            |    2 +-
 8 files changed, 22 insertions(+), 27 deletions(-)

[PATCH 1/2] DSPBRIDGE: Use enable_off_mode variable instead of dsp_test_sleepstate

[Felipe Contreras]

From: Jouni Hogander <jouni.hogander@nokia.com>

Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/rmgr/drv_interface.c |    5 -----
 drivers/dsp/bridge/wmd/tiomap3430_pwr.c |    9 ++++-----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
index a02a32a..b20f77e 100644
--- a/drivers/dsp/bridge/rmgr/drv_interface.c
+++ b/drivers/dsp/bridge/rmgr/drv_interface.c
@@ -78,8 +78,6 @@ s32 dsp_debug;
 
 struct platform_device *omap_dspbridge_dev;
 
-/* This is a test variable used by Bridge to test different sleep states */
-s32 dsp_test_sleepstate;
 struct bridge_dev {
 	struct cdev cdev;
 };
@@ -129,9 +127,6 @@ module_param(dsp_debug, int, 0);
 MODULE_PARM_DESC(dsp_debug, "Wait after loading DSP image. default = false");
 #endif
 
-module_param(dsp_test_sleepstate, int, 0);
-MODULE_PARM_DESC(dsp_test_sleepstate, "DSP Sleep state = 0");
-
 module_param(base_img, charp, 0);
 MODULE_PARM_DESC(base_img, "DSP base image, default = NULL");
 
diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
index 084f406..c0e0994 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
@@ -54,10 +54,9 @@
 #include <mach-omap2/cm-regbits-34xx.h>
 
 #ifdef CONFIG_PM
-extern s32 dsp_test_sleepstate;
+extern unsigned short enable_off_mode;
 #endif
 extern struct MAILBOX_CONTEXT mboxsetting;
-
 /*
  *  ======== handle_constraints_set ========
  *  	Sets new DSP constraint
@@ -196,7 +195,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
 	switch (pDevContext->dwBrdState) {
 	case BRD_RUNNING:
 		status = HW_MBOX_saveSettings(resources.dwMboxBase);
-		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
+		if (enable_off_mode) {
 			CHNLSM_InterruptDSP2(pDevContext,
 					     MBX_PM_DSPHIBERNATE);
 			DBG_Trace(DBG_LEVEL7,
@@ -211,7 +210,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
 		break;
 	case BRD_RETENTION:
 		status = HW_MBOX_saveSettings(resources.dwMboxBase);
-		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
+		if (enable_off_mode) {
 			CHNLSM_InterruptDSP2(pDevContext,
 					     MBX_PM_DSPHIBERNATE);
 			targetPwrState = HW_PWR_STATE_OFF;
@@ -261,7 +260,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
 			 pwrState);
 
 		/* Update the Bridger Driver state */
-		if (dsp_test_sleepstate == HW_PWR_STATE_OFF)
+		if (enable_off_mode)
 			pDevContext->dwBrdState = BRD_HIBERNATION;
 		else
 			pDevContext->dwBrdState = BRD_RETENTION;
-- 
1.6.6.1

[PATCH 2/2] DSPBRIDGE: Various compile warning fixes

[Felipe Contreras]

From: Mika Kukkonen <mika.kukkonen@nokia.com>

This patch contains indentation fixes and cleans up various warnings
uncovered with extra warning flags:
  - empty if() bodies
  - incorrect use of unsigned variables
  - bad comparison of pointer value
  - pointless check of unsigned value being smaller than zero
  - keyword 'extern' has to be first one in variable declaration

Signed-off-by: Mika Kukkonen <mika.kukkonen@nokia.com>
Signed-off-by: Ameya Palande <ameya.palande@nokia.com>
---
 arch/arm/plat-omap/include/dspbridge/dbc.h |    6 +++---
 arch/arm/plat-omap/include/dspbridge/dbg.h |    4 ++--
 arch/arm/plat-omap/include/dspbridge/gt.h  |   16 +++++++++-------
 arch/arm/plat-omap/include/dspbridge/mem.h |    2 +-
 drivers/dsp/bridge/wmd/io_sm.c             |    5 ++---
 drivers/dsp/bridge/wmd/ue_deh.c            |    2 +-
 6 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/dbc.h b/arch/arm/plat-omap/include/dspbridge/dbc.h
index ac5d178..1b3ac44 100644
--- a/arch/arm/plat-omap/include/dspbridge/dbc.h
+++ b/arch/arm/plat-omap/include/dspbridge/dbc.h
@@ -43,9 +43,9 @@
 
 #else
 
-#define DBC_Assert(exp)
-#define DBC_Require(exp)
-#define DBC_Ensure(exp)
+#define DBC_Assert(exp) {}
+#define DBC_Require(exp) {}
+#define DBC_Ensure(exp) {}
 
 #endif				/* DEBUG */
 
diff --git a/arch/arm/plat-omap/include/dspbridge/dbg.h b/arch/arm/plat-omap/include/dspbridge/dbg.h
index 2f61dab..4d01eca 100644
--- a/arch/arm/plat-omap/include/dspbridge/dbg.h
+++ b/arch/arm/plat-omap/include/dspbridge/dbg.h
@@ -80,9 +80,9 @@
 	extern DSP_STATUS DBG_Trace(IN u8 bLevel, IN char *pstrFormat, ...);
 #else
 
-#define DBG_Exit(void)
+#define DBG_Exit(void) do {} while (0)
 #define DBG_Init(void) true
-#define DBG_Trace(bLevel, pstrFormat, args...)
+#define DBG_Trace(bLevel, pstrFormat, args...) do {} while (0)
 
 #endif	/* (CONFIG_BRIDGE_DEBUG || DDSP_DEBUG_PRODUCT) && GT_TRACE */
 
diff --git a/arch/arm/plat-omap/include/dspbridge/gt.h b/arch/arm/plat-omap/include/dspbridge/gt.h
index 6082d15..9097910 100644
--- a/arch/arm/plat-omap/include/dspbridge/gt.h
+++ b/arch/arm/plat-omap/include/dspbridge/gt.h
@@ -252,13 +252,15 @@ extern struct GT_Config _GT_params;
 
 #define GT_query(mask, class)     false
 
-#define GT_0trace(mask, class, format)
-#define GT_1trace(mask, class, format, arg1)
-#define GT_2trace(mask, class, format, arg1, arg2)
-#define GT_3trace(mask, class, format, arg1, arg2, arg3)
-#define GT_4trace(mask, class, format, arg1, arg2, arg3, arg4)
-#define GT_5trace(mask, class, format, arg1, arg2, arg3, arg4, arg5)
-#define GT_6trace(mask, class, format, arg1, arg2, arg3, arg4, arg5, arg6)
+#define GT_0trace(mask, class, format) do {} while (0)
+#define GT_1trace(mask, class, format, arg1) do {} while (0)
+#define GT_2trace(mask, class, format, arg1, arg2) do {} while (0)
+#define GT_3trace(mask, class, format, arg1, arg2, arg3) do {} while (0)
+#define GT_4trace(mask, class, format, arg1, arg2, arg3, arg4) do {} while (0)
+#define GT_5trace(mask, class, format, arg1, arg2, arg3, arg4, arg5) \
+	do {} while (0)
+#define GT_6trace(mask, class, format, arg1, arg2, arg3, arg4, arg5, arg6) \
+	do {} while (0)
 
 #else				/* GT_TRACE == 1 */
 
diff --git a/arch/arm/plat-omap/include/dspbridge/mem.h b/arch/arm/plat-omap/include/dspbridge/mem.h
index 03b419a..353ffb0 100644
--- a/arch/arm/plat-omap/include/dspbridge/mem.h
+++ b/arch/arm/plat-omap/include/dspbridge/mem.h
@@ -286,7 +286,7 @@
  *  Ensures:
  *      - pBaseAddr no longer points to a valid linear address.
  */
-#define MEM_UnmapLinearAddress(pBaseAddr)
+#define MEM_UnmapLinearAddress(pBaseAddr) {}
 
 /*
  *  ======== MEM_ExtPhysPoolInit ========
diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index e35ce57..587dc6c 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -221,8 +221,7 @@ DSP_STATUS WMD_IO_Create(OUT struct IO_MGR **phIOMgr,
 		bridge_workqueue = create_workqueue("bridge_work-queue");
 
 	if (bridge_workqueue <= 0)
-		DBG_Trace(DBG_LEVEL1, "Workque Create failed 0x%d \n",
-						bridge_workqueue);
+		DBG_Trace(DBG_LEVEL1, "Workqueue creation failed!\n");
 
 	/* Allocate IO manager object */
 	MEM_AllocObject(pIOMgr, struct IO_MGR, IO_MGRSIGNATURE);
@@ -1210,7 +1209,7 @@ static void InputChnl(struct IO_MGR *pIOMgr, struct CHNL_OBJECT *pChnl,
 			    pChnlMgr->uWordSize;
 	chnlId = IO_GetValue(pIOMgr->hWmdContext, struct SHM, sm, inputId);
 	dwArg = IO_GetLong(pIOMgr->hWmdContext, struct SHM, sm, arg);
-	if (!(chnlId >= 0) || !(chnlId < CHNL_MAXCHANNELS)) {
+	if (chnlId >= CHNL_MAXCHANNELS) {
 		/* Shouldn't be here: would indicate corrupted SHM. */
 		DBC_Assert(chnlId);
 		goto func_end;
diff --git a/drivers/dsp/bridge/wmd/ue_deh.c b/drivers/dsp/bridge/wmd/ue_deh.c
index 6166d97..2c3a2cd 100644
--- a/drivers/dsp/bridge/wmd/ue_deh.c
+++ b/drivers/dsp/bridge/wmd/ue_deh.c
@@ -189,7 +189,7 @@ void WMD_DEH_Notify(struct DEH_MGR *hDehMgr, u32 ulEventMask,
 	DSP_STATUS status = DSP_SOK;
 	u32 memPhysical = 0;
 	u32 HW_MMU_MAX_TLB_COUNT = 31;
-	u32 extern faultAddr;
+	extern u32 faultAddr;
 	struct CFG_HOSTRES resources;
 	HW_STATUS hwStatus;
 
-- 
1.6.6.1

Re: [PATCH 1/2] DSPBRIDGE: Use enable_off_mode variable instead of dsp_test_sleepstate

[Omar Ramirez Luna]

Hi,

On 1/23/2010 4:19 AM, Felipe Contreras wrote:
> From: Jouni Hogander<jouni.hogander@nokia.com>
>
> Signed-off-by: Jouni Hogander<jouni.hogander@nokia.com>
> Signed-off-by: Hiroshi DOYU<Hiroshi.DOYU@nokia.com>

This patch is missing to export enable_off_mode otherwise it will fail 
while linking.

FTR, I don't like to be exporting symbols of other subsystems, guess the 
same applies to:

http://marc.info/?l=linux-omap&m=126450677527042&w=2

> ---
>   drivers/dsp/bridge/rmgr/drv_interface.c |    5 -----
>   drivers/dsp/bridge/wmd/tiomap3430_pwr.c |    9 ++++-----
>   2 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
> index a02a32a..b20f77e 100644
> --- a/drivers/dsp/bridge/rmgr/drv_interface.c
> +++ b/drivers/dsp/bridge/rmgr/drv_interface.c
> @@ -78,8 +78,6 @@ s32 dsp_debug;
>
>   struct platform_device *omap_dspbridge_dev;
>
> -/* This is a test variable used by Bridge to test different sleep states */
> -s32 dsp_test_sleepstate;
>   struct bridge_dev {
>   	struct cdev cdev;
>   };
> @@ -129,9 +127,6 @@ module_param(dsp_debug, int, 0);
>   MODULE_PARM_DESC(dsp_debug, "Wait after loading DSP image. default = false");
>   #endif
>
> -module_param(dsp_test_sleepstate, int, 0);
> -MODULE_PARM_DESC(dsp_test_sleepstate, "DSP Sleep state = 0");
> -
>   module_param(base_img, charp, 0);
>   MODULE_PARM_DESC(base_img, "DSP base image, default = NULL");
>
> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> index 084f406..c0e0994 100644
> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> @@ -54,10 +54,9 @@
>   #include<mach-omap2/cm-regbits-34xx.h>
>
>   #ifdef CONFIG_PM
> -extern s32 dsp_test_sleepstate;
> +extern unsigned short enable_off_mode;
>   #endif
>   extern struct MAILBOX_CONTEXT mboxsetting;
> -
>   /*
>    *  ======== handle_constraints_set ========
>    *  	Sets new DSP constraint
> @@ -196,7 +195,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>   	switch (pDevContext->dwBrdState) {
>   	case BRD_RUNNING:
>   		status = HW_MBOX_saveSettings(resources.dwMboxBase);
> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
> +		if (enable_off_mode) {
>   			CHNLSM_InterruptDSP2(pDevContext,
>   					     MBX_PM_DSPHIBERNATE);
>   			DBG_Trace(DBG_LEVEL7,
> @@ -211,7 +210,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>   		break;
>   	case BRD_RETENTION:
>   		status = HW_MBOX_saveSettings(resources.dwMboxBase);
> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
> +		if (enable_off_mode) {
>   			CHNLSM_InterruptDSP2(pDevContext,
>   					     MBX_PM_DSPHIBERNATE);
>   			targetPwrState = HW_PWR_STATE_OFF;
> @@ -261,7 +260,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>   			 pwrState);
>
>   		/* Update the Bridger Driver state */
> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF)
> +		if (enable_off_mode)
>   			pDevContext->dwBrdState = BRD_HIBERNATION;
>   		else
>   			pDevContext->dwBrdState = BRD_RETENTION;

- omar

Re: [PATCH 1/2] DSPBRIDGE: Use enable_off_mode variable instead of dsp_test_sleepstate

[Felipe Contreras]

On Wed, Jan 27, 2010 at 12:23 AM, Omar Ramirez Luna <omar.ramirez@ti.com> wrote:
> Hi,
>
> On 1/23/2010 4:19 AM, Felipe Contreras wrote:
>>
>> From: Jouni Hogander<jouni.hogander@nokia.com>
>>
>> Signed-off-by: Jouni Hogander<jouni.hogander@nokia.com>
>> Signed-off-by: Hiroshi DOYU<Hiroshi.DOYU@nokia.com>
>
> This patch is missing to export enable_off_mode otherwise it will fail while
> linking.
>
> FTR, I don't like to be exporting symbols of other subsystems, guess the
> same applies to:

I don't know, I was just making sure this patch wasn't lost.

But to me, it looks like a hint that perhaps
drivers/dsp/bridge/wmd/tiomap3430_pwr.c
should move to:
arch/arm/mach-omap2/dspbridge.c

Then you could #include "pm.h", just like in
arch/arm/mach-omap2/serial.c

-- 
Felipe Contreras

Re: [PATCH 1/2] DSPBRIDGE: Use enable_off_mode variable instead of dsp_test_sleepstate

[Nishanth Menon]

Omar Ramirez Luna had written, on 01/26/2010 04:23 PM, the following:
> Hi,
> 
> On 1/23/2010 4:19 AM, Felipe Contreras wrote:
>> From: Jouni Hogander<jouni.hogander@nokia.com>
>>
>> Signed-off-by: Jouni Hogander<jouni.hogander@nokia.com>
>> Signed-off-by: Hiroshi DOYU<Hiroshi.DOYU@nokia.com>
> 
> This patch is missing to export enable_off_mode otherwise it will fail 
> while linking.
> 
> FTR, I don't like to be exporting symbols of other subsystems, guess the 
> same applies to:
> 
> http://marc.info/?l=linux-omap&m=126450677527042&w=2
looking at the patch itself few questions arise:

> 
>> ---
>>   drivers/dsp/bridge/rmgr/drv_interface.c |    5 -----
>>   drivers/dsp/bridge/wmd/tiomap3430_pwr.c |    9 ++++-----
>>   2 files changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
>> index a02a32a..b20f77e 100644
>> --- a/drivers/dsp/bridge/rmgr/drv_interface.c
>> +++ b/drivers/dsp/bridge/rmgr/drv_interface.c
>> @@ -78,8 +78,6 @@ s32 dsp_debug;
>>
>>   struct platform_device *omap_dspbridge_dev;
>>
>> -/* This is a test variable used by Bridge to test different sleep states */
>> -s32 dsp_test_sleepstate;
>>   struct bridge_dev {
>>   	struct cdev cdev;
>>   };
>> @@ -129,9 +127,6 @@ module_param(dsp_debug, int, 0);
>>   MODULE_PARM_DESC(dsp_debug, "Wait after loading DSP image. default = false");
>>   #endif
>>
>> -module_param(dsp_test_sleepstate, int, 0);
>> -MODULE_PARM_DESC(dsp_test_sleepstate, "DSP Sleep state = 0");
>> -
>>   module_param(base_img, charp, 0);
>>   MODULE_PARM_DESC(base_img, "DSP base image, default = NULL");
>>
>> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>> index 084f406..c0e0994 100644
>> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>> @@ -54,10 +54,9 @@
>>   #include<mach-omap2/cm-regbits-34xx.h>
>>
>>   #ifdef CONFIG_PM
>> -extern s32 dsp_test_sleepstate;
>> +extern unsigned short enable_off_mode;
>>   #endif
>>   extern struct MAILBOX_CONTEXT mboxsetting;
>> -
>>   /*
>>    *  ======== handle_constraints_set ========
>>    *  	Sets new DSP constraint
>> @@ -196,7 +195,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>   	switch (pDevContext->dwBrdState) {
>>   	case BRD_RUNNING:
>>   		status = HW_MBOX_saveSettings(resources.dwMboxBase);
>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
>> +		if (enable_off_mode) {
>>   			CHNLSM_InterruptDSP2(pDevContext,
>>   					     MBX_PM_DSPHIBERNATE);
>>   			DBG_Trace(DBG_LEVEL7,
>> @@ -211,7 +210,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>   		break;
>>   	case BRD_RETENTION:
>>   		status = HW_MBOX_saveSettings(resources.dwMboxBase);
>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
>> +		if (enable_off_mode) {
>>   			CHNLSM_InterruptDSP2(pDevContext,
>>   					     MBX_PM_DSPHIBERNATE);
>>   			targetPwrState = HW_PWR_STATE_OFF;
>> @@ -261,7 +260,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>   			 pwrState);
>>
>>   		/* Update the Bridger Driver state */
>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF)
>> +		if (enable_off_mode)
>>   			pDevContext->dwBrdState = BRD_HIBERNATION;
>>   		else
>>   			pDevContext->dwBrdState = BRD_RETENTION;
IMHO it changes behavior:
before:
IF dsp_test_sleepstate was set to HW_PWR_STATE_OFF DSP was told to go to 
hibernate every time SleepDSP is called else the mbox event
default behavior is to ask BIOS for retention.

with the patch:
If sysfs entry(meant for MPU) has enabled off mode, THEN DSP will be 
send to off mode every time we get message to SleepDSP. In other words 
MPU and DSP retention/OFF are tied together.

The questions then are:
a) why was this done so?
b) is the intention of test_sleepstate param meant only as a test?
c) was it intended that SleepDSP should put IVA to sleep no matter what 
MPU decisions are?

-- 
Regards,
Nishanth Menon

Re: [PATCH 1/2] DSPBRIDGE: Use enable_off_mode variable instead of dsp_test_sleepstate

[Omar Ramirez Luna]

On 1/27/2010 6:46 PM, Menon, Nishanth wrote:
> Omar Ramirez Luna had written, on 01/26/2010 04:23 PM, the following:
>> Hi,
>>
>> On 1/23/2010 4:19 AM, Felipe Contreras wrote:
>>> From: Jouni Hogander<jouni.hogander@nokia.com>
>>>
>>> Signed-off-by: Jouni Hogander<jouni.hogander@nokia.com>
>>> Signed-off-by: Hiroshi DOYU<Hiroshi.DOYU@nokia.com>
>>
>> This patch is missing to export enable_off_mode otherwise it will fail
>> while linking.
>>
>> FTR, I don't like to be exporting symbols of other subsystems, guess the
>> same applies to:
>>
>> http://marc.info/?l=linux-omap&m=126450677527042&w=2
> looking at the patch itself few questions arise:
>
>>
>>> ---
>>>    drivers/dsp/bridge/rmgr/drv_interface.c |    5 -----
>>>    drivers/dsp/bridge/wmd/tiomap3430_pwr.c |    9 ++++-----
>>>    2 files changed, 4 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/dsp/bridge/rmgr/drv_interface.c b/drivers/dsp/bridge/rmgr/drv_interface.c
>>> index a02a32a..b20f77e 100644
>>> --- a/drivers/dsp/bridge/rmgr/drv_interface.c
>>> +++ b/drivers/dsp/bridge/rmgr/drv_interface.c
>>> @@ -78,8 +78,6 @@ s32 dsp_debug;
>>>
>>>    struct platform_device *omap_dspbridge_dev;
>>>
>>> -/* This is a test variable used by Bridge to test different sleep states */
>>> -s32 dsp_test_sleepstate;
>>>    struct bridge_dev {
>>>    	struct cdev cdev;
>>>    };
>>> @@ -129,9 +127,6 @@ module_param(dsp_debug, int, 0);
>>>    MODULE_PARM_DESC(dsp_debug, "Wait after loading DSP image. default = false");
>>>    #endif
>>>
>>> -module_param(dsp_test_sleepstate, int, 0);
>>> -MODULE_PARM_DESC(dsp_test_sleepstate, "DSP Sleep state = 0");
>>> -
>>>    module_param(base_img, charp, 0);
>>>    MODULE_PARM_DESC(base_img, "DSP base image, default = NULL");
>>>
>>> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>>> index 084f406..c0e0994 100644
>>> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>>> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
>>> @@ -54,10 +54,9 @@
>>>    #include<mach-omap2/cm-regbits-34xx.h>
>>>
>>>    #ifdef CONFIG_PM
>>> -extern s32 dsp_test_sleepstate;
>>> +extern unsigned short enable_off_mode;
>>>    #endif
>>>    extern struct MAILBOX_CONTEXT mboxsetting;
>>> -
>>>    /*
>>>     *  ======== handle_constraints_set ========
>>>     *  	Sets new DSP constraint
>>> @@ -196,7 +195,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>>    	switch (pDevContext->dwBrdState) {
>>>    	case BRD_RUNNING:
>>>    		status = HW_MBOX_saveSettings(resources.dwMboxBase);
>>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
>>> +		if (enable_off_mode) {
>>>    			CHNLSM_InterruptDSP2(pDevContext,
>>>    					     MBX_PM_DSPHIBERNATE);
>>>    			DBG_Trace(DBG_LEVEL7,
>>> @@ -211,7 +210,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>>    		break;
>>>    	case BRD_RETENTION:
>>>    		status = HW_MBOX_saveSettings(resources.dwMboxBase);
>>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF) {
>>> +		if (enable_off_mode) {
>>>    			CHNLSM_InterruptDSP2(pDevContext,
>>>    					     MBX_PM_DSPHIBERNATE);
>>>    			targetPwrState = HW_PWR_STATE_OFF;
>>> @@ -261,7 +260,7 @@ DSP_STATUS SleepDSP(struct WMD_DEV_CONTEXT *pDevContext, IN u32 dwCmd,
>>>    			 pwrState);
>>>
>>>    		/* Update the Bridger Driver state */
>>> -		if (dsp_test_sleepstate == HW_PWR_STATE_OFF)
>>> +		if (enable_off_mode)
>>>    			pDevContext->dwBrdState = BRD_HIBERNATION;
>>>    		else
>>>    			pDevContext->dwBrdState = BRD_RETENTION;
> IMHO it changes behavior:
> before:
> IF dsp_test_sleepstate was set to HW_PWR_STATE_OFF DSP was told to go to
> hibernate every time SleepDSP is called else the mbox event
> default behavior is to ask BIOS for retention.

dsp_test_sleepstate is initialized always to 0

>
> with the patch:
> If sysfs entry(meant for MPU) has enabled off mode, THEN DSP will be
> send to off mode every time we get message to SleepDSP. In other words
> MPU and DSP retention/OFF are tied together.

(-->) means mbx bridge to dsp
(...) means do not disturb dsp while sleeping

Suspend coming for this scenarios:

if DSP = ON, enable_off_mode = 1 --> OFF (HIB)
if DSP = ON, enable_off_mode = 0 --> RET
if DSP = OFF, enable_off_mode = 1 ...
if DSP = OFF, enable_off_mode = 0 ...

>
> The questions then are:
> a) why was this done so?

always target the next lower state (according to the specified), but DO 
NOT disturb the dsp if it already went to sleep

> b) is the intention of test_sleepstate param meant only as a test?

should be clear enough:
/* This is a test variable used by Bridge to test different sleep states */

> c) was it intended that SleepDSP should put IVA to sleep no matter what
> MPU decisions are?

if the DSP is already OFF and MPU wants to be in RET why MPU can be 
smart enough to tell ok this guy went down, print a message like "this 
might be wrong" but it is acceptable.

the other tendency claims that the dsp should be woken and then told to 
go to next-power-state (if OFF not enabled).

bottom line is that we listen to what mpu wants if the dsp is awake but 
ignore if the dsp has gone to off.

- omar