Re: [PATCH v4] regulator: qcom-saw: Add support for SAW regulators

[Stephen Boyd <sboyd@codeaurora.org>]

On 02/11, Georgi Djakov wrote:
> On 02/11/2016 12:46 AM, Stephen Boyd wrote:
> > On 02/10, Mark Brown wrote:
> >> On Wed, Feb 10, 2016 at 11:04:36AM -0800, Stephen Boyd wrote:
> >>
> [..]
> >>> 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.
> > 
> [..]
> > 
> > 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!
> > 
> 
> 8064 uses SSBI instead of SPMI and we currently do not have any
> existing regulator support upstream yet. So this driver is not
> duplicating any existing regulator. We should decide whether to
> keep this driver or to replace it with a new ssbi-regulator driver
> and bindings instead, where we can avoid the split-bus fun at
> least to some extent. Maybe the latter is the better option?

Yes I think having an ssbi/spmi regulator driver may be a better
approach. The SAW code can monitor the regulator for voltage
changes with a notifier and then stick the restore voltage into
the SAW registers. There's only one sticking point below.

> 
> > 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.
> 
> This might be a problem. I guess we can't change this hardware
> behaviour?

Right, we can't change how this hardware works. The way this SAW
regulator driver is written works for this problem though. It
modifies the SAW registers to set the voltage on the CPU that is
using the regulator, thereby preventing the CPU from going idle
or hitting suspend when the voltage is changed. If we were to use
the SSBI/SPMI regulator driver we would need to do something
similar so that the SPM is guaranteed to not be running during
the voltage switch. So I guess schedule a work on the CPU that's
affected by the voltage switch and hope that the CPU doesn't go
offline during that time?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project