Messages in this thread | | | Date | Thu, 12 Mar 2015 14:48:59 -0600 | From | Lina Iyer <> | Subject | Re: [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 >
| |