RE: [PATCH v2 3/4] dsp-bridge: cleanup and remove HW_MBOX_IsFull

2009-02-23 Thread Kanigeri, Hari 
Hi Felipe,

 HW_MBOX_IsFull has many convoluted macros and is used only once. Clean
 it up so it's easier to see what it's actually doing.

-- Is this the reason you added a new function that checks if fifo is full or 
is there any issue with the existing function ?

Thank you,
Best regards,
Hari

 -Original Message-
 From: Felipe Contreras [mailto:felipe.contre...@gmail.com]
 Sent: Sunday, February 22, 2009 3:32 AM
 To: linux-omap@vger.kernel.org
 Cc: Hiroshi DOYU; Kanigeri, Hari; ameya.pala...@nokia.com; Felipe
 Contreras
 Subject: [PATCH v2 3/4] dsp-bridge: cleanup and remove HW_MBOX_IsFull
 
 HW_MBOX_IsFull has many convoluted macros and is used only once. Clean
 it up so it's easier to see what it's actually doing.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  drivers/dsp/bridge/hw/hw_mbox.c|   25 -
  drivers/dsp/bridge/hw/hw_mbox.h|   35 ---
 
  drivers/dsp/bridge/wmd/tiomap_sm.c |   15 +--
  3 files changed, 9 insertions(+), 66 deletions(-)
 
 diff --git a/drivers/dsp/bridge/hw/hw_mbox.c
 b/drivers/dsp/bridge/hw/hw_mbox.c
 index 2c14ade..31b890a 100644
 --- a/drivers/dsp/bridge/hw/hw_mbox.c
 +++ b/drivers/dsp/bridge/hw/hw_mbox.c
 @@ -104,31 +104,6 @@ HW_STATUS HW_MBOX_MsgWrite(const u32 baseAddress,
 const HW_MBOX_Id_t mailBoxId,
   return status;
  }
 
 -/* Reads the full status register for mailbox. */
 -HW_STATUS HW_MBOX_IsFull(const u32 baseAddress, const HW_MBOX_Id_t
 mailBoxId,
 - u32 *const pIsFull)
 -{
 - 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,
 - (u32)mailBoxId);
 -
 - /* fill in return parameter */
 - *pIsFull = (fullStatus  0xFF);
 -
 - return status;
 -}
 -
  /* Gets number of messages in a specified mailbox. */
  HW_STATUS HW_MBOX_NumMsgGet(const u32 baseAddress, const HW_MBOX_Id_t
 mailBoxId,
   u32 *const pNumMsg)
 diff --git a/drivers/dsp/bridge/hw/hw_mbox.h
 b/drivers/dsp/bridge/hw/hw_mbox.h
 index 225fb40..d2981d3 100644
 --- a/drivers/dsp/bridge/hw/hw_mbox.h
 +++ b/drivers/dsp/bridge/hw/hw_mbox.h
 @@ -130,41 +130,6 @@ extern HW_STATUS HW_MBOX_MsgWrite(
 );
 
  /*
 -* FUNCTION  : HW_MBOX_IsFull
 -*
 -* INPUTS:
 -*
 -*   Identifier  : baseAddress
 -*   Type : const u32
 -*   Description : Base Address of instance of Mailbox module
 -*
 -*   Identifier  : mailBoxId
 -*   Type : const HW_MBOX_Id_t
 -*   Description : Mail Box Sub module Id to check
 -*
 -* OUTPUTS:
 -*
 -*   Identifier  : pIsFull
 -*   Type : u32 *const
 -*   Description : false means mail box not Full
 -* true means mailbox full.
 -*
 -* RETURNS:
 -*
 -*   Type : ReturnCode_t
 -*   Description : RET_OK   No errors occured
 -* RET_BAD_NULL_PARAM  Address/pointer Paramater was set to
 0/NULL
 -* RET_INVALID_ID  Invalid Id used
 -*
 -* PURPOSE:  : this function reads the full status register for
 mailbox.
 -*/
 -extern HW_STATUS HW_MBOX_IsFull(
 -   const u32  baseAddress,
 -   const HW_MBOX_Id_t   mailBoxId,
 -   u32 *constpIsFull
 -   );
 -
 -/*
  * FUNCTION  : HW_MBOX_NumMsgGet
  *
  * INPUTS:
 diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c
 b/drivers/dsp/bridge/wmd/tiomap_sm.c
 index 2f381c8..4ad893b 100644
 --- a/drivers/dsp/bridge/wmd/tiomap_sm.c
 +++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
 @@ -156,6 +156,13 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct
 WMD_DEV_CONTEXT *hDevContext)
   return status;
  }
 
 +#define MAILBOX_FIFOSTATUS(m) (0x80 + 4 * (m))
 +
 +static inline unsigned int fifo_full(void __iomem *mbox_base, int
 mbox_id)
 +{
 + return __raw_readl(mbox_base + MAILBOX_FIFOSTATUS(mbox_id))  0x1;
 +}
 +
  /*
   *   CHNLSM_InterruptDSP 
   *  Send an interrupt to the DSP processor(s).
 @@ -171,7 +178,6 @@ DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT
 *hDevContext)
   u32 opplevel = 0;
  #endif
   HW_STATUS hwStatus;
 - u32 mbxFull;
   struct CFG_HOSTRES resources;
   u16 cnt = 10;
   u32 temp;
 @@ -214,12 +220,9 @@ DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT
 *hDevContext)
   pDevContext-dwBrdState = BRD_RUNNING;
   }
   while (--cnt) {
 - hwStatus

Re: [PATCH v2 3/4] dsp-bridge: cleanup and remove HW_MBOX_IsFull

2009-02-23 Thread Felipe Contreras 
On Tue, Feb 24, 2009 at 12:06 AM, Kanigeri, Hari h-kanige...@ti.com wrote:
 Hi Felipe,

 HW_MBOX_IsFull has many convoluted macros and is used only once. Clean
 it up so it's easier to see what it's actually doing.

 -- Is this the reason you added a new function that checks if fifo is full or 
 is there any issue with the existing function ?

fifo_full should do exactly the same as HW_MBOX_IsFulll, I just
removed all the macros and used __raw_readl instead. I could have
removed the fifo_full function and do the __raw_readl directly, but I
thought this way it was more readable.

The main reason I cleaned the function was for performance reasons;
remove unnecessary checks, extra steps, and the function call (now
inline).

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

[PATCH v2 3/4] dsp-bridge: cleanup and remove HW_MBOX_IsFull

2009-02-22 Thread Felipe Contreras 
HW_MBOX_IsFull has many convoluted macros and is used only once. Clean
it up so it's easier to see what it's actually doing.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 drivers/dsp/bridge/hw/hw_mbox.c|   25 -
 drivers/dsp/bridge/hw/hw_mbox.h|   35 ---
 drivers/dsp/bridge/wmd/tiomap_sm.c |   15 +--
 3 files changed, 9 insertions(+), 66 deletions(-)

diff --git a/drivers/dsp/bridge/hw/hw_mbox.c b/drivers/dsp/bridge/hw/hw_mbox.c
index 2c14ade..31b890a 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.c
+++ b/drivers/dsp/bridge/hw/hw_mbox.c
@@ -104,31 +104,6 @@ HW_STATUS HW_MBOX_MsgWrite(const u32 baseAddress, const 
HW_MBOX_Id_t mailBoxId,
return status;
 }
 
-/* Reads the full status register for mailbox. */
-HW_STATUS HW_MBOX_IsFull(const u32 baseAddress, const HW_MBOX_Id_t mailBoxId,
-   u32 *const pIsFull)
-{
-   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,
-   (u32)mailBoxId);
-
-   /* fill in return parameter */
-   *pIsFull = (fullStatus  0xFF);
-
-   return status;
-}
-
 /* Gets number of messages in a specified mailbox. */
 HW_STATUS HW_MBOX_NumMsgGet(const u32 baseAddress, const HW_MBOX_Id_t 
mailBoxId,
u32 *const pNumMsg)
diff --git a/drivers/dsp/bridge/hw/hw_mbox.h b/drivers/dsp/bridge/hw/hw_mbox.h
index 225fb40..d2981d3 100644
--- a/drivers/dsp/bridge/hw/hw_mbox.h
+++ b/drivers/dsp/bridge/hw/hw_mbox.h
@@ -130,41 +130,6 @@ extern HW_STATUS HW_MBOX_MsgWrite(
  );
 
 /*
-* FUNCTION  : HW_MBOX_IsFull
-*
-* INPUTS:
-*
-*   Identifier  : baseAddress
-*   Type   : const u32
-*   Description : Base Address of instance of Mailbox module
-*
-*   Identifier  : mailBoxId
-*   Type   : const HW_MBOX_Id_t
-*   Description : Mail Box Sub module Id to check
-*
-* OUTPUTS:
-*
-*   Identifier  : pIsFull
-*   Type   : u32 *const
-*   Description : false means mail box not Full
-*   true means mailbox full.
-*
-* RETURNS:
-*
-*   Type   : ReturnCode_t
-*   Description : RET_OK No errors occured
-*   RET_BAD_NULL_PARAM  Address/pointer Paramater was set to 0/NULL
-*   RET_INVALID_ID  Invalid Id used
-*
-* PURPOSE:  : this function reads the full status register for mailbox.
-*/
-extern HW_STATUS HW_MBOX_IsFull(
- const u32  baseAddress,
- const HW_MBOX_Id_t   mailBoxId,
- u32 *constpIsFull
- );
-
-/*
 * FUNCTION  : HW_MBOX_NumMsgGet
 *
 * INPUTS:
diff --git a/drivers/dsp/bridge/wmd/tiomap_sm.c 
b/drivers/dsp/bridge/wmd/tiomap_sm.c
index 2f381c8..4ad893b 100644
--- a/drivers/dsp/bridge/wmd/tiomap_sm.c
+++ b/drivers/dsp/bridge/wmd/tiomap_sm.c
@@ -156,6 +156,13 @@ DSP_STATUS CHNLSM_DisableInterrupt(struct WMD_DEV_CONTEXT 
*hDevContext)
return status;
 }
 
+#define MAILBOX_FIFOSTATUS(m) (0x80 + 4 * (m))
+
+static inline unsigned int fifo_full(void __iomem *mbox_base, int mbox_id)
+{
+   return __raw_readl(mbox_base + MAILBOX_FIFOSTATUS(mbox_id))  0x1;
+}
+
 /*
  *   CHNLSM_InterruptDSP 
  *  Send an interrupt to the DSP processor(s).
@@ -171,7 +178,6 @@ DSP_STATUS CHNLSM_InterruptDSP(struct WMD_DEV_CONTEXT 
*hDevContext)
u32 opplevel = 0;
 #endif
HW_STATUS hwStatus;
-   u32 mbxFull;
struct CFG_HOSTRES resources;
u16 cnt = 10;
u32 temp;
@@ -214,12 +220,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)
-   mdelay(1);
-   else
+   if (!fifo_full((void __iomem *) resources.dwMboxBase, 0))
break;
+   mdelay(1);
}
if (!cnt) {
DBG_Trace(DBG_LEVEL7, Timed out waiting for DSP mailbox \n);
-- 
1.6.1.3

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