Re: [PATCH RFC v2 0/2] hwspinlock: Introduce raw capability for hwspinlock_device

Lina Iyer
2015-07-02T20:30:38Z

On Sat, Jun 27 2015 at 05:25 -0600, Ohad Ben-Cohen wrote:
>Hi Lina,
>
>On Sat, Jun 27, 2015 at 6:05 AM, Lina Iyer <lina.iyer@linaro.org> wrote:
>> Hi Ohad,
>>
>> Any comments?
>
>Sorry, I was under the impression the discussion with Bjorn is still open.
>
I am of the opinion that the platform driver and the framework should
handle this request. This variation is still within the bounds of proper
usage of the hw remote lock. hwspinlock frameowkr imposes a s/w spinlock
around access to every hw remote lock and the current QCOM platform
driver assumes that the value written into the hardware, has to be a
constant. Both of these are assumptions are the limitations in Linux
and is not a hw remote lock behavior.

I do not agree that the cpuidle driver has to memory map a hwspinlock
region and treat it as a register write, because we dont want to
complicate the hwspinlock platform driver.

>Like Bjorn, I'm not so sure too we want to bind a specific lock to the
>RAW capability since this is not a lock-specific hardware detail.
>
You are right, RAW capability is not lock specific. But we dont want to
impose this on every lock in the bank either. Drivers rely on the
framework's s/w spinlock to ensure that different processes in Linux,
trying to lock the same hwspinlock may correctly acquire. The framework
shall guarantee that the hwspinlock is correctly acquired for regular
usecases (where a constant value is written to the h/w t olock). The RAW
capability assumes that the driver acquiring the RAW lock, knows that
the platform will write a unique value to the h/w and therefore the
correctness of locking is assured by the h/w.

>As far as I can see, the hardware-specific differences (if any) are at
>the vendor level and not at the lock level, therefore it might make
>more sense to add the caps member to hwspinlock_device rather than to
>the hwspinlock struct (Jeffrey commented about this too).
>
Jeff's comment is about my commit text pointing to the wrong structure.
I believe he is fine with the implementation. We debated this idea,
before I came up with this patch.

Thanks,
Lina