Ameya,

Patch looks good to me.

Thank you,
Best regards,
Hari

> -----Original Message-----
> From: [email protected] [mailto:linux-omap-
> [email protected]] On Behalf Of Ameya Palande
> Sent: Wednesday, April 15, 2009 7:19 AM
> To: [email protected]
> Subject: [PATCH] DSPBRIDGE: CLK_Enable and CLK_Disable Code cleanup
> 
> This patch does following things:
> 1. Instead of GT_2trace() use pr_err(), since failure to enable or disable
> a
>    clock is error which should be notified.
> 2. There is no need to check the return value of CLK_Enable and
> CLK_Disable
>    and print error message, since these functions internally print the
> error.
> 3. Indentation changes and a typo fix.
> 
> Signed-off-by: Ameya Palande <[email protected]>
> ---
>  drivers/dsp/bridge/services/clk.c       |   42 ++++++++++++++------------
> ----
>  drivers/dsp/bridge/wmd/tiomap3430.c     |   13 +--------
>  drivers/dsp/bridge/wmd/tiomap3430_pwr.c |   22 ++++-----------
>  3 files changed, 28 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/services/clk.c
> b/drivers/dsp/bridge/services/clk.c
> index 440706f..fbbde72 100644
> --- a/drivers/dsp/bridge/services/clk.c
> +++ b/drivers/dsp/bridge/services/clk.c
> @@ -188,7 +188,7 @@ DSP_STATUS CLK_Enable(IN enum SERVICES_ClkId clk_id)
>       struct clk *pClk;
> 
>       DBC_Require(clk_id < SERVICESCLK_NOT_DEFINED);
> -       GT_2trace(CLK_debugMask, GT_6CLASS, "CLK_Enable: CLK %s, "
> +     GT_2trace(CLK_debugMask, GT_6CLASS, "CLK_Enable: CLK %s, "
>               "CLK dev id = %d\n", SERVICES_Clks[clk_id].clk_name,
>               SERVICES_Clks[clk_id].id);
> 
> @@ -197,18 +197,16 @@ DSP_STATUS CLK_Enable(IN enum SERVICES_ClkId clk_id)
>               if (clk_enable(pClk) == 0x0) {
>                       /* Success ? */
>               } else {
> -                    GT_2trace(CLK_debugMask, GT_7CLASS,
> -                              "CLK_Enable: failed to Enable CLK %s, "
> -                              "CLK dev id = %d\n",
> -                              SERVICES_Clks[clk_id].clk_name,
> -                              SERVICES_Clks[clk_id].id);
> +                     pr_err("CLK_Enable: failed to Enable CLK %s, "
> +                                     "CLK dev id = %d\n",
> +                                     SERVICES_Clks[clk_id].clk_name,
> +                                     SERVICES_Clks[clk_id].id);
>                       status = DSP_EFAIL;
>               }
>       } else {
> -            GT_2trace(CLK_debugMask, GT_7CLASS,
> -                      "CLK_Enable: failed to get CLK %s, CLK dev id = %d\n",
> -                      SERVICES_Clks[clk_id].clk_name,
> -                      SERVICES_Clks[clk_id].id);
> +             pr_err("CLK_Enable: failed to get CLK %s, CLK dev id = %d\n",
> +                                     SERVICES_Clks[clk_id].clk_name,
> +                                     SERVICES_Clks[clk_id].id);
>               status = DSP_EFAIL;
>       }
>       /* The SSI module need to configured not to have the Forced idle for
> @@ -274,15 +272,15 @@ DSP_STATUS CLK_Disable(IN enum SERVICES_ClkId
> clk_id)
> 
>       clkUseCnt = CLK_Get_UseCnt(clk_id);
>       if (clkUseCnt == -1) {
> -            GT_2trace(CLK_debugMask, GT_7CLASS, "CLK_Disable: failed to "
> -                     "get CLK Use count for CLK %s, CLK dev id = %d\n",
> -                     SERVICES_Clks[clk_id].clk_name,
> -                     SERVICES_Clks[clk_id].id);
> +             pr_err("CLK_Disable: failed to get CLK Use count for CLK %s,
> +                             CLK dev id = %d\n",
> +                             SERVICES_Clks[clk_id].clk_name,
> +                             SERVICES_Clks[clk_id].id);
>       } else if (clkUseCnt == 0) {
> -            GT_2trace(CLK_debugMask, GT_7CLASS, "CLK_Disable: CLK %s, "
> -                     "CLK dev id= %d is already disabled\n",
> -                     SERVICES_Clks[clk_id].clk_name,
> -                     SERVICES_Clks[clk_id].id);
> +             pr_err("CLK_Disable: CLK %s, CLK dev id= %d is already
> +                             disabled\n",
> +                             SERVICES_Clks[clk_id].clk_name,
> +                             SERVICES_Clks[clk_id].id);
>                return status;
>       }
>       if (clk_id == SERVICESCLK_ssi_ick)
> @@ -291,10 +289,10 @@ DSP_STATUS CLK_Disable(IN enum SERVICES_ClkId
> clk_id)
>               if (pClk) {
>                       clk_disable(pClk);
>               } else {
> -                    GT_2trace(CLK_debugMask, GT_7CLASS, "CLK_Disable: "
> -                             "failed to get CLK %s, CLK dev id = %d\n",
> -                             SERVICES_Clks[clk_id].clk_name,
> -                             SERVICES_Clks[clk_id].id);
> +                     pr_err("CLK_Disable: failed to get CLK %s,
> +                                     CLK dev id = %d\n",
> +                                     SERVICES_Clks[clk_id].clk_name,
> +                                     SERVICES_Clks[clk_id].id);
>                       status = DSP_EFAIL;
>               }
>       return status;
> diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c
> b/drivers/dsp/bridge/wmd/tiomap3430.c
> index 7fa6f8e..606de3c 100644
> --- a/drivers/dsp/bridge/wmd/tiomap3430.c
> +++ b/drivers/dsp/bridge/wmd/tiomap3430.c
> @@ -2119,7 +2119,6 @@ static DSP_STATUS run_IdleBoot(u32 prm_base, u32
> cm_base,
>  {
>       u32 temp;
>       DSP_STATUS status = DSP_SOK;
> -     DSP_STATUS clk_status = DSP_SOK;
>       enum HW_PwrState_t    pwrState;
> 
>       /* Read PM_PWSTST_IVA2 */
> @@ -2134,11 +2133,7 @@ static DSP_STATUS run_IdleBoot(u32 prm_base, u32
> cm_base,
>               /* Wait until the state has moved to ON */
>               HW_PWR_IVA2StateGet(prm_base, HW_PWR_DOMAIN_DSP, &pwrState);
>       }
> -     clk_status = CLK_Disable(SERVICESCLK_iva2_ck);
> -     if (DSP_FAILED(clk_status)) {
> -             DBG_Trace(DBG_LEVEL6, "CLK_Disbale failed for clk = 0x%x \n",
> -                       SERVICESCLK_iva2_ck);
> -     }
> +     CLK_Disable(SERVICESCLK_iva2_ck);
>       udelay(10);
>       /* Assert IVA2-RST1 and IVA2-RST2  */
>       *((REG_UWORD32 *)((u32)(prm_base) + 0x50)) = (u32)0x07;
> @@ -2155,11 +2150,7 @@ static DSP_STATUS run_IdleBoot(u32 prm_base, u32
> cm_base,
>                 temp =  (temp & 0xFFFFFC8) | 0x37;
>                 *((REG_UWORD32 *) ((u32) (cm_base) + 0x4))
>                         (u32) temp;
> -     clk_status = CLK_Enable(SERVICESCLK_iva2_ck);
> -     if (DSP_FAILED(clk_status)) {
> -             DBG_Trace(DBG_LEVEL6, "CLK_Enable failed for clk = 0x%x \n",
> -                       SERVICESCLK_iva2_ck);
> -     }
> +     CLK_Enable(SERVICESCLK_iva2_ck);
>       udelay(20);
>       GetHWRegs(prm_base, cm_base);
>       /* Release Reset1 and Reset2 */
> diff --git a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> index 488a512..1ad1565 100644
> --- a/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> +++ b/drivers/dsp/bridge/wmd/tiomap3430_pwr.c
> @@ -285,7 +285,7 @@ DSP_STATUS WakeDSP(struct WMD_DEV_CONTEXT
> *pDevContext, IN void *pArgs)
>  #ifdef CONFIG_PM
>       struct CFG_HOSTRES resources;
>       enum HW_PwrState_t pwrState;
> -       u32 temp;
> +     u32 temp;
> 
>       status = CFG_GetHostResources(
>                (struct CFG_DEVNODE *)DRV_GetFirstDevExtension(),
> &resources);
> @@ -306,7 +306,7 @@ DSP_STATUS WakeDSP(struct WMD_DEV_CONTEXT
> *pDevContext, IN void *pArgs)
>                pDevContext->uDspPerClks);
>       status = DSP_PeripheralClocks_Enable(pDevContext, NULL);
> 
> -       /* Enablifg Dppll in lock mode*/
> +     /* Enabling Dppll in lock mode */
>                 temp = (u32) *((REG_UWORD32 *)
>                         ((u32) (resources.dwCmBase) + 0x34));
>                 temp = (temp & 0xFFFFFFFE) | 0x1;
> @@ -546,27 +546,17 @@ DSP_STATUS DSP_PeripheralClocks_Enable(struct
> WMD_DEV_CONTEXT *pDevContext,
>                                     IN void *pArgs)
>  {
>       u32 clkIdx;
> -     DSP_STATUS status = DSP_SOK;
> +     DSP_STATUS int_clk_status = DSP_EFAIL, fun_clk_status = DSP_EFAIL;
> 
>       for (clkIdx = 0; clkIdx < MBX_PM_MAX_RESOURCES; clkIdx++) {
>               if (((pDevContext->uDspPerClks) >> clkIdx) & 0x01) {
>                       /* Enable the interface clock of the peripheral */
> -                     status = CLK_Enable(BPWR_Clks[clkIdx].intClk);
> -                     if (DSP_FAILED(status)) {
> -                             DBG_Trace(DBG_LEVEL7,
> -                                      "Failed to Enable the DSP Peripheral"
> -                                      "Clk 0x%x \n", BPWR_Clks[clkIdx]);
> -                     }
> +                     int_clk_status = CLK_Enable(BPWR_Clks[clkIdx].intClk);
>                       /* Enable the functional clock of the periphearl */
> -                     status = CLK_Enable(BPWR_Clks[clkIdx].funClk);
> -                     if (DSP_FAILED(status)) {
> -                             DBG_Trace(DBG_LEVEL7,
> -                                      "Failed to Enable the DSP Peripheral"
> -                                      "Clk 0x%x \n", BPWR_Clks[clkIdx]);
> -                     }
> +                     fun_clk_status = CLK_Enable(BPWR_Clks[clkIdx].funClk);
>               }
>       }
> -     return status;
> +     return ((int_clk_status | fun_clk_status) != DSP_OK) ? DSP_EFAIL :
> DSP_OK;
>  }
> 
>  void DSPClkWakeupEventCtrl(u32 ClkId, bool enable)
> --
> 1.6.2.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to [email protected]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Reply via email to