clk: npcm7xx: get fixed input clks from DT

clk: npcm7xx: get fixed input clks from DT

[Tali Perry]

[PATCH v1 0/1] clk: npcm7xx: get fixed clocks from DT
[PATCH v1 1/1] clk: npcm: get fixed input clks from DT

[PATCH v1 0/1] clk: npcm7xx: get fixed clocks from DT

[Tali Perry]

Nuvoton NPCM7XX Clock Controller



The npcm7xx clock module includes 4 PLLs and then a tree of muxes and dividers. 
All muxes and dividers are preset before Linux boots. The presetting can change
according to the board. 
Linux drivers need to know what the frequencies are, but they cannot change it, 
so this whole driver is intended as read only mechanism for clocks. 

Before this change PLLs input clk value was defined inside the driver.
With this fix clock will be on the DT.


Signed-off-by: Tali Perry <tali.perry1@gmail.com>
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

---

Tali Perry (1):
  clk: npcm: get fixed input clks from DT

 drivers/clk/clk-npcm7xx.c | 64 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 11 deletions(-)

-- 
2.14.1

[PATCH v1 1/1] clk: npcm: get fixed input clks from DT

[Tali Perry]

---
 drivers/clk/clk-npcm7xx.c | 64 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/clk-npcm7xx.c b/drivers/clk/clk-npcm7xx.c
index 27a86b7a34db..25f8ccaa3d2f 100644
--- a/drivers/clk/clk-npcm7xx.c
+++ b/drivers/clk/clk-npcm7xx.c
@@ -8,17 +8,23 @@
  */
 
 #include <linux/module.h>
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
+#include <linux/device.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
 #include <linux/of_address.h>
+#include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/err.h>
+#include <linux/rational.h>
 #include <linux/bitfield.h>
-
 #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
 
+
 struct npcm7xx_clk_pll {
 	struct clk_hw	hw;
 	void __iomem	*pllcon;
@@ -44,7 +50,8 @@ static unsigned long npcm7xx_clk_pll_recalc_rate(struct clk_hw *hw,
 	u64 ret;
 
 	if (parent_rate == 0) {
-		pr_err("%s: parent rate is zero", __func__);
+		pr_err("%s: parent rate is zero. reg=%s\n", __func__,
+		       hw->init->name);
 		return 0;
 	}
 
@@ -65,9 +72,10 @@ static const struct clk_ops npcm7xx_clk_pll_ops = {
 	.recalc_rate = npcm7xx_clk_pll_recalc_rate,
 };
 
-static struct clk_hw *
-npcm7xx_clk_register_pll(void __iomem *pllcon, const char *name,
-			 const char *parent_name, unsigned long flags)
+static struct clk_hw *npcm7xx_clk_register_pll(void __iomem *pllcon,
+					       const char *name,
+					       const char *parent_name,
+					       unsigned long flags)
 {
 	struct npcm7xx_clk_pll *pll;
 	struct clk_init_data init;
@@ -544,12 +552,14 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
 	void __iomem *clk_base;
 	struct resource res;
 	struct clk_hw *hw;
+	struct clk *clk;
 	int ret;
 	int i;
 
+	clk_base = NULL;
 	ret = of_address_to_resource(clk_np, 0, &res);
 	if (ret) {
-		pr_err("%pOFn: failed to get resource, ret %d\n", clk_np,
+		pr_err("%s: failed to get resource, ret %d\n", clk_np->name,
 			ret);
 		return;
 	}
@@ -558,9 +568,10 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
 	if (!clk_base)
 		goto npcm7xx_init_error;
 
-	npcm7xx_clk_data = kzalloc(struct_size(npcm7xx_clk_data, hws,
-				   NPCM7XX_NUM_CLOCKS), GFP_KERNEL);
-	if (!npcm7xx_clk_data)
+	npcm7xx_clk_data = kzalloc(sizeof(*npcm7xx_clk_data->hws) *
+		NPCM7XX_NUM_CLOCKS + sizeof(npcm7xx_clk_data), GFP_KERNEL);
+
+	if (!npcm7xx_clk_data->hws)
 		goto npcm7xx_init_np_err;
 
 	npcm7xx_clk_data->num = NPCM7XX_NUM_CLOCKS;
@@ -568,6 +579,31 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
 	for (i = 0; i < NPCM7XX_NUM_CLOCKS; i++)
 		npcm7xx_clk_data->hws[i] = ERR_PTR(-EPROBE_DEFER);
 
+	/* Read fixed clocks. These 3 clocks must be defined in DT */
+	clk = of_clk_get_by_name(clk_np, NPCM7XX_CLK_S_REFCLK);
+	if (IS_ERR(clk)) {
+		pr_err("failed to find external REFCLK on device tree, err=%ld\n",
+			PTR_ERR(clk));
+		clk_put(clk);
+		goto npcm7xx_init_fail_no_clk_on_dt;
+	}
+
+	clk = of_clk_get_by_name(clk_np, NPCM7XX_CLK_S_SYSBYPCK);
+	if (IS_ERR(clk)) {
+		pr_err("failed to find external SYSBYPCK on device tree, err=%ld\n",
+			PTR_ERR(clk));
+		clk_put(clk);
+		goto npcm7xx_init_fail_no_clk_on_dt;
+	}
+
+	clk = of_clk_get_by_name(clk_np, NPCM7XX_CLK_S_MCBYPCK);
+	if (IS_ERR(clk)) {
+		pr_err("failed to find external MCBYPCK on device tree, err=%ld\n",
+			PTR_ERR(clk));
+		clk_put(clk);
+		goto npcm7xx_init_fail_no_clk_on_dt;
+	}
+
 	/* Register plls */
 	for (i = 0; i < ARRAY_SIZE(npcm7xx_plls); i++) {
 		const struct npcm7xx_clk_pll_data *pll_data = &npcm7xx_plls[i];
@@ -646,11 +682,17 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
 
 	return;
 
+npcm7xx_init_fail_no_clk_on_dt:
+	pr_err("see Documentation/devicetree/bindings/clock/");
+	pr_err("nuvoton,npcm750-clk.txt  for details\n");
 npcm7xx_init_fail:
-	kfree(npcm7xx_clk_data->hws);
+	if (npcm7xx_clk_data->num)
+		kfree(npcm7xx_clk_data->hws);
 npcm7xx_init_np_err:
-	iounmap(clk_base);
+	if (clk_base != NULL)
+		iounmap(clk_base);
 npcm7xx_init_error:
 	of_node_put(clk_np);
+	pr_err("clk setup fail\n");
 }
 CLK_OF_DECLARE(npcm7xx_clk_init, "nuvoton,npcm750-clk", npcm7xx_clk_init);
-- 
2.14.1

Re: [PATCH v1 1/1] clk: npcm: get fixed input clks from DT

[Stephen Boyd]

Quoting Tali Perry (2018-12-05 11:47:02)
> ---

No commit text? No Signed-off-by?

Sorry, I don't understand what this patch is for or what's going on.

>  drivers/clk/clk-npcm7xx.c | 64 +++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 53 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/clk/clk-npcm7xx.c b/drivers/clk/clk-npcm7xx.c
> index 27a86b7a34db..25f8ccaa3d2f 100644
> --- a/drivers/clk/clk-npcm7xx.c
> +++ b/drivers/clk/clk-npcm7xx.c
> @@ -8,17 +8,23 @@
>   */
>  
>  #include <linux/module.h>
> +#include <linux/clk.h>
>  #include <linux/clk-provider.h>
> +#include <linux/device.h>
>  #include <linux/io.h>
>  #include <linux/kernel.h>
>  #include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
>  #include <linux/of_address.h>
> +#include <linux/platform_device.h>
>  #include <linux/slab.h>
>  #include <linux/err.h>
> +#include <linux/rational.h>
>  #include <linux/bitfield.h>
> -
>  #include <dt-bindings/clock/nuvoton,npcm7xx-clock.h>
>  
> +

I don't get it. Please don't send random whitespace changes in a patch.

>  struct npcm7xx_clk_pll {
>         struct clk_hw   hw;
>         void __iomem    *pllcon;
> @@ -44,7 +50,8 @@ static unsigned long npcm7xx_clk_pll_recalc_rate(struct clk_hw *hw,
>         u64 ret;
>  
>         if (parent_rate == 0) {
> -               pr_err("%s: parent rate is zero", __func__);
> +               pr_err("%s: parent rate is zero. reg=%s\n", __func__,
> +                      hw->init->name);

Why do we care?

>                 return 0;
>         }
>  
> @@ -544,12 +552,14 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
>         void __iomem *clk_base;
>         struct resource res;
>         struct clk_hw *hw;
> +       struct clk *clk;
>         int ret;
>         int i;
>  
> +       clk_base = NULL;
>         ret = of_address_to_resource(clk_np, 0, &res);
>         if (ret) {
> -               pr_err("%pOFn: failed to get resource, ret %d\n", clk_np,
> +               pr_err("%s: failed to get resource, ret %d\n", clk_np->name,
>                         ret);
>                 return;
>         }

Drop this change as it's making things worse.

> @@ -558,9 +568,10 @@ static void __init npcm7xx_clk_init(struct device_node *clk_np)
>         if (!clk_base)
>                 goto npcm7xx_init_error;
>  
> -       npcm7xx_clk_data = kzalloc(struct_size(npcm7xx_clk_data, hws,
> -                                  NPCM7XX_NUM_CLOCKS), GFP_KERNEL);
> -       if (!npcm7xx_clk_data)
> +       npcm7xx_clk_data = kzalloc(sizeof(*npcm7xx_clk_data->hws) *
> +               NPCM7XX_NUM_CLOCKS + sizeof(npcm7xx_clk_data), GFP_KERNEL);
> +
> +       if (!npcm7xx_clk_data->hws)
>                 goto npcm7xx_init_np_err;
>  
>         npcm7xx_clk_data->num = NPCM7XX_NUM_CLOCKS;

Drop this hunk.