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/
