Re: [PATCH RFC v2 2/2] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jun 10 2015 at 11:33 -0600, Bjorn Andersson wrote:
On Tue, Jun 9, 2015 at 9:23 AM, Lina Iyer <lina.iyer@xxxxxxxxxx> wrote:
Hwspinlocks are widely used between processors in an SoC, and also
between elevation levels within in the same processor.  QCOM SoC's use
hwspinlock to serialize entry into a low power mode when the context
switches from Linux to secure monitor.

Lock #7 has been assigned for this purpose. In order to differentiate
between one cpu core holding a lock while another cpu is contending for
the same lock, the proc id written into the lock is (128 + cpu id). This
makes it unique value among the cpu cores and therefore when a core
locks the hwspinlock, other cores would wait for the lock to be released
since they would have a different proc id.  This value is specific for
the lock #7 only.

Declare lock #7 as raw capable, so the hwspinlock framework would not
enfore acquiring a s/w spinlock before acquiring the hwspinlock.


Hi Lina,

Very sorry for slacking off and missing v1 of this.

No worries. Thanks for reviewing.

I'm puzzled to the concept of using the hwspinlock framework for
lock-only locks. The patch your proposed is rather clean and as long
as there's no lock-debugging added to the framework it would work...


Blindly declaring lock #7 as special on all Qualcomm hwspinlocks I do
however not like at all. There's nothing in either the SFPB nor TCSR
mutex hardware that dictates this fact, it's a system configuration
fact. As such this "requirement" should be described in the device
tree.

Its not a mutable entity, but sure.

The puzzling part of the value to be written is strongly cpuidle
implementation defined makes me wonder if it belong in this driver at
all.

At least this should be configured/flagged by some devicetree
property. "qcom,lock-by-cpu-id-locks = <7, ...>"?

Okay.


The other alternative to these patches would be to just consume the
syscon in cpuidle and opencode the locking there. It isolates the
cpuidle specifics of this to the original place and it isn't using
only one side of the hwspinlock framework...

Well, ultimately a hwspinlock is just a writel, so that is a
possibility, if we want. But it is a hwspinlock, therefore the use of
the framework seems appropriate, even amidst the unique behavior of the
lock.

Thanks,
Lina


--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux