I'm sorry. Authors of the code may delete this e-mail.
More flame and suggestions.
Ameya Palande wrote:
diff --git a/drivers/dsp/bridge/services/clk.c b/drivers/dsp/bridge/services/clk.c
index 440706f..b45603f 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);
Is it possible to get rid of this crap? I mean trace.
First of all, if you pretend your code is production-ready,
you should not need that much of this. Second of all, we
have tools like ftrace.
@@ -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);
Is it possible to wake up, and realize you are writing
linux kernel code, and realize you should socialize?
Namely, these marginal naming conventions you use are
absolutely anti-social. DSPBRIGE is around for quite a
time already, its time to start socializing.
} 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)
Is it possible to stop using excessive enums? This is not
C++, and C does not do any type-checking. Named enums
make almost zero sense.
diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c
index df350c6..fb71e96 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430.c
@@ -2240,7 +2240,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;
Is it possible to remove these insane types. Use normal types.
enum HW_PwrState_t pwrState;
Similar. What is the point of enums like this? Obfuscate
the code?
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..ffe2e3c 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;
I'm not 100% sure about this particular case, but overall,
the code excessively uses these u32 types. This makse few
sense. Unless you work with registers, packets, or something
which needs some strict types, use int or long. Use natural
C types. Do not try to be too smart, do not try to limit
CPU and compiler by u32, if it is not necessary.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
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
