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
| ||
|
Date: Thu, 21 Feb 2019 13:04:04 -0800 From: Stephen Boyd <sboyd@...nel.org> To: Abel Vesa <abel.vesa@....com>, Aisheng Dong <aisheng.dong@....com>, Anson Huang <anson.huang@....com>, Fabio Estevam <fabio.estevam@....com>, Fabio Estevam <festevam@...il.com>, Jacky Bai <ping.bai@....com>, Lucas Stach <l.stach@...gutronix.de>, Mike Turquette <mturquette@...libre.com>, Sascha Hauer <kernel@...gutronix.de>, Shawn Guo <shawnguo@...nel.org> Cc: dl-linux-imx <linux-imx@....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 Mailing List <linux-kernel@...r.kernel.org>, Abel Vesa <abel.vesa@....com> Subject: Re: [RESEND] clk: imx: Refactor entire sccg pll clk Quoting Abel Vesa (2019-02-18 01:24:33) > Make the entire combination of plls to be one single clock. The parents used > for bypasses are specified each as an index in the parents list. > The determine_rate does a lookup throughout all the possible combinations > for all the divs and returns the best possible 'setup' which in turn is used > by set_rate later to set up all the divs and bypasses. > > Signed-off-by: Abel Vesa <abel.vesa@....com> > Tested-by: Lucas Stach <l.stach@...gutronix.de> > Acked-by: Lucas Stach <l.stach@...gutronix.de> > --- > > This is basically just a resend of the following: > https://lkml.org/lkml/2019/1/29/874 > > There are no changes. Just added T-b and A-b tags from Lucas. > > Waiting for review. Spoke too soon. I've kicked this out of the tree. This doesn't compile and spews a bunch of sparse warnings on arm32. Presumably because the use of unsigned long was assuming 64 bits in size? Please write portable code and fix these sparse warnings. Probably all the unsigned long usage should be u64? drivers/clk/imx/clk-sccg-pll.c:142:53: warning: decimal constant 2400000000 is between LONG_MAX and ULONG_MAX. For C99 that means long long, C90 compilers are very likely to produce unsigned long (and a warning) here drivers/clk/imx/clk-sccg-pll.c:199:48: warning: decimal constant 2400000000 is between LONG_MAX and ULONG_MAX. For C99 that means long long, C90 compilers are very likely to produce unsigned long (and a warning) here drivers/clk/imx/clk-sccg-pll.c:138:17: error: incompatible types in comparison expression (different type sizes) drivers/clk/imx/clk-sccg-pll.c:144:25: error: incompatible types in comparison expression (different type sizes) drivers/clk/imx/clk-sccg-pll.c:180:17: error: incompatible types in comparison expression (different type sizes) drivers/clk/imx/clk-sccg-pll.c:217:17: error: incompatible types in comparison expression (different type sizes) drivers/clk/imx/clk-sccg-pll.c:239:17: error: incompatible types in comparison expression (different type sizes) drivers/clk/imx/clk-sccg-pll.c:138:17: warning: shift too big (32) for type unsigned long drivers/clk/imx/clk-sccg-pll.c:144:25: warning: shift too big (32) for type unsigned long drivers/clk/imx/clk-sccg-pll.c:180:17: warning: shift too big (32) for type unsigned long drivers/clk/imx/clk-sccg-pll.c:217:17: warning: shift too big (32) for type long drivers/clk/imx/clk-sccg-pll.c:239:17: warning: shift too big (32) for type unsigned long In file included from arch/arm/include/asm/div64.h:127:0, from include/linux/kernel.h:207, from include/asm-generic/bug.h:18, from arch/arm/include/asm/bug.h:60, from include/linux/bug.h:5, from include/linux/io.h:23, from include/linux/clk-provider.h:9, from drivers/clk/imx/clk-sccg-pll.c:11: drivers/clk/imx/clk-sccg-pll.c: In function 'clk_sccg_divq_lookup': include/asm-generic/div64.h:222:28: warning: comparison of distinct pointer types lacks a cast (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ ^ > @@ -347,14 +319,12 @@ static int imx8mq_clocks_probe(struct platform_device *pdev) > clks[IMX8MQ_AUDIO_PLL2_BYPASS] = imx_clk_mux("audio_pll2_bypass", base + 0x8, 14, 1, audio_pll2_bypass_sels, ARRAY_SIZE(audio_pll2_bypass_sels)); > clks[IMX8MQ_VIDEO_PLL1_BYPASS] = imx_clk_mux("video_pll1_bypass", base + 0x10, 14, 1, video_pll1_bypass_sels, ARRAY_SIZE(video_pll1_bypass_sels)); > > - clks[IMX8MQ_SYS1_PLL1_OUT] = imx_clk_mux("sys1_pll1_out", base + 0x30, 5, 1, sys1_pll1_out_sels, ARRAY_SIZE(sys1_pll1_out_sels)); > - clks[IMX8MQ_SYS2_PLL1_OUT] = imx_clk_mux("sys2_pll1_out", base + 0x3c, 5, 1, sys2_pll1_out_sels, ARRAY_SIZE(sys2_pll1_out_sels)); > - clks[IMX8MQ_SYS3_PLL1_OUT] = imx_clk_mux("sys3_pll1_out", base + 0x48, 5, 1, sys3_pll1_out_sels, ARRAY_SIZE(sys3_pll1_out_sels)); > - clks[IMX8MQ_DRAM_PLL1_OUT] = imx_clk_mux("dram_pll1_out", base + 0x60, 5, 1, dram_pll1_out_sels, ARRAY_SIZE(dram_pll1_out_sels)); > - clks[IMX8MQ_SYS1_PLL2_OUT] = imx_clk_mux("sys1_pll2_out", base + 0x30, 4, 1, sys1_pll2_out_sels, ARRAY_SIZE(sys1_pll2_out_sels)); > - clks[IMX8MQ_SYS2_PLL2_OUT] = imx_clk_mux("sys2_pll2_out", base + 0x3c, 4, 1, sys2_pll2_out_sels, ARRAY_SIZE(sys2_pll2_out_sels)); > - clks[IMX8MQ_SYS3_PLL2_OUT] = imx_clk_mux("sys3_pll2_out", base + 0x48, 4, 1, sys3_pll2_out_sels, ARRAY_SIZE(sys3_pll2_out_sels)); > - clks[IMX8MQ_DRAM_PLL2_OUT] = imx_clk_mux("dram_pll2_out", base + 0x60, 4, 1, dram_pll2_out_sels, ARRAY_SIZE(dram_pll2_out_sels)); > + /* unbypass all the plls */ > + clk_set_parent(clks[IMX8MQ_GPU_PLL_BYPASS], clks[IMX8MQ_GPU_PLL]); > + clk_set_parent(clks[IMX8MQ_VPU_PLL_BYPASS], clks[IMX8MQ_VPU_PLL]); > + clk_set_parent(clks[IMX8MQ_AUDIO_PLL1_BYPASS], clks[IMX8MQ_AUDIO_PLL1]); > + clk_set_parent(clks[IMX8MQ_AUDIO_PLL2_BYPASS], clks[IMX8MQ_AUDIO_PLL2]); > + clk_set_parent(clks[IMX8MQ_VIDEO_PLL1_BYPASS], clks[IMX8MQ_VIDEO_PLL1]); Why? Can you just write the hardware registers directly instead of going through the clk APIs during this clk registration routine? While it's obviously easier to just call consumer APIs, it makes it weird because this is a clk provider driver which 1) shouldn't use the consumer APIs and 2) makes probe ordering and other things complicated to figure out. Plus one more extra, it means that changing this code to use the clk_hw APIs is harder. > > /* PLL OUT GATE */ > clks[IMX8MQ_ARM_PLL_OUT] = imx_clk_gate("arm_pll_out", "arm_pll_bypass", base + 0x28, 21);
Powered by blists - more mailing lists