Messages in this thread |  | | Subject | Re: [PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP | From | Stephen Boyd <> | Date | Mon, 10 Feb 2020 14:38:37 -0800 |
| |
Quoting peng.fan@nxp.com (2020-02-04 05:34:34) > From: Peng Fan <peng.fan@nxp.com> > > Add a clk api for i.MX7ULP ARM core clk usage. > imx_hw_clk_cpu could not be reused, because i.MX7ULP ARM core > clk has totally different design. To simplify ARM core clk > change logic, add a new clk api. > > A draft picture to show the ARM core clock. > |-sirc > |-> run(500MHz) -> div -> mux -------------|-firc > ARM| | > |-> hsrun(720MHz) -> hs div -> hs mux -------|-spll pfd > |-.... > > Need to configure PMC when ARM core runs in HSRUN or RUN mode. > > RUN and HSRUN related registers are not same, but their > mux has same clocks as input. > > The API takes arm core, div, hs div, mux, hs mux, mux parent, pfd, step > as params for switch clk freq. > > When set rate, need to switch mux to take firc as input, then > set spll pfd freq, then switch back mux to spll pfd as parent. > > Per i.MX7ULP requirement, when clk runs in HSRUN mode, it could > only support arm core wfi idle, so add pm qos to support it. > > Signed-off-by: Peng Fan <peng.fan@nxp.com> > --- > drivers/clk/imx/Makefile | 1 + > drivers/clk/imx/clk-cpuv2.c | 137 ++++++++++++++++++++++++++++++++++++++++++++ > drivers/clk/imx/clk.h | 9 +++ > 3 files changed, 147 insertions(+) > create mode 100644 drivers/clk/imx/clk-cpuv2.c > > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile > index 928f874c73d2..9707fef8da98 100644 > --- a/drivers/clk/imx/Makefile > +++ b/drivers/clk/imx/Makefile > @@ -5,6 +5,7 @@ obj-$(CONFIG_MXC_CLK) += \ > clk-busy.o \ > clk-composite-8m.o \ > clk-cpu.o \ > + clk-cpuv2.o \ > clk-composite-7ulp.o \ > clk-divider-gate.o \ > clk-fixup-div.o \ > diff --git a/drivers/clk/imx/clk-cpuv2.c b/drivers/clk/imx/clk-cpuv2.c > new file mode 100644 > index 000000000000..a73d97a782aa > --- /dev/null > +++ b/drivers/clk/imx/clk-cpuv2.c > @@ -0,0 +1,137 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright 2020 NXP > + * > + * Peng Fan <peng.fan@nxp.com> > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <linux/slab.h> > +#include <linux/pm_qos.h> > +#include "clk.h" > + > +static struct pm_qos_request pm_qos_hsrun; > + > +#define MAX_NORMAL_RUN_FREQ 528000000 > + > +struct clk_cpu { > + struct clk_hw hw; > + struct clk_hw *core; > + struct clk_hw *div_nor; > + struct clk_hw *div_hs; > + struct clk_hw *mux_nor; > + struct clk_hw *mux_hs; > + struct clk_hw *mux_parent; > + struct clk_hw *pfd; > + struct clk_hw *step; > +}; > + > +static inline struct clk_cpu *to_clk_cpu(struct clk_hw *hw) > +{ > + return container_of(hw, struct clk_cpu, hw); > +} > + > +static unsigned long clk_cpu_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct clk_cpu *cpu = to_clk_cpu(hw); > + > + return clk_hw_get_rate(cpu->core); > +} > + > +static long clk_cpu_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *prate) > +{ > + return rate; > +} > + > +static int clk_cpu_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_cpu *cpu = to_clk_cpu(hw); > + int ret; > + struct clk_hw *div, *mux_now; > + unsigned long old_rate = clk_hw_get_rate(cpu->core); > + > + div = clk_hw_get_parent(cpu->core); > + > + if (div == cpu->div_nor) > + mux_now = cpu->mux_nor; > + else > + mux_now = cpu->mux_hs; > + > + ret = clk_hw_set_parent(mux_now, cpu->step); > + if (ret) > + return ret; > + > + ret = clk_set_rate(cpu->pfd->clk, rate); > + if (ret) { > + clk_hw_set_parent(mux_now, cpu->mux_parent); > + return ret; > + } > + > + if (rate > MAX_NORMAL_RUN_FREQ) { > + pm_qos_add_request(&pm_qos_hsrun, PM_QOS_CPU_DMA_LATENCY, 0); > + clk_hw_set_parent(cpu->mux_hs, cpu->mux_parent); > + clk_hw_set_parent(cpu->core, cpu->div_hs); > + } else { > + clk_hw_set_parent(cpu->mux_nor, cpu->mux_parent); > + clk_hw_set_parent(cpu->core, cpu->div_nor); > + if (old_rate > MAX_NORMAL_RUN_FREQ) > + pm_qos_remove_request(&pm_qos_hsrun); > + } > +
This is a cpufreq driver. Please write a cpufreq driver instead of trying to make "clk_set_rate()" conform to the requirements that cpufreq-dt mandates, which is that one clk exists and that clk rate change changes the frequency of the CPU.
If cpufreq-dt can work with the clk framework is up to the implementation of the hardware and the software. From what I see here, the clk framework is being subverted through the use of clk_hw_set_parent() and clk_set_rate() calls from within the framework to make the cpufreq-dt driver happy. Don't do that. Either write a proper clk driver for the clks that are there and have that manage the clk tree when clk_set_rate() is called, or write a cpufreq driver that controls various clks and adjusts their frequencies.
I assume there is a mux or something that eventually clks the CPU, so that can certainly be modeled as a clk with the parents set some way. That will make clk_set_rate() mostly work as long as you can hardcode a min/max value to change the parents, etc. Should work!
The use of pm_qos_add_request() is also pretty horrifying. We're deep in the clk framework implementation here and we're calling out to pm_qos. That shouldn't need to happen. If anything, we should do that from the core framework, but I don't know why we would. It's probably some sort of cpufreq thing.
|  |