Re: [PATCH 1/2] cpufreq: mediatek: Add MT8173 cpufreq driver

[Pi-Cheng Chen]

On Fri, Apr 24, 2015 at 8:55 PM, Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxx>
>> >> ---
>> >>  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 |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/