Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755392AbbDTOR4 (ORCPT ); Mon, 20 Apr 2015 10:17:56 -0400 Received: from skprod3.natinst.com ([130.164.80.24]:45469 "EHLO ni.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754864AbbDTORw (ORCPT ); Mon, 20 Apr 2015 10:17:52 -0400 Date: Mon, 20 Apr 2015 09:17:19 -0500 From: Josh Cartwright To: "pi-cheng.chen" Cc: Viresh Kumar , Mike Turquette , Matthias Brugger , devicetree@vger.kernel.org, linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, fan.chen@mediatek.com, Howard Chen , linux-mediatek@lists.infradead.org, "Joe.C" , Eddie Huang , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/2] cpufreq: mediatek: Add MT8173 cpufreq driver Message-ID: <20150420141719.GF27115@jcartwri.amer.corp.natinst.com> References: <1429522047-16675-1-git-send-email-pi-cheng.chen@linaro.org> <1429522047-16675-2-git-send-email-pi-cheng.chen@linaro.org> MIME-Version: 1.0 In-Reply-To: <1429522047-16675-2-git-send-email-pi-cheng.chen@linaro.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-MIMETrack: Itemize by SMTP Server on US-AUS-MGWOut2/AUS/H/NIC(Release 8.5.3FP6|November 21, 2013) at 04/20/2015 09:17:19 AM, Serialize by Router on US-AUS-MGWOut2/AUS/H/NIC(Release 8.5.3FP6|November 21, 2013) at 04/20/2015 09:17:19 AM, Serialize complete at 04/20/2015 09:17:19 AM Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2015-04-20_02:,, signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7183 Lines: 211 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 > --- > 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 > > diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm > index 1b06fc4..25643c7 100644 > --- a/drivers/cpufreq/Kconfig.arm > +++ b/drivers/cpufreq/Kconfig.arm > @@ -132,6 +132,12 @@ config ARM_KIRKWOOD_CPUFREQ > This adds the CPUFreq driver for Marvell Kirkwood > SoCs. > > +config ARM_MT8173_CPUFREQ > + bool "Mediatek MT8173 CPUFreq support" > + depends on ARCH_MEDIATEK && REGULATOR I think you want to 'select REGULATOR' here; because REGULATOR isn't a user-visible option. > + help > + This adds the CPUFreq driver support for Mediatek MT8173 SoC. > + > config ARM_OMAP2PLUS_CPUFREQ > bool "TI OMAP2+" > depends on ARCH_OMAP2PLUS > diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile > index 82a1821..da9d616 100644 > --- a/drivers/cpufreq/Makefile > +++ b/drivers/cpufreq/Makefile > @@ -62,6 +62,7 @@ obj-$(CONFIG_ARM_HIGHBANK_CPUFREQ) += highbank-cpufreq.o > obj-$(CONFIG_ARM_IMX6Q_CPUFREQ) += imx6q-cpufreq.o > obj-$(CONFIG_ARM_INTEGRATOR) += integrator-cpufreq.o > obj-$(CONFIG_ARM_KIRKWOOD_CPUFREQ) += kirkwood-cpufreq.o > +obj-$(CONFIG_ARM_MT8173_CPUFREQ) += mt8173-cpufreq.o > obj-$(CONFIG_ARM_OMAP2PLUS_CPUFREQ) += omap-cpufreq.o > obj-$(CONFIG_ARM_PXA2xx_CPUFREQ) += pxa2xx-cpufreq.o > obj-$(CONFIG_PXA3xx) += pxa3xx-cpufreq.o > diff --git a/drivers/cpufreq/mt8173-cpufreq.c b/drivers/cpufreq/mt8173-cpufreq.c > new file mode 100644 > index 0000000..a310e72 > --- /dev/null > +++ b/drivers/cpufreq/mt8173-cpufreq.c > @@ -0,0 +1,509 @@ > +/* > +* Copyright (c) 2015 Linaro Ltd. > +* Author: Pi-Cheng Chen > +* > +* This program is free software; you can redistribute it and/or modify > +* it under the terms of the GNU General Public License version 2 as > +* published by the Free Software Foundation. > +* > +* This program is distributed in the hope that it will be useful, > +* but WITHOUT ANY WARRANTY; without even the implied warranty of > +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +* GNU General Public License for more details. > +*/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MIN_VOLT_SHIFT 100000 > +#define MAX_VOLT_SHIFT 200000 > + > +#define OPP(f, vp, vs) { \ > + .freq = f, \ > + .vproc = vp, \ > + .vsram = vs, \ > + } > + > +struct mtk_cpu_opp { > + unsigned int freq; > + int vproc; > + int vsram; > +}; > + > +/* > + * The struct cpu_dvfs_info holds necessary information for doing CPU DVFS of > + * each cluster. For Mediatek SoCs, each CPU cluster in SoC has two voltage > + * inputs, Vproc and Vsram. For some cluster in SoC, the two voltage inputs are > + * supplied by different PMICs. In this case, when scaling up/down the voltage > + * of Vsram and Vproc, the two voltage inputs need to be controlled under a > + * hardware limitation: 100mV < Vsram - Vproc < 200mV > + * When scaling up/down the clock frequency of a cluster, the clock source need > + * to be switched to another stable PLL clock temporarily, and switched back to > + * the original PLL after the it becomes stable at target frequency. > + * Hence the voltage inputs of cluster need to be set to an intermediate voltage > + * before the clock frequency being scaled up/down. > + */ > + > +struct cpu_dvfs_info { > + cpumask_t cpus; > + > + struct mtk_cpu_opp *opp_tbl; > + struct mtk_cpu_opp *intermediate_opp; > + int nr_opp; > + > + struct regulator *proc_reg; > + struct regulator *sram_reg; > + struct clk *cpu_clk; > + struct clk *inter_pll; > +}; > + > +/* > + * This is a temporary solution until we have new OPPv2 bindings. Therefore we > + * could describe the OPPs with (freq, volt, volt) tuple properly in device > + * tree. > + */ > + > +/* OPP table for LITTLE cores of MT8173 */ > +struct mtk_cpu_opp mt8173_l_opp[] = { static const? > + OPP(507000000, 859000, 0), > + OPP(702000000, 908000, 0), > + OPP(1001000000, 983000, 0), > + OPP(1105000000, 1009000, 0), > + OPP(1183000000, 1028000, 0), > + OPP(1404000000, 1083000, 0), > + OPP(1508000000, 1109000, 0), > + OPP(1573000000, 1125000, 0), > +}; > + > +/* OPP table for big cores of MT8173 */ > +struct mtk_cpu_opp mt8173_b_opp[] = { same here? > + OPP(507000000, 828000, 928000), > + OPP(702000000, 867000, 967000), > + OPP(1001000000, 927000, 1027000), > + OPP(1209000000, 968000, 1068000), > + OPP(1404000000, 1007000, 1107000), > + OPP(1612000000, 1049000, 1149000), > + OPP(1807000000, 1089000, 1150000), > + OPP(1989000000, 1125000, 1150000), > +}; > + [..] > +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; > + > + vproc = (new_vsram == vsram) ? > + new_vproc : vsram - MIN_VOLT_SHIFT; > + vproc = get_regulator_voltage_ceil(proc_reg, vproc); > + > + ret = regulator_set_voltage(proc_reg, vproc, vproc); > + if (ret) { > + regulator_set_voltage(sram_reg, old_vsram, old_vsram); > + return ret; > + } > + > + if (new_vproc == vproc && new_vsram == vsram) > + return 0; > + > + old_vproc = vproc; > + goto next_up_step; Perhaps a naive question: but, is this the correct place to do this? I would expect this stepping behavior to be implemented in the driver controlling the regulator you are consuming. It seems strange to do it here. Josh -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/