2020-01-10 07:19:00

by Peng Fan

[permalink] [raw]
Subject: [PATCH V2 0/4] clk: imx: imx8m: introduce imx8m_clk_hw_composite_core

From: Peng Fan <[email protected]>

V2:
Rename imx8m_clk_hw_core_composite to imx8m_clk_hw_composite_core
Add Abel's tag

To i.MX8M family, there are different types of clock slices,
bus/core/ip and etc. Currently, the imx8m_clk_hw_composite
api could only handle bus and ip clock slice, it could
not handle core slice. The difference is core slice not have
pre divider and the width of post divider is 3 bits.

To simplify code and reuse imx8m_clk_hw_composite, introduce a
flag IMX_COMPOSITE_CORE to differentiate the slices.

With this new helper, we could simplify i.MX8M SoC clk drivers.

Peng Fan (4):
clk: imx: composite-8m: add imx8m_clk_hw_composite_core
clk: imx: imx8mq: use imx8m_clk_hw_composite_core
clk: imx: imx8mm: use imx8m_clk_hw_composite_core
clk: imx: imx8mn: use imx8m_clk_hw_composite_core

drivers/clk/imx/clk-composite-8m.c | 18 ++++++++++++++----
drivers/clk/imx/clk-imx8mm.c | 17 +++++------------
drivers/clk/imx/clk-imx8mn.c | 10 +++-------
drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
drivers/clk/imx/clk.h | 12 ++++++++++--
5 files changed, 37 insertions(+), 39 deletions(-)

--
2.16.4


2020-01-10 07:19:05

by Peng Fan

[permalink] [raw]
Subject: [PATCH V2 1/4] clk: imx: composite-8m: add imx8m_clk_hw_composite_core

From: Peng Fan <[email protected]>

There are several clock slices, current composite code
only support bus/ip clock slices, it could not support core
slice.

So introduce a new API imx8m_clk_hw_composite_core to support
core slice. To core slice, post divider with 3 bits width and
no pre divider. Other fields are same as bus/ip slices.

Add a flag IMX_COMPOSITE_CORE for the usecase.

Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-composite-8m.c | 18 ++++++++++++++----
drivers/clk/imx/clk.h | 12 ++++++++++--
2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c b/drivers/clk/imx/clk-composite-8m.c
index 20f7c91c03d2..4869c16376bf 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -15,6 +15,7 @@
#define PCG_PREDIV_MAX 8

#define PCG_DIV_SHIFT 0
+#define PCG_CORE_DIV_WIDTH 3
#define PCG_DIV_WIDTH 6
#define PCG_DIV_MAX 64

@@ -126,6 +127,7 @@ static const struct clk_ops imx8m_clk_composite_divider_ops = {
struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
const char * const *parent_names,
int num_parents, void __iomem *reg,
+ u32 composite_flags,
unsigned long flags)
{
struct clk_hw *hw = ERR_PTR(-ENOMEM), *mux_hw;
@@ -133,6 +135,7 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
struct clk_divider *div = NULL;
struct clk_gate *gate = NULL;
struct clk_mux *mux = NULL;
+ const struct clk_ops *divider_ops;

mux = kzalloc(sizeof(*mux), GFP_KERNEL);
if (!mux)
@@ -150,8 +153,16 @@ struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,

div_hw = &div->hw;
div->reg = reg;
- div->shift = PCG_PREDIV_SHIFT;
- div->width = PCG_PREDIV_WIDTH;
+ if (composite_flags & IMX_COMPOSITE_CORE) {
+ div->shift = PCG_DIV_SHIFT;
+ div->width = PCG_CORE_DIV_WIDTH;
+ divider_ops = &clk_divider_ops;
+ } else {
+ div->shift = PCG_PREDIV_SHIFT;
+ div->width = PCG_PREDIV_WIDTH;
+ divider_ops = &imx8m_clk_composite_divider_ops;
+ }
+
div->lock = &imx_ccm_lock;
div->flags = CLK_DIVIDER_ROUND_CLOSEST;

@@ -166,8 +177,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, &clk_mux_ops, div_hw,
- &imx8m_clk_composite_divider_ops,
- gate_hw, &clk_gate_ops, flags);
+ divider_ops, gate_hw, &clk_gate_ops, flags);
if (IS_ERR(hw))
goto fail;

diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h
index 65d80c675aa9..6f0d5e8fbbac 100644
--- a/drivers/clk/imx/clk.h
+++ b/drivers/clk/imx/clk.h
@@ -474,20 +474,28 @@ struct clk_hw *imx_clk_hw_cpu(const char *name, const char *parent_name,
struct clk *div, struct clk *mux, struct clk *pll,
struct clk *step);

+#define IMX_COMPOSITE_CORE BIT(0)
+
struct clk_hw *imx8m_clk_hw_composite_flags(const char *name,
const char * const *parent_names,
int num_parents,
void __iomem *reg,
+ u32 composite_flags,
unsigned long flags);

+#define imx8m_clk_hw_composite_core(name, parent_names, reg) \
+ imx8m_clk_hw_composite_flags(name, parent_names, \
+ ARRAY_SIZE(parent_names), reg, \
+ IMX_COMPOSITE_CORE, 0)
+
#define imx8m_clk_composite_flags(name, parent_names, num_parents, reg, \
flags) \
to_clk(imx8m_clk_hw_composite_flags(name, parent_names, \
- num_parents, reg, flags))
+ num_parents, reg, 0, flags))

#define __imx8m_clk_hw_composite(name, parent_names, reg, flags) \
imx8m_clk_hw_composite_flags(name, parent_names, \
- ARRAY_SIZE(parent_names), reg, \
+ ARRAY_SIZE(parent_names), reg, 0, \
flags | CLK_SET_RATE_NO_REPARENT | CLK_OPS_PARENT_ENABLE)

#define __imx8m_clk_composite(name, parent_names, reg, flags) \
--
2.16.4

2020-01-10 07:19:25

by Peng Fan

[permalink] [raw]
Subject: [PATCH V2 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

From: Peng Fan <[email protected]>

Use imx8m_clk_hw_composite_core to simplify code.

Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4c0edca1a6d0..b031183ff427 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -403,22 +403,13 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)

/* CORE */
hws[IMX8MQ_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels));
- hws[IMX8MQ_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mq_arm_m4_sels, ARRAY_SIZE(imx8mq_arm_m4_sels));
- hws[IMX8MQ_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mq_vpu_sels, ARRAY_SIZE(imx8mq_vpu_sels));
- hws[IMX8MQ_CLK_GPU_CORE_SRC] = imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3, imx8mq_gpu_core_sels, ARRAY_SIZE(imx8mq_gpu_core_sels));
- hws[IMX8MQ_CLK_GPU_SHADER_SRC] = imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3, imx8mq_gpu_shader_sels, ARRAY_SIZE(imx8mq_gpu_shader_sels));
-
hws[IMX8MQ_CLK_A53_CG] = imx_clk_hw_gate3_flags("arm_a53_cg", "arm_a53_src", base + 0x8000, 28, CLK_IS_CRITICAL);
- hws[IMX8MQ_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
- hws[IMX8MQ_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
- hws[IMX8MQ_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg", "gpu_core_src", base + 0x8180, 28);
- hws[IMX8MQ_CLK_GPU_SHADER_CG] = imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
-
hws[IMX8MQ_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
- hws[IMX8MQ_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
- hws[IMX8MQ_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
- hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
- hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0, 3);
+
+ hws[IMX8MQ_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mq_arm_m4_sels, base + 0x8080);
+ hws[IMX8MQ_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mq_vpu_sels, base + 0x8100);
+ hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx8m_clk_hw_composite_core("gpu_core_div", imx8mq_gpu_core_sels, base + 0x8180);
+ hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx8m_clk_hw_composite("gpu_shader_div", imx8mq_gpu_shader_sels, base + 0x8200);

/* BUS */
hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);
--
2.16.4

2020-01-10 07:19:48

by Peng Fan

[permalink] [raw]
Subject: [PATCH V2 3/4] clk: imx: imx8mm: use imx8m_clk_hw_composite_core

From: Peng Fan <[email protected]>

Use imx8m_clk_hw_composite_core to simplify code.

Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-imx8mm.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 2ed93fc25087..197ba2cdab7d 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -414,20 +414,13 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)

/* Core Slice */
hws[IMX8MM_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mm_a53_sels, ARRAY_SIZE(imx8mm_a53_sels));
- hws[IMX8MM_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mm_m4_sels, ARRAY_SIZE(imx8mm_m4_sels));
- hws[IMX8MM_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mm_vpu_sels, ARRAY_SIZE(imx8mm_vpu_sels));
- hws[IMX8MM_CLK_GPU3D_SRC] = imx_clk_hw_mux2("gpu3d_src", base + 0x8180, 24, 3, imx8mm_gpu3d_sels, ARRAY_SIZE(imx8mm_gpu3d_sels));
- hws[IMX8MM_CLK_GPU2D_SRC] = imx_clk_hw_mux2("gpu2d_src", base + 0x8200, 24, 3, imx8mm_gpu2d_sels, ARRAY_SIZE(imx8mm_gpu2d_sels));
hws[IMX8MM_CLK_A53_CG] = imx_clk_hw_gate3("arm_a53_cg", "arm_a53_src", base + 0x8000, 28);
- hws[IMX8MM_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
- hws[IMX8MM_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
- hws[IMX8MM_CLK_GPU3D_CG] = imx_clk_hw_gate3("gpu3d_cg", "gpu3d_src", base + 0x8180, 28);
- hws[IMX8MM_CLK_GPU2D_CG] = imx_clk_hw_gate3("gpu2d_cg", "gpu2d_src", base + 0x8200, 28);
hws[IMX8MM_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
- hws[IMX8MM_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
- hws[IMX8MM_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
- hws[IMX8MM_CLK_GPU3D_DIV] = imx_clk_hw_divider2("gpu3d_div", "gpu3d_cg", base + 0x8180, 0, 3);
- hws[IMX8MM_CLK_GPU2D_DIV] = imx_clk_hw_divider2("gpu2d_div", "gpu2d_cg", base + 0x8200, 0, 3);
+
+ hws[IMX8MM_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mm_m4_sels, base + 0x8080);
+ hws[IMX8MM_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mm_vpu_sels, base + 0x8100);
+ hws[IMX8MM_CLK_GPU3D_DIV] = imx8m_clk_hw_composite_core("gpu3d_div", imx8mm_gpu3d_sels, base + 0x8180);
+ hws[IMX8MM_CLK_GPU2D_DIV] = imx8m_clk_hw_composite_core("gpu2d_div", imx8mm_gpu2d_sels, base + 0x8200);

/* BUS */
hws[IMX8MM_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mm_main_axi_sels, base + 0x8800);
--
2.16.4

2020-01-10 07:20:08

by Peng Fan

[permalink] [raw]
Subject: [PATCH V2 4/4] clk: imx: imx8mn: use imx8m_clk_hw_composite_core

From: Peng Fan <[email protected]>

Use imx8m_clk_hw_composite_core to simplify code.

Reviewed-by: Abel Vesa <[email protected]>
Signed-off-by: Peng Fan <[email protected]>
---
drivers/clk/imx/clk-imx8mn.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index c5e7316b4c66..ce2ba3dce483 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -413,15 +413,11 @@ static int imx8mn_clocks_probe(struct platform_device *pdev)

/* CORE */
hws[IMX8MN_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mn_a53_sels, ARRAY_SIZE(imx8mn_a53_sels));
- hws[IMX8MN_CLK_GPU_CORE_SRC] = imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3, imx8mn_gpu_core_sels, ARRAY_SIZE(imx8mn_gpu_core_sels));
- hws[IMX8MN_CLK_GPU_SHADER_SRC] = imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3, imx8mn_gpu_shader_sels, ARRAY_SIZE(imx8mn_gpu_shader_sels));
hws[IMX8MN_CLK_A53_CG] = imx_clk_hw_gate3("arm_a53_cg", "arm_a53_src", base + 0x8000, 28);
- hws[IMX8MN_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg", "gpu_core_src", base + 0x8180, 28);
- hws[IMX8MN_CLK_GPU_SHADER_CG] = imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
-
hws[IMX8MN_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
- hws[IMX8MN_CLK_GPU_CORE_DIV] = imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
- hws[IMX8MN_CLK_GPU_SHADER_DIV] = imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0, 3);
+
+ hws[IMX8MN_CLK_GPU_CORE_DIV] = imx8m_clk_hw_composite_core("gpu_core_div", imx8mn_gpu_core_sels, base + 0x8180);
+ hws[IMX8MN_CLK_GPU_SHADER_DIV] = imx8m_clk_hw_composite_core("gpu_shader_div", imx8mn_gpu_shader_sels, base + 0x8200);

/* BUS */
hws[IMX8MN_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mn_main_axi_sels, base + 0x8800);
--
2.16.4

2020-01-13 21:50:11

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH V2 3/4] clk: imx: imx8mm: use imx8m_clk_hw_composite_core

On 10.01.2020 09:18, Peng Fan wrote:
> From: Peng Fan <[email protected]>
>
> Use imx8m_clk_hw_composite_core to simplify code.
>
> Reviewed-by: Abel Vesa <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>

This seems to break imx8mm-evk boot in next-20200113. Board boots fine
if I revert commit 15a8b30ba79f ("clk: imx: imx8mm: use
imx8m_clk_hw_composite_core").

It works on imx8mq-evk tough, not clear why this is happening.

> ---
> drivers/clk/imx/clk-imx8mm.c | 17 +++++------------
> 1 file changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
> index 2ed93fc25087..197ba2cdab7d 100644
> --- a/drivers/clk/imx/clk-imx8mm.c
> +++ b/drivers/clk/imx/clk-imx8mm.c
> @@ -414,20 +414,13 @@ static int imx8mm_clocks_probe(struct platform_device *pdev)
>
> /* Core Slice */
> hws[IMX8MM_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mm_a53_sels, ARRAY_SIZE(imx8mm_a53_sels));
> - hws[IMX8MM_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mm_m4_sels, ARRAY_SIZE(imx8mm_m4_sels));
> - hws[IMX8MM_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mm_vpu_sels, ARRAY_SIZE(imx8mm_vpu_sels));
> - hws[IMX8MM_CLK_GPU3D_SRC] = imx_clk_hw_mux2("gpu3d_src", base + 0x8180, 24, 3, imx8mm_gpu3d_sels, ARRAY_SIZE(imx8mm_gpu3d_sels));
> - hws[IMX8MM_CLK_GPU2D_SRC] = imx_clk_hw_mux2("gpu2d_src", base + 0x8200, 24, 3, imx8mm_gpu2d_sels, ARRAY_SIZE(imx8mm_gpu2d_sels));
> hws[IMX8MM_CLK_A53_CG] = imx_clk_hw_gate3("arm_a53_cg", "arm_a53_src", base + 0x8000, 28);
> - hws[IMX8MM_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
> - hws[IMX8MM_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
> - hws[IMX8MM_CLK_GPU3D_CG] = imx_clk_hw_gate3("gpu3d_cg", "gpu3d_src", base + 0x8180, 28);
> - hws[IMX8MM_CLK_GPU2D_CG] = imx_clk_hw_gate3("gpu2d_cg", "gpu2d_src", base + 0x8200, 28);
> hws[IMX8MM_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
> - hws[IMX8MM_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
> - hws[IMX8MM_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
> - hws[IMX8MM_CLK_GPU3D_DIV] = imx_clk_hw_divider2("gpu3d_div", "gpu3d_cg", base + 0x8180, 0, 3);
> - hws[IMX8MM_CLK_GPU2D_DIV] = imx_clk_hw_divider2("gpu2d_div", "gpu2d_cg", base + 0x8200, 0, 3);
> +
> + hws[IMX8MM_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mm_m4_sels, base + 0x8080);
> + hws[IMX8MM_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mm_vpu_sels, base + 0x8100);
> + hws[IMX8MM_CLK_GPU3D_DIV] = imx8m_clk_hw_composite_core("gpu3d_div", imx8mm_gpu3d_sels, base + 0x8180);
> + hws[IMX8MM_CLK_GPU2D_DIV] = imx8m_clk_hw_composite_core("gpu2d_div", imx8mm_gpu2d_sels, base + 0x8200);
>
> /* BUS */
> hws[IMX8MM_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mm_main_axi_sels, base + 0x8800);
>

2020-01-14 16:50:26

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH V2 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

On 10.01.2020 09:17, Peng Fan wrote:
> From: Peng Fan <[email protected]>
>
> Use imx8m_clk_hw_composite_core to simplify code.
>
> Reviewed-by: Abel Vesa <[email protected]>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
> 1 file changed, 5 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index 4c0edca1a6d0..b031183ff427 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -403,22 +403,13 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
>
> /* CORE */
> hws[IMX8MQ_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels));
> - hws[IMX8MQ_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mq_arm_m4_sels, ARRAY_SIZE(imx8mq_arm_m4_sels));
> - hws[IMX8MQ_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mq_vpu_sels, ARRAY_SIZE(imx8mq_vpu_sels));
> - hws[IMX8MQ_CLK_GPU_CORE_SRC] = imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3, imx8mq_gpu_core_sels, ARRAY_SIZE(imx8mq_gpu_core_sels));
> - hws[IMX8MQ_CLK_GPU_SHADER_SRC] = imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3, imx8mq_gpu_shader_sels, ARRAY_SIZE(imx8mq_gpu_shader_sels));
> -
> hws[IMX8MQ_CLK_A53_CG] = imx_clk_hw_gate3_flags("arm_a53_cg", "arm_a53_src", base + 0x8000, 28, CLK_IS_CRITICAL);
> - hws[IMX8MQ_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
> - hws[IMX8MQ_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
> - hws[IMX8MQ_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg", "gpu_core_src", base + 0x8180, 28);
> - hws[IMX8MQ_CLK_GPU_SHADER_CG] = imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
> -
> hws[IMX8MQ_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
> - hws[IMX8MQ_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
> - hws[IMX8MQ_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
> - hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
> - hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0, 3);
> +
> + hws[IMX8MQ_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mq_arm_m4_sels, base + 0x8080);
> + hws[IMX8MQ_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mq_vpu_sels, base + 0x8100);
> + hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx8m_clk_hw_composite_core("gpu_core_div", imx8mq_gpu_core_sels, base + 0x8180);
> + hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx8m_clk_hw_composite("gpu_shader_div", imx8mq_gpu_shader_sels, base + 0x8200);
>
> /* BUS */
> hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);

Collapsing _SRC _CG into _DIV is an useful simplification but it
technically breaks DT compatibility rules.

Inside imx8mq.dtsi there are clock assignments for
IMX8MQ_CLK_GPU_CORE_SRC and IMX8MQ_CLK_GPU_SHADER_SRC which no longer
exist so those assignments don't take effect.

I think patching dts would be reasonable fix.

--
Regards,
Leonard

2020-01-15 11:14:02

by Shawn Guo

[permalink] [raw]
Subject: Re: [PATCH V2 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

On Tue, Jan 14, 2020 at 04:49:20PM +0000, Leonard Crestez wrote:
> On 10.01.2020 09:17, Peng Fan wrote:
> > From: Peng Fan <[email protected]>
> >
> > Use imx8m_clk_hw_composite_core to simplify code.
> >
> > Reviewed-by: Abel Vesa <[email protected]>
> > Signed-off-by: Peng Fan <[email protected]>
> > ---
> > drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
> > 1 file changed, 5 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> > index 4c0edca1a6d0..b031183ff427 100644
> > --- a/drivers/clk/imx/clk-imx8mq.c
> > +++ b/drivers/clk/imx/clk-imx8mq.c
> > @@ -403,22 +403,13 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
> >
> > /* CORE */
> > hws[IMX8MQ_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels));
> > - hws[IMX8MQ_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mq_arm_m4_sels, ARRAY_SIZE(imx8mq_arm_m4_sels));
> > - hws[IMX8MQ_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mq_vpu_sels, ARRAY_SIZE(imx8mq_vpu_sels));
> > - hws[IMX8MQ_CLK_GPU_CORE_SRC] = imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3, imx8mq_gpu_core_sels, ARRAY_SIZE(imx8mq_gpu_core_sels));
> > - hws[IMX8MQ_CLK_GPU_SHADER_SRC] = imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3, imx8mq_gpu_shader_sels, ARRAY_SIZE(imx8mq_gpu_shader_sels));
> > -
> > hws[IMX8MQ_CLK_A53_CG] = imx_clk_hw_gate3_flags("arm_a53_cg", "arm_a53_src", base + 0x8000, 28, CLK_IS_CRITICAL);
> > - hws[IMX8MQ_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
> > - hws[IMX8MQ_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
> > - hws[IMX8MQ_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg", "gpu_core_src", base + 0x8180, 28);
> > - hws[IMX8MQ_CLK_GPU_SHADER_CG] = imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
> > -
> > hws[IMX8MQ_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
> > - hws[IMX8MQ_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
> > - hws[IMX8MQ_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
> > - hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
> > - hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0, 3);
> > +
> > + hws[IMX8MQ_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mq_arm_m4_sels, base + 0x8080);
> > + hws[IMX8MQ_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mq_vpu_sels, base + 0x8100);
> > + hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx8m_clk_hw_composite_core("gpu_core_div", imx8mq_gpu_core_sels, base + 0x8180);
> > + hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx8m_clk_hw_composite("gpu_shader_div", imx8mq_gpu_shader_sels, base + 0x8200);
> >
> > /* BUS */
> > hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);
>
> Collapsing _SRC _CG into _DIV is an useful simplification but it
> technically breaks DT compatibility rules.
>
> Inside imx8mq.dtsi there are clock assignments for
> IMX8MQ_CLK_GPU_CORE_SRC and IMX8MQ_CLK_GPU_SHADER_SRC which no longer
> exist so those assignments don't take effect.

We do not want to break existing DTBs for this case. Patches dropped.

Shawn

2020-01-15 11:49:27

by Leonard Crestez

[permalink] [raw]
Subject: Re: [PATCH V2 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

On 2020-01-15 1:12 PM, Shawn Guo wrote:
> On Tue, Jan 14, 2020 at 04:49:20PM +0000, Leonard Crestez wrote:
>> On 10.01.2020 09:17, Peng Fan wrote:
>>> From: Peng Fan <[email protected]>
>>>
>>> Use imx8m_clk_hw_composite_core to simplify code.
>>>
>>> Reviewed-by: Abel Vesa <[email protected]>
>>> Signed-off-by: Peng Fan <[email protected]>
>>> ---
>>> drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
>>> 1 file changed, 5 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
>>> index 4c0edca1a6d0..b031183ff427 100644
>>> --- a/drivers/clk/imx/clk-imx8mq.c
>>> +++ b/drivers/clk/imx/clk-imx8mq.c
>>> @@ -403,22 +403,13 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
>>>
>>> /* CORE */
>>> hws[IMX8MQ_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src", base + 0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels));
>>> - hws[IMX8MQ_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base + 0x8080, 24, 3, imx8mq_arm_m4_sels, ARRAY_SIZE(imx8mq_arm_m4_sels));
>>> - hws[IMX8MQ_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base + 0x8100, 24, 3, imx8mq_vpu_sels, ARRAY_SIZE(imx8mq_vpu_sels));
>>> - hws[IMX8MQ_CLK_GPU_CORE_SRC] = imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3, imx8mq_gpu_core_sels, ARRAY_SIZE(imx8mq_gpu_core_sels));
>>> - hws[IMX8MQ_CLK_GPU_SHADER_SRC] = imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3, imx8mq_gpu_shader_sels, ARRAY_SIZE(imx8mq_gpu_shader_sels));
>>> -
>>> hws[IMX8MQ_CLK_A53_CG] = imx_clk_hw_gate3_flags("arm_a53_cg", "arm_a53_src", base + 0x8000, 28, CLK_IS_CRITICAL);
>>> - hws[IMX8MQ_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg", "arm_m4_src", base + 0x8080, 28);
>>> - hws[IMX8MQ_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src", base + 0x8100, 28);
>>> - hws[IMX8MQ_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg", "gpu_core_src", base + 0x8180, 28);
>>> - hws[IMX8MQ_CLK_GPU_SHADER_CG] = imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
>>> -
>>> hws[IMX8MQ_CLK_A53_DIV] = imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
>>> - hws[IMX8MQ_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div", "arm_m4_cg", base + 0x8080, 0, 3);
>>> - hws[IMX8MQ_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div", "vpu_cg", base + 0x8100, 0, 3);
>>> - hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
>>> - hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0, 3);
>>> +
>>> + hws[IMX8MQ_CLK_M4_DIV] = imx8m_clk_hw_composite_core("arm_m4_div", imx8mq_arm_m4_sels, base + 0x8080);
>>> + hws[IMX8MQ_CLK_VPU_DIV] = imx8m_clk_hw_composite_core("vpu_div", imx8mq_vpu_sels, base + 0x8100);
>>> + hws[IMX8MQ_CLK_GPU_CORE_DIV] = imx8m_clk_hw_composite_core("gpu_core_div", imx8mq_gpu_core_sels, base + 0x8180);
>>> + hws[IMX8MQ_CLK_GPU_SHADER_DIV] = imx8m_clk_hw_composite("gpu_shader_div", imx8mq_gpu_shader_sels, base + 0x8200);
>>>
>>> /* BUS */
>>> hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);
>>
>> Collapsing _SRC _CG into _DIV is an useful simplification but it
>> technically breaks DT compatibility rules.
>>
>> Inside imx8mq.dtsi there are clock assignments for
>> IMX8MQ_CLK_GPU_CORE_SRC and IMX8MQ_CLK_GPU_SHADER_SRC which no longer
>> exist so those assignments don't take effect.
>
> We do not want to break existing DTBs for this case. Patches dropped.

Sorry. I think DT compatibility could be maintained by adding
assignments like this:

hws[IMX8MQ_CLK_GPU_CORE_SRC] = hws[IMX8MQ_CLK_GPU_CORE_DIV];

If all the old clocks are aliased to the new composite then all
currently valid set_rate and set_parent calls would still have the same
effect.

It would also mean that you can set_rate on the mux and set_parent on
the div but that should be harmless: an enhancement instead of a
compat-breaking change.

--
Regards,
Leonard

2020-01-16 02:31:34

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH V2 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

> Subject: Re: [PATCH V2 2/4] clk: imx: imx8mq: use
> imx8m_clk_hw_composite_core
>
> On Tue, Jan 14, 2020 at 04:49:20PM +0000, Leonard Crestez wrote:
> > On 10.01.2020 09:17, Peng Fan wrote:
> > > From: Peng Fan <[email protected]>
> > >
> > > Use imx8m_clk_hw_composite_core to simplify code.
> > >
> > > Reviewed-by: Abel Vesa <[email protected]>
> > > Signed-off-by: Peng Fan <[email protected]>
> > > ---
> > > drivers/clk/imx/clk-imx8mq.c | 19 +++++--------------
> > > 1 file changed, 5 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-imx8mq.c
> > > b/drivers/clk/imx/clk-imx8mq.c index 4c0edca1a6d0..b031183ff427
> > > 100644
> > > --- a/drivers/clk/imx/clk-imx8mq.c
> > > +++ b/drivers/clk/imx/clk-imx8mq.c
> > > @@ -403,22 +403,13 @@ static int imx8mq_clocks_probe(struct
> > > platform_device *pdev)
> > >
> > > /* CORE */
> > > hws[IMX8MQ_CLK_A53_SRC] = imx_clk_hw_mux2("arm_a53_src",
> base + 0x8000, 24, 3, imx8mq_a53_sels, ARRAY_SIZE(imx8mq_a53_sels));
> > > - hws[IMX8MQ_CLK_M4_SRC] = imx_clk_hw_mux2("arm_m4_src", base
> + 0x8080, 24, 3, imx8mq_arm_m4_sels, ARRAY_SIZE(imx8mq_arm_m4_sels));
> > > - hws[IMX8MQ_CLK_VPU_SRC] = imx_clk_hw_mux2("vpu_src", base +
> 0x8100, 24, 3, imx8mq_vpu_sels, ARRAY_SIZE(imx8mq_vpu_sels));
> > > - hws[IMX8MQ_CLK_GPU_CORE_SRC] =
> imx_clk_hw_mux2("gpu_core_src", base + 0x8180, 24, 3,
> imx8mq_gpu_core_sels, ARRAY_SIZE(imx8mq_gpu_core_sels));
> > > - hws[IMX8MQ_CLK_GPU_SHADER_SRC] =
> imx_clk_hw_mux2("gpu_shader_src", base + 0x8200, 24, 3,
> imx8mq_gpu_shader_sels, ARRAY_SIZE(imx8mq_gpu_shader_sels));
> > > -
> > > hws[IMX8MQ_CLK_A53_CG] =
> imx_clk_hw_gate3_flags("arm_a53_cg", "arm_a53_src", base + 0x8000, 28,
> CLK_IS_CRITICAL);
> > > - hws[IMX8MQ_CLK_M4_CG] = imx_clk_hw_gate3("arm_m4_cg",
> "arm_m4_src", base + 0x8080, 28);
> > > - hws[IMX8MQ_CLK_VPU_CG] = imx_clk_hw_gate3("vpu_cg", "vpu_src",
> base + 0x8100, 28);
> > > - hws[IMX8MQ_CLK_GPU_CORE_CG] = imx_clk_hw_gate3("gpu_core_cg",
> "gpu_core_src", base + 0x8180, 28);
> > > - hws[IMX8MQ_CLK_GPU_SHADER_CG] =
> imx_clk_hw_gate3("gpu_shader_cg", "gpu_shader_src", base + 0x8200, 28);
> > > -
> > > hws[IMX8MQ_CLK_A53_DIV] =
> imx_clk_hw_divider2("arm_a53_div", "arm_a53_cg", base + 0x8000, 0, 3);
> > > - hws[IMX8MQ_CLK_M4_DIV] = imx_clk_hw_divider2("arm_m4_div",
> "arm_m4_cg", base + 0x8080, 0, 3);
> > > - hws[IMX8MQ_CLK_VPU_DIV] = imx_clk_hw_divider2("vpu_div",
> "vpu_cg", base + 0x8100, 0, 3);
> > > - hws[IMX8MQ_CLK_GPU_CORE_DIV] =
> imx_clk_hw_divider2("gpu_core_div", "gpu_core_cg", base + 0x8180, 0, 3);
> > > - hws[IMX8MQ_CLK_GPU_SHADER_DIV] =
> imx_clk_hw_divider2("gpu_shader_div", "gpu_shader_cg", base + 0x8200, 0,
> 3);
> > > +
> > > + hws[IMX8MQ_CLK_M4_DIV] =
> imx8m_clk_hw_composite_core("arm_m4_div", imx8mq_arm_m4_sels, base
> + 0x8080);
> > > + hws[IMX8MQ_CLK_VPU_DIV] =
> imx8m_clk_hw_composite_core("vpu_div", imx8mq_vpu_sels, base +
> 0x8100);
> > > + hws[IMX8MQ_CLK_GPU_CORE_DIV] =
> imx8m_clk_hw_composite_core("gpu_core_div", imx8mq_gpu_core_sels,
> base + 0x8180);
> > > + hws[IMX8MQ_CLK_GPU_SHADER_DIV] =
> > > +imx8m_clk_hw_composite("gpu_shader_div", imx8mq_gpu_shader_sels,
> > > +base + 0x8200);
> > >
> > > /* BUS */
> > > hws[IMX8MQ_CLK_MAIN_AXI] =
> > > imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels,
> > > base + 0x8800);
> >
> > Collapsing _SRC _CG into _DIV is an useful simplification but it
> > technically breaks DT compatibility rules.
> >
> > Inside imx8mq.dtsi there are clock assignments for
> > IMX8MQ_CLK_GPU_CORE_SRC and IMX8MQ_CLK_GPU_SHADER_SRC
> which no longer
> > exist so those assignments don't take effect.
>
> We do not want to break existing DTBs for this case. Patches dropped.

I'll send a v3 to address the issues. Sorry.

Thanks,
Peng.

>
> Shawn