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?

If there did have some features that current function didn't contain, can w=
e expend the current one to include it?

BTW: if you did need this, please update the binding as well, if any:
Documentation/devicetree/bindings/clock/qoriq-clock.txt

Thanks,
Yuantian

-----Original Message-----
From: Emil Medve [mailto:Emilian.Medve@Freescale.com]
Sent: Wednesday, January 21, 2015 5:03 PM
To: Tang Yuantian-B29983; linuxppc-dev@lists.ozlabs.org; Wood Scott-B0742=

1;

mturquette@linaro.org; haokexin@gmail.com
Subject: Re: [PATCH 8/8] clk: ppc-corenet: Add support for the platform P=

LL

=20
Hello Yuan-Tian,
=20
=20
On 01/21/2015 02:35 AM, Tang Yuantian-B29983 wrote:

quoted

Hi Emil,

I don't think it is the best to add a function that is very similar to =

existing one.

quoted

If you think the function name is not appropriate, rename it.

=20
It's not a naming matter. As I said, core_pll_init() assumptions and deci=

sions

based on the number of clocks in the DT. My hunch is some of these assump=

tions

are not necessary and/or should be explicit based on the node/device comp=

atible.

Having a standalone platform PLL initialization function wasn't a unilate=

ral lack of

foresight
=20
=20
Cheers,
=20
=20

quoted

Thanks,
Yuantian

quoted

-----Original Message-----
From: Emil Medve [mailto:Emilian.Medve@Freescale.com]
Sent: Wednesday, January 21, 2015 4:20 PM
To: Tang Yuantian-B29983; linuxppc-dev@lists.ozlabs.org; Wood
Scott-B07421; mturquette@linaro.org; haokexin@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:

quoted

Which platform are you trying to use this on?

CoreNet chassis v1 and v2 SoC(s)

quoted

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,

quoted

Thanks,
Yuantian

quoted

-----Original Message-----
From: Emil Medve [mailto:Emilian.Medve@Freescale.com]
Sent: Tuesday, January 20, 2015 6:10 PM
To: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421;
mturquette@linaro.org; haokexin@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 <redacted>
---
 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)

quoted

quoted

 		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 =3D of_iomap(np, 0);
+	if (!base) {
+		pr_err("%s(): %s: of_iomap() failed\n", __func__, np->name);
+		return;
+	}
+
+	/* Get the multiple of PLL */
+	mult =3D 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 =3D (mult & GENMASK(6, 1)) >> 1;
+
+	parent_name =3D 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 =3D of_property_count_strings(np, "clock-output-names");
+	if (i < 0) {
+		pr_err("%s(): %s: of_property_count_strings(clock-output-names)
=3D %d\n",
+		       __func__, np->name, i);
+		return;
+	}
+
+	cod =3D kmalloc(sizeof(*cod) + i * sizeof(struct clk *), GFP_KERNE=

L);

quoted

quoted

quoted

quoted

+	if (!cod)
+		return;
+	cod->clks =3D (struct clk **)(cod + 1);
+	cod->clk_num =3D i;
+
+	for (i =3D 0; i < cod->clk_num; i++) {
+		_errno =3D of_property_read_string_index(np,

"clock-output-names",

quoted

quoted

quoted

quoted

+						       i, &clk_name);
+		if (_errno < 0) {
+			pr_err("%s(): %s:
of_property_read_string_index(clock-output-names) =3D %d\n",
+			       __func__, np->name, _errno);
+			goto return_clk_unregister;
+		}
+
+		cod->clks[i] =3D 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) =3D %ld\n",
+			       __func__, np->name,
+			       clk_name, PTR_ERR(cod->clks[i]));
+			goto return_clk_unregister;
+		}
+	}
+
+	_errno =3D of_clk_add_provider(np, of_clk_src_onecell_get, cod);
+	if (_errno < 0) {
+		pr_err("%s(): %s: of_clk_add_provider() =3D %d\n",
+		       __func__, np->name, _errno);
+		goto return_clk_unregister;
+	}
+
+	return;
+
+return_clk_unregister:
+	while (--i >=3D 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