[PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support

[PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

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

[PATCH 1/7] clk: imx: Fix division by zero warning on pfdv2

[peng.fan]

From: Anson Huang <Anson.Huang@nxp.com>

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 <Anson.Huang@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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

[PATCH 2/7] clk: imx: pfdv2: switch to use determine_rate

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

Per clk_ops, compared with round_rate, determine_rate could optionally
support the parent clock that should be used to provide the clock rate.

In this patch, the parent clock is just parent->rate as round_rate.

The following patch will calculate the best parent clock.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-pfdv2.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-pfdv2.c b/drivers/clk/imx/clk-pfdv2.c
index f8707278aad9..28b5f208ced9 100644
--- a/drivers/clk/imx/clk-pfdv2.c
+++ b/drivers/clk/imx/clk-pfdv2.c
@@ -98,10 +98,11 @@ static unsigned long clk_pfdv2_recalc_rate(struct clk_hw *hw,
 	return tmp;
 }
 
-static long clk_pfdv2_round_rate(struct clk_hw *hw, unsigned long rate,
-				 unsigned long *prate)
+static int clk_pfdv2_determine_rate(struct clk_hw *hw,
+				    struct clk_rate_request *req)
 {
-	u64 tmp = *prate;
+	u64 tmp = req->best_parent_rate;
+	u64 rate = req->rate;
 	u8 frac;
 
 	tmp = tmp * 18 + rate / 2;
@@ -113,11 +114,13 @@ static long clk_pfdv2_round_rate(struct clk_hw *hw, unsigned long rate,
 	else if (frac > 35)
 		frac = 35;
 
-	tmp = *prate;
+	tmp = req->best_parent_rate;
 	tmp *= 18;
 	do_div(tmp, frac);
 
-	return tmp;
+	req->rate = tmp;
+
+	return 0;
 }
 
 static int clk_pfdv2_is_enabled(struct clk_hw *hw)
@@ -167,7 +170,7 @@ static const struct clk_ops clk_pfdv2_ops = {
 	.enable		= clk_pfdv2_enable,
 	.disable	= clk_pfdv2_disable,
 	.recalc_rate	= clk_pfdv2_recalc_rate,
-	.round_rate	= clk_pfdv2_round_rate,
+	.determine_rate	= clk_pfdv2_determine_rate,
 	.set_rate	= clk_pfdv2_set_rate,
 	.is_enabled     = clk_pfdv2_is_enabled,
 };
-- 
2.16.4

[PATCH 3/7] clk: imx: pfdv2: determine best parent rate

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

pfdv2 is only used in i.MX7ULP. To get best pfd output, the i.MX7ULP
Datasheet defines two best PLL rate and pfd frac.

Per Datasheel
All PLLs on i.MX 7ULP either have VCO base frequency of
480 MHz or 528 MHz. So when determine best rate, we also
determine best parent rate which could match the requirement.

For some reason the current parent might not be 480MHz or 528MHz,
so we still take current parent rate as a choice.

And we also enable flag CLK_SET_RATE_PARENT to let parent rate
to be configured.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/imx/clk-pfdv2.c | 50 ++++++++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/imx/clk-pfdv2.c b/drivers/clk/imx/clk-pfdv2.c
index 28b5f208ced9..78e1f7641aaa 100644
--- a/drivers/clk/imx/clk-pfdv2.c
+++ b/drivers/clk/imx/clk-pfdv2.c
@@ -101,24 +101,40 @@ static unsigned long clk_pfdv2_recalc_rate(struct clk_hw *hw,
 static int clk_pfdv2_determine_rate(struct clk_hw *hw,
 				    struct clk_rate_request *req)
 {
-	u64 tmp = req->best_parent_rate;
-	u64 rate = req->rate;
+	unsigned long parent_rates[] = {
+					480000000,
+					528000000,
+					req->best_parent_rate
+				       };
+	unsigned long best_rate = -1UL, rate = req->rate;
+	unsigned long best_parent_rate = req->best_parent_rate;
+	u64 tmp;
 	u8 frac;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(parent_rates); i++) {
+		tmp = parent_rates[i];
+		tmp = tmp * 18 + rate / 2;
+		do_div(tmp, rate);
+		frac = tmp;
+
+		if (frac < 12)
+			frac = 12;
+		else if (frac > 35)
+			frac = 35;
+
+		tmp = parent_rates[i];
+		tmp *= 18;
+		do_div(tmp, frac);
+
+		if (abs(tmp - req->rate) < abs(best_rate - req->rate)) {
+			best_rate = tmp;
+			best_parent_rate = parent_rates[i];
+		}
+	}
 
-	tmp = tmp * 18 + rate / 2;
-	do_div(tmp, rate);
-	frac = tmp;
-
-	if (frac < 12)
-		frac = 12;
-	else if (frac > 35)
-		frac = 35;
-
-	tmp = req->best_parent_rate;
-	tmp *= 18;
-	do_div(tmp, frac);
-
-	req->rate = tmp;
+	req->best_parent_rate = best_parent_rate;
+	req->rate = best_rate;
 
 	return 0;
 }
@@ -198,7 +214,7 @@ struct clk_hw *imx_clk_hw_pfdv2(const char *name, const char *parent_name,
 	init.ops = &clk_pfdv2_ops;
 	init.parent_names = &parent_name;
 	init.num_parents = 1;
-	init.flags = CLK_SET_RATE_GATE;
+	init.flags = CLK_SET_RATE_GATE | CLK_SET_RATE_PARENT;
 
 	pfd->hw.init = &init;
 
-- 
2.16.4

[PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP

[peng.fan]

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);
+	}
+
+	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

Re: [PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP

[Stephen Boyd]

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.

RE: [PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP

[Peng Fan]

> Subject: Re: [PATCH 4/7] clk: imx: add imx_hw_clk_cpuv2 for i.MX7ULP

+ Viresh

Hi Stephen,

> 
> 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.

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.

[PATCH 5/7] clk: imx: imx7ulp: add IMX7ULP_CLK_ARM_FREQ clk

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---
 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

[PATCH 6/7] ARM: imx: imx7ulp: support HSRUN mode

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

Configure pmprot to let ARM core could run into HSRUN mode.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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

Re: [PATCH 6/7] ARM: imx: imx7ulp: support HSRUN mode

[Fabio Estevam]

Hi Peng,

On Tue, Feb 4, 2020 at 10:41 AM <peng.fan@nxp.com> wrote:
>
> From: Peng Fan <peng.fan@nxp.com>
>
> Configure pmprot to let ARM core could run into HSRUN mode.
>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---

> +       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.

RE: [PATCH 6/7] ARM: imx: imx7ulp: support HSRUN mode

[Peng Fan]

> Subject: Re: [PATCH 6/7] ARM: imx: imx7ulp: support HSRUN mode
> 
> Hi Peng,
> 
> On Tue, Feb 4, 2020 at 10:41 AM <peng.fan@nxp.com> wrote:
> >
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Configure pmprot to let ARM core could run into HSRUN mode.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> 
> > +       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.

[PATCH 7/7] ARM: imx: imx7ulp: create cpufreq device

[peng.fan]

From: Peng Fan <peng.fan@nxp.com>

Create cpufreq device to let cpufreq driver could probe

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 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

Re: [PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support

[Fabio Estevam]

Hi Peng,

On Tue, Feb 4, 2020 at 10:41 AM <peng.fan@nxp.com> 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

RE: [PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support

[Peng Fan]

> Subject: Re: [PATCH 0/7] ARM: imx: imx7ulp: add cpufreq support
> 
> Hi Peng,
> 
> On Tue, Feb 4, 2020 at 10:41 AM <peng.fan@nxp.com> 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