Re: [PATCHv5 RFC 12/15] hwspinlock/core: add OF helper to parse reserved locks

[Suman Anna]

Hi Josh,

On 05/05/2014 04:54 PM, Josh Cartwright wrote:
> On Mon, May 05, 2014 at 04:44:25PM -0500, Suman Anna wrote:
>> Hi Rob,
>>
>> On 04/30/2014 07:34 PM, Suman Anna wrote:
>>> The property 'hwlock-reserved-locks' will be used to represent
>>> the number of locks to be reserved for clients that would need
>>> to request/operate on specific locks. A new OF helper function,
>>> of_hwspin_lock_get_num_reserved_locks(), is added to minimize
>>> duplication in different platform implementations.
>>>
>>> The function will return a value of 0 if the property is not
>>> defined, so as to support a default behavior of marking all
>>> locks as unused and open for anonymous allocations.
>>>
>>> Signed-off-by: Suman Anna <s-anna@xxxxxx>
>>> ---
>>>  .../devicetree/bindings/hwlock/hwlock.txt          |  3 +++
>>>  drivers/hwspinlock/hwspinlock_core.c               | 25 ++++++++++++++++++++++
>>>  include/linux/hwspinlock.h                         |  1 +
>>>  3 files changed, 29 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwlock/hwlock.txt b/Documentation/devicetree/bindings/hwlock/hwlock.txt
>>> index d538a9b..88d16d2 100644
>>> --- a/Documentation/devicetree/bindings/hwlock/hwlock.txt
>>> +++ b/Documentation/devicetree/bindings/hwlock/hwlock.txt
>>> @@ -18,6 +18,9 @@ Common properties:
>>>  			property is needed on hwlock devices, where the number
>>>  			of supported locks within a hwlock device cannot be
>>>  			read from a register.
>>> +- hwlock-reserved-locks: Number of locks to reserve for clients requiring
>>> +			specific locks. This value cannot exceed the value of
>>> +			hwlock-num-locks.
>>
>> Any suggestions here on the approach? This property falls into a gray
>> area as well, as the current approach is somewhat limiting (it doesn't
>> support sparse reserved locks, and expects them at the beginning of the
>> lock range).
> 
> Is it possible to implement a pinctrl-like hogging approach, whereby the
> specific locks that need to be reserved are consumed by the controller
> itself?
> 

Thanks for the suggestion. I did take a look at pinctrl and while it is
possible to implement something similar, I feel it is a bit heavy for
hwspinlock framework with no added advantages. It requires that the
controller and clients always need to be updated together. Ohad had
already brought this up [1]. Here's an alternate approach that does not
require any additional property to the controller itself, while the
client node usage is as before. The logic is based on parsing through
the DT blob and marking only those that are used by any clients. The RFC
patch below is a replacement for Patches 11 to 15, and do not require
any changes to platform implementations or additional DT properties.

It currently marks locks as reserved even for disabled client nodes
(very easy to change that behavior). It will also impose a standard
property name "hwlocks" on the client nodes. What do you think of this
approach?

regards
Suman

[1] http://marc.info/?l=linux-omap&m=139514977622964&w=2

----