linux-kernel - Re: [PATCH V3 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <VI1PR04MB7023DD8E3D07543FB8F5CD50EE0B0@VI1PR04MB7023.eurprd04.prod.outlook.com>
Date:   Mon, 27 Jan 2020 20:33:10 +0000
From:   Leonard Crestez <leonard.crestez@....com>
To:     Peng Fan <peng.fan@....com>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
CC:     "sboyd@...nel.org" <sboyd@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "festevam@...il.com" <festevam@...il.com>,
        Abel Vesa <abel.vesa@....com>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        dl-linux-imx <linux-imx@....com>,
        Aisheng Dong <aisheng.dong@....com>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Anson Huang <anson.huang@....com>,
        Jacky Bai <ping.bai@....com>,
        "l.stach@...gutronix.de" <l.stach@...gutronix.de>
Subject: Re: [PATCH V3 2/4] clk: imx: imx8mq: use imx8m_clk_hw_composite_core

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 <peng.fan@....com>
>>>
>>> Use imx8m_clk_hw_composite_core to simplify code.
>>>
>>> Reviewed-by: Abel Vesa <abel.vesa@....com>
>>> Signed-off-by: Peng Fan <peng.fan@....com>
>>> ---
>>>    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