From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933873AbbFJUNb (ORCPT ); Wed, 10 Jun 2015 16:13:31 -0400 Received: from mail-pd0-f173.google.com ([209.85.192.173]:34425 "EHLO mail-pd0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751688AbbFJUNW (ORCPT ); Wed, 10 Jun 2015 16:13:22 -0400 Date: Wed, 10 Jun 2015 14:13:19 -0600 From: Lina Iyer To: Bjorn Andersson Cc: Ohad Ben-Cohen , linux-arm-msm , "linux-kernel@vger.kernel.org" , Jeffrey Hugo , Bjorn Andersson , Andy Gross Subject: Re: [PATCH RFC v2 2/2] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id Message-ID: <20150610201319.GA7715@linaro.org> References: <1433867020-7746-1-git-send-email-lina.iyer@linaro.org> <1433867020-7746-3-git-send-email-lina.iyer@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 10 2015 at 11:33 -0600, Bjorn Andersson wrote: >On Tue, Jun 9, 2015 at 9:23 AM, Lina Iyer 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