lkml.org 
[lkml]   [2015]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] Lock 7 is cpuidle specific, use non-generic value for locking
On Thu, Mar 12 2015 at 14:35 -0600, Stephen Boyd wrote:
>On 03/12/15 12:38, Lina Iyer wrote:
>> ---
>
>sign off?
>
:) I was just hacking it to make it easier to understand. Sure.

>> drivers/hwspinlock/qcom_hwspinlock.c | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwspinlock/qcom_hwspinlock.c b/drivers/hwspinlock/qcom_hwspinlock.c
>> index 93b62e0..7642524 100644
>> --- a/drivers/hwspinlock/qcom_hwspinlock.c
>> +++ b/drivers/hwspinlock/qcom_hwspinlock.c
>> @@ -25,16 +25,23 @@
>>
>> #include "hwspinlock_internal.h"
>>
>> -#define QCOM_MUTEX_APPS_PROC_ID 1
>> -#define QCOM_MUTEX_NUM_LOCKS 32
>> +#define QCOM_MUTEX_APPS_PROC_ID 1
>> +#define QCOM_MUTEX_CPUIDLE_OFFSET 128
>> +#define QCOM_CPUIDLE_LOCK 7
>> +#define QCOM_MUTEX_NUM_LOCKS 32
>>
>> static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
>> {
>> struct regmap_field *field = lock->priv;
>> u32 lock_owner;
>> int ret;
>> + u32 proc_id;
>>
>> - ret = regmap_field_write(field, QCOM_MUTEX_APPS_PROC_ID);
>> + proc_id = hwspin_lock_get_id(lock) == QCOM_CPUIDLE_LOCK ?
>> + QCOM_MUTEX_CPUIDLE_OFFSET + smp_processor_id():
>
>So we assume that the caller will always be the CPU that is locking the
>lock? Also, do we assume that the remote side knows our CPU scheme here?
>smp_processor_id() returns the logical CPU and not the physical CPU
>number so hopefully the remote side doesn't care about logical CPU
>numbers being written to the lock value.

The remote side (SCM) doesnt care the value written. We use 128+cpu to
be unique in Linux(128 is to make sure it doesnt clash with predefined
values used across by other processors.

>
>Perhaps it would be better to have a way to tell the hwspinlock
>framework what value we want written to the lock value.
>
That would be good, if there is value in that for other platforms, I
will gladly make the change.

Thoughts?

>> + QCOM_MUTEX_APPS_PROC_ID;
>> +
>> + ret = regmap_field_write(field, proc_id);
>> if (ret)
>> return ret;
>>
>> @@ -42,7 +49,7 @@ static int qcom_hwspinlock_trylock(struct hwspinlock *lock)
>> if (ret)
>> return ret;
>>
>> - return lock_owner == QCOM_MUTEX_APPS_PROC_ID;
>> + return lock_owner == proc_id;
>> }
>>
>> static void qcom_hwspinlock_unlock(struct hwspinlock *lock)
>
>The unlock path checks proc_id so we need to update the path there too.
>
Good point. I missed it.

>--
>Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
>a Linux Foundation Collaborative Project
>


\
 
 \ /
  Last update: 2015-03-12 22:01    [W:0.079 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site