From: Peng Fan <[email protected]>
This patchset aims to use cpufreq-dt for i.MX7ULP to avoid
plaform specific cpufreq driver. To use cpufreq-dt, we need
a ARM core clock that could be easy to support freq change.
However i.MX7ULP has some specific design that we could
reuse imx_hw_clk_cpu that used on i.MX7D/8M. So
introduced a new api imx_hw_clk_cpuv2 to add a virtual clk
that could support ARM core freq change easily.
Patch 1,2 is to change pfdv2 to make it could determine
best parent clk, then we could directly configure pfdv2
to get the best clk per i.MX7ULP datasheet "6.2.4 PLL PFD output"
I have tested with following diff applied, and mark fsl,imx7ulp as
blacklist in cpufreq-dt driver(I also send out when this patchset
is ok)
diff --git a/arch/arm/boot/dts/imx7ulp.dtsi b/arch/arm/boot/dts/imx7ulp.dtsi
index ab91c98f2124..11085b06506e 100644
--- a/arch/arm/boot/dts/imx7ulp.dtsi
+++ b/arch/arm/boot/dts/imx7ulp.dtsi
@@ -41,9 +41,29 @@
compatible = "arm,cortex-a7";
device_type = "cpu";
reg = <0xf00>;
+ clocks = <&smc1 IMX7ULP_CLK_ARM_FREQ>;
+ clock-frequency = <500210000>;
+ operating-points-v2 = <&cpu0_opp_table>;
};
};
+ cpu0_opp_table: opp-table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-500210000 {
+ opp-hz = /bits/ 64 <500210000>;
+ /*opp-microvolt = <1025000>;*/
+ clock-latency-ns = <150000>;
+ };
+
+ opp-720000000 {
+ opp-hz = /bits/ 64 <720000000>;
+ /*opp-microvolt = <1125000>;*/
+ clock-latency-ns = <150000>;
+ };
+ };
+
I not include the voltage configuration, because imx-rpmsg
and pf1550 rpmsg driver still not upstreamed.
So I not included dts in this patchset, when imx-rpmsg and pf1550
ready, the dts part could be added then.
Anson Huang (1):
clk: imx: Fix division by zero warning on pfdv2
Peng Fan (6):
clk: imx: pfdv2: switch to use determine_rate
clk: imx: pfdv2: determine best parent rate
clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP
clk: imx: imx7ulp: add IMX7ULP_CLK_ARM_FREQ clk
ARM: imx: imx7ulp: support HSRUN mode
ARM: imx: imx7ulp: create cpufreq device
arch/arm/mach-imx/mach-imx7ulp.c | 2 +
arch/arm/mach-imx/pm-imx7ulp.c | 4 +
drivers/clk/imx/Makefile | 1 +
drivers/clk/imx/clk-cpuv2.c | 137 ++++++++++++++++++++++++++++++
drivers/clk/imx/clk-imx7ulp.c | 15 +++-
drivers/clk/imx/clk-pfdv2.c | 61 +++++++++----
drivers/clk/imx/clk.h | 9 ++
include/dt-bindings/clock/imx7ulp-clock.h | 3 +-
8 files changed, 212 insertions(+), 20 deletions(-)
create mode 100644 drivers/clk/imx/clk-cpuv2.c
--
2.16.4
From: Peng Fan <[email protected]>
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 <[email protected]>
---
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 <[email protected]>
+ */
+
+#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);
+ }
+
+ return 0;
+}
+
+static const struct clk_ops clk_cpu_ops = {
+ .recalc_rate = clk_cpu_recalc_rate,
+ .round_rate = clk_cpu_round_rate,
+ .set_rate = clk_cpu_set_rate,
+};
+
+struct clk_hw *imx_clk_hw_cpuv2(const char *name, const char *parent_names,
+ 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,
+ unsigned long flags,
+ unsigned long plat_flags)
+{
+ struct clk_cpu *cpu;
+ struct clk_hw *hw;
+ struct clk_init_data init;
+ int ret;
+
+ cpu = kzalloc(sizeof(*cpu), GFP_KERNEL);
+ if (!cpu)
+ return ERR_PTR(-ENOMEM);
+
+ cpu->core = core;
+ cpu->div_nor = div_nor;
+ cpu->div_hs = div_hs;
+ cpu->mux_nor = mux_nor;
+ cpu->mux_hs = mux_hs;
+ cpu->mux_parent = mux_parent;
+ cpu->pfd = pfd;
+ cpu->step = step;
+
+ init.name = name;
+ init.ops = &clk_cpu_ops;
+ init.flags = flags;
+ init.parent_names = &parent_names;
+ init.num_parents = 1;
+
+ cpu->hw.init = &init;
+ hw = &cpu->hw;
+
+ ret = clk_hw_register(NULL, hw);
+ if (ret) {
+ kfree(cpu);
+ return ERR_PTR(ret);
+ }
+
+ return hw;
+}
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index f074dd8ec42e..7deaba2e525c 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -521,4 +521,13 @@ struct clk_hw *imx_clk_hw_divider_gate(const char *name, const char *parent_name
unsigned long flags, void __iomem *reg, u8 shift, u8 width,
u8 clk_divider_flags, const struct clk_div_table *table,
spinlock_t *lock);
+
+struct clk_hw *imx_clk_hw_cpuv2(const char *name, const char *parent_names,
+ 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,
+ unsigned long flags,
+ unsigned long plat_flags);
#endif
--
2.16.4
From: Peng Fan <[email protected]>
Add IMX7ULP_CLK_ARM_FREQ clk entry for cpufreq usage.
The cpu in device tree needs use this index as clock.
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-imx7ulp.c | 15 ++++++++++++++-
include/dt-bindings/clock/imx7ulp-clock.h | 3 ++-
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/imx/clk-imx7ulp.c b/drivers/clk/imx/clk-imx7ulp.c
index 0620d6c8c072..daa770432bc8 100644
--- a/drivers/clk/imx/clk-imx7ulp.c
+++ b/drivers/clk/imx/clk-imx7ulp.c
@@ -56,6 +56,7 @@ static const int pcc3_uart_clk_ids[] __initconst = {
static struct clk **pcc2_uart_clks[ARRAY_SIZE(pcc2_uart_clk_ids) + 1] __initdata;
static struct clk **pcc3_uart_clks[ARRAY_SIZE(pcc3_uart_clk_ids) + 1] __initdata;
+static struct clk_hw **hws_scg1;
static void __init imx7ulp_clk_scg1_init(struct device_node *np)
{
struct clk_hw_onecell_data *clk_data;
@@ -139,6 +140,8 @@ static void __init imx7ulp_clk_scg1_init(struct device_node *np)
imx_check_clk_hws(hws, clk_data->num);
+ hws_scg1 = hws;
+
of_clk_add_hw_provider(np, of_clk_hw_onecell_get, clk_data);
}
CLK_OF_DECLARE(imx7ulp_clk_scg1, "fsl,imx7ulp-scg1", imx7ulp_clk_scg1_init);
@@ -270,7 +273,17 @@ static void __init imx7ulp_clk_smc1_init(struct device_node *np)
base = of_iomap(np, 0);
WARN_ON(!base);
- hws[IMX7ULP_CLK_ARM] = imx_clk_hw_mux_flags("arm", base + 0x10, 8, 2, arm_sels, ARRAY_SIZE(arm_sels), CLK_IS_CRITICAL);
+ hws[IMX7ULP_CLK_ARM] = imx_clk_hw_mux_flags("arm", base + 0x10, 8, 2, arm_sels, ARRAY_SIZE(arm_sels), 0);
+
+ hws[IMX7ULP_CLK_ARM_FREQ] = imx_clk_hw_cpuv2("arm_freq", "arm",
+ hws[IMX7ULP_CLK_ARM],
+ hws_scg1[IMX7ULP_CLK_CORE_DIV],
+ hws_scg1[IMX7ULP_CLK_HSRUN_CORE_DIV],
+ hws_scg1[IMX7ULP_CLK_SYS_SEL],
+ hws_scg1[IMX7ULP_CLK_HSRUN_SYS_SEL],
+ hws_scg1[IMX7ULP_CLK_SPLL_SEL],
+ hws_scg1[IMX7ULP_CLK_SPLL_PFD0],
+ hws_scg1[IMX7ULP_CLK_FIRC], CLK_IS_CRITICAL, 0);
imx_check_clk_hws(hws, clk_data->num);
diff --git a/include/dt-bindings/clock/imx7ulp-clock.h b/include/dt-bindings/clock/imx7ulp-clock.h
index 38145bdcd975..ecd832dd1c9c 100644
--- a/include/dt-bindings/clock/imx7ulp-clock.h
+++ b/include/dt-bindings/clock/imx7ulp-clock.h
@@ -110,7 +110,8 @@
/* SMC1 */
#define IMX7ULP_CLK_ARM 0
+#define IMX7ULP_CLK_ARM_FREQ 1
-#define IMX7ULP_CLK_SMC1_END 1
+#define IMX7ULP_CLK_SMC1_END 2
#endif /* __DT_BINDINGS_CLOCK_IMX7ULP_H */
--
2.16.4
From: Anson Huang <[email protected]>
Fix below division by zero warning:
[ 3.176443] Division by zero in kernel.
[ 3.181809] CPU: 0 PID: 88 Comm: kworker/0:2 Not tainted 5.3.0-rc2-next-20190730-63758-ge08da51-dirty #124
[ 3.191817] Hardware name: Freescale i.MX7ULP (Device Tree)
[ 3.197821] Workqueue: events dbs_work_handler
[ 3.202849] [<c01127d8>] (unwind_backtrace) from [<c010cd80>] (show_stack+0x10/0x14)
[ 3.211058] [<c010cd80>] (show_stack) from [<c0c77e68>] (dump_stack+0xd8/0x110)
[ 3.218820] [<c0c77e68>] (dump_stack) from [<c0c753c0>] (Ldiv0_64+0x8/0x18)
[ 3.226263] [<c0c753c0>] (Ldiv0_64) from [<c05984b4>] (clk_pfdv2_set_rate+0x54/0xac)
[ 3.234487] [<c05984b4>] (clk_pfdv2_set_rate) from [<c059192c>] (clk_change_rate+0x1a4/0x698)
[ 3.243468] [<c059192c>] (clk_change_rate) from [<c0591a08>] (clk_change_rate+0x280/0x698)
[ 3.252180] [<c0591a08>] (clk_change_rate) from [<c0591fc0>] (clk_core_set_rate_nolock+0x1a0/0x278)
[ 3.261679] [<c0591fc0>] (clk_core_set_rate_nolock) from [<c05920c8>] (clk_set_rate+0x30/0x64)
[ 3.270743] [<c05920c8>] (clk_set_rate) from [<c089cb88>] (imx7ulp_set_target+0x184/0x2a4)
[ 3.279501] [<c089cb88>] (imx7ulp_set_target) from [<c0896358>] (__cpufreq_driver_target+0x188/0x514)
[ 3.289196] [<c0896358>] (__cpufreq_driver_target) from [<c0899b0c>] (od_dbs_update+0x130/0x15c)
[ 3.298438] [<c0899b0c>] (od_dbs_update) from [<c089a5d0>] (dbs_work_handler+0x2c/0x5c)
[ 3.306914] [<c089a5d0>] (dbs_work_handler) from [<c0156858>] (process_one_work+0x2ac/0x704)
[ 3.315826] [<c0156858>] (process_one_work) from [<c0156cdc>] (worker_thread+0x2c/0x574)
[ 3.324404] [<c0156cdc>] (worker_thread) from [<c015cfe8>] (kthread+0x134/0x148)
[ 3.332278] [<c015cfe8>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
[ 3.339858] Exception stack(0xe82d5fb0 to 0xe82d5ff8)
[ 3.345314] 5fa0: 00000000 00000000 00000000 00000000
[ 3.353926] 5fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
[ 3.362519] 5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
Signed-off-by: Anson Huang <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-pfdv2.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/clk/imx/clk-pfdv2.c b/drivers/clk/imx/clk-pfdv2.c
index de93ce73101b..f8707278aad9 100644
--- a/drivers/clk/imx/clk-pfdv2.c
+++ b/drivers/clk/imx/clk-pfdv2.c
@@ -139,6 +139,12 @@ static int clk_pfdv2_set_rate(struct clk_hw *hw, unsigned long rate,
u32 val;
u8 frac;
+ if (!rate)
+ return -EINVAL;
+
+ /* PFD can NOT change rate without gating */
+ WARN_ON(clk_pfdv2_is_enabled(hw));
+
tmp = tmp * 18 + rate / 2;
do_div(tmp, rate);
frac = tmp;
--
2.16.4
From: Peng Fan <[email protected]>
Create cpufreq device to let cpufreq driver could probe
Signed-off-by: Peng Fan <[email protected]>
---
arch/arm/mach-imx/mach-imx7ulp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/mach-imx/mach-imx7ulp.c b/arch/arm/mach-imx/mach-imx7ulp.c
index 128cf4c92aab..fc44fb4ca48b 100644
--- a/arch/arm/mach-imx/mach-imx7ulp.c
+++ b/arch/arm/mach-imx/mach-imx7ulp.c
@@ -67,6 +67,8 @@ static const char *const imx7ulp_dt_compat[] __initconst = {
static void __init imx7ulp_init_late(void)
{
+ platform_device_register_simple("cpufreq-dt", -1, NULL, 0);
+
imx7ulp_cpuidle_init();
}
--
2.16.4
From: Peng Fan <[email protected]>
Configure pmprot to let ARM core could run into HSRUN mode.
Signed-off-by: Peng Fan <[email protected]>
---
arch/arm/mach-imx/pm-imx7ulp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/pm-imx7ulp.c b/arch/arm/mach-imx/pm-imx7ulp.c
index 7b2f7387e662..dc2bc6f2cadd 100644
--- a/arch/arm/mach-imx/pm-imx7ulp.c
+++ b/arch/arm/mach-imx/pm-imx7ulp.c
@@ -11,6 +11,7 @@
#include "common.h"
+#define SMC_PMPROT 0x8
#define SMC_PMCTRL 0x10
#define BP_PMCTRL_PSTOPO 16
#define PSTOPO_PSTOP3 0x3
@@ -25,6 +26,8 @@
#define BM_PMCTRL_RUNM (3 << BP_PMCTRL_RUNM)
#define BM_PMCTRL_STOPM (7 << BP_PMCTRL_STOPM)
+#define BM_PMPROT_AHSRUN BIT(7)
+
static void __iomem *smc1_base;
int imx7ulp_set_lpm(enum ulp_cpu_pwr_mode mode)
@@ -51,6 +54,7 @@ int imx7ulp_set_lpm(enum ulp_cpu_pwr_mode mode)
return -EINVAL;
}
+ writel_relaxed(BM_PMPROT_AHSRUN, smc1_base + SMC_PMPROT);
writel_relaxed(val, smc1_base + SMC_PMCTRL);
return 0;
--
2.16.4
Hi Peng,
On Tue, Feb 4, 2020 at 10:41 AM <[email protected]> wrote:
> I not include the voltage configuration, because imx-rpmsg
> and pf1550 rpmsg driver still not upstreamed.
Any plans for upstreaming imx-rpmsg? I assume this will go into the
remoteproc framework.
Without this driver, the i.MX7ULP support in mainline is very limited
in functionality.
Thanks
Hi Peng,
On Tue, Feb 4, 2020 at 10:41 AM <[email protected]> wrote:
>
> From: Peng Fan <[email protected]>
>
> Configure pmprot to let ARM core could run into HSRUN mode.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> + writel_relaxed(BM_PMPROT_AHSRUN, smc1_base + SMC_PMPROT);
HSRUN cannot be configured unconditionally because if i.MX7ULP runs
with LDO-enabled it cannot run in HSRUN mode.
> Subject: Re: [PATCH 6/7] ARM: imx: imx7ulp: support HSRUN mode
>
> Hi Peng,
>
> On Tue, Feb 4, 2020 at 10:41 AM <[email protected]> wrote:
> >
> > From: Peng Fan <[email protected]>
> >
> > Configure pmprot to let ARM core could run into HSRUN mode.
> >
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
>
> > + writel_relaxed(BM_PMPROT_AHSRUN, smc1_base +
> SMC_PMPROT);
>
> HSRUN cannot be configured unconditionally because if i.MX7ULP runs with
> LDO-enabled it cannot run in HSRUN mode.
Thanks, I'll update to add a check LDO mode here.
Thanks,
Peng.
> Subject: Re: [PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support
>
> Hi Peng,
>
> On Tue, Feb 4, 2020 at 10:41 AM <[email protected]> wrote:
>
> > I not include the voltage configuration, because imx-rpmsg and pf1550
> > rpmsg driver still not upstreamed.
>
> Any plans for upstreaming imx-rpmsg? I assume this will go into the
> remoteproc framework.
I need check with Richard first, if no plan, I'll take it.
>
> Without this driver, the i.MX7ULP support in mainline is very limited in
> functionality.
To test only clk change, rpmsg driver is not a must. I have tested that,
mhz could correctly show 500MHz and 720Mhz with the diff applied
in cover letter.
I'll try to push forward with rpmsg and regulator part, but both not
go through shawn's tree. This patchset without the dts part, cpufreq
also not take effect, so it is safe to be in if got reviewed. After
the rpmsg/regulator part got in, we could add the dts patch to
switch on cpufreq for i.MX7ULP.
Thanks,
Peng.
>
> Thanks
Quoting [email protected] (2020-02-04 05:34:34)
> From: Peng Fan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
> + */
> +
> +#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.
> Subject: Re: [PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP
+ Viresh
Hi Stephen,
>
> Quoting [email protected] (2020-02-04 05:34:34)
> > From: Peng Fan <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > + */
> > +
> > +#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.
There was an old thread, https://lkml.org/lkml/2017/9/20/931
Aisheng proposed a opp->set_clk implemented, but rejected by Viresh.
Viresh suggested resolve this issue in clk driver to let clk driver handle
the cpu clk freq change.
>
> 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.
ok
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,
Do you have any suggestions if put the cpu clk freq change in clk driver?
i.MX7ULP has some special requirements, it has RUN and HSRUN registers
which match 500MHz and 720MHz, to run at 500MHz, need configure RUN
related registers; to run at 720MHz, need configure HSRUN related registers.
So the cpu clk is mux marked with CLK_IS_CRITICAL. However in its parent
tree, pll and pfd needs CLK_SET_RATE_GATE. So after pll/pfd prepared,
its freq could not be changed, because of protected.
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!
As described above, the pll/pfd flag has CLK_SET_RATE_GATE, the mux
has flag CLK_IS_CRITICAL. So the pll/pfd freq could not be changed because
clk framework marked pll/pfd protected.
Anyway I'll try to find more to see any new solutions.
>
> 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.
When run at 720MHz, cpu are only allowed cpu core wfi. So I add pm qos
to block low power idle.
Thanks,
Peng.