[PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL

These patches are based on:

http://patchwork.ozlabs.org/patch/417297
http://patchwork.ozlabs.org/patch/417295
http://patchwork.ozlabs.org/patch/417292

The first 5 patches below are checkpatch/static analysis fixes

Emil Medve (8):
  clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT
  clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY
  clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE
  clk: ppc-corenet: Make local symbol 'static'
  clk: ppc-corenet: Replace kzalloc() with kmalloc()
  powerpc/corenet: Enable CLK_PPC_CORENET
  clk: ppc-corenet: Add support for the platform PLL

 arch/powerpc/configs/corenet32_smp_defconfig |   1 +
 arch/powerpc/configs/corenet64_smp_defconfig |   1 +
 drivers/clk/clk-ppc-corenet.c                | 120 ++++++++++++++++++++++-----
 3 files changed, 101 insertions(+), 21 deletions(-)

-- 
2.2.2

CHECK:ALLOC_SIZEOF_STRUCT: Prefer kzalloc(sizeof(*cmux_clk)...) over kzalloc(sizeof(struct cmux_clk)...)
+       cmux_clk = kzalloc(sizeof(struct cmux_clk), GFP_KERNEL);

CHECK:ALLOC_SIZEOF_STRUCT: Prefer kzalloc(sizeof(*onecell_data)...) over kzalloc(sizeof(struct clk_onecell_data)...)
+       onecell_data = kzalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index efa74aa..a439f52 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -94,7 +94,7 @@ static void __init core_mux_init(struct device_node *np)
 	for (i = 0; i < count; i++)
 		parent_names[i] = of_clk_get_parent_name(np, i);
 
-	cmux_clk = kzalloc(sizeof(struct cmux_clk), GFP_KERNEL);
+	cmux_clk = kzalloc(sizeof(*cmux_clk), GFP_KERNEL);
 	if (!cmux_clk) {
 		pr_err("%s: could not allocate cmux_clk\n", __func__);
 		goto err_name;

@@ -190,7 +190,7 @@ static void __init core_pll_init(struct device_node *np)
 		goto err_map;
 	}
 
-	onecell_data = kzalloc(sizeof(struct clk_onecell_data), GFP_KERNEL);
+	onecell_data = kzalloc(sizeof(*onecell_data), GFP_KERNEL);
 	if (!onecell_data) {
 		pr_err("%s: could not allocate onecell_data\n", __func__);
 		goto err_clks;

-- 
2.2.2

drivers/clk/clk-ppc-corenet.c:63:22: warning: symbol 'cmux_ops' was not declared. Should it be static?

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index 6a9dbaa..d84a7f0 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -57,7 +57,7 @@ static u8 cmux_get_parent(struct clk_hw *hw)
 	return clksel;
 }
 
-const struct clk_ops cmux_ops = {
+static const struct clk_ops cmux_ops = {
 	.get_parent = cmux_get_parent,
 	.set_parent = cmux_set_parent,
 };

-- 
2.2.2

WARNING:ALLOC_WITH_MULTIPLY: Prefer kcalloc over kzalloc with multiply
+       subclks = kzalloc(sizeof(struct clk *) * count, GFP_KERNEL);

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index 7cb8f23..efa74aa 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -85,7 +85,7 @@ static void __init core_mux_init(struct device_node *np)
 		pr_err("%s: get clock count error\n", np->name);
 		return;
 	}
-	parent_names = kzalloc((sizeof(char *) * count), GFP_KERNEL);
+	parent_names = kcalloc(count, sizeof(char *), GFP_KERNEL);
 	if (!parent_names) {
 		pr_err("%s: could not allocate parent_names\n", __func__);
 		return;

@@ -184,7 +184,7 @@ static void __init core_pll_init(struct device_node *np)
 	/* output clock number per PLL */
 	clocks_per_pll = count;
 
-	subclks = kzalloc(sizeof(struct clk *) * count, GFP_KERNEL);
+	subclks = kcalloc(count, sizeof(struct clk *), GFP_KERNEL);
 	if (!subclks) {
 		pr_err("%s: could not allocate subclks\n", __func__);
 		goto err_map;

-- 
2.2.2

Change-Id: I1a80ad7b9f6854791bd270b746f93a91439155a6
Signed-off-by: Emil Medve <redacted>
---
 arch/powerpc/configs/corenet32_smp_defconfig | 1 +
 arch/powerpc/configs/corenet64_smp_defconfig | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/powerpc/configs/corenet32_smp_defconfig b/arch/powerpc/configs/corenet32_smp_defconfig
index 611efe9..93c38d2 100644
--- a/arch/powerpc/configs/corenet32_smp_defconfig
+++ b/arch/powerpc/configs/corenet32_smp_defconfig

@@ -147,6 +147,7 @@ CONFIG_STAGING=y
 CONFIG_MEMORY=y
 CONFIG_VIRT_DRIVERS=y
 CONFIG_FSL_HV_MANAGER=y
+CONFIG_CLK_PPC_CORENET=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 # CONFIG_EXT3_DEFAULTS_TO_ORDERED is not set

diff --git a/arch/powerpc/configs/corenet64_smp_defconfig b/arch/powerpc/configs/corenet64_smp_defconfig
index be24a18..b9d75ae 100644
--- a/arch/powerpc/configs/corenet64_smp_defconfig
+++ b/arch/powerpc/configs/corenet64_smp_defconfig

@@ -119,6 +119,7 @@ CONFIG_VIRT_DRIVERS=y
 CONFIG_FSL_HV_MANAGER=y
 CONFIG_FSL_CORENET_CF=y
 CONFIG_MEMORY=y
+CONFIG_CLK_PPC_CORENET=y
 CONFIG_EXT2_FS=y
 CONFIG_EXT3_FS=y
 CONFIG_ISO9660_FS=m

-- 
2.2.2

Where the memset() is not necessary

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index d84a7f0..91816b1 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -185,7 +185,7 @@ static void __init core_pll_init(struct device_node *np)
 	if (!subclks)
 		goto err_map;
 
-	onecell_data = kzalloc(sizeof(*onecell_data), GFP_KERNEL);
+	onecell_data = kmalloc(sizeof(*onecell_data), GFP_KERNEL);
 	if (!onecell_data)
 		goto err_clks;
 

-- 
2.2.2

WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
+       if (!parent_names) {
+               pr_err("%s: could not allocate parent_names\n", __func__);

WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
+       if (!cmux_clk) {
+               pr_err("%s: could not allocate cmux_clk\n", __func__);

WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
+       if (!subclks) {
+               pr_err("%s: could not allocate subclks\n", __func__);

WARNING:OOM_MESSAGE: Possible unnecessary 'out of memory' message
+       if (!onecell_data) {
+               pr_err("%s: could not allocate onecell_data\n", __func__);

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index a439f52..6a9dbaa 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -86,19 +86,16 @@ static void __init core_mux_init(struct device_node *np)
 		return;
 	}
 	parent_names = kcalloc(count, sizeof(char *), GFP_KERNEL);
-	if (!parent_names) {
-		pr_err("%s: could not allocate parent_names\n", __func__);
+	if (!parent_names)
 		return;
-	}
 
 	for (i = 0; i < count; i++)
 		parent_names[i] = of_clk_get_parent_name(np, i);
 
 	cmux_clk = kzalloc(sizeof(*cmux_clk), GFP_KERNEL);
-	if (!cmux_clk) {
-		pr_err("%s: could not allocate cmux_clk\n", __func__);
+	if (!cmux_clk)
 		goto err_name;
-	}
+
 	cmux_clk->reg = of_iomap(np, 0);
 	if (!cmux_clk->reg) {
 		pr_err("%s: could not map register\n", __func__);

@@ -185,16 +182,12 @@ static void __init core_pll_init(struct device_node *np)
 	clocks_per_pll = count;
 
 	subclks = kcalloc(count, sizeof(struct clk *), GFP_KERNEL);
-	if (!subclks) {
-		pr_err("%s: could not allocate subclks\n", __func__);
+	if (!subclks)
 		goto err_map;
-	}
 
 	onecell_data = kzalloc(sizeof(*onecell_data), GFP_KERNEL);
-	if (!onecell_data) {
-		pr_err("%s: could not allocate onecell_data\n", __func__);
+	if (!onecell_data)
 		goto err_clks;
-	}
 
 	for (i = 0; i < count; i++) {
 		rc = of_property_read_string_index(np, "clock-output-names",

-- 
2.2.2

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+       rc = of_property_read_string_index(np, "clock-output-names",
+                       0, &clk_name);

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+               pr_err("Could not register clock provider for node:%s\n",
+                        np->name);

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+               rc = of_property_read_string_index(np, "clock-output-names",
+                               i, &clk_name);

CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
+               pr_err("Could not register clk provider for node:%s\n",
+                        np->name);

Signed-off-by: Emil Medve <redacted>
---
 drivers/clk/clk-ppc-corenet.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c
index bf0fe56..7cb8f23 100644
--- a/drivers/clk/clk-ppc-corenet.c
+++ b/drivers/clk/clk-ppc-corenet.c

@@ -110,7 +110,7 @@ static void __init core_mux_init(struct device_node *np)
 		cmux_clk->flags = CLKSEL_ADJUST;
 
 	rc = of_property_read_string_index(np, "clock-output-names",
-			0, &clk_name);
+					   0, &clk_name);
 	if (rc) {
 		pr_err("%s: read clock names error\n", np->name);
 		goto err_clk;

@@ -132,7 +132,7 @@ static void __init core_mux_init(struct device_node *np)
 	rc = of_clk_add_provider(np, of_clk_src_simple_get, clk);
 	if (rc) {
 		pr_err("Could not register clock provider for node:%s\n",
-			 np->name);
+		       np->name);
 		goto err_clk;
 	}
 	goto err_name;

@@ -198,7 +198,7 @@ static void __init core_pll_init(struct device_node *np)
 
 	for (i = 0; i < count; i++) {
 		rc = of_property_read_string_index(np, "clock-output-names",
-				i, &clk_name);
+						   i, &clk_name);
 		if (rc) {
 			pr_err("%s: could not get clock names\n", np->name);
 			goto err_cell;

@@ -230,7 +230,7 @@ static void __init core_pll_init(struct device_node *np)
 	rc = of_clk_add_provider(np, of_clk_src_onecell_get, onecell_data);
 	if (rc) {
 		pr_err("Could not register clk provider for node:%s\n",
-			 np->name);
+		       np->name);
 		goto err_cell;
 	}
 

-- 
2.2.2

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)
 		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

Hi Emil,

Thanks for fixing those warnings. The patch set you based on is merged.
I sent another two patches and one of them got merged. You probably need to=
 rebase your patches.
My patch link:
http://patchwork.ozlabs.org/patch/429257/
http://patchwork.ozlabs.org/patch/429258/

Thanks,
Yuantian

-----Original Message-----
From: Emil Medve [mailto:Emilian.Medve@Freescale.com]
Sent: Tuesday, January 20, 2015 6:09 PM
To: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; mturquette@linaro.o=

rg;

haokexin@gmail.com; Tang Yuantian-B29983
Cc: Medve Emilian-EMMEDVE1
Subject: [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL
=20
These patches are based on:
=20
http://patchwork.ozlabs.org/patch/417297
http://patchwork.ozlabs.org/patch/417295
http://patchwork.ozlabs.org/patch/417292
=20
The first 5 patches below are checkpatch/static analysis fixes
=20
Emil Medve (8):
  clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT
  clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY
  clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE
  clk: ppc-corenet: Make local symbol 'static'
  clk: ppc-corenet: Replace kzalloc() with kmalloc()
  powerpc/corenet: Enable CLK_PPC_CORENET
  clk: ppc-corenet: Add support for the platform PLL
=20
 arch/powerpc/configs/corenet32_smp_defconfig |   1 +
 arch/powerpc/configs/corenet64_smp_defconfig |   1 +
 drivers/clk/clk-ppc-corenet.c                | 120
++++++++++++++++++++++-----
 3 files changed, 101 insertions(+), 21 deletions(-)
=20
--
2.2.2

Which platform are you trying to use this on? Can this be initialized by co=
re pll function core_pll_init()?
I just saw most of this function is silimar to the core_pll_init().

Thanks,
Yuantian

-----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.o=

rg;

c index

ode)

 		of_clk_add_provider(np, of_clk_src_simple_get, clk);  }
=20
+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_KERNEL);
+	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",
+						       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

Hello Yuan-Tian,


On 01/20/2015 07:40 PM, Tang Yuantian-B29983 wrote:

Hi Emil,

Thanks for fixing those warnings. The patch set you based on is merged.
I sent another two patches and one of them got merged. You probably need to rebase your patches.
My patch link:
http://patchwork.ozlabs.org/patch/429257/

I'll rebase on top of this and resubmit


Cheers,

http://patchwork.ozlabs.org/patch/429258/

Thanks,
Yuantian

quoted

-----Original Message-----
From: Emil Medve [mailto:Emilian.Medve@Freescale.com]
Sent: Tuesday, January 20, 2015 6:09 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 0/8] clk: ppc-corenet: Add support for the platform PLL

These patches are based on:

http://patchwork.ozlabs.org/patch/417297
http://patchwork.ozlabs.org/patch/417295
http://patchwork.ozlabs.org/patch/417292

The first 5 patches below are checkpatch/static analysis fixes

Emil Medve (8):
  clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT
  clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY
  clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE
  clk: ppc-corenet: Make local symbol 'static'
  clk: ppc-corenet: Replace kzalloc() with kmalloc()
  powerpc/corenet: Enable CLK_PPC_CORENET
  clk: ppc-corenet: Add support for the platform PLL

 arch/powerpc/configs/corenet32_smp_defconfig |   1 +
 arch/powerpc/configs/corenet64_smp_defconfig |   1 +
 drivers/clk/clk-ppc-corenet.c                | 120
++++++++++++++++++++++-----
 3 files changed, 101 insertions(+), 21 deletions(-)

--
2.2.2

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

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)
 		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

Hi Emil,

I don't think it is the best to add a function that is very similar to exis=
ting one.=20
If you think the function name is not appropriate, rename it.

Thanks,
Yuantian

-----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-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/20/2015 11:42 PM, Tang Yuantian-B29983 wrote:

quoted

Which platform are you trying to use this on?

=20
CoreNet chassis v1 and v2 SoC(s)
=20

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().

=20
Yes, the flow is similar, but core_pll_init() makes assumptions that it s=

houldn't or

are not relevant to the platform PLL
=20
=20
Cheers,
=20
=20

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_KERNEL)=

;

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",
+						       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

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

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 = 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

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

Hello Yuan-Tian,


On 01/21/2015 03:37 AM, Tang Yuantian-B29983 wrote:

Hello Emil,

quoted

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

quoted

-----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-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/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.
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,

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 = 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",

quoted

quoted

quoted

quoted

+						       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

On Tue, 2015-01-20 at 04:09 -0600, Emil Medve wrote:

Change-Id: I1a80ad7b9f6854791bd270b746f93a91439155a6
Signed-off-by: Emil Medve <redacted>

No Change-Id, and don't bundle patches meant for my tree in the same
patchset as patches meant for other trees.  There's no dependency
between them.

-Scott

On Tue, 2015-01-20 at 04:09 -0600, Emil Medve wrote:

I think it's better to use kzalloc always, outside of
performance-sensitive allocations.  E.g. what if a new field is added to
the struct later?

-Scott