RE: [PATCH] DSPBRIDGE: CLK_Enable and CLK_Disable Code cleanup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ameya,

Patch looks good to me.

Thank you,
Best regards,
Hari

> -----Original Message-----
> From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-
> owner@xxxxxxxxxxxxxxx] On Behalf Of Ameya Palande
> Sent: Wednesday, April 15, 2009 7:19 AM
> To: linux-omap@xxxxxxxxxxxxxxx
> 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 <ameya.palande@xxxxxxxxx>
> ---
>  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 majordomo@xxxxxxxxxxxxxxx
> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux