Hello Yuan-Tian,
On 01/21/2015 03:37 AM, Tang Yuantian-B29983 wrote: > Hello Emil, > >>From the RM and your code, I didn't see any difference between the two type >>of PLL. > Could you provide some use cases or feature that prove this is necessary? I said the DT makes the core_pll_init() code stand out. Yes, the register layout is identical, but that's not the entire story about the place of these PLL(s) in the SoC clocking hierarchy/tree > If there did have some features that current function didn't > contain, can we expend the current one to include it? Once core_pll_init() gets fixed/cleaned up somebody should look into unifying *_pll_init() > BTW: if you did need this, please update the binding as well, if any: > Documentation/devicetree/bindings/clock/qoriq-clock.txt The platform PLL binding is already there Cheers, > Thanks, > Yuantian > >> -----Original Message----- >> From: Emil Medve [mailto:emilian.me...@freescale.com] >> Sent: Wednesday, January 21, 2015 5:03 PM >> To: Tang Yuantian-B29983; linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; >> mturque...@linaro.org; haoke...@gmail.com >> Subject: Re: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL >> >> Hello Yuan-Tian, >> >> >> On 01/21/2015 02:35 AM, Tang Yuantian-B29983 wrote: >>> Hi Emil, >>> >>> I don't think it is the best to add a function that is very similar to >>> existing one. >>> If you think the function name is not appropriate, rename it. >> >> It's not a naming matter. As I said, core_pll_init() assumptions and >> decisions >> based on the number of clocks in the DT. My hunch is some of these >> assumptions >> are not necessary and/or should be explicit based on the node/device >> compatible. >> Having a standalone platform PLL initialization function wasn't a unilateral >> lack of >> foresight >> >> >> Cheers, >> >> >>> Thanks, >>> Yuantian >>> >>>> -----Original Message----- >>>> From: Emil Medve [mailto:emilian.me...@freescale.com] >>>> Sent: Wednesday, January 21, 2015 4:20 PM >>>> To: Tang Yuantian-B29983; linuxppc-dev@lists.ozlabs.org; Wood >>>> Scott-B07421; mturque...@linaro.org; haoke...@gmail.com >>>> Subject: Re: [PATCH 8/8] clk: ppc-corenet: Add support for the >>>> platform PLL >>>> >>>> Hello Yuan-Tian, >>>> >>>> >>>> On 01/20/2015 11:42 PM, Tang Yuantian-B29983 wrote: >>>>> Which platform are you trying to use this on? >>>> >>>> CoreNet chassis v1 and v2 SoC(s) >>>> >>>>> Can this be initialized by core pll function core_pll_init()? >>>>> I just saw most of this function is silimar to the core_pll_init(). >>>> >>>> Yes, the flow is similar, but core_pll_init() makes assumptions that >>>> it shouldn't or are not relevant to the platform PLL >>>> >>>> >>>> Cheers, >>>> >>>> >>>>> Thanks, >>>>> Yuantian >>>>> >>>>>> -----Original Message----- >>>>>> From: Emil Medve [mailto:emilian.me...@freescale.com] >>>>>> Sent: Tuesday, January 20, 2015 6:10 PM >>>>>> To: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; >>>>>> mturque...@linaro.org; haoke...@gmail.com; Tang Yuantian-B29983 >>>>>> Cc: Medve Emilian-EMMEDVE1 >>>>>> Subject: [PATCH 8/8] clk: ppc-corenet: Add support for the platform >>>>>> PLL >>>>>> >>>>>> Change-Id: Iac11ed95f274485a86d2c11f32a3dc502bcd020f >>>>>> Signed-off-by: Emil Medve <emilian.me...@freescale.com> >>>>>> --- >>>>>> drivers/clk/clk-ppc-corenet.c | 85 >>>>>> +++++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 85 insertions(+) >>>>>> >>>>>> diff --git a/drivers/clk/clk-ppc-corenet.c >>>>>> b/drivers/clk/clk-ppc-corenet.c index >>>>>> 91816b1..ff425e1 100644 >>>>>> --- a/drivers/clk/clk-ppc-corenet.c >>>>>> +++ b/drivers/clk/clk-ppc-corenet.c >>>>>> @@ -7,6 +7,9 @@ >>>>>> * >>>>>> * clock driver for Freescale PowerPC corenet SoCs. >>>>>> */ >>>>>> + >>>>>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >>>>>> + >>>>>> #include <linux/clk-provider.h> >>>>>> #include <linux/io.h> >>>>>> #include <linux/kernel.h> >>>>>> @@ -261,9 +264,91 @@ static void __init sysclk_init(struct >>>>>> device_node >>>> *node) >>>>>> of_clk_add_provider(np, of_clk_src_simple_get, clk); } >>>>>> >>>>>> +static void __init pltfrm_pll_init(struct device_node *np) { >>>>>> + void __iomem *base; >>>>>> + uint32_t mult; >>>>>> + const char *parent_name, *clk_name; >>>>>> + int i, _errno; >>>>>> + struct clk_onecell_data *cod; >>>>>> + >>>>>> + base = of_iomap(np, 0); >>>>>> + if (!base) { >>>>>> + pr_err("%s(): %s: of_iomap() failed\n", __func__, >>>>>> np->name); >>>>>> + return; >>>>>> + } >>>>>> + >>>>>> + /* Get the multiple of PLL */ >>>>>> + mult = ioread32be(base); >>>>>> + >>>>>> + iounmap(base); >>>>>> + >>>>>> + /* Check if this PLL is disabled */ >>>>>> + if (mult & PLL_KILL) { >>>>>> + pr_debug("%s(): %s: Disabled\n", __func__, np->name); >>>>>> + return; >>>>>> + } >>>>>> + mult = (mult & GENMASK(6, 1)) >> 1; >>>>>> + >>>>>> + parent_name = of_clk_get_parent_name(np, 0); >>>>>> + if (!parent_name) { >>>>>> + pr_err("%s(): %s: of_clk_get_parent_name() failed\n", >>>>>> + __func__, np->name); >>>>>> + return; >>>>>> + } >>>>>> + >>>>>> + i = of_property_count_strings(np, "clock-output-names"); >>>>>> + if (i < 0) { >>>>>> + pr_err("%s(): %s: >>>>>> of_property_count_strings(clock-output-names) >>>>>> = %d\n", >>>>>> + __func__, np->name, i); >>>>>> + return; >>>>>> + } >>>>>> + >>>>>> + cod = kmalloc(sizeof(*cod) + i * sizeof(struct clk *), >>>>>> GFP_KERNEL); >>>>>> + if (!cod) >>>>>> + return; >>>>>> + cod->clks = (struct clk **)(cod + 1); >>>>>> + cod->clk_num = i; >>>>>> + >>>>>> + for (i = 0; i < cod->clk_num; i++) { >>>>>> + _errno = of_property_read_string_index(np, >> "clock-output-names", >>>>>> + i, &clk_name); >>>>>> + if (_errno < 0) { >>>>>> + pr_err("%s(): %s: >>>>>> of_property_read_string_index(clock-output-names) = %d\n", >>>>>> + __func__, np->name, _errno); >>>>>> + goto return_clk_unregister; >>>>>> + } >>>>>> + >>>>>> + cod->clks[i] = clk_register_fixed_factor(NULL, clk_name, >>>>>> + parent_name, 0, mult, 1 >>>>>> + i); >>>>>> + if (IS_ERR(cod->clks[i])) { >>>>>> + pr_err("%s(): %s: clk_register_fixed_factor(%s) >>>>>> = %ld\n", >>>>>> + __func__, np->name, >>>>>> + clk_name, PTR_ERR(cod->clks[i])); >>>>>> + goto return_clk_unregister; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + _errno = of_clk_add_provider(np, of_clk_src_onecell_get, cod); >>>>>> + if (_errno < 0) { >>>>>> + pr_err("%s(): %s: of_clk_add_provider() = %d\n", >>>>>> + __func__, np->name, _errno); >>>>>> + goto return_clk_unregister; >>>>>> + } >>>>>> + >>>>>> + return; >>>>>> + >>>>>> +return_clk_unregister: >>>>>> + while (--i >= 0) >>>>>> + clk_unregister(cod->clks[i]); >>>>>> + kfree(cod); >>>>>> +} >>>>>> + >>>>>> CLK_OF_DECLARE(qoriq_sysclk_1, "fsl,qoriq-sysclk-1.0", >>>>>> sysclk_init); CLK_OF_DECLARE(qoriq_sysclk_2, >>>>>> "fsl,qoriq-sysclk-2.0", sysclk_init); >>>>>> CLK_OF_DECLARE(qoriq_core_pll_1, "fsl,qoriq-core-pll-1.0", >>>>>> core_pll_init); CLK_OF_DECLARE(qoriq_core_pll_2, >>>>>> "fsl,qoriq-core-pll-2.0", core_pll_init); >>>>>> CLK_OF_DECLARE(qoriq_core_mux_1, "fsl,qoriq-core-mux-1.0", >>>>>> core_mux_init); CLK_OF_DECLARE(qoriq_core_mux_2, >>>>>> "fsl,qoriq-core-mux-2.0", core_mux_init); >>>>>> +CLK_OF_DECLARE(qoriq_pltfrm_pll_1, "fsl,qoriq-platform-pll-1.0", >>>>>> +pltfrm_pll_init); CLK_OF_DECLARE(qoriq_pltfrm_pll_2, >>>>>> +"fsl,qoriq-platform-pll-2.0", pltfrm_pll_init); >>>>>> -- >>>>>> 2.2.2 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
