linux-kernel - RE: [PATCH V2 2/4] clk: imx: imx8mn: Switch to clk

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: <AM0PR04MB4481F5F07FF493E75CFD1747884C0@AM0PR04MB4481.eurprd04.prod.outlook.com>
Date:   Tue, 19 Nov 2019 01:06:46 +0000
From:   Peng Fan <peng.fan@....com>
To:     Leonard Crestez <leonard.crestez@....com>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>
CC:     "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>,
        Anson Huang <anson.huang@....com>,
        Jacky Bai <ping.bai@....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>
Subject: RE: [PATCH V2 2/4] clk: imx: imx8mn: Switch to clk_hw based API

> Subject: Re: [PATCH V2 2/4] clk: imx: imx8mn: Switch to clk_hw based API
> 
> On 2019-11-04 7:46 AM, Peng Fan wrote:
> > From: Peng Fan <peng.fan@....com>
> >
> > Switch the entire clk-imx8mn driver to clk_hw based API.
> > This allows us to move closer to a clear split between consumer and
> > provider clk APIs.
> 
> > -	clks[IMX8MN_AUDIO_PLL1] = imx_clk_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx_1443x_pll);
> > -	clks[IMX8MN_AUDIO_PLL2] = imx_clk_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx_1443x_pll);
> > -	clks[IMX8MN_VIDEO_PLL1] = imx_clk_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx_1443x_pll);
> > -	clks[IMX8MN_DRAM_PLL] = imx_clk_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx_1443x_pll);
> > -	clks[IMX8MN_GPU_PLL] = imx_clk_pll14xx("gpu_pll", "gpu_pll_ref_sel",
> base + 0x64, &imx_1416x_pll);
> > -	clks[IMX8MN_VPU_PLL] = imx_clk_pll14xx("vpu_pll", "vpu_pll_ref_sel",
> base + 0x74, &imx_1416x_pll);
> > -	clks[IMX8MN_ARM_PLL] = imx_clk_pll14xx("arm_pll", "arm_pll_ref_sel",
> base + 0x84, &imx_1416x_pll);
> > -	clks[IMX8MN_SYS_PLL1] = imx_clk_fixed("sys_pll1", 800000000);
> > -	clks[IMX8MN_SYS_PLL2] = imx_clk_fixed("sys_pll2", 1000000000);
> > -	clks[IMX8MN_SYS_PLL3] = imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel",
> base + 0x114, &imx_1416x_pll);
> > +	clks[IMX8MN_AUDIO_PLL1] = imx_clk_hw_pll14xx("audio_pll1",
> "audio_pll1_ref_sel", base, &imx_1416x_pll);
> > +	clks[IMX8MN_AUDIO_PLL2] = imx_clk_hw_pll14xx("audio_pll2",
> "audio_pll2_ref_sel", base + 0x14, &imx_1416x_pll);
> > +	clks[IMX8MN_VIDEO_PLL1] = imx_clk_hw_pll14xx("video_pll1",
> "video_pll1_ref_sel", base + 0x28, &imx_1416x_pll);
> > +	clks[IMX8MN_DRAM_PLL] = imx_clk_hw_pll14xx("dram_pll",
> "dram_pll_ref_sel", base + 0x50, &imx_1416x_pll);
> > +	clks[IMX8MN_GPU_PLL] = imx_clk_hw_pll14xx("gpu_pll",
> "gpu_pll_ref_sel", base + 0x64, &imx_1416x_pll);
> > +	clks[IMX8MN_VPU_PLL] = imx_clk_hw_pll14xx("vpu_pll",
> "vpu_pll_ref_sel", base + 0x74, &imx_1416x_pll);
> > +	clks[IMX8MN_ARM_PLL] = imx_clk_hw_pll14xx("arm_pll",
> "arm_pll_ref_sel", base + 0x84, &imx_1416x_pll);
> > +	clks[IMX8MN_SYS_PLL1] = imx_clk_hw_fixed("sys_pll1", 800000000);
> > +	clks[IMX8MN_SYS_PLL2] = imx_clk_hw_fixed("sys_pll2", 1000000000);
> > +	clks[IMX8MN_SYS_PLL3] = imx_clk_hw_pll14xx("sys_pll3",
> > +"sys_pll3_ref_sel", base + 0x114, &imx_1416x_pll);
> 
> You are switching audio/video/dram PLL from imx_1443x_pll to
> imx_1416x_pll, are you sure this is correct?

That is a mistaken, copy/paste error. I'll fix in V3.

> 
> If this is intentional it should be an separate patch.
> 
> > -	clks[IMX8MN_CLK_ARM] = imx_clk_cpu("arm", "arm_a53_div",
> > -					   clks[IMX8MN_CLK_A53_DIV],
> > -					   clks[IMX8MN_CLK_A53_SRC],
> > -					   clks[IMX8MN_ARM_PLL_OUT],
> > -					   clks[IMX8MN_CLK_24M]);
> 
> > +	clks[IMX8MN_CLK_ARM] = imx_clk_hw_cpu("arm", "arm_a53_div",
> > +					      clks[IMX8MN_CLK_A53_DIV]->clk,
> > +					      clks[IMX8MN_CLK_A53_SRC]->clk,
> > +					      clks[IMX8MN_ARM_PLL_OUT]->clk,
> > +					      clks[IMX8MN_CLK_24M]->clk);
> 
> This series seems to be against Shawn's clk/imx but there is an additional
> patch in Stephen's tree which changes this 24M to PLL1_800M.
> Maybe that should be pulled into clk/imx? Otherwise it might spawn an
> unreadable merge conflicts since almost the entire file is rewritten.

Yes.

Thanks,
Peng.

> 
> --
> Regards,
> Leonard