Re: [PATCH 3/5] omap:hwspinlock-added hwspinlock driver

["Cousson, Benoit" <b-cousson@ti.com>]

On 7/29/2010 3:14 PM, Kanigeri, Hari wrote:
> Yogesh,
>
> Nice to see your email.
>
>>>> +/* Release a spinlock */
>>>> +int hwspinlock_unlock(struct hwspinlock *handle)
>>>> +{
>>>> +     if (WARN_ON(handle == NULL))
>>>> +             return -EINVAL;
>>>> +
>>>> +     /* Release it by writing 0 to it */
>>>> +     writel(0, handle->lock_reg);
>>
>> [[Yogesh Marathe]] Releasing the spinlock without knowing who owns it is
>> risky. There should be a check for ownership and if authenticated user has
>> called this api  only then it should be released otherwise permission
>> denied error should be returned.
>
> -- I think if there is another Kernel client that is trying to release that is not owned by it then that Kernel client itself is buggy and needs to be fixed. Please share your thoughts on how we can ensure that we can add some protection.

Yes, I think so as well. The hwspinlock itself was done to be simple. 
The HW is not even checking that the processor that is taking the 
spinlock is the one that will release it.

This is maybe not the ideal and safe way to do that, but I'm not sure 
that we need to add some more complexity than needed for such a basic IP.

BTW, is a Linux kernel spinlock taking care of ownership?

Benoit

>
>>
>>>> +
>>>> +     pm_runtime_put(&handle->pdev->dev);
>>>> +
>>>> +     return 0;
>
> Thank you,
> Best regards,
> Hari