On Fri, Oct 19, 2012 at 11:29 AM, Shiraz Hashim <shiraz.has...@st.com> wrote: > On Thu, Oct 18, 2012 at 11:11:06PM +0530, viresh kumar wrote: >> On Thu, Oct 18, 2012 at 4:58 PM, Shiraz Hashim <shiraz.has...@st.com> wrote:
>> > +static int __devexit spear_pwm_remove(struct platform_device *pdev) >> > +{ >> > + struct spear_pwm_chip *pc = platform_get_drvdata(pdev); >> > + int i; >> > + >> > + if (WARN_ON(!pc)) >> > + return -ENODEV; >> > + >> > + for (i = 0; i < NUM_PWM; i++) { >> > + struct pwm_device *pwmd = &pc->chip.pwms[i]; >> > + >> > + if (!test_bit(PWMF_ENABLED, &pwmd->flags)) One point here: If i am not wrong you want to disable pwmd if it is enabled. Shouldn't you check for if (test_bit(PWMF_ENABLED, &pwmd->flags)) instead? >> > + if (clk_prepare_enable(pc->clk) < 0) >> > + continue; >> > + >> > + spear_pwm_writel(pc, i, PWMCR, 0); >> > + clk_disable_unprepare(pc->clk); >> > + } >> >> You are enabling/disabling clock N times here and each of these will >> write to an register. Do something better. >> > > I need to shut down all active pwms, how else would you suggest that ? Sorry, i misread the code on the second go :( I am proposing something like: static int __devexit spear_pwm_remove(struct platform_device *pdev) { struct spear_pwm_chip *pc = platform_get_drvdata(pdev); int i, clk_enabled = 0; if (WARN_ON(!pc)) return -ENODEV; for (i = 0; i < NUM_PWM; i++) { struct pwm_device *pwmd = &pc->chip.pwms[i]; if (!test_bit(PWMF_ENABLED, &pwmd->flags) && !clk_enabled) if (clk_prepare_enable(pc->clk) < 0) continue; else clk_enabled++; spear_pwm_writel(pc, i, PWMCR, 0); } if (clk_enabled) clk_disable_unprepare(pc->clk); ... } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/