linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL
@ 2015-01-20 10:09 Emil Medve
  2015-01-20 10:09 ` [PATCH 1/8] clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT Emil Medve
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/8] clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-20 10:09 ` [PATCH 2/8] clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY Emil Medve
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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 <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/8] clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
  2015-01-20 10:09 ` [PATCH 1/8] clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-20 10:09 ` [PATCH 3/8] clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT Emil Medve
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/8] clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
  2015-01-20 10:09 ` [PATCH 1/8] clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT Emil Medve
  2015-01-20 10:09 ` [PATCH 2/8] clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-20 10:09 ` [PATCH 4/8] clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE Emil Medve
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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 <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/8] clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (2 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 3/8] clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-20 10:09 ` [PATCH 5/8] clk: ppc-corenet: Make local symbol 'static' Emil Medve
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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 <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/8] clk: ppc-corenet: Make local symbol 'static'
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (3 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 4/8] clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-20 10:09 ` [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc() Emil Medve
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

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

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc()
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (4 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 5/8] clk: ppc-corenet: Make local symbol 'static' Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-02-25  1:17   ` Scott Wood
  2015-01-20 10:09 ` [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET Emil Medve
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

Where the memset() is not necessary

Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (5 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc() Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-02-25  1:14   ` Scott Wood
  2015-01-20 10:09 ` [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
  2015-01-21  1:40 ` [PATCH 0/8] " Yuantian Tang
  8 siblings, 1 reply; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

Change-Id: I1a80ad7b9f6854791bd270b746f93a91439155a6
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (6 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET Emil Medve
@ 2015-01-20 10:09 ` Emil Medve
  2015-01-21  5:42   ` Yuantian Tang
  2015-01-21  1:40 ` [PATCH 0/8] " Yuantian Tang
  8 siblings, 1 reply; 19+ messages in thread
From: Emil Medve @ 2015-01-20 10:09 UTC (permalink / raw)
  To: linuxppc-dev, scottwood, mturquette, haokexin, yuantian.tang; +Cc: Emil Medve

Change-Id: Iac11ed95f274485a86d2c11f32a3dc502bcd020f
Signed-off-by: Emil Medve <Emilian.Medve@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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* RE: [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
                   ` (7 preceding siblings ...)
  2015-01-20 10:09 ` [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
@ 2015-01-21  1:40 ` Yuantian Tang
  2015-01-21  6:24   ` Emil Medve
  8 siblings, 1 reply; 19+ messages in thread
From: Yuantian Tang @ 2015-01-21  1:40 UTC (permalink / raw)
  To: Emilian Medve, linuxppc-dev@lists.ozlabs.org, Scott Wood,
	mturquette@linaro.org, haokexin@gmail.com
  Cc: Emilian Medve

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-20 10:09 ` [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
@ 2015-01-21  5:42   ` Yuantian Tang
  2015-01-21  8:20     ` Emil Medve
  0 siblings, 1 reply; 19+ messages in thread
From: Yuantian Tang @ 2015-01-21  5:42 UTC (permalink / raw)
  To: Emilian Medve, linuxppc-dev@lists.ozlabs.org, Scott Wood,
	mturquette@linaro.org, haokexin@gmail.com
  Cc: Emilian Medve

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;
> haokexin@gmail.com; Tang Yuantian-B29983
> Cc: Medve Emilian-EMMEDVE1
> Subject: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
>=20
> Change-Id: Iac11ed95f274485a86d2c11f32a3dc502bcd020f
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> ---
>  drivers/clk/clk-ppc-corenet.c | 85
> +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
>=20
> 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 *n=
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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  1:40 ` [PATCH 0/8] " Yuantian Tang
@ 2015-01-21  6:24   ` Emil Medve
  0 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-21  6:24 UTC (permalink / raw)
  To: Tang Yuantian-B29983, linuxppc-dev@lists.ozlabs.org,
	Wood Scott-B07421, mturquette@linaro.org, haokexin@gmail.com

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  5:42   ` Yuantian Tang
@ 2015-01-21  8:20     ` Emil Medve
  2015-01-21  8:35       ` Yuantian Tang
  0 siblings, 1 reply; 19+ messages in thread
From: Emil Medve @ 2015-01-21  8:20 UTC (permalink / raw)
  To: Tang Yuantian-B29983, linuxppc-dev@lists.ozlabs.org,
	Wood Scott-B07421, mturquette@linaro.org, haokexin@gmail.com

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@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 <Emilian.Medve@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  8:20     ` Emil Medve
@ 2015-01-21  8:35       ` Yuantian Tang
  2015-01-21  9:02         ` Emil Medve
  0 siblings, 1 reply; 19+ messages in thread
From: Yuantian Tang @ 2015-01-21  8:35 UTC (permalink / raw)
  To: Emilian Medve, linuxppc-dev@lists.ozlabs.org, Scott Wood,
	mturquette@linaro.org, haokexin@gmail.com

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:
> > Which platform are you trying to use this on?
>=20
> CoreNet chassis v1 and v2 SoC(s)
>=20
> > 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
> > 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.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 <Emilian.Medve@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 =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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  8:35       ` Yuantian Tang
@ 2015-01-21  9:02         ` Emil Medve
  2015-01-21  9:37           ` Yuantian Tang
  0 siblings, 1 reply; 19+ messages in thread
From: Emil Medve @ 2015-01-21  9:02 UTC (permalink / raw)
  To: Tang Yuantian-B29983, linuxppc-dev@lists.ozlabs.org,
	Wood Scott-B07421, mturquette@linaro.org, haokexin@gmail.com

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@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:
>>> 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@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 <Emilian.Medve@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  9:02         ` Emil Medve
@ 2015-01-21  9:37           ` Yuantian Tang
  2015-01-21  9:58             ` Emil Medve
  0 siblings, 1 reply; 19+ messages in thread
From: Yuantian Tang @ 2015-01-21  9:37 UTC (permalink / raw)
  To: Emilian Medve, linuxppc-dev@lists.ozlabs.org, Scott Wood,
	mturquette@linaro.org, haokexin@gmail.com

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:
> > 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.
>=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
> > 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-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:
> >>> 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@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 <Emilian.Medve@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 =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);
> >>>> +	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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL
  2015-01-21  9:37           ` Yuantian Tang
@ 2015-01-21  9:58             ` Emil Medve
  0 siblings, 0 replies; 19+ messages in thread
From: Emil Medve @ 2015-01-21  9:58 UTC (permalink / raw)
  To: Tang Yuantian-B29983, linuxppc-dev@lists.ozlabs.org,
	Wood Scott-B07421, mturquette@linaro.org, haokexin@gmail.com

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@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:
>>> 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@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:
>>>>> 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@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 <Emilian.Medve@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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET
  2015-01-20 10:09 ` [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET Emil Medve
@ 2015-02-25  1:14   ` Scott Wood
  0 siblings, 0 replies; 19+ messages in thread
From: Scott Wood @ 2015-02-25  1:14 UTC (permalink / raw)
  To: Emil Medve; +Cc: mturquette, haokexin, linuxppc-dev, yuantian.tang

On Tue, 2015-01-20 at 04:09 -0600, Emil Medve wrote:
> Change-Id: I1a80ad7b9f6854791bd270b746f93a91439155a6
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc()
  2015-01-20 10:09 ` [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc() Emil Medve
@ 2015-02-25  1:17   ` Scott Wood
  0 siblings, 0 replies; 19+ messages in thread
From: Scott Wood @ 2015-02-25  1:17 UTC (permalink / raw)
  To: Emil Medve; +Cc: mturquette, haokexin, linuxppc-dev, yuantian.tang

On Tue, 2015-01-20 at 04:09 -0600, Emil Medve wrote:
> Where the memset() is not necessary
> 
> Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
> ---
>  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;
>  

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-02-25  1:17 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-20 10:09 [PATCH 0/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
2015-01-20 10:09 ` [PATCH 1/8] clk: ppc-corenet: Fix checkpatch type PARENTHESIS_ALIGNMENT Emil Medve
2015-01-20 10:09 ` [PATCH 2/8] clk: ppc-corenet: Fix checkpatch type ALLOC_WITH_MULTIPLY Emil Medve
2015-01-20 10:09 ` [PATCH 3/8] clk: ppc-corenet: Fix checkpatch type ALLOC_SIZEOF_STRUCT Emil Medve
2015-01-20 10:09 ` [PATCH 4/8] clk: ppc-corenet: Fix checkpatch type OOM_MESSAGE Emil Medve
2015-01-20 10:09 ` [PATCH 5/8] clk: ppc-corenet: Make local symbol 'static' Emil Medve
2015-01-20 10:09 ` [PATCH 6/8] clk: ppc-corenet: Replace kzalloc() with kmalloc() Emil Medve
2015-02-25  1:17   ` Scott Wood
2015-01-20 10:09 ` [PATCH 7/8] powerpc/corenet: Enable CLK_PPC_CORENET Emil Medve
2015-02-25  1:14   ` Scott Wood
2015-01-20 10:09 ` [PATCH 8/8] clk: ppc-corenet: Add support for the platform PLL Emil Medve
2015-01-21  5:42   ` Yuantian Tang
2015-01-21  8:20     ` Emil Medve
2015-01-21  8:35       ` Yuantian Tang
2015-01-21  9:02         ` Emil Medve
2015-01-21  9:37           ` Yuantian Tang
2015-01-21  9:58             ` Emil Medve
2015-01-21  1:40 ` [PATCH 0/8] " Yuantian Tang
2015-01-21  6:24   ` Emil Medve

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).