On Tue, Sep 25, 2012 at 12:32:37PM +0300, Tero Kristo wrote:
> diff --git a/arch/arm/mach-omap2/powerdomain.c 
> b/arch/arm/mach-omap2/powerdomain.c
> index ba49029..ca54aec 100644
> --- a/arch/arm/mach-omap2/powerdomain.c
> +++ b/arch/arm/mach-omap2/powerdomain.c
> @@ -1475,10 +1477,16 @@ int pwrdm_state_switch(struct powerdomain *pwrdm)
>   */
>  void pwrdm_clkdm_enable(struct powerdomain *pwrdm)
>  {
> +     unsigned long flags;
> +
>       if (!pwrdm)
>               return;
>  
> -     atomic_inc(&pwrdm->usecount);
> +     if (atomic_inc_return(&pwrdm->usecount) == 1) {
> +             spin_lock_irqsave(&pwrdm->lock, flags);
> +             voltdm_pwrdm_enable(pwrdm->voltdm.ptr);
> +             spin_unlock_irqrestore(&pwrdm->lock, flags);
> +     }

This looks like the classic "I like atomic types because they have magic
properties" brain-deadness.

What would happen to users of this if you had this sequence:

pwrdm->usecount starts off as 1.

Thread0                         Thread1
atomic_inc_return() (returns 1)
                                atomic_inc_return() (returns 2)
                                starts using stuff in power domain
spin_lock_irqsave()
voltdm_pwrdm_enable()
spin_unlock_irqrestore()

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

Reply via email to