From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Boyd Subject: Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators Date: Wed, 10 Feb 2016 14:46:28 -0800 Message-ID: <20160210224628.GK30978@codeaurora.org> References: <1455023549-30836-1-git-send-email-georgi.djakov@linaro.org> <20160209222154.GB1646@linaro.org> <56BB32A9.7090002@linaro.org> <20160210183651.GH30978@codeaurora.org> <20160210184327.GB13270@sirena.org.uk> <20160210190436.GI30978@codeaurora.org> <20160210192101.GE13270@sirena.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from smtp.codeaurora.org ([198.145.29.96]:50899 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751438AbcBJWqa (ORCPT ); Wed, 10 Feb 2016 17:46:30 -0500 Content-Disposition: inline In-Reply-To: <20160210192101.GE13270@sirena.org.uk> Sender: linux-arm-msm-owner@vger.kernel.org List-Id: linux-arm-msm@vger.kernel.org To: Mark Brown Cc: Georgi Djakov , Lina Iyer , lgirdwood@gmail.com, andy.gross@linaro.org, linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org On 02/10, Mark Brown wrote: > On Wed, Feb 10, 2016 at 11:04:36AM -0800, Stephen Boyd wrote: > > > I don't follow the rest of your mail though. Are you suggesting > > that in this case we put the regulator control into the PMIC > > regulator driver (qcom_spmi-regulator.c) and then use a > > syscon/regmap there to change the voltages inside the MMIO bus? > > That may work but we're going to need to update the binding for > > the SPMI regulator driver then. > > No, why would you want to do that? I'm saying that if the device has a > SPMI interface make that the primary interface for the driver. > Presumably the SPMI bus abstraction is capable of representing this > fairly directly. > > > I'm not really excited about the binding we have here either. > > We're going to have two places in DT where we've created a > > regulator for the same physical regulator. One will be a child of > > the SAW node on the MMIO bus, and another will be a child of the > > PMIC on the SPMI/SSBI bus. In the end, they're both the same > > regulator, so any constraints on one node will need to be applied > > to the other node as well. > > Are you saying that this is a problem with the driver that just got > merged? We got to v4 before I applied the driver... My understanding > was that this is a driver for a new regulator type not a duplicate > interface for existing regulator. Yeah I haven't had the time to properly review this patch. From what I can tell though, if this driver probes first and then the spm driver probes second, the CPU could go idle and then wakeup with a lower voltage than is required. The hardware works like this: there's a regulator on the PMIC that's dedicated to the CPU. Let's call this regulator S1. The PMIC is on the SPMI bus. There's an SPMI master controller inside the SoC. This controller can be used to perform SPMI transactions when software does MMIO read/writes or it can be perform transactions on the behalf of some hardware entity. The controller is called the "PMIC arbiter" because it's arbitrating access to the PMIC between all the sw entities (linux, modem, dsp, etc.) and the hw entities (various SAWs in the system). Typical control of an SPMI regulator can be seen in the qcom_spmi-regulator.c driver. That's a standard SPMI bus regulator driver. The SAWs are "hardwired" to a PMIC regulator based on some silicon configuration. In this example, the SAW would be configured to send commands to the PMIC for the S1 regulator. The SAW hardware will basically takes whatever voltage is written to it in the MMIO space and tacks on an address for S1 before handing that over to the PMIC arbiter to send it to the PMIC. One way to think of this is that each SAW has its own SPMI regulator driver for the one regulator it cares about, except we have to format the payload in the same way that the PMIC would expect it. Now this is all pretty much useless hardware if all we care about is to set voltages from software. Obviously we could just use the SPMI regulator driver that we already have. It's written for the bus that the hardware is actually on and it works! The problem there is that the SAW register is also used during idle/suspend when software isn't running. The CPU power down sequence usually turns off the regulator, but it may also change the voltage to something lower, depending on how deep of idle/suspend we're trying to achieve. This is all done without software intervention. When the CPU wakes up due to some interrupt, the SPM runs the CPU power on sequence which takes whatever is in the SAW register and sends it off to the PMIC to restore the CPU voltage. Again, this is all hardware doing this. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project