[RFC 0/4] clk: imx: Register the dram_apb and dram_alt as read-only

[RFC 0/4] clk: imx: Register the dram_apb and dram_alt as read-only

[Abel Vesa]

On i.MX8M platforms the dram_apb and dram_alt are controlled from EL3.
So in order to keep track of the actual clock tree in kernel, we need
to actually declare the clocks but never write to any of their registes.
We do that by registering the clocks with only the ops that read but
never write the registers.

Abel Vesa (4):
  clk: Add CLK_GET_PARENT_NOCACHE flag
  clk: Add clk_gate_ro_ops for read-only gate clocks
  clk: imx: composite-8m: Add DRAM clock registration variant
  clk: imx8m: Use dram variant registration for dram clocks

 drivers/clk/clk-gate.c             |  5 +++++
 drivers/clk/clk.c                  | 31 +++++++++++++++++--------------
 drivers/clk/imx/clk-composite-8m.c | 12 +++++++++++-
 drivers/clk/imx/clk-imx8mm.c       |  4 ++--
 drivers/clk/imx/clk-imx8mn.c       |  4 ++--
 drivers/clk/imx/clk-imx8mp.c       |  4 ++--
 drivers/clk/imx/clk-imx8mq.c       |  4 ++--
 drivers/clk/imx/clk.h              |  7 +++++++
 include/linux/clk-provider.h       |  2 ++
 9 files changed, 50 insertions(+), 23 deletions(-)

-- 
2.7.4

[RFC 1/4] clk: Add CLK_GET_PARENT_NOCACHE flag

[Abel Vesa]

This can be used by the clocks that have their parents changed from EL3.
This way the clk_get_parent will read the value from register instead of
using the value stored in the core framework.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
Suggested-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/clk/clk.c            | 31 +++++++++++++++++--------------
 include/linux/clk-provider.h |  1 +
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 90d2373..474315d 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2401,6 +2401,16 @@ int clk_set_max_rate(struct clk *clk, unsigned long rate)
 }
 EXPORT_SYMBOL_GPL(clk_set_max_rate);
 
+static struct clk_core *__clk_get_parent(struct clk_core *core)
+{
+	u8 index = 0;
+
+	if (core->num_parents > 1 && core->ops->get_parent)
+		index = core->ops->get_parent(core->hw);
+
+	return clk_core_get_parent_by_index(core, index);
+}
+
 /**
  * clk_get_parent - return the parent of a clk
  * @clk: the clk whose parent gets returned
@@ -2415,24 +2425,17 @@ struct clk *clk_get_parent(struct clk *clk)
 		return NULL;
 
 	clk_prepare_lock();
-	/* TODO: Create a per-user clk and change callers to call clk_put */
-	parent = !clk->core->parent ? NULL : clk->core->parent->hw->clk;
+	if (clk->core && (clk->core->flags & CLK_GET_PARENT_NOCACHE))
+		parent = __clk_get_parent(clk->core)->hw->clk;
+	else
+		/* TODO: Create a per-user clk and change callers to call clk_put */
+		parent = !clk->core->parent ? NULL : clk->core->parent->hw->clk;
 	clk_prepare_unlock();
 
 	return parent;
 }
 EXPORT_SYMBOL_GPL(clk_get_parent);
 
-static struct clk_core *__clk_init_parent(struct clk_core *core)
-{
-	u8 index = 0;
-
-	if (core->num_parents > 1 && core->ops->get_parent)
-		index = core->ops->get_parent(core->hw);
-
-	return clk_core_get_parent_by_index(core, index);
-}
-
 static void clk_core_reparent(struct clk_core *core,
 				  struct clk_core *new_parent)
 {
@@ -3353,7 +3356,7 @@ static void clk_core_reparent_orphans_nolock(void)
 	 * parent.
 	 */
 	hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
-		struct clk_core *parent = __clk_init_parent(orphan);
+		struct clk_core *parent = __clk_get_parent(orphan);
 
 		/*
 		 * We need to use __clk_set_parent_before() and _after() to
@@ -3454,7 +3457,7 @@ static int __clk_core_init(struct clk_core *core)
 			goto out;
 	}
 
-	parent = core->parent = __clk_init_parent(core);
+	parent = core->parent = __clk_get_parent(core);
 
 	/*
 	 * Populate core->parent if parent has already been clk_core_init'd. If
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index d98e41f..94a78b3 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -33,6 +33,7 @@
 /* duty cycle call may be forwarded to the parent clock */
 #define CLK_DUTY_CYCLE_PARENT	BIT(13)
 #define CLK_SET_PARENT_NOCACHE	BIT(14) /* do not use the cached clk parent */
+#define CLK_GET_PARENT_NOCACHE	BIT(15) /* read the parent from reg */
 
 struct clk;
 struct clk_hw;
-- 
2.7.4

[RFC 2/4] clk: Add clk_gate_ro_ops for read-only gate clocks

[Abel Vesa]

The clocks that can be changed from outside of the clock common framework
scope (for example, EL3) need to have only the .is_enabled gate op.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/clk/clk-gate.c       | 5 +++++
 include/linux/clk-provider.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index 070dc47..41ca887 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -123,6 +123,11 @@ const struct clk_ops clk_gate_ops = {
 };
 EXPORT_SYMBOL_GPL(clk_gate_ops);
 
+const struct clk_ops clk_gate_ro_ops = {
+	.is_enabled = clk_gate_is_enabled,
+};
+EXPORT_SYMBOL_GPL(clk_gate_ro_ops);
+
 struct clk_hw *__clk_hw_register_gate(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 94a78b3..6668d2d 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -485,6 +485,7 @@ struct clk_gate {
 #define CLK_GATE_BIG_ENDIAN		BIT(2)
 
 extern const struct clk_ops clk_gate_ops;
+extern const struct clk_ops clk_gate_ro_ops;
 struct clk_hw *__clk_hw_register_gate(struct device *dev,
 		struct device_node *np, const char *name,
 		const char *parent_name, const struct clk_hw *parent_hw,
-- 
2.7.4

[RFC 3/4] clk: imx: composite-8m: Add DRAM clock registration variant

[Abel Vesa]

The switch between parents for dram_apb and dram_alt is done in EL3,
so make all the ops read-only. That means none of the ops that write
any of the registers is used for such a clock.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/clk/imx/clk-composite-8m.c | 12 +++++++++++-
 drivers/clk/imx/clk.h              |  7 +++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index b8cd0f0..6734a4a 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -184,6 +184,7 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 	struct clk_mux *mux = NULL;
 	const struct clk_ops *divider_ops;
 	const struct clk_ops *mux_ops;
+	const struct clk_ops *gate_ops;
 
 	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
 	if (!mux)
@@ -206,16 +207,25 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 		div->width = PCG_CORE_DIV_WIDTH;
 		divider_ops = &clk_divider_ops;
 		mux_ops = &imx8m_clk_composite_mux_ops;
+		gate_ops = &clk_gate_ops;
 	} else if (composite_flags & IMX_COMPOSITE_BUS) {
 		div->shift = PCG_PREDIV_SHIFT;
 		div->width = PCG_PREDIV_WIDTH;
 		divider_ops = &imx8m_clk_composite_divider_ops;
 		mux_ops = &imx8m_clk_composite_mux_ops;
+		gate_ops = &clk_gate_ops;
+	} else if (composite_flags & IMX_COMPOSITE_RO) {
+		div->shift = PCG_PREDIV_SHIFT;
+		div->width = PCG_PREDIV_WIDTH;
+		divider_ops = &clk_divider_ro_ops;
+		mux_ops = &clk_mux_ro_ops;
+		gate_ops = &clk_gate_ro_ops;
 	} else {
 		div->shift = PCG_PREDIV_SHIFT;
 		div->width = PCG_PREDIV_WIDTH;
 		divider_ops = &imx8m_clk_composite_divider_ops;
 		mux_ops = &clk_mux_ops;
+		gate_ops = &clk_gate_ops;
 		flags |= CLK_SET_PARENT_GATE;
 	}
 
@@ -238,7 +248,7 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 
 	hw = clk_hw_register_composite(NULL, name, parent_names, num_parents,
 			mux_hw, mux_ops, div_hw,
-			divider_ops, gate_hw, &clk_gate_ops, flags);
+			divider_ops, gate_hw, gate_ops, flags);
 	if (IS_ERR(hw))
 		goto fail;
 
diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index a997049..8096585 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -580,6 +580,7 @@ struct clk_hw *imx_clk_hw_cpu(const char *name, const char *parent_name,
 
 #define IMX_COMPOSITE_CORE	BIT(0)
 #define IMX_COMPOSITE_BUS	BIT(1)
+#define IMX_COMPOSITE_RO	BIT(2)
 
 struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 					    const char * const *parent_names,
@@ -600,6 +601,12 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
 			IMX_COMPOSITE_CORE, \
 			CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE)
 
+#define imx8m_clk_hw_composite_dram(name, parent_names, reg) \
+	imx8m_clk_hw_composite_flags(name, parent_names, \
+		ARRAY_SIZE(parent_names), reg, IMX_COMPOSITE_RO, \
+		CLK_GET_RATE_NOCACHE | CLK_GET_PARENT_NOCACHE \
+		| CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE)
+
 #define imx8m_clk_composite_flags(name, parent_names, num_parents, reg, \
 				  flags) \
 	to_clk(imx8m_clk_hw_composite_flags(name, parent_names, \
-- 
2.7.4

[RFC 4/4] clk: imx8m: Use dram variant registration for dram clocks

[Abel Vesa]

Both dram_apb and dram_alt are controlled by EL3. Using the dram
variant registration of the composite-8m clock, the mux and the
divider will be read only. Do this for all i.MX8M platforms.

Signed-off-by: Abel Vesa <abel.vesa@nxp.com>
---
 drivers/clk/imx/clk-imx8mm.c | 4 ++--
 drivers/clk/imx/clk-imx8mn.c | 4 ++--
 drivers/clk/imx/clk-imx8mp.c | 4 ++--
 drivers/clk/imx/clk-imx8mq.c | 4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index cd807fa..7f9ef21 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -477,8 +477,8 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
 	 * DRAM clocks are manipulated from TF-A outside clock framework.
 	 * Mark with GET_RATE_NOCACHE to always read div value from hardware
 	 */
-	hws[IMX8MM_CLK_DRAM_ALT] = __imx8m_clk_hw_composite("dram_alt", imx8mm_dram_alt_sels, base + 0xa000, CLK_GET_RATE_NOCACHE);
-	hws[IMX8MM_CLK_DRAM_APB] = __imx8m_clk_hw_composite("dram_apb", imx8mm_dram_apb_sels, base + 0xa080, CLK_IS_CRITICAL | CLK_GET_RATE_NOCACHE);
+	hws[IMX8MM_CLK_DRAM_ALT] = imx8m_clk_hw_composite_dram("dram_alt", imx8mm_dram_alt_sels, base + 0xa000);
+	hws[IMX8MM_CLK_DRAM_APB] = imx8m_clk_hw_composite_dram("dram_apb", imx8mm_dram_apb_sels, base + 0xa080);
 
 	/* IP */
 	hws[IMX8MM_CLK_VPU_G1] = imx8m_clk_hw_composite("vpu_g1", imx8mm_vpu_g1_sels, base + 0xa100);
diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index b65f60d..d8000ee 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -459,8 +459,8 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)
 	 * DRAM clocks are manipulated from TF-A outside clock framework.
 	 * Mark with GET_RATE_NOCACHE to always read div value from hardware
 	 */
-	hws[IMX8MN_CLK_DRAM_ALT] = __imx8m_clk_hw_composite("dram_alt", imx8mn_dram_alt_sels, base + 0xa000, CLK_GET_RATE_NOCACHE);
-	hws[IMX8MN_CLK_DRAM_APB] = __imx8m_clk_hw_composite("dram_apb", imx8mn_dram_apb_sels, base + 0xa080, CLK_IS_CRITICAL | CLK_GET_RATE_NOCACHE);
+	hws[IMX8MN_CLK_DRAM_ALT] = imx8m_clk_hw_composite_dram("dram_alt", imx8mn_dram_alt_sels, base + 0xa000);
+	hws[IMX8MN_CLK_DRAM_APB] = imx8m_clk_hw_composite_dram("dram_apb", imx8mn_dram_apb_sels, base + 0xa080);
 
 	hws[IMX8MN_CLK_DISP_PIXEL] = imx8m_clk_hw_composite("disp_pixel", imx8mn_disp_pixel_sels, base + 0xa500);
 	hws[IMX8MN_CLK_SAI2] = imx8m_clk_hw_composite("sai2", imx8mn_sai2_sels, base + 0xa600);
diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 5b9026b..bf13566 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -846,8 +846,8 @@ static int imx8mp_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MP_CLK_IPG_ROOT] = imx_clk_hw_divider2("ipg_root", "ahb_root", ccm_base + 0x9080, 0, 1);
 	hws[IMX8MP_CLK_IPG_AUDIO_ROOT] = imx_clk_hw_divider2("ipg_audio_root", "audio_ahb", ccm_base + 0x9180, 0, 1);
 
-	hws[IMX8MP_CLK_DRAM_ALT] = imx8m_clk_hw_composite("dram_alt", imx8mp_dram_alt_sels, ccm_base + 0xa000);
-	hws[IMX8MP_CLK_DRAM_APB] = imx8m_clk_hw_composite_critical("dram_apb", imx8mp_dram_apb_sels, ccm_base + 0xa080);
+	hws[IMX8MP_CLK_DRAM_ALT] = imx8m_clk_hw_composite_dram("dram_alt", imx8mp_dram_alt_sels, ccm_base + 0xa000);
+	hws[IMX8MP_CLK_DRAM_APB] = imx8m_clk_hw_composite_dram("dram_apb", imx8mp_dram_apb_sels, ccm_base + 0xa080);
 	hws[IMX8MP_CLK_VPU_G1] = imx8m_clk_hw_composite("vpu_g1", imx8mp_vpu_g1_sels, ccm_base + 0xa100);
 	hws[IMX8MP_CLK_VPU_G2] = imx8m_clk_hw_composite("vpu_g2", imx8mp_vpu_g2_sels, ccm_base + 0xa180);
 	hws[IMX8MP_CLK_CAN1] = imx8m_clk_hw_composite("can1", imx8mp_can1_sels, ccm_base + 0xa200);
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index e5d2da4..57db0cc 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -473,8 +473,8 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	 * Mark with GET_RATE_NOCACHE to always read div value from hardware
 	 */
 	hws[IMX8MQ_CLK_DRAM_CORE] = imx_clk_hw_mux2_flags("dram_core_clk", base + 0x9800, 24, 1, imx8mq_dram_core_sels, ARRAY_SIZE(imx8mq_dram_core_sels), CLK_IS_CRITICAL);
-	hws[IMX8MQ_CLK_DRAM_ALT] = __imx8m_clk_hw_composite("dram_alt", imx8mq_dram_alt_sels, base + 0xa000, CLK_GET_RATE_NOCACHE);
-	hws[IMX8MQ_CLK_DRAM_APB] = __imx8m_clk_hw_composite("dram_apb", imx8mq_dram_apb_sels, base + 0xa080, CLK_IS_CRITICAL | CLK_GET_RATE_NOCACHE);
+	hws[IMX8MQ_CLK_DRAM_ALT] = imx8m_clk_hw_composite_dram("dram_alt", imx8mq_dram_alt_sels, base + 0xa000);
+	hws[IMX8MQ_CLK_DRAM_APB] = imx8m_clk_hw_composite_dram("dram_apb", imx8mq_dram_apb_sels, base + 0xa080);
 
 	/* IP */
 	hws[IMX8MQ_CLK_VPU_G1] = imx8m_clk_hw_composite("vpu_g1", imx8mq_vpu_g1_sels, base + 0xa100);
-- 
2.7.4

RE: [RFC 0/4] clk: imx: Register the dram_apb and dram_alt as read-only

[Peng Fan]

> Subject: [RFC 0/4] clk: imx: Register the dram_apb and dram_alt as read-only

Any follow up about this patchset?

Regards,
Peng.

> 
> On i.MX8M platforms the dram_apb and dram_alt are controlled from EL3.
> So in order to keep track of the actual clock tree in kernel, we need to
> actually declare the clocks but never write to any of their registes.
> We do that by registering the clocks with only the ops that read but never
> write the registers.
> 
> Abel Vesa (4):
>   clk: Add CLK_GET_PARENT_NOCACHE flag
>   clk: Add clk_gate_ro_ops for read-only gate clocks
>   clk: imx: composite-8m: Add DRAM clock registration variant
>   clk: imx8m: Use dram variant registration for dram clocks
> 
>  drivers/clk/clk-gate.c             |  5 +++++
>  drivers/clk/clk.c                  | 31 +++++++++++++++++--------------
>  drivers/clk/imx/clk-composite-8m.c | 12 +++++++++++-
>  drivers/clk/imx/clk-imx8mm.c       |  4 ++--
>  drivers/clk/imx/clk-imx8mn.c       |  4 ++--
>  drivers/clk/imx/clk-imx8mp.c       |  4 ++--
>  drivers/clk/imx/clk-imx8mq.c       |  4 ++--
>  drivers/clk/imx/clk.h              |  7 +++++++
>  include/linux/clk-provider.h       |  2 ++
>  9 files changed, 50 insertions(+), 23 deletions(-)
> 
> --
> 2.7.4