| Message ID | 1548700381-22376-1-git-send-email-jorge.ramirez-ortiz@linaro.org |
|---|---|
| Headers | show |
| Series | Support CPU frequency scaling on QCS404 | expand |
On 1/30/19 05:41, Vinod Koul wrote: > On 28-01-19, 19:32, Jorge Ramirez-Ortiz wrote: >> Add a CPU OPP table to qcs404 >> >> Co-developed-by: Niklas Cassel <niklas.cassel@linaro.org> >> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> >> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/qcs404.dtsi | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi >> index 9b5c165..4594fea7 100644 >> --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi >> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi >> @@ -62,6 +62,21 @@ >> }; >> }; >> >> + cpu_opp_table: cpu_opp_table { > > This should be: > cpu_opp_table: cpu-opp-table yes. um I thought I fixed this ..sorry about it. > > IIRC node names are not supposed to have _ and tags not supposed to have > -, please compile with W=12 to trigger these warnings :) > >> + compatible = "operating-points-v2"; >> + opp-shared; >> + >> + opp-1094400000 { >> + opp-hz = /bits/ 64 <1094400000>; >> + }; >> + opp-1248000000 { >> + opp-hz = /bits/ 64 <1248000000>; >> + }; also need to remove the frequency below. I might have sent the wrong version of this file >> + opp-1401600000 { >> + opp-hz = /bits/ 64 <1401600000>; >> + }; >> + }; >> + >> firmware { >> scm: scm { >> compatible = "qcom,scm-qcs404", "qcom,scm"; >> -- >> 2.7.4 >
Quoting Jorge Ramirez-Ortiz (2019-01-28 10:32:54) > Make the output of the high frequency pll a clock provider. > On the QCS404 this PLL controls cpu frequency scaling. > > Co-developed-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> > --- Acked-by: Stephen Boyd <sboyd@kernel.org>
Quoting Jorge Ramirez-Ortiz (2019-01-28 10:32:48) > Limit the GPLL0_AO_OUT_MAIN operating frequency as per its hardware > specifications. > > Co-developed-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> Acked-by: Stephen Boyd <sboyd@kernel.org> > diff --git a/drivers/clk/qcom/gcc-qcs404.c b/drivers/clk/qcom/gcc-qcs404.c > index 64da032..7de4fcf 100644 > --- a/drivers/clk/qcom/gcc-qcs404.c > +++ b/drivers/clk/qcom/gcc-qcs404.c > @@ -304,6 +304,7 @@ static struct clk_alpha_pll gpll0_out_main = { > }, > }; > > + Please remove this stray hunk though. > static struct clk_alpha_pll gpll0_ao_out_main = { > .offset = 0x21000, > .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_DEFAULT],
Quoting Jorge Ramirez-Ortiz (2019-01-28 10:32:55) > When COMMON_CLK_DISABLED_UNUSED is set, in an effort to save power and > to keep the software model of the clock in line with reality, the > framework transverses the clock tree and disables those clocks that > were enabled by the firmware but have not been enabled by any device > driver. > > If CPUFREQ is enabled, early during the system boot, it might attempt > to change the CPU frequency ("set_rate"). If the HFPLL is selected as > a provider, it will then change the rate for this clock. > > As boot continues, clk_disable_unused_subtree will run. Since it wont > find a valid counter (enable_count) for a clock that is actually > enabled it will attempt to disable it which will cause the CPU to > stop. Notice that in this driver, calls to check whether the clock is > enabled are routed via the is_enabled callback which queries the > hardware. > > The following commit, rather than marking the clock critical and > forcing the clock to be always enabled, addresses the above scenario > making sure the clock is not disabled but it continues to rely on the > firmware to enable the clock. > > Co-developed-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> > --- > drivers/clk/qcom/hfpll.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/clk/qcom/hfpll.c b/drivers/clk/qcom/hfpll.c > index 0ffed0d..9d92f5d 100644 > --- a/drivers/clk/qcom/hfpll.c > +++ b/drivers/clk/qcom/hfpll.c > @@ -58,6 +58,7 @@ static int qcom_hfpll_probe(struct platform_device *pdev) > .parent_names = (const char *[]){ "xo" }, > .num_parents = 1, > .ops = &clk_ops_hfpll, > + .flags = CLK_IGNORE_UNUSED, Please put some sort of similar comment in the code so we can find it without git history digging.
The following patchset enables CPU frequency scaling support on the QCS404. Patch 8 "clk: qcom: hfpll: CLK_IGNORE_UNUSED" is a bit controversial; in this platform, this PLL provides the clock signal to a CPU core. But in others it might not. I opted for the minimal ammount of changes without affecting the default functionality: simply bypassing the COMMON_CLK_DISABLE_UNUSED framework and letting the firwmare chose whether to enable or disable the clock at boot. However maybe a DT property and marking the clock as critical would be more appropriate for this PLL. I'd appreciate the maintainer's input on this topic. v2: - dts: ms8916: apcs mux/divider: new bindings (the driver can still support the old bindings) - qcs404.dtsi fix apcs-hfpll definition fix cpu_opp_table definition - GPLL0_AO_OUT operating frequency define new alpha_pll_fixed_ops to limit the operating frequency Co-developed-by: Niklas Cassel <niklas.cassel@linaro.org> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org> Signed-off-by: Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org> Jorge Ramirez-Ortiz (14): clk: qcom: gcc: limit GPLL0_AO_OUT operating frequency mbox: qcom: add APCS child device for QCS404 mbox: qcom: replace integer with valid macro dt-bindings: mailbox: qcom: Add clock-name optional property clk: qcom: apcs-msm8916: get parent clock names from DT clk: qcom: hfpll: get parent clock names from DT clk: qcom: hfpll: register as clock provider clk: qcom: hfpll: CLK_IGNORE_UNUSED arm64: dts: qcom: msm8916: Add the clocks for the APCS mux/divider arm64: dts: qcom: qcs404: Add OPP table arm64: dts: qcom: qcs404: Add HFPLL node arm64: dts: qcom: qcs404: Add the clocks for APCS mux/divider arm64: dts: qcom: qcs404: Add cpufreq support arm64: defconfig: Enable HFPLL .../bindings/mailbox/qcom,apcs-kpss-global.txt | 24 +++++++++++++-- arch/arm64/boot/dts/qcom/msm8916.dtsi | 3 +- arch/arm64/boot/dts/qcom/qcs404.dtsi | 35 ++++++++++++++++++++++ arch/arm64/configs/defconfig | 1 + drivers/clk/qcom/apcs-msm8916.c | 32 +++++++++++++++----- drivers/clk/qcom/clk-alpha-pll.c | 8 +++++ drivers/clk/qcom/clk-alpha-pll.h | 1 + drivers/clk/qcom/gcc-qcs404.c | 3 +- drivers/clk/qcom/hfpll.c | 19 +++++++++++- drivers/mailbox/qcom-apcs-ipc-mailbox.c | 21 ++++++++----- 10 files changed, 125 insertions(+), 22 deletions(-) -- 2.7.4