[PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

[PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

[Omar Ramirez Luna]

From: Shivananda Hebbar <x0hebbar@ti.com>

This patch removes the conditional check which can result in
message skip.

Discovered-by: Bhavin Shah <bshah@ti.com>
Signed-off-by: Shivananda Hebbar <x0hebbar@ti.com>
---
 drivers/dsp/bridge/wmd/io_sm.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index 39b37a6..b2092a2 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -1328,7 +1328,7 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
 	fInputEmpty = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl,
 				 bufEmpty);
 	uMsgs = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl, size);
-	if (fInputEmpty || uMsgs >= hMsgMgr->uMaxMsgs)
+	if (fInputEmpty)
 		goto func_end;
 
 	pMsgInput = pIOMgr->pMsgInput;
@@ -1356,9 +1356,6 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
 		 * input locations have been set up. If all nodes were
 		 * cleaned up, hMsgMgr->uMaxMsgs should be 0.
 		 */
-		if (hMsgQueue && uMsgs > hMsgMgr->uMaxMsgs)
-			goto func_end;
-
 		while (hMsgQueue != NULL) {
 			if (msg.dwId == hMsgQueue->dwId) {
 				/* Found it */
-- 
1.6.2.4

Re: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

[Ameya Palande]

Hi Shivananda,

On Wed, 2010-01-27 at 02:44 +0100, ext Omar Ramirez Luna wrote:
> From: Shivananda Hebbar <x0hebbar@ti.com>
> 
> This patch removes the conditional check which can result in
> message skip.
> 
> Discovered-by: Bhavin Shah <bshah@ti.com>
> Signed-off-by: Shivananda Hebbar <x0hebbar@ti.com>
> ---
>  drivers/dsp/bridge/wmd/io_sm.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
> index 39b37a6..b2092a2 100644
> --- a/drivers/dsp/bridge/wmd/io_sm.c
> +++ b/drivers/dsp/bridge/wmd/io_sm.c
> @@ -1328,7 +1328,7 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
>  	fInputEmpty = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl,
>  				 bufEmpty);
>  	uMsgs = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl, size);
> -	if (fInputEmpty || uMsgs >= hMsgMgr->uMaxMsgs)
> +	if (fInputEmpty)
>  		goto func_end;
>  
>  	pMsgInput = pIOMgr->pMsgInput;
> @@ -1356,9 +1356,6 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
>  		 * input locations have been set up. If all nodes were
>  		 * cleaned up, hMsgMgr->uMaxMsgs should be 0.
>  		 */
> -		if (hMsgQueue && uMsgs > hMsgMgr->uMaxMsgs)
> -			goto func_end;
> -
>  		while (hMsgQueue != NULL) {
>  			if (msg.dwId == hMsgQueue->dwId) {
>  				/* Found it */

Forgive me for the stupid question, but now since uMsgs >=
hMsgMgr->uMaxMsgs condition is removed, what is the side effect of that?
At least I am not able to understand how it affects the logic just by
seeing your patch. I guess if you can modify the patch description to
reflect the logic behind this then it will be nice :)

Cheers,
Ameya.

RE: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

[Hebbar, Shivananda]

> -----Original Message-----
> From: Ameya Palande [mailto:ameya.palande@nokia.com] 
> Sent: Wednesday, January 27, 2010 3:44 AM
> To: Ramirez Luna, Omar
> Cc: linux-omap; Doyu Hiroshi (Nokia-D/Helsinki); Contreras Felipe (Nokia-> >D/Helsinki); Menon, Nishanth; Hebbar, Shivananda
>Subject: Re: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg >function

>Hi Shivananda,

On Wed, 2010-01-27 at 02:44 +0100, ext Omar Ramirez Luna wrote:
> From: Shivananda Hebbar <x0hebbar@ti.com>
> 
> This patch removes the conditional check which can result in
> message skip.
> 
> Discovered-by: Bhavin Shah <bshah@ti.com>
> Signed-off-by: Shivananda Hebbar <x0hebbar@ti.com>
> ---
>  drivers/dsp/bridge/wmd/io_sm.c |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
> index 39b37a6..b2092a2 100644
> --- a/drivers/dsp/bridge/wmd/io_sm.c
> +++ b/drivers/dsp/bridge/wmd/io_sm.c
> @@ -1328,7 +1328,7 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
>  	fInputEmpty = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl,
>  				 bufEmpty);
>  	uMsgs = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl, size);
> -	if (fInputEmpty || uMsgs >= hMsgMgr->uMaxMsgs)
> +	if (fInputEmpty)
>  		goto func_end;
>  
>  	pMsgInput = pIOMgr->pMsgInput;
> @@ -1356,9 +1356,6 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
>  		 * input locations have been set up. If all nodes were
>  		 * cleaned up, hMsgMgr->uMaxMsgs should be 0.
>  		 */
> -		if (hMsgQueue && uMsgs > hMsgMgr->uMaxMsgs)
> -			goto func_end;
> -
>  		while (hMsgQueue != NULL) {
>  			if (msg.dwId == hMsgQueue->dwId) {
>  				/* Found it */

>Forgive me for the stupid question, but now since uMsgs >=
>hMsgMgr->uMaxMsgs condition is removed, what is the side effect of that?
>At least I am not able to understand how it affects the logic just by
>seeing your patch. I guess if you can modify the patch description to
>reflect the logic behind this then it will be nice :)

I don't see any side effects with the removal of this conditional check.
Intention of the patch was to remove the redundant check on uMaxMsgs as the DSP side of the bridge code check will ensure that uMsgs can't exceed uMaxMsgs.
If you are ok with this explanation, I will update the description and resend it.

And the initial patch description was written with the intention that in case the uMsgs exceeds the uMaxMessages (), then we shouldn't discard it by returning from function but process it. 


Regards,
Shivananda

RE: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

[Ameya Palande]

Hi Shivananda,

On Wed, 2010-01-27 at 18:04 +0100, ext Hebbar, Shivananda wrote:
> > -----Original Message-----
> > From: Ameya Palande [mailto:ameya.palande@nokia.com] 
> > Sent: Wednesday, January 27, 2010 3:44 AM
> > To: Ramirez Luna, Omar
> > Cc: linux-omap; Doyu Hiroshi (Nokia-D/Helsinki); Contreras Felipe (Nokia-> >D/Helsinki); Menon, Nishanth; Hebbar, Shivananda
> >Subject: Re: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg >function
> 
> >Hi Shivananda,
> 
> On Wed, 2010-01-27 at 02:44 +0100, ext Omar Ramirez Luna wrote:
> > From: Shivananda Hebbar <x0hebbar@ti.com>
> > 
> > This patch removes the conditional check which can result in
> > message skip.
> > 
> > Discovered-by: Bhavin Shah <bshah@ti.com>
> > Signed-off-by: Shivananda Hebbar <x0hebbar@ti.com>
> > ---
> >  drivers/dsp/bridge/wmd/io_sm.c |    5 +----
> >  1 files changed, 1 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
> > index 39b37a6..b2092a2 100644
> > --- a/drivers/dsp/bridge/wmd/io_sm.c
> > +++ b/drivers/dsp/bridge/wmd/io_sm.c
> > @@ -1328,7 +1328,7 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
> >  	fInputEmpty = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl,
> >  				 bufEmpty);
> >  	uMsgs = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl, size);
> > -	if (fInputEmpty || uMsgs >= hMsgMgr->uMaxMsgs)
> > +	if (fInputEmpty)
> >  		goto func_end;
> >  
> >  	pMsgInput = pIOMgr->pMsgInput;
> > @@ -1356,9 +1356,6 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
> >  		 * input locations have been set up. If all nodes were
> >  		 * cleaned up, hMsgMgr->uMaxMsgs should be 0.
> >  		 */
> > -		if (hMsgQueue && uMsgs > hMsgMgr->uMaxMsgs)
> > -			goto func_end;
> > -
> >  		while (hMsgQueue != NULL) {
> >  			if (msg.dwId == hMsgQueue->dwId) {
> >  				/* Found it */
> 
> >Forgive me for the stupid question, but now since uMsgs >=
> >hMsgMgr->uMaxMsgs condition is removed, what is the side effect of that?
> >At least I am not able to understand how it affects the logic just by
> >seeing your patch. I guess if you can modify the patch description to
> >reflect the logic behind this then it will be nice :)
> 
> I don't see any side effects with the removal of this conditional check.
> Intention of the patch was to remove the redundant check on uMaxMsgs as the DSP side of the bridge code check will ensure that uMsgs can't exceed uMaxMsgs.
> If you are ok with this explanation, I will update the description and resend it.
> 
> And the initial patch description was written with the intention that in case the uMsgs exceeds the uMaxMessages (), then we shouldn't discard it by returning from function but process it. 
> 
> 
> Regards,
> Shivananda

Ya! if you can write down the logic in patch description then it will be
great!

Cheers,
Ameya.

[PATCH ] DSPBRIDGE:Remove conditional check from the InputMsg function

[Hebbar, Shivananda]

>From 03b5f57849eda16fb659c4288ae2dc9a2d1582a9 Mon Sep 17 00:00:00 2001
From: Shivananda Hebbar <x0hebbar@ti.com>
Date: Thu, 17 Dec 2009 17:00:22 -0600
Subject: [PATCH] DSPBRIDGE: Remove conditional check from the InputMsg function

This patch removes the conditional check which can result in
message skip.

Discovered-by: Bhavin Shah	 <bshah@ti.com>
Signed-off-by: Shivananda Hebbar <x0hebbar@ti.com>
---
 drivers/dsp/bridge/wmd/io_sm.c |   13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/dsp/bridge/wmd/io_sm.c b/drivers/dsp/bridge/wmd/io_sm.c
index a6c8e31..5b7b004 100644
--- a/drivers/dsp/bridge/wmd/io_sm.c
+++ b/drivers/dsp/bridge/wmd/io_sm.c
@@ -1308,7 +1308,7 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
 	fInputEmpty = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl,
 				 bufEmpty);
 	uMsgs = IO_GetValue(pIOMgr->hWmdContext, struct MSG, pCtrl, size);
-	if (fInputEmpty || uMsgs >= hMsgMgr->uMaxMsgs)
+	if (fInputEmpty)
 		goto func_end;
 
 	pMsgInput = pIOMgr->pMsgInput;
@@ -1331,12 +1331,11 @@ static void InputMsg(struct IO_MGR *pIOMgr, struct MSG_MGR *hMsgMgr)
 		DBG_Trace(DBG_LEVEL7, "InputMsg RECVD: dwCmd=0x%x dwArg1=0x%x "
 			 "dwArg2=0x%x dwId=0x%x \n", msg.msg.dwCmd,
 			 msg.msg.dwArg1, msg.msg.dwArg2, msg.dwId);
-		 /*  Interrupt may occur before shared memory and message
-		 *  input locations have been set up. If all nodes were
-		 *  cleaned up, hMsgMgr->uMaxMsgs should be 0.  */
-               if (hMsgQueue && uMsgs > hMsgMgr->uMaxMsgs)
-                       goto func_end;
-
+		/*
+		 * Interrupt may occur before shared memory and message
+		 * input locations have been set up. If all nodes were
+		 * cleaned up, hMsgMgr->uMaxMsgs should be 0.
+		 */
 		while (hMsgQueue != NULL) {
 			if (msg.dwId == hMsgQueue->dwId) {
 				/* Found it */
-- 
1.6.0.4