Hi Hari,

On Thu, Aug 12, 2010 at 12:44 AM, Kanigeri, Hari <h-kanige...@ti.com> wrote:
>> > +/* Attempt to acquire a spinlock once */
>> > +int hwspinlock_trylock(struct hwspinlock *handle)
>> > +{
>> > +       int retval = 0;
>> > +
>> > +       if (WARN_ON(handle == NULL))
>> > +               return -EINVAL;
>> > +
>> > +       if (WARN_ON(in_irq()))
>> > +               return -EPERM;
>> > +
>> > +       if (pm_runtime_get(&handle->pdev->dev) < 0)
>> > +               return -ENODEV;
>> > +
>> > +       /* Attempt to acquire the lock by reading from it */
>> > +       retval = readl(handle->lock_reg);
>> > +
>> > +       if (retval == HWSPINLOCK_BUSY)
>> > +               pm_runtime_put(&handle->pdev->dev);
>> Any reason for using pm_runtime_put instead of pm_runtime_put_sync?
>>
>> Using pm_runtime_gett_sync & pm_runtime_put_sync have been recommended by
>> Kevin Hilman.
>
> Actually is there a need to call pm_runtime_put_sync for hwspinlock ? 
> Spinlocks are used by the co-processors and we have to ensure that the device 
> doesn't enter some low power mode without the knowledge of Co-processor. I 
> don't think run time PM is needed for hwspinlock.
>
> Just doing pm_runtime_get_sync at probe time for all the spinlock instances 
> should be good.


It would probably make more sense to call pm_runtime_get_sync during
hwspinlock_request{_specific}, and then call pm_runtime_put during
hwspinlock_free.

This way the runtime PM's usage_count will reflect the number of locks
that are actually used, and if that number drops to (or never go
beyond) zero, it is desirable to have the hwspinlock's clock disabled.

This is also safe since no other core will use the hwspinlock if it
wasn't requested by the MPU beforehand (and if it does, we better know
about it and fix it).

Thanks,
Ohad.
--
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