Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 8BIT
To: Mark Brown <broonie@kernel.org>,
        "Georgi Djakov" <georgi.djakov@linaro.org>
From: Stephen Boyd <stephen.boyd@linaro.org>
In-Reply-To: <20160330173658.GN2350@sirena.org.uk>
Cc: linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
References: <1459347792-13515-1-git-send-email-georgi.djakov@linaro.org>
 <20160330173658.GN2350@sirena.org.uk>
Message-ID: <20160330181709.5175.51228@sboyd-linaro>
User-Agent: alot/0.3.7
Subject: Re: [PATCH] regulator: Support ramp-up delay for drivers with get_voltage()
Date: Wed, 30 Mar 2016 11:17:09 -0700
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1310
Lines: 27

Quoting Mark Brown (2016-03-30 10:36:58)
> On Wed, Mar 30, 2016 at 05:23:12PM +0300, Georgi Djakov wrote:
> 
> > +     } else if (_regulator_is_enabled(rdev) &&
> > +                rdev->desc->ops->set_voltage_time_sel &&
> > +                rdev->desc->ops->get_voltage) {
> > +             int uV = rdev->desc->ops->get_voltage(rdev);
> > +
> > +             if (uV > 0) {
> > +                     old_selector = regulator_map_voltage(rdev, uV, uV);
> > +                     if (old_selector < 0)
> > +                             return old_selector;
> > +             }
> 
> If a driver is using selectors it should use selectors uninformly, it
> should not mix and match selector and raw voltage interfaces.  If we
> the set and get operations are not symmetric I'd expect we're going to
> run into problems sooner rather than later.

This is for the qcom spmi regulator driver. I seem to have put in the
set_voltage_time_sel op but missed the fact that the regulator core
wasn't calling that op to find out how much time to delay. So we have
raw voltage set and get ops and this selector based delay op.

Do we need to change the ops to be selector based if we want the
regulator core to delay after changing voltages? Or do we need to put
the delay directly into the set_voltage() op in the driver?