| Message ID | 1235295144-22097-3-git-send-email-felipe.contreras@gmail.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Headers | show |
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.contreras@gmail.com] > Sent: Sunday, February 22, 2009 3:32 AM > To: linux-omap@vger.kernel.org > Cc: Hiroshi DOYU; Kanigeri, Hari; ameya.palande@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.contreras@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 *const pIsFull > - ); > - > -/* > * 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 24, 2009 at 12:06 AM, Kanigeri, Hari <h-kanigeri2@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).
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 *const pIsFull - ); - -/* * 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");
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.contreras@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(-)