LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Pi-Cheng Chen <pi-cheng.chen@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
	Mike Turquette <mturquette@linaro.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	"Joe.C" <yingjoe.chen@mediatek.com>,
	Eddie Huang <eddie.huang@mediatek.com>,
	Howard Chen <ibanezchen@gmail.com>,
	Chen Fan <fan.chen@mediatek.com>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	Linaro Kernel Mailman List <linaro-kernel@lists.linaro.org>,
	linux-mediatek@lists.infradead.org
Subject: Re: [PATCH 1/2] cpufreq: mediatek: Add MT8173 cpufreq driver
Date: Fri, 24 Apr 2015 14:55:21 +0200	[thread overview]
Message-ID: <20150424125521.GU6325@pengutronix.de> (raw)
In-Reply-To: <CALx668VsT60+kx7o-ZM5Wxk7Nn-rXS21O5SDYr++EeY-zEJ6ew@mail.gmail.com>

On Fri, Apr 24, 2015 at 02:46:25PM +0800, Pi-Cheng Chen wrote:
> Hi Sascha,
> 
> Thanks for reviewing.
> 
> On Thu, Apr 23, 2015 at 8:01 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Apr 20, 2015 at 05:27:26PM +0800, pi-cheng.chen wrote:
> >> This patch implements MT8173 specific cpufreq driver with OPP table defined
> >> in the driver code.
> >>
> >> Signed-off-by: pi-cheng.chen <pi-cheng.chen@linaro.org>
> >> ---
> >>  drivers/cpufreq/Kconfig.arm      |   6 +
> >>  drivers/cpufreq/Makefile         |   1 +
> >>  drivers/cpufreq/mt8173-cpufreq.c | 509 +++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 516 insertions(+)
> >>  create mode 100644 drivers/cpufreq/mt8173-cpufreq.c
> >>
> >> +static int mtk_cpufreq_voltage_trace(struct cpu_dvfs_info *info,
> >> +                                  struct mtk_cpu_opp *opp)
> >> +{
> >> +     struct regulator *proc_reg = info->proc_reg;
> >> +     struct regulator *sram_reg = info->sram_reg;
> >> +     int old_vproc, new_vproc, old_vsram, new_vsram, vsram, vproc, ret;
> >> +
> >> +     old_vproc = regulator_get_voltage(proc_reg);
> >> +     old_vsram = regulator_get_voltage(sram_reg);
> >> +
> >> +     new_vproc = opp->vproc;
> >> +     new_vsram = opp->vsram;
> >> +
> >> +     /*
> >> +      * In the case the voltage is going to be scaled up, Vsram and Vproc
> >> +      * need to be scaled up step by step. In each step, Vsram needs to be
> >> +      * set to (Vproc + 200mV) first, then Vproc is set to (Vsram - 100mV).
> >> +      * Repeat the step until Vsram and Vproc are set to target voltage.
> >> +      */
> >> +     if (old_vproc < new_vproc) {
> >> +next_up_step:
> >> +             old_vsram = regulator_get_voltage(sram_reg);
> >> +
> >> +             vsram = (new_vsram - old_vproc < MAX_VOLT_SHIFT) ?
> >> +                     new_vsram : old_vproc + MAX_VOLT_SHIFT;
> >> +             vsram = get_regulator_voltage_floor(sram_reg, vsram);
> >> +
> >> +             ret = regulator_set_voltage(sram_reg, vsram, vsram);
> >> +             if (ret)
> >> +                     return ret;
> >
> > This introspecting the regulators for possible voltages looks hacky and
> > unnecessary. regulator_set_voltage() can be passed minimum and maximum
> > values, why don't you use it to increase the voltage within sensible
> > limit, like
> >
> >         regulator_set_voltage(sram_reg, old_vsram + 100000, old_vsram + 200000);
> >
> > or similar?
> 
> I am sorry I don't understand how could I do it. Would you elaborate?

You try to set the OPPs to the exact voltages, then next use functions
to determine the next exact higher voltage and set the regulator voltage
to an exact value. Instead of doing this you can let the ability to
specify a voltage range work for you, something like:

	int tolerance = 50000;

	while (vproc < new_vproc) {
		int next = min(new_vproc - vproc, 200000);
		int next_sram = next + 100000;

		regulator_set_voltage(sram_reg, next_sram - tolerance, next_sram + tolerance);
		regulator_set_voltage(vproc_reg, next - tolerance, next + tolerance);
		vproc = regulator_get_voltage(vproc_reg);
	}

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2015-04-24 12:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-20  9:27 [PATCH v3 0/2] cpufreq: add cpufreq driver for Mediatek MT8173 SoC pi-cheng.chen
2015-04-20  9:27 ` [PATCH 1/2] cpufreq: mediatek: Add MT8173 cpufreq driver pi-cheng.chen
2015-04-20 14:17   ` Josh Cartwright
2015-04-20 18:31     ` Paul Bolle
2015-04-22  3:11     ` Pi-Cheng Chen
2015-04-22 14:33       ` Josh Cartwright
2015-04-20 18:28   ` Paul Bolle
2015-04-22  3:14     ` Pi-Cheng Chen
2015-04-23 12:01   ` Sascha Hauer
2015-04-24  6:46     ` Pi-Cheng Chen
2015-04-24 12:55       ` Sascha Hauer [this message]
2015-05-07  9:42         ` Pi-Cheng Chen
2015-04-23 12:56   ` Mark Rutland
2015-04-24  6:50     ` Pi-Cheng Chen
2015-04-29  6:06       ` Viresh Kumar
2015-04-30  7:42   ` Sascha Hauer
2015-05-07  9:40     ` Pi-Cheng Chen
2015-04-20  9:27 ` [PATCH 2/2] ARM64: mt8173: dts Add MT8173 cpufreq driver support pi-cheng.chen
2015-04-23 12:53   ` Mark Rutland
2015-04-24  7:09     ` Pi-Cheng Chen
2015-05-18 13:29       ` Pi-Cheng Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150424125521.GU6325@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=devicetree@vger.kernel.org \
    --cc=eddie.huang@mediatek.com \
    --cc=fan.chen@mediatek.com \
    --cc=ibanezchen@gmail.com \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=matthias.bgg@gmail.com \
    --cc=mturquette@linaro.org \
    --cc=pi-cheng.chen@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=yingjoe.chen@mediatek.com \
    --subject='Re: [PATCH 1/2] cpufreq: mediatek: Add MT8173 cpufreq driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).