From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932687AbaICQe6 (ORCPT ); Wed, 3 Sep 2014 12:34:58 -0400 Received: from seldrel01.sonyericsson.com ([212.209.106.2]:2792 "EHLO seldrel01.sonyericsson.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751989AbaICQe4 convert rfc822-to-8bit (ORCPT ); Wed, 3 Sep 2014 12:34:56 -0400 Date: Wed, 3 Sep 2014 09:34:55 -0700 From: Bjorn Andersson To: Kumar Gala CC: Ohad Ben-Cohen , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Grant Likely , Suman Anna , "open list:OPEN FIRMWARE AND..." , "linux-kernel@vger.kernel.org" , linux-arm-msm , Jeffrey Hugo , Eric Holmberg , "Cavin, Courtney" Subject: Re: [PATCH v3] hwspinlock: qcom: Add support for Qualcomm HW Mutex block Message-ID: <20140903163448.GB16352@sonymobile.com> References: <1409688246-16530-1-git-send-email-bjorn.andersson@sonymobile.com> <2B6A9AA9-E69F-47DF-A1E7-5C95B4E6FA94@codeaurora.org> <20140903145508.GA16352@sonymobile.com> <5B5C3B14-0FC8-4B4B-AE33-69030B7863D8@codeaurora.org> MIME-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Disposition: inline Content-Transfer-Encoding: 8BIT In-Reply-To: <5B5C3B14-0FC8-4B4B-AE33-69030B7863D8@codeaurora.org> User-Agent: Mutt/1.5.22 (2013-10-16) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed 03 Sep 08:22 PDT 2014, Kumar Gala wrote: > > On Sep 3, 2014, at 9:55 AM, Bjorn Andersson wrote: > > > On Wed 03 Sep 05:49 PDT 2014, Kumar Gala wrote: > > > >> > >> On Sep 2, 2014, at 3:04 PM, Bjorn Andersson wrote: > >> > >>> Changes since v2: > >>> - MODULE_DEVICE_TABLE > >>> - Changed prefix to qcom > >>> - Cleaned up includes > >>> - Rely on reg and num-locks to figure out stride, instead of of_match data > >> > >> I know Jeff prefers this method of computing stride, but I’m not a fan as > >> there isn’t a reason one could adjust qcom,num-locks in the dt for some > >> reason and leave regs alone. > >> > > > > All the current platform it's 32 consecutive mutexes with either 4 or 128 byte > > stride, so encoding it as data either way works fine. The hardware you're > > trying to describe with your dt is the addresses that spans your mutex > > registers and how many there are. So from the HW/dts pov I don't see why you > > would like to do this. > > > > Then looking in the caf code, there is a limit of max 8 mutexes. So apparently > > there is some sort of usecase, I just don't know what or if it's valid from a > > dt pov. > > I believe not all the mutexes are meant for the cores running linux. > However, I think we just expect linux to play nice and not touch anything it > isn’t using explicitly. > I would expect so too. One problem I see is that it's not very robust relying on the relationship between reg and num-locks. I consider this an implementation detail leaking into the dt binding and it's not described enough currently... > > Going to that future awesome SoCs where it's still called tcsr-mutex, but with > > a stride of 4096 bytes makes me wonder; is that really a consecutive 128kb with > > nothing else in-between that we can ioremap? > > think 64-bit machines with more address space to burn and wanting to separate > resources to use MMUs for protection. > Makes sense, I just don't have any documentation verifying this. > > I.e. can we really reuse this driver straight off for that SoC? > > I dont see why not. > As long as the space inbetween is just wasted, there is no issue. > >>> diff --git a/Documentation/devicetree/bindings/hwlock/qcom-hwspinlock.txt b/Documentation/devicetree/bindings/hwlock/qcom-hwspinlock.txt > >>> +- compatible: > >>> + Usage: required > >>> + Value type: > >>> + Definition: must be one of: > >>> + "qcom,sfpb-mutex", > >>> + "qcom,tcsr-mutex” > >> > >> I dont get the purpose of having different compatible strings if there is no > >> difference in the code between them. > >> > > > > The semantics are the same, but there are no mutex registers in the tcsr block > > in e.g 8960, so the name is just missleading. I assume that's why you didn't > > follow caf and used the compatible "sfpb" in the first place? > > What do you expect the 8960 dt node to look like? I’m not 100% against ‘sfpb’ > > I’m feel like we we should use compat for stride, so we’d end up with something like: > > qcom,sfpb-mutex: stride 4 bytes, base: 0x01200604, reset: 0x01200600 > qcom,tcsr-mutex: stride 128 bytes, base: 0xFD484000, reset: 0xFD485380 > qcom,tcsr-4k-mutex: stride 4k bytes, base: 0x740000, reset: 0x767000 > Maybe these are the best names for the 3 hw blocks after all. The alternative would be either to encode the platform name in the compatible or adding the stride as a separate property. The first is waste and the second doesn't describe how hw is connected. So netiher are good alternatives. I think your suggestion is reasonable and will move the stride back into compat. It's the most robust solution. Is the 4k block finalized (don't see it in 8994 docs)? Should I add it to the driver now as well? Thanks for your input! Regards, Bjorn