From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933986AbbFJRdL (ORCPT ); Wed, 10 Jun 2015 13:33:11 -0400 Received: from mail-lb0-f180.google.com ([209.85.217.180]:36284 "EHLO mail-lb0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933847AbbFJRdH (ORCPT ); Wed, 10 Jun 2015 13:33:07 -0400 MIME-Version: 1.0 In-Reply-To: <1433867020-7746-3-git-send-email-lina.iyer@linaro.org> References: <1433867020-7746-1-git-send-email-lina.iyer@linaro.org> <1433867020-7746-3-git-send-email-lina.iyer@linaro.org> Date: Wed, 10 Jun 2015 10:33:06 -0700 Message-ID: Subject: Re: [PATCH RFC v2 2/2] hwspinlock: qcom: Lock #7 is special lock, uses dynamic proc_id From: Bjorn Andersson To: Lina Iyer Cc: Ohad Ben-Cohen , linux-arm-msm , "linux-kernel@vger.kernel.org" , Jeffrey Hugo , Bjorn Andersson , Andy Gross Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. 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. 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, ...>"? 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... Regards, Bjorn