From: Peng Fan <[email protected]>
Leonard,
Please help review this V3 patchset.
V3:
Add CLK_SET_RATE_NO_REPARENT and CLK_OPS_PARENT_ENABLE for core
Avoid break DT for i.MX8MQ
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 | 22 ++++++++--------------
drivers/clk/imx/clk.h | 13 +++++++++++--
5 files changed, 41 insertions(+), 39 deletions(-)
--
2.16.4
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 | 22 ++++++++--------------
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4c0edca1a6d0..e928c1355ad8 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -403,22 +403,16 @@ 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);
+ /* For DTS which still assign parents for gpu core src clk */
+ hws[IMX8MQ_CLK_GPU_CORE_SRC] = hws[IMX8MQ_CLK_GPU_CORE_DIV];
+ hws[IMX8MQ_CLK_GPU_SHADER_SRC] = hws[IMX8MQ_CLK_GPU_SHADER_DIV];
/* BUS */
hws[IMX8MQ_CLK_MAIN_AXI] = imx8m_clk_hw_composite_critical("main_axi", imx8mq_main_axi_sels, base + 0x8800);
--
2.16.4
On 16.01.2020 04:15, 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 | 22 ++++++++--------------
> 1 file changed, 8 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
> index 4c0edca1a6d0..e928c1355ad8 100644
> --- a/drivers/clk/imx/clk-imx8mq.c
> +++ b/drivers/clk/imx/clk-imx8mq.c
> @@ -403,22 +403,16 @@ 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);
> + /* For DTS which still assign parents for gpu core src clk */
> + hws[IMX8MQ_CLK_GPU_CORE_SRC] = hws[IMX8MQ_CLK_GPU_CORE_DIV];
> + hws[IMX8MQ_CLK_GPU_SHADER_SRC] = hws[IMX8MQ_CLK_GPU_SHADER_DIV];
Why not assign to all the old clocks?
> Subject: Re: [PATCH V3 2/4] clk: imx: imx8mq: use
> imx8m_clk_hw_composite_core
>
> On 16.01.2020 04:15, 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 | 22 ++++++++--------------
> > 1 file changed, 8 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mq.c
> > b/drivers/clk/imx/clk-imx8mq.c index 4c0edca1a6d0..e928c1355ad8
> 100644
> > --- a/drivers/clk/imx/clk-imx8mq.c
> > +++ b/drivers/clk/imx/clk-imx8mq.c
> > @@ -403,22 +403,16 @@ 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);
>
> > + /* For DTS which still assign parents for gpu core src clk */
> > + hws[IMX8MQ_CLK_GPU_CORE_SRC] =
> hws[IMX8MQ_CLK_GPU_CORE_DIV];
> > + hws[IMX8MQ_CLK_GPU_SHADER_SRC] =
> hws[IMX8MQ_CLK_GPU_SHADER_DIV];
>
> Why not assign to all the old clocks?
Are those clocks expect the GPU ones needed?
Currently only the gpu clocks are needed, others are not used in dts.
For dts update to use the SRC clocks should be avoided in future for Linux,
DIV clocks should be used.
How do you think?
Thanks,
Peng.
On 27.01.2020 07:00, Peng Fan wrote:
>> Subject: Re: [PATCH V3 2/4] clk: imx: imx8mq: use
>> imx8m_clk_hw_composite_core
>>
>> On 16.01.2020 04:15, 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 | 22 ++++++++--------------
>>> 1 file changed, 8 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/drivers/clk/imx/clk-imx8mq.c
>>> b/drivers/clk/imx/clk-imx8mq.c index 4c0edca1a6d0..e928c1355ad8
>> 100644
>>> --- a/drivers/clk/imx/clk-imx8mq.c
>>> +++ b/drivers/clk/imx/clk-imx8mq.c
>>> @@ -403,22 +403,16 @@ 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);
>>
>>> + /* For DTS which still assign parents for gpu core src clk */
>>> + hws[IMX8MQ_CLK_GPU_CORE_SRC] =
>> hws[IMX8MQ_CLK_GPU_CORE_DIV];
>>> + hws[IMX8MQ_CLK_GPU_SHADER_SRC] =
>> hws[IMX8MQ_CLK_GPU_SHADER_DIV];
>>
>> Why not assign to all the old clocks?
>
> Are those clocks expect the GPU ones needed?
>
> Currently only the gpu clocks are needed, others are not used in dts.
>
> For dts update to use the SRC clocks should be avoided in future for Linux,
> DIV clocks should be used.
>
> How do you think?
In theory backwards compatibility should be supported at the level of
"DT bindings", not just hacked for a particular DT. In this case it's
easy to add aliases for everything, just slightly verbose.
It might also make sense to add new defines for the composite clock so
that IMX8MQ_CLK_GPU_CORE looks like IMX8MQ_CLK_DSI_CORE and all the _SRC
_DIV _CG stuff is just aliases to the unsuffixed composite.
--
Regards,
Leonard