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