From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753420AbbHMPZr (ORCPT ); Thu, 13 Aug 2015 11:25:47 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:40085 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752012AbbHMPZL (ORCPT ); Thu, 13 Aug 2015 11:25:11 -0400 Date: Thu, 13 Aug 2015 10:25:08 -0500 From: Andy Gross To: Ohad Ben-Cohen Cc: Lina Iyer , Bjorn Andersson , Jeffrey Hugo , "linux-arm-msm@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH RFC v2 0/2] hwspinlock: Introduce raw capability for hwspinlock_device Message-ID: <20150813152508.GA4627@qualcomm.com> References: <1433867020-7746-1-git-send-email-lina.iyer@linaro.org> <20150627030514.GA893@linaro.org> <20150702203028.GA4711@linaro.org> <20150728215122.GE51847@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 13, 2015 at 09:34:13AM +0300, Ohad Ben-Cohen wrote: > On Wed, Jul 29, 2015 at 12:51 AM, Lina Iyer wrote: > >> Let's not make this more complicated than needed, so please add the > >> hwcaps member to hwspinlock_device instead of to hwspinlock struct. We > >> could always change this later if it proves to be insufficient. > >> > > But this could yield wrong locking scenarios. If banks are allowed RAW > > capability and is not enforced on a per-lock basis, a driver may lock > > using non-raw lock using the _raw API, while another driver may > > 'acquire' the lock (since the value written to the lock would be the > > same as raw api would). That is why you should have the capability on > > hwspinlock and not on hwspinlock_device. Locks that are defined are RAW > > capable should be used as RAW only. > > > > QCOM platform hwlock #7 is unique that different CPUs trying to acquire > > the lock would write different values and hence would be fine. But, the > > same is not true for other locks in the bank. > > As far as I understand, there is nothing special about QCOM's hwlock > #7 in terms of hardware. It's exactly the same lock as all the others. > > The only difference in hwlock #7 is the way you use it, and that > sounds like a decision the driver should be able to make. It's a > policy, and I'm not sure we should put it in the DT. I'm also not sure > we need this hwlock-specific complexity in the hwspinlock framework. The issue in hardwiring this into the driver itself means forfeiting extensibility. So on one side (w/ raw support), we get the ability to deal with the lock number changing. On the other side (w/o raw), we'd have to probably tie this to chip compat to figure out which lock is the 'special' if it ever changes. > > The driver already makes a decision whether to disable the interrupts > or not and whether to save their state or not. So it can also make a > decision whether to take a sw spinlock at all or not --- if the > hardware allows it. and that if should be encoded in an accessible > vendor specific (not hwlock specific) struct, which is setup by the > underlying vendor specific hwspinlock driver (no DT involved). It's arbitrary right now. The remote processor selected a number, not the processor running Linux. > > Let's go over your aforementioned concerns: > > But this could yield wrong locking scenarios. If banks are allowed RAW > > capability and is not enforced on a per-lock basis, a driver may lock > > using non-raw lock using the _raw API > > If this is allowed by the hardware, then this is a valid scenario. > There's no such thing a non-raw lock: a lock is raw if a raw > functionality is required. > > > while another driver may > > 'acquire' the lock (since the value written to the lock would be the > > same as raw api would). > > Not sure I understand this one. If a lock has already been assigned to > a driver, it cannot be re-assigned to another driver. > -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project