On 24/10/14 13:07, Chanwoo Choi wrote: > This patch adds the new clock driver of Exynos4415 SoC based on Cortex-A9 > using common clock framework. The CMU (Clock Management Unit) of Exynos4415 > controls PLLs(Phase Locked Loops) and generates system clocks for CPU, buses > and function clocks for individual IPs. > > Cc: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> > Cc: Tomasz Figa <tomasz.figa@xxxxxxxxx> > Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx> > Signed-off-by: Tomasz Figa <tomasz.figa@xxxxxxxxx> > Signed-off-by: Seung-Woo Kim <sw0312.kim@xxxxxxxxxxx> > Acked-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx> Thanks for the update, there are still couple issues pointed out by checkpatch.pl unfortunately, please see below. Please fix the errors, I added also some more comments inline below. In future please put DT binding documentation patch first in the series, before the actual driver patch. WARNING: kfree(NULL) is safe this check is probably not required #252: FILE: drivers/clk/samsung/clk-exynos4415.c:252: + if (clk_regs) + kfree(clk_regs); ERROR: space required after that ',' (ctx:VxV) #423: FILE: drivers/clk/samsung/clk-exynos4415.c:423: + 0,4), ^ WARNING: line over 80 characters #726: FILE: drivers/clk/samsung/clk-exynos4415.c:726: + "div_pxlasync_csis0_fimc", GATE_SCLK_CAM, 10, CLK_SET_RATE_PARENT, 0), WARNING: line over 80 characters #817: FILE: drivers/clk/samsung/clk-exynos4415.c:817: + GATE(CLK_SMMUFIMC_LITE2, "smmufimc_lite2", "div_aclk_160", GATE_IP_CAM, 22, WARNING: line over 80 characters #875: FILE: drivers/clk/samsung/clk-exynos4415.c:875: + GATE(CLK_USBDEVICE, "usbdevice", "div_aclk_200", GATE_IP_FSYS, 13, 0, 0), ERROR: space prohibited after that open parenthesis '(' #920: FILE: drivers/clk/samsung/clk-exynos4415.c:920: + PLL_35XX_RATE( 960000000, 320, 4, 1), ERROR: space prohibited after that open parenthesis '(' #921: FILE: drivers/clk/samsung/clk-exynos4415.c:921: + PLL_35XX_RATE( 900000000, 300, 4, 1), ERROR: space prohibited after that open parenthesis '(' #922: FILE: drivers/clk/samsung/clk-exynos4415.c:922: + PLL_35XX_RATE( 850000000, 425, 6, 1), ERROR: space prohibited after that open parenthesis '(' #923: FILE: drivers/clk/samsung/clk-exynos4415.c:923: + PLL_35XX_RATE( 800000000, 200, 3, 1), ERROR: space prohibited after that open parenthesis '(' #924: FILE: drivers/clk/samsung/clk-exynos4415.c:924: + PLL_35XX_RATE( 700000000, 175, 3, 1), ERROR: space prohibited after that open parenthesis '(' #925: FILE: drivers/clk/samsung/clk-exynos4415.c:925: + PLL_35XX_RATE( 667000000, 667, 12, 1), ERROR: space prohibited after that open parenthesis '(' #926: FILE: drivers/clk/samsung/clk-exynos4415.c:926: + PLL_35XX_RATE( 600000000, 400, 4, 2), ERROR: space prohibited after that open parenthesis '(' #927: FILE: drivers/clk/samsung/clk-exynos4415.c:927: + PLL_35XX_RATE( 550000000, 275, 3, 2), ERROR: space prohibited after that open parenthesis '(' #928: FILE: drivers/clk/samsung/clk-exynos4415.c:928: + PLL_35XX_RATE( 533000000, 533, 6, 2), ERROR: space prohibited after that open parenthesis '(' #929: FILE: drivers/clk/samsung/clk-exynos4415.c:929: + PLL_35XX_RATE( 520000000, 260, 3, 2), ERROR: space prohibited after that open parenthesis '(' #930: FILE: drivers/clk/samsung/clk-exynos4415.c:930: + PLL_35XX_RATE( 500000000, 250, 3, 2), ERROR: space prohibited after that open parenthesis '(' #931: FILE: drivers/clk/samsung/clk-exynos4415.c:931: + PLL_35XX_RATE( 440000000, 220, 3, 2), ERROR: space prohibited after that open parenthesis '(' #932: FILE: drivers/clk/samsung/clk-exynos4415.c:932: + PLL_35XX_RATE( 400000000, 200, 3, 2), ERROR: space prohibited after that open parenthesis '(' #933: FILE: drivers/clk/samsung/clk-exynos4415.c:933: + PLL_35XX_RATE( 350000000, 175, 3, 2), ERROR: space prohibited after that open parenthesis '(' #934: FILE: drivers/clk/samsung/clk-exynos4415.c:934: + PLL_35XX_RATE( 300000000, 300, 3, 3), #935: FILE: drivers/clk/samsung/clk-exynos4415.c:935: + PLL_35XX_RATE( 266000000, 266, 3, 3), ERROR: space prohibited after that open parenthesis '(' #936: FILE: drivers/clk/samsung/clk-exynos4415.c:936: + PLL_35XX_RATE( 200000000, 200, 3, 3), ERROR: space prohibited after that open parenthesis '(' #937: FILE: drivers/clk/samsung/clk-exynos4415.c:937: + PLL_35XX_RATE( 160000000, 160, 3, 3), ERROR: space prohibited after that open parenthesis '(' #938: FILE: drivers/clk/samsung/clk-exynos4415.c:938: + PLL_35XX_RATE( 100000000, 200, 3, 4), ERROR: space prohibited after that open parenthesis '(' #948: FILE: drivers/clk/samsung/clk-exynos4415.c:948: + PLL_36XX_RATE( 96000000, 128, 2, 4, 0), ERROR: space prohibited after that open parenthesis '(' #949: FILE: drivers/clk/samsung/clk-exynos4415.c:949: + PLL_36XX_RATE( 84000000, 112, 2, 4, 0), ERROR: space prohibited after that open parenthesis '(' #950: FILE: drivers/clk/samsung/clk-exynos4415.c:950: + PLL_36XX_RATE( 80750011, 107, 2, 4, 43691), ERROR: space prohibited after that open parenthesis '(' #951: FILE: drivers/clk/samsung/clk-exynos4415.c:951: + PLL_36XX_RATE( 73728004, 98, 2, 4, 19923), ERROR: space prohibited after that open parenthesis '(' #952: FILE: drivers/clk/samsung/clk-exynos4415.c:952: + PLL_36XX_RATE( 67987602, 271, 3, 5, 62285), ERROR: space prohibited after that open parenthesis '(' #953: FILE: drivers/clk/samsung/clk-exynos4415.c:953: + PLL_36XX_RATE( 65911004, 175, 2, 5, 49982), ERROR: space prohibited after that open parenthesis '(' #954: FILE: drivers/clk/samsung/clk-exynos4415.c:954: + PLL_36XX_RATE( 50000000, 200, 3, 5, 0), ERROR: space prohibited after that open parenthesis '(' #955: FILE: drivers/clk/samsung/clk-exynos4415.c:955: + PLL_36XX_RATE( 49152003, 131, 2, 5, 4719), ERROR: space prohibited after that open parenthesis '(' #956: FILE: drivers/clk/samsung/clk-exynos4415.c:956: + PLL_36XX_RATE( 48000000, 128, 2, 5, 0), ERROR: space prohibited after that open parenthesis '(' #957: FILE: drivers/clk/samsung/clk-exynos4415.c:957: + PLL_36XX_RATE( 45250000, 181, 3, 5, 0), total: 30 errors, 4 warnings, 1133 lines checked drivers/clk/samsung/clk-exynos4415.c has style problems, please review. > --- > drivers/clk/samsung/Makefile | 1 + > drivers/clk/samsung/clk-exynos4415.c | 1133 ++++++++++++++++++++++++++++++++ > include/dt-bindings/clock/exynos4415.h | 360 ++++++++++ > 3 files changed, 1494 insertions(+) > create mode 100644 drivers/clk/samsung/clk-exynos4415.c > create mode 100644 include/dt-bindings/clock/exynos4415.h > diff --git a/drivers/clk/samsung/clk-exynos4415.c b/drivers/clk/samsung/clk-exynos4415.c > new file mode 100644 > index 0000000..a4b6211 > --- /dev/null > +++ b/drivers/clk/samsung/clk-exynos4415.c > @@ -0,0 +1,1133 @@ > +static struct samsung_clk_reg_dump *clk_regs; > +static struct samsung_clk_provider *ctx; > + > +static unsigned long cmu_clk_regs[] __initdata = { > +}; > + > +static int exynos_clk_suspend(void) Please prefix such function (and above variable) names with exynos4415, as is done for other SoC drivers. > +{ > + samsung_clk_save(ctx->reg_base, clk_regs, ARRAY_SIZE(cmu_clk_regs)); > + > + return 0; > +} > + > +static void exynos_clk_resume(void) exynos4415_clk_resume ? > +{ > + samsung_clk_restore(ctx->reg_base, clk_regs, ARRAY_SIZE(cmu_clk_regs)); > +} > + > +static struct syscore_ops exynos_clk_syscore_ops = { exynos4415_clk_syscore_ops ? > + .suspend = exynos_clk_suspend, > + .resume = exynos_clk_resume, > +}; > + > +static void exynos_clk_sleep_init(void) exynos4415_clk_sleep_init ? > +{ > + clk_regs = samsung_clk_alloc_reg_dump(cmu_clk_regs, > + ARRAY_SIZE(cmu_clk_regs)); > + if (!clk_regs) { > + pr_warn("%s: Failed to allocate sleep save data\n", __func__); > + goto err; You could just return here. > + } > + > + register_syscore_ops(&exynos_clk_syscore_ops); > + > + return; > +err: > + if (clk_regs) > + kfree(clk_regs); kfree() can be removed (and the condition could never be true anyway). > +} > +#else > +static inline void exynos_clk_sleep_init(void) { } exynos4415_clk_sleep_init ? > +#endif How about prefixing the table names below with "exynos4415", rather than "samsung" ? > +static struct samsung_fixed_factor_clock fixed_factor_clks[] __initdata = { > +}; > + > +static struct samsung_fixed_rate_clock fixed_rate_clks[] __initdata = { > + FRATE(CLK_SCLK_HDMIPHY, "sclk_hdmiphy", NULL, CLK_IS_ROOT, 27000000), > +}; > + > +static struct samsung_mux_clock mux_clks[] __initdata = { > +}; > + > +static struct samsung_div_clock div_clks[] __initdata = { > +}; > + > +static struct samsung_gate_clock gate_clks[] __initdata = { > +}; > + > +/* > + * APLL & MPLL & BPLL & ISP_PLL & DISP_PLL & G3D_PLL > + */ > +static struct samsung_pll_rate_table exynos4415_pll_rates[] = { > + PLL_35XX_RATE(1600000000, 400, 3, 1), > + PLL_35XX_RATE(1500000000, 250, 2, 1), > + PLL_35XX_RATE(1400000000, 175, 3, 0), > + PLL_35XX_RATE(1300000000, 325, 3, 1), > + PLL_35XX_RATE(1200000000, 400, 4, 1), > + PLL_35XX_RATE(1100000000, 275, 3, 1), > + PLL_35XX_RATE(1066000000, 533, 6, 1), > + PLL_35XX_RATE(1000000000, 250, 3, 1), > + PLL_35XX_RATE( 960000000, 320, 4, 1), > + PLL_35XX_RATE( 900000000, 300, 4, 1), > + PLL_35XX_RATE( 850000000, 425, 6, 1), > + PLL_35XX_RATE( 800000000, 200, 3, 1), > + PLL_35XX_RATE( 700000000, 175, 3, 1), > + PLL_35XX_RATE( 667000000, 667, 12, 1), > + PLL_35XX_RATE( 600000000, 400, 4, 2), > + PLL_35XX_RATE( 550000000, 275, 3, 2), > + PLL_35XX_RATE( 533000000, 533, 6, 2), > + PLL_35XX_RATE( 520000000, 260, 3, 2), > + PLL_35XX_RATE( 500000000, 250, 3, 2), > + PLL_35XX_RATE( 440000000, 220, 3, 2), > + PLL_35XX_RATE( 400000000, 200, 3, 2), > + PLL_35XX_RATE( 350000000, 175, 3, 2), > + PLL_35XX_RATE( 300000000, 300, 3, 3), > + PLL_35XX_RATE( 266000000, 266, 3, 3), > + PLL_35XX_RATE( 200000000, 200, 3, 3), > + PLL_35XX_RATE( 160000000, 160, 3, 3), > + PLL_35XX_RATE( 100000000, 200, 3, 4), > + { /* sentinel */ } > +}; > + > +/* EPLL */ > +static struct samsung_pll_rate_table exynos4415_epll_rates[] = { > + PLL_36XX_RATE(800000000, 200, 3, 1, 0), > + PLL_36XX_RATE(288000000, 96, 2, 2, 0), > + PLL_36XX_RATE(192000000, 128, 2, 3, 0), > + PLL_36XX_RATE(144000000, 96, 2, 3, 0), > + PLL_36XX_RATE( 96000000, 128, 2, 4, 0), > + PLL_36XX_RATE( 84000000, 112, 2, 4, 0), > + PLL_36XX_RATE( 80750011, 107, 2, 4, 43691), > + PLL_36XX_RATE( 73728004, 98, 2, 4, 19923), > + PLL_36XX_RATE( 67987602, 271, 3, 5, 62285), > + PLL_36XX_RATE( 65911004, 175, 2, 5, 49982), > + PLL_36XX_RATE( 50000000, 200, 3, 5, 0), > + PLL_36XX_RATE( 49152003, 131, 2, 5, 4719), > + PLL_36XX_RATE( 48000000, 128, 2, 5, 0), > + PLL_36XX_RATE( 45250000, 181, 3, 5, 0), > + { /* sentinel */ } > +}; > + > +static struct samsung_pll_clock plls[nr_plls] __initdata = { > +}; > + > +static void __init exynos4415_cmu_init(struct device_node *np) > +{ > +} > +CLK_OF_DECLARE(exynos4415_cmu, "samsung,exynos4415-cmu", exynos4415_cmu_init); > + > +/* > + * CMU DMC > + */ > + > +#ifdef CONFIG_PM_SLEEP Similarly for dmc, can we have function, table and below two static variable names prefixed with exynos4415, like in exynos3250 driver ? > +static struct samsung_clk_reg_dump *dmc_clk_regs; > +static struct samsung_clk_provider *dmc_ctx; > + > +static unsigned long dmc_save_list[] __initdata = { > +}; > + > +static int dmc_clk_suspend(void) > +{ > + samsung_clk_save(dmc_ctx->reg_base, > + dmc_clk_regs, ARRAY_SIZE(dmc_save_list)); > + return 0; > +} > + > +static void dmc_clk_resume(void) > +{ > + samsung_clk_restore(dmc_ctx->reg_base, > + dmc_clk_regs, ARRAY_SIZE(dmc_save_list)); > +} > + > +static struct syscore_ops dmc_clk_syscore_ops = { > + .suspend = dmc_clk_suspend, > + .resume = dmc_clk_resume, > +}; > + > +static void dmc_clk_sleep_init(void) > +{ > +} > +#else > +static inline void dmc_clk_sleep_init(void) { } > +#endif /* CONFIG_PM_SLEEP */ > + > +static struct samsung_mux_clock dmc_mux_clks[] __initdata = { > +}; > + > +static struct samsung_div_clock dmc_div_clks[] __initdata = { > +}; > + > +static struct samsung_pll_clock dmc_plls[nr_dmc_plls] __initdata = { > +}; -- Thanks, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html