diff mbox

[OMAPZOOM,5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull

Message ID 496565EC904933469F292DDA3F1663E60287B84DFA@dlee06.ent.ti.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Guzman Lugo, Fernando March 3, 2009, 1:35 a.m. UTC 
From db89674b7316d7490e71c131091758eb7ef0eca2 Mon Sep 17 00:00:00 2001
From: Fernando Guzman Lugo <x0095840@ti.com>
Date: Wed, 25 Feb 2009 19:27:41 -0600
Subject: [PATCH] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull function

This patch removes some unnecesarry checks in HW_MBOX_IsFull
function
Signed-off-by: Guzman Lugo Fernando <x0095840@ti.com>
---
 drivers/dsp/bridge/hw/hw_mbox.c    |   22 ++--------------------
 drivers/dsp/bridge/hw/hw_mbox.h    |    6 ++----
 drivers/dsp/bridge/wmd/tiomap_sm.c |   14 ++++----------
 3 files changed, 8 insertions(+), 34 deletions(-)

Comments

Felipe Contreras March 3, 2009, 7:22 a.m. UTC | #1 
On Tue, Mar 3, 2009 at 3:35 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
> From db89674b7316d7490e71c131091758eb7ef0eca2 Mon Sep 17 00:00:00 2001
> From: Fernando Guzman Lugo <x0095840@ti.com>
> Date: Wed, 25 Feb 2009 19:27:41 -0600
> Subject: [PATCH] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull function
>
> This patch removes some unnecesarry checks in HW_MBOX_IsFull
> function

Is that all this patch is doing?

> -       u16 cnt = 10;
> +       u16 cnt = 1000;

> -       while (--cnt) {
> -               hwStatus = HW_MBOX_IsFull(resources.dwMboxBase,
> -                                          MBOX_ARM2DSP, &mbxFull);
> -               if (mbxFull)
> -                       UTIL_Wait(1000);        /* wait for 1 ms)      */
> -               else
> -                       break;
> -       }
> +       while (--cnt && HW_MBOX_IsFull(resources.dwMboxBase, MBOX_ARM2DSP))
> +               udelay(1);
> +
Guzman Lugo, Fernando March 3, 2009, 4:34 p.m. UTC | #2 
Hi Felipe,

	This patch is base on your patch about increase the performance waiting less and checking more the mail box.

Regards,
Fernando.

-----Original Message-----
From: Felipe Contreras [mailto:felipe.contreras@gmail.com] 
Sent: Tuesday, March 03, 2009 1:22 AM
To: Guzman Lugo, Fernando
Cc: Pandita, Vikram; Kanigeri, Hari; Aguilar Pena, Leed; linux-omap@vger.kernel.org
Subject: Re: [OMAPZOOM][PATCH 5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull

On Tue, Mar 3, 2009 at 3:35 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
> From db89674b7316d7490e71c131091758eb7ef0eca2 Mon Sep 17 00:00:00 2001
> From: Fernando Guzman Lugo <x0095840@ti.com>
> Date: Wed, 25 Feb 2009 19:27:41 -0600
> Subject: [PATCH] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull function
>
> This patch removes some unnecesarry checks in HW_MBOX_IsFull
> function

Is that all this patch is doing?

> -       u16 cnt = 10;
> +       u16 cnt = 1000;

> -       while (--cnt) {
> -               hwStatus = HW_MBOX_IsFull(resources.dwMboxBase,
> -                                          MBOX_ARM2DSP, &mbxFull);
> -               if (mbxFull)
> -                       UTIL_Wait(1000);        /* wait for 1 ms)      */
> -               else
> -                       break;
> -       }
> +       while (--cnt && HW_MBOX_IsFull(resources.dwMboxBase, MBOX_ARM2DSP))
> +               udelay(1);
> +
Guzman Lugo, Fernando March 3, 2009, 4:41 p.m. UTC | #3 
Felipe,

	This patch also remove some check no needed; this part

-	/* Check input parameters */
-	CHECK_INPUT_PARAM(baseAddress, 0, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_PARAM(pIsFull,  NULL, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_RANGE_MIN0(mailBoxId, HW_MBOX_ID_MAX, RET_INVALID_ID,
-			RES_MBOX_BASE + RES_INVALID_INPUT_PARAM);
-

And change the function to inline.

Please let me know if you thing it needs something else.

Regards,
Fernando.

-----Original Message-----
From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Guzman Lugo, Fernando
Sent: Tuesday, March 03, 2009 10:35 AM
To: Felipe Contreras
Cc: Pandita, Vikram; Kanigeri, Hari; Aguilar Pena, Leed; linux-omap@vger.kernel.org
Subject: RE: [OMAPZOOM][PATCH 5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull


Hi Felipe,

	This patch is base on your patch about increase the performance waiting less and checking more the mail box.

Regards,
Fernando.

-----Original Message-----
From: Felipe Contreras [mailto:felipe.contreras@gmail.com] 
Sent: Tuesday, March 03, 2009 1:22 AM
To: Guzman Lugo, Fernando
Cc: Pandita, Vikram; Kanigeri, Hari; Aguilar Pena, Leed; linux-omap@vger.kernel.org
Subject: Re: [OMAPZOOM][PATCH 5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull

On Tue, Mar 3, 2009 at 3:35 AM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
> From db89674b7316d7490e71c131091758eb7ef0eca2 Mon Sep 17 00:00:00 2001
> From: Fernando Guzman Lugo <x0095840@ti.com>
> Date: Wed, 25 Feb 2009 19:27:41 -0600
> Subject: [PATCH] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull function
>
> This patch removes some unnecesarry checks in HW_MBOX_IsFull
> function

Is that all this patch is doing?

> -       u16 cnt = 10;
> +       u16 cnt = 1000;

> -       while (--cnt) {
> -               hwStatus = HW_MBOX_IsFull(resources.dwMboxBase,
> -                                          MBOX_ARM2DSP, &mbxFull);
> -               if (mbxFull)
> -                       UTIL_Wait(1000);        /* wait for 1 ms)      */
> -               else
> -                       break;
> -       }
> +       while (--cnt && HW_MBOX_IsFull(resources.dwMboxBase, MBOX_ARM2DSP))
> +               udelay(1);
> +
Felipe Contreras March 3, 2009, 6:16 p.m. UTC | #4 
On Tue, Mar 3, 2009 at 6:34 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
>
> Hi Felipe,
>
>        This patch is base on your patch about increase the performance waiting less and checking more the mail box.

Exactly, but that's not what the commit message says.

Besides, you are are doing multiple things in the same patch, which is
the same mistake I did in the original patch that Hiroshi pointed out:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg09875.html
Guzman Lugo, Fernando March 3, 2009, 7:39 p.m. UTC | #5 
Ok no problem, I am going to split this patch into two patches and I will send them.

Regards,
Fernando.

-----Original Message-----
From: Felipe Contreras [mailto:felipe.contreras@gmail.com] 
Sent: Tuesday, March 03, 2009 12:16 PM
To: Guzman Lugo, Fernando
Cc: Pandita, Vikram; Kanigeri, Hari; Aguilar Pena, Leed; linux-omap@vger.kernel.org
Subject: Re: [OMAPZOOM][PATCH 5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull

On Tue, Mar 3, 2009 at 6:34 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
>
> Hi Felipe,
>
>        This patch is base on your patch about increase the performance waiting less and checking more the mail box.

Exactly, but that's not what the commit message says.

Besides, you are are doing multiple things in the same patch, which is
the same mistake I did in the original patch that Hiroshi pointed out:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg09875.html
Guzman Lugo, Fernando March 4, 2009, 5:59 p.m. UTC | #6 
This patch has been spitted in two patches I will send them as PATCH 5a/5 DSPBRIDGE: cleanup of HW_MBOX_IsFull function and PATCH 5b/5 DSPBRIDGE: wait less and check the mailbox more.

Regards,
Fernando
Guzman
Lugo.

-----Original Message-----
From: Felipe Contreras [mailto:felipe.contreras@gmail.com] 
Sent: Tuesday, March 03, 2009 12:16 PM
To: Guzman Lugo, Fernando
Cc: Pandita, Vikram; Kanigeri, Hari; Aguilar Pena, Leed; linux-omap@vger.kernel.org
Subject: Re: [OMAPZOOM][PATCH 5/5] DSPBRIDGE: Remove unnecessary checks in HW_MBOX_IsFull

On Tue, Mar 3, 2009 at 6:34 PM, Guzman Lugo, Fernando <x0095840@ti.com> wrote:
>
> Hi Felipe,
>
>        This patch is base on your patch about increase the performance waiting less and checking more the mail box.

Exactly, but that's not what the commit message says.

Besides, you are are doing multiple things in the same patch, which is
the same mistake I did in the original patch that Hiroshi pointed out:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg09875.html
diff mbox

Patch

diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c
index bc61d64..3a539f5
--- a/drivers/dsp/bridge/hw/hw_mbox.c
+++ b/drivers/dsp/bridge/hw/hw_mbox.c
@@ -105,28 +105,11 @@  HW_STATUS HW_MBOX_MsgWrite(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId,
 }
 
 /* Reads the full status register for mailbox. */
-HW_STATUS HW_MBOX_IsFull(const void __iomem *baseAddress,
-	const HW_MBOX_Id_t mailBoxId, u32 *const pIsFull)
+inline bool HW_MBOX_IsFull(const void __iomem *baseAddress,
+	const HW_MBOX_Id_t mailBoxId)
 {
-	HW_STATUS status = RET_OK;
-	u32 fullStatus;
-
-	/* Check input parameters */
-	CHECK_INPUT_PARAM(baseAddress, 0, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_PARAM(pIsFull,  NULL, RET_BAD_NULL_PARAM, RES_MBOX_BASE +
-			RES_INVALID_INPUT_PARAM);
-	CHECK_INPUT_RANGE_MIN0(mailBoxId, HW_MBOX_ID_MAX, RET_INVALID_ID,
-			RES_MBOX_BASE + RES_INVALID_INPUT_PARAM);
-
-	/* read the is full status parameter for Mailbox */
-	fullStatus = MLBMAILBOX_FIFOSTATUS___0_15FifoFullMBmRead32(baseAddress,
+	return MLBMAILBOX_FIFOSTATUS___0_15FifoFullMBmRead32(baseAddress,
 							(u32)mailBoxId);
-
-	/* fill in return parameter */
-	*pIsFull = (fullStatus & 0xFF);
-
-	return status;
 }
 
 /* Gets number of messages in a specified mailbox. */
diff --git a/drivers/dsp/bridge/hw/hw_mbox.h b/drivers/dsp/bridge/hw/hw_mbox.h
index 225fb40..50a3746
--- a/drivers/dsp/bridge/hw/hw_mbox.h
+++ b/drivers/dsp/bridge/hw/hw_mbox.h
@@ -158,11 +158,9 @@  extern HW_STATUS HW_MBOX_MsgWrite(
 *
 * PURPOSE:      : this function reads the full status register for mailbox.
 */
-extern HW_STATUS HW_MBOX_IsFull(
+extern inline bool HW_MBOX_IsFull(
 		      const void __iomem *baseAddress,
-		      const HW_MBOX_Id_t   mailBoxId,
-		      u32 *const	pIsFull
-		  );
+		      const HW_MBOX_Id_t   mailBoxId);
 
 /*
 * FUNCTION      : HW_MBOX_NumMsgGet
diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c b/drivers/dsp/bridge/wmd/tiomap_sm.c
index edc3bcf..2843788
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -178,9 +178,8 @@  DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT *hDevContext)
 #endif
 #endif
 	HW_STATUS hwStatus;
-	u32 mbxFull;
 	struct CFG_HOSTRES resources;
-	u16 cnt = 10;
+	u16 cnt = 1000;
 	u32 temp;
 	/* We are waiting indefinitely here. This needs to be fixed in the
 	 * second phase */
@@ -241,14 +240,9 @@  DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT *hDevContext)
 
 		pDevContext->dwBrdState = BRD_RUNNING;
 	}
-	while (--cnt) {
-		hwStatus = HW_MBOX_IsFull(resources.dwMboxBase,
-					   MBOX_ARM2DSP, &mbxFull);
-		if (mbxFull)
-			UTIL_Wait(1000);	/* wait for 1 ms)      */
-		else
-			break;
-	}
+	while (--cnt && HW_MBOX_IsFull(resources.dwMboxBase, MBOX_ARM2DSP))
+		udelay(1);
+
 	if (!cnt) {
 		DBG_Trace(DBG_LEVEL7, "Timed out waiting for DSP mailbox \n");
 		status = WMD_E_TIMEOUT;