[PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
Emil Medve
Emilian.Medve at Freescale.com
Wed Jan 21 20:58:31 AEDT 2015
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.Medve at Freescale.com]
>> Sent: Wednesday, January 21, 2015 5:03 PM
>> To: Tang Yuantian-B29983; linuxppc-dev at lists.ozlabs.org; Wood Scott-B07421;
>> mturquette at linaro.org; haokexin at 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.Medve at Freescale.com]
>>>> Sent: Wednesday, January 21, 2015 4:20 PM
>>>> To: Tang Yuantian-B29983; linuxppc-dev at lists.ozlabs.org; Wood
>>>> Scott-B07421; mturquette at linaro.org; haokexin at 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.Medve at Freescale.com]
>>>>>> Sent: Tuesday, January 20, 2015 6:10 PM
>>>>>> To: linuxppc-dev at lists.ozlabs.org; Wood Scott-B07421;
>>>>>> mturquette at linaro.org; haokexin at 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.Medve at 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
More information about the Linuxppc-dev
mailing list