LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Pi-Cheng Chen <pi-cheng.chen@linaro.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
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: Thu, 7 May 2015 17:42:44 +0800	[thread overview]
Message-ID: <CALx668V4Ri_7cEefY8pZW7YkGcnf1JU-VD1bka0Rzc1xjC9dqw@mail.gmail.com> (raw)
In-Reply-To: <20150424125521.GU6325@pengutronix.de>

On Fri, Apr 24, 2015 at 8:55 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 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);
>         }

Thanks for your explanation.
I'll try it to get rid of those functions to find out the exact voltages.

Best Regards,
Pi-Cheng

>
> 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-05-07  9:42 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
2015-05-07  9:42         ` Pi-Cheng Chen [this message]
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=CALx668V4Ri_7cEefY8pZW7YkGcnf1JU-VD1bka0Rzc1xjC9dqw@mail.gmail.com \
    --to=pi-cheng.chen@linaro.org \
    --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=s.hauer@pengutronix.de \
    --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).