[PATCH 0/4] Introduce of_init_clk_data() for DT clock parsing

[PATCH 0/4] Introduce of_init_clk_data() for DT clock parsing

[Stephen Boyd]

I see the same pattern repeated a few times to parse the common clock
bindings for a particular clock. Instead of wrting this code another
time, let's consolidate the logic in one place and standardize how
clocks will be named from their bindings.

After this series there are no more users of of_clk_get_parent_name()
so we may want to delete it.

Stephen Boyd (4):
  clk: Add of_init_clk_data() to parse common clock bindings
  clk: highbank: Use of_init_clk_data()
  clk: vt8500: Use of_init_clk_data()
  clk: zynq: Use of_init_clk_data()

 drivers/clk/clk-highbank.c   | 21 ++++++----------
 drivers/clk/clk-vt8500.c     | 39 ++++++++++++-----------------
 drivers/clk/clk-zynq.c       | 28 +++++++--------------
 drivers/clk/clk.c            | 58 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/clk-provider.h |  2 ++
 5 files changed, 91 insertions(+), 57 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 1/4] clk: Add of_init_clk_data() to parse common clock bindings

[Stephen Boyd]

Consolidate DT parsing for the common bits of a clock binding in
one place to simplify clock drivers. This also has the added
benefit of standardizing how the clock names used by the common
clock framework are generated from the DT bindings. We always use
the first clock-output-names string if it exists, otherwise we
fall back to the node name.

To be slightly more efficient and make the caller's life easier,
we introduce a shallow copy flag so that the clock core knows to
just copy the pointers to the strings and not the string
contents. Otherwise the callers of this function would have to
free the strings allocated here which could be cumbersome.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 drivers/clk/clk.c            | 58 +++++++++++++++++++++++++++++++++++++++++++-
 include/linux/clk-provider.h |  2 ++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 251e45d..95380a9 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1383,6 +1383,10 @@ static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk)
 {
 	int i, ret;
 
+	hw->clk = clk;
+	if (hw->init->flags & CLK_SHALLOW_COPY)
+		return PTR_RET(__clk_register(dev, hw));
+
 	clk->name = kstrdup(hw->init->name, GFP_KERNEL);
 	if (!clk->name) {
 		pr_err("%s: could not allocate clk->name\n", __func__);
@@ -1393,7 +1397,6 @@ static int _clk_register(struct device *dev, struct clk_hw *hw, struct clk *clk)
 	clk->hw = hw;
 	clk->flags = hw->init->flags;
 	clk->num_parents = hw->init->num_parents;
-	hw->clk = clk;
 
 	/* allocate local copy in case parent_names is __initdata */
 	clk->parent_names = kzalloc((sizeof(char*) * clk->num_parents),
@@ -1797,4 +1800,57 @@ void __init of_clk_init(const struct of_device_id *matches)
 		clk_init_cb(np);
 	}
 }
+
+/**
+ * of_init_clk_data() - Initialize a clk_init_data struct from a DT node
+ * @np: node to initialize struct from
+ * @init: struct to initialize
+ *
+ * Populates the clk_init_data struct by parsing the device node for
+ * properties matching the common clock binding. Returns 0 on success
+ * and a negative error code on failure.
+ */
+int of_init_clk_data(struct device_node *np, struct clk_init_data *init)
+{
+	struct of_phandle_args s;
+	const char **names = NULL, **p;
+	const char *name;
+	int i;
+
+	if (of_property_read_string(np, "clock-output-names", &name) < 0)
+		name = np->name;
+	init->name = kstrdup(name, GFP_KERNEL);
+	if (!init->name)
+		return -ENOMEM;
+
+	for (i = 0; of_parse_phandle_with_args(np, "clocks", "#clock-cells",
+				i, &s) == 0; i++) {
+		p = krealloc(names, sizeof(*names) * (i + 1), GFP_KERNEL);
+		if (!p)
+			goto err;
+		names = p;
+
+		if (of_property_read_string(s.np, "clock-output-names",
+					&name) < 0)
+			name = s.np->name;
+		names[i] = kstrdup(name, GFP_KERNEL);
+		if (!names[i])
+			goto err;
+		of_node_put(s.np);
+	}
+
+	init->parent_names = names;
+	init->num_parents = i;
+	init->flags = CLK_SHALLOW_COPY;
+
+	return 0;
+err:
+	of_node_put(s.np);
+	while (--i >= 0)
+		kfree(names[i]);
+	kfree(names);
+	kfree(init->name);
+	return -ENOMEM;
+}
+EXPORT_SYMBOL_GPL(of_init_clk_data);
 #endif
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 4989b8a..9d3db2b 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -27,6 +27,7 @@
 #define CLK_IS_ROOT		BIT(4) /* root clk, has no parent */
 #define CLK_IS_BASIC		BIT(5) /* Basic clk, can't do a to_clk_foo() */
 #define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
+#define CLK_SHALLOW_COPY	BIT(7) /* don't copy the initdata strings */
 
 struct clk_hw;
 
@@ -380,6 +381,7 @@ struct clk_onecell_data {
 struct clk *of_clk_src_onecell_get(struct of_phandle_args *clkspec, void *data);
 const char *of_clk_get_parent_name(struct device_node *np, int index);
 void of_clk_init(const struct of_device_id *matches);
+int of_init_clk_data(struct device_node *np, struct clk_init_data *init);
 
 #endif /* CONFIG_COMMON_CLK */
 #endif /* CLK_PROVIDER_H */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 2/4] clk: highbank: Use of_init_clk_data()

[Stephen Boyd]

Reduce lines of code and simplify this driver by using the
generic clock binding parsing function.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Rob Herring <rob.herring@calxeda.com>
---
 drivers/clk/clk-highbank.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/clk-highbank.c b/drivers/clk/clk-highbank.c
index 52fecad..5dbbf62 100644
--- a/drivers/clk/clk-highbank.c
+++ b/drivers/clk/clk-highbank.c
@@ -275,8 +275,6 @@ static __init struct clk *hb_clk_init(struct device_node *node, const struct clk
 	u32 reg;
 	struct clk *clk;
 	struct hb_clk *hb_clk;
-	const char *clk_name = node->name;
-	const char *parent_name;
 	struct clk_init_data init;
 	int rc;
 
@@ -290,24 +288,21 @@ static __init struct clk *hb_clk_init(struct device_node *node, const struct clk
 
 	hb_clk->reg = sregs_base + reg;
 
-	of_property_read_string(node, "clock-output-names", &clk_name);
-
-	init.name = clk_name;
 	init.ops = ops;
-	init.flags = 0;
-	parent_name = of_clk_get_parent_name(node, 0);
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
+	rc = of_init_clk_data(node, &init);
+	if (WARN_ON(rc))
+		goto err;
 
 	hb_clk->hw.init = &init;
 
 	clk = clk_register(NULL, &hb_clk->hw);
-	if (WARN_ON(IS_ERR(clk))) {
-		kfree(hb_clk);
-		return NULL;
-	}
+	if (WARN_ON(IS_ERR(clk)))
+		goto err;
 	rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
 	return clk;
+err:
+	kfree(hb_clk);
+	return NULL;
 }
 
 static void __init hb_pll_init(struct device_node *node)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 3/4] clk: vt8500: Use of_init_clk_data()

[Stephen Boyd]

Reduce lines of code and simplify this driver by using the
generic clock binding parsing function.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Tony Prisk <linux@prisktech.co.nz>
---
 drivers/clk/clk-vt8500.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/clk/clk-vt8500.c b/drivers/clk/clk-vt8500.c
index fe25570..454efa7 100644
--- a/drivers/clk/clk-vt8500.c
+++ b/drivers/clk/clk-vt8500.c
@@ -200,8 +200,6 @@ static __init void vtwm_device_clk_init(struct device_node *node)
 	u32 en_reg, div_reg;
 	struct clk *clk;
 	struct clk_device *dev_clk;
-	const char *clk_name = node->name;
-	const char *parent_name;
 	struct clk_init_data init;
 	int rc;
 	int clk_init_flags = 0;
@@ -237,8 +235,6 @@ static __init void vtwm_device_clk_init(struct device_node *node)
 		clk_init_flags |= CLK_INIT_DIVISOR;
 	}
 
-	of_property_read_string(node, "clock-output-names", &clk_name);
-
 	switch (clk_init_flags) {
 	case CLK_INIT_GATED:
 		init.ops = &vt8500_gated_clk_ops;
@@ -256,11 +252,11 @@ static __init void vtwm_device_clk_init(struct device_node *node)
 		return;
 	}
 
-	init.name = clk_name;
-	init.flags = 0;
-	parent_name = of_clk_get_parent_name(node, 0);
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
+	rc = of_init_clk_data(node, &init);
+	if (WARN_ON(rc)) {
+		kfree(dev_clk);
+		return;
+	}
 
 	dev_clk->hw.init = &init;
 
@@ -270,7 +266,7 @@ static __init void vtwm_device_clk_init(struct device_node *node)
 		return;
 	}
 	rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
+	clk_register_clkdev(clk, init.name, NULL);
 }
 
 
@@ -458,8 +454,6 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type)
 	u32 reg;
 	struct clk *clk;
 	struct clk_pll *pll_clk;
-	const char *clk_name = node->name;
-	const char *parent_name;
 	struct clk_init_data init;
 	int rc;
 
@@ -475,24 +469,21 @@ static __init void vtwm_pll_clk_init(struct device_node *node, int pll_type)
 	pll_clk->lock = &_lock;
 	pll_clk->type = pll_type;
 
-	of_property_read_string(node, "clock-output-names", &clk_name);
-
-	init.name = clk_name;
 	init.ops = &vtwm_pll_ops;
-	init.flags = 0;
-	parent_name = of_clk_get_parent_name(node, 0);
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
+	rc = of_init_clk_data(node, &init);
+	if (WARN_ON(rc))
+		goto err;
 
 	pll_clk->hw.init = &init;
 
 	clk = clk_register(NULL, &pll_clk->hw);
-	if (WARN_ON(IS_ERR(clk))) {
-		kfree(pll_clk);
-		return;
-	}
+	if (WARN_ON(IS_ERR(clk)))
+		goto err;
 	rc = of_clk_add_provider(node, of_clk_src_simple_get, clk);
-	clk_register_clkdev(clk, clk_name, NULL);
+	clk_register_clkdev(clk, init.name, NULL);
+	return;
+err:
+	kfree(pll_clk);
 }
 
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Stephen Boyd]

Reduce lines of code and simplify this driver by using the
generic clock binding parsing function. This also fixes a bug
where the 'flags' member of the init struct is not initialized.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Cc: Josh Cartwright <josh.cartwright@ni.com>
Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/clk/clk-zynq.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/clk/clk-zynq.c b/drivers/clk/clk-zynq.c
index 37a3051..0fcc23d 100644
--- a/drivers/clk/clk-zynq.c
+++ b/drivers/clk/clk-zynq.c
@@ -48,7 +48,6 @@ static void __init zynq_pll_clk_setup(struct device_node *np)
 {
 	struct clk_init_data init;
 	struct zynq_pll_clk *pll;
-	const char *parent_name;
 	struct clk *clk;
 	u32 regs[2];
 	int ret;
@@ -64,12 +63,10 @@ static void __init zynq_pll_clk_setup(struct device_node *np)
 	pll->pll_ctrl = slcr_base + regs[0];
 	pll->pll_cfg  = slcr_base + regs[1];
 
-	of_property_read_string(np, "clock-output-names", &init.name);
-
+	ret = of_init_clk_data(np, &init);
+	if (WARN_ON(ret))
+		return;
 	init.ops = &zynq_pll_clk_ops;
-	parent_name = of_clk_get_parent_name(np, 0);
-	init.parent_names = &parent_name;
-	init.num_parents = 1;
 
 	pll->hw.init = &init;
 
@@ -119,13 +116,11 @@ static const struct clk_ops zynq_periph_clk_ops = {
 static void __init zynq_periph_clk_setup(struct device_node *np)
 {
 	struct zynq_periph_clk *periph;
-	const char *parent_names[3];
 	struct clk_init_data init;
 	int clk_num = 0, err;
 	const char *name;
 	struct clk *clk;
 	u32 reg;
-	int i;
 
 	err = of_property_read_u32(np, "reg", &reg);
 	if (WARN_ON(err))
@@ -138,12 +133,10 @@ static void __init zynq_periph_clk_setup(struct device_node *np)
 	periph->clk_ctrl = slcr_base + reg;
 	spin_lock_init(&periph->clkact_lock);
 
-	init.name = np->name;
+	err = of_init_clk_data(np, &init);
+	if (WARN_ON(err))
+		return;
 	init.ops = &zynq_periph_clk_ops;
-	for (i = 0; i < ARRAY_SIZE(parent_names); i++)
-		parent_names[i] = of_clk_get_parent_name(np, i);
-	init.parent_names = parent_names;
-	init.num_parents = ARRAY_SIZE(parent_names);
 
 	periph->hw.init = &init;
 
@@ -315,7 +308,6 @@ err_read_output_name:
 static void __init zynq_cpu_clk_setup(struct device_node *np)
 {
 	struct zynq_cpu_clk *cpuclk;
-	const char *parent_names[3];
 	struct clk_init_data init;
 	void __iomem *clk_621;
 	struct clk *clk;
@@ -335,12 +327,10 @@ static void __init zynq_cpu_clk_setup(struct device_node *np)
 	clk_621 = slcr_base + reg[1];
 	spin_lock_init(&cpuclk->clkact_lock);
 
-	init.name = np->name;
+	err = of_init_clk_data(np, &init);
+	if (WARN_ON(err))
+		return;
 	init.ops = &zynq_cpu_clk_ops;
-	for (i = 0; i < ARRAY_SIZE(parent_names); i++)
-		parent_names[i] = of_clk_get_parent_name(np, i);
-	init.parent_names = parent_names;
-	init.num_parents = ARRAY_SIZE(parent_names);
 
 	cpuclk->hw.init = &init;
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 3/4] clk: vt8500: Use of_init_clk_data()

[Tony Prisk]

On Mon, 2012-12-17 at 13:02 -0800, Stephen Boyd wrote:
> Reduce lines of code and simplify this driver by using the
> generic clock binding parsing function.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Tony Prisk <linux@prisktech.co.nz>
> ---
>  drivers/clk/clk-vt8500.c | 39 +++++++++++++++------------------------
>  1 file changed, 15 insertions(+), 24 deletions(-)

Looks fine, and compiles without errors/warnings.

Acked-by: Tony Prisk <linux@prisktech.co.nz>

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Josh Cartwright]

On Mon, Dec 17, 2012 at 01:02:15PM -0800, Stephen Boyd wrote:
> Reduce lines of code and simplify this driver by using the
> generic clock binding parsing function. This also fixes a bug
> where the 'flags' member of the init struct is not initialized.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Josh Cartwright <josh.cartwright@ni.com>
> Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
>  drivers/clk/clk-zynq.c | 28 +++++++++-------------------
>  1 file changed, 9 insertions(+), 19 deletions(-)

Stephen-

Is there a particular tree I should be testing this against?  Booting this set
on top of linus/master gives me:

   Linux version 3.7.0-10837-g3bb3ebf (joshc@beefymiracle) (gcc version 4.6.3 (Sourcery CodeBench Lite 2012.03-57) ) #3 Wed Dec 19 11:19:20 CST 2012
   CPU: ARMv7 Processor [413fc090] revision 0 (ARMv7), cr=18c53c7d
   CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
   Machine: Xilinx Zynq Platform, model: NI Zynq Prototype Board
   Memory policy: ECC disabled, Data cache writeback
   Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 130048
   Kernel command line: console=ttyPS1,115200
   PID hash table entries: 2048 (order: 1, 8192 bytes)
   Dentry cache hash table entries: 65536 (order: 6, 262144 bytes)
   Inode-cache hash table entries: 32768 (order: 5, 131072 bytes)
   __ex_table already sorted, skipping sort
   Memory: 512MB = 512MB total
   Memory: 514304k/514304k available, 9984k reserved, 0K highmem
   Virtual kernel memory layout:
       vector  : 0xffff0000 - 0xffff1000   (   4 kB)
       fixmap  : 0xfff00000 - 0xfffe0000   ( 896 kB)
       vmalloc : 0xe0800000 - 0xff000000   ( 488 MB)
       lowmem  : 0xc0000000 - 0xe0000000   ( 512 MB)
         .text : 0xc0008000 - 0xc0182e58   (1516 kB)
         .init : 0xc0183000 - 0xc01992cc   (  89 kB)
         .data : 0xc019a000 - 0xc01ac040   (  73 kB)
          .bss : 0xc01ac064 - 0xc01d30fc   ( 157 kB)
   SLUB: Genslabs=11, HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
   NR_IRQS:16 nr_irqs:16 16
   ------------[ cut here ]------------
   WARNING: at drivers/clk/clk-zynq.c:159 zynq_periph_clk_setup+0x190/0x248()
   [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
   [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
   [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193520>] (zynq_periph_clk_setup+0x190/0x248)
   [<c0193520>] (zynq_periph_clk_setup+0x190/0x248) from [<c0192944>] (of_clk_init+0x30/0x58)
   [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
   [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
   [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
   [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
   ---[ end trace 1b75b31a2719ed1c ]---
   ------------[ cut here ]------------
   WARNING: at drivers/clk/clk-zynq.c:296 zynq_cpu_clk_setup+0x1e0/0x26c()
   [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
   [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
   [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c)
   [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
   [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
   [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
   [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
   [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
   ---[ end trace 1b75b31a2719ed1d ]---
   ------------[ cut here ]------------
   WARNING: at drivers/clk/clk-zynq.c:347 zynq_cpu_clk_setup+0x210/0x26c()
   [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
   [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
   [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c)
   [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
   [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
   [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
   [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
   [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
   ---[ end trace 1b75b31a2719ed1e ]---
   sched_clock: 32 bits at 100 Hz, resolution 10000000ns, wraps every 4294967286ms
   Calibrating delay loop... 3991.96 BogoMIPS (lpj=19959808)
   pid_max: default: 32768 minimum: 301
   Mount-cache hash table entries: 512
   CPU: Testing write buffer coherency: ok
   Setting up static identity map for 0x139e00 - 0x139e58
   devtmpfs: initialized
   DMA: preallocated 256 KiB pool for atomic coherent allocations
   L310 cache controller enabled
   l2x0: 8 ways, CACHE_ID 0x000000c0, AUX_CTRL 0x02060000, Cache size: 524288 B

Josh

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Stephen Boyd]

On 12/19/12 09:26, Josh Cartwright wrote:
> On Mon, Dec 17, 2012 at 01:02:15PM -0800, Stephen Boyd wrote:
>> Reduce lines of code and simplify this driver by using the
>> generic clock binding parsing function. This also fixes a bug
>> where the 'flags' member of the init struct is not initialized.
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> Cc: Josh Cartwright <josh.cartwright@ni.com>
>> Cc: Soren Brinkmann <soren.brinkmann@xilinx.com>
>> ---
>>  drivers/clk/clk-zynq.c | 28 +++++++++-------------------
>>  1 file changed, 9 insertions(+), 19 deletions(-)
> Stephen-
>
> Is there a particular tree I should be testing this against?  Booting this set
> on top of linus/master gives me:

I based the patches off of linux-next-20121217 although it seems like it
applied fine for you on linus/master.

>    ------------[ cut here ]------------
>    WARNING: at drivers/clk/clk-zynq.c:159 zynq_periph_clk_setup+0x190/0x248()
>    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
>    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
>    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193520>] (zynq_periph_clk_setup+0x190/0x248)
>    [<c0193520>] (zynq_periph_clk_setup+0x190/0x248) from [<c0192944>] (of_clk_init+0x30/0x58)
>    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
>    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
>    [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
>    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
>    ---[ end trace 1b75b31a2719ed1c ]---
>    ------------[ cut here ]------------
>    WARNING: at drivers/clk/clk-zynq.c:296 zynq_cpu_clk_setup+0x1e0/0x26c()
>    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
>    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
>    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c)
>    [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
>    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
>    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
>    [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
>    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
>    ---[ end trace 1b75b31a2719ed1d ]---
>    ------------[ cut here ]------------
>    WARNING: at drivers/clk/clk-zynq.c:347 zynq_cpu_clk_setup+0x210/0x26c()
>    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
>    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
>    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c)
>    [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
>    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
>    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
>    [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
>    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
>    ---[ end trace 1b75b31a2719ed1e ]---

Can you show the code at those line numbers? There are quite a few
WARN_ONs in that code and it's possible the WARN_ON is the one
introduced in this patch.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Josh Cartwright]

On Wed, Dec 19, 2012 at 10:20:30AM -0800, Stephen Boyd wrote:
> On 12/19/12 09:26, Josh Cartwright wrote:
> > On Mon, Dec 17, 2012 at 01:02:15PM -0800, Stephen Boyd wrote:
[..]
> 
> Can you show the code at those line numbers? There are quite a few
> WARN_ONs in that code and it's possible the WARN_ON is the one
> introduced in this patch.

It looks like we're not hitting the WARN_ON() you added, but several of
the other ones.

> >    ------------[ cut here ]------------
> >    WARNING: at drivers/clk/clk-zynq.c:159 zynq_periph_clk_setup+0x190/0x248()
> >    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
> >    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
> >    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193520>] (zynq_periph_clk_setup+0x190/0x248)
> >    [<c0193520>] (zynq_periph_clk_setup+0x190/0x248) from [<c0192944>] (of_clk_init+0x30/0x58)
> >    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
> >    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
> >    [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
> >    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
> >    ---[ end trace 1b75b31a2719ed1c ]---

zynq_periph_clk_setup:
156 >-------periph->gates[0] = clk_register_gate(NULL, name, np->name, 0,
157 >------->------->------->------->-------     periph->clk_ctrl, 0, 0,
158 >------->------->------->------->-------     &periph->clkact_lock);
159 >-------if (WARN_ON(IS_ERR(periph->gates[0])))
160 >------->-------return;

> >    ------------[ cut here ]------------
> >    WARNING: at drivers/clk/clk-zynq.c:296 zynq_cpu_clk_setup+0x1e0/0x26c()
> >    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
> >    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
> >    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c)
> >    [<c0193304>] (zynq_cpu_clk_setup+0x1e0/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
> >    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
> >    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
> >    [<c0185d54>] (time_init+0x28/0x30) from [<c0183650>] (start_kernel+0x1a8/0x2c0)
> >    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
> >    ---[ end trace 1b75b31a2719ed1d ]---

zynq_cpu_subclk_setup:
295 >-------clk = clk_register(NULL, &subclk->hw);
296 >-------if (WARN_ON(IS_ERR(clk)))
297 >------->-------goto err_clk_register;

> >    ------------[ cut here ]------------
> >    WARNING: at drivers/clk/clk-zynq.c:347 zynq_cpu_clk_setup+0x210/0x26c()
> >    [<c000d814>] (unwind_backtrace+0x0/0xf8) from [<c0016808>] (warn_slowpath_common+0x50/0x60)
> >    [<c0016808>] (warn_slowpath_common+0x50/0x60) from [<c00168e0>] (warn_slowpath_null+0x1c/0x24)
> >    [<c00168e0>] (warn_slowpath_null+0x1c/0x24) from [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c)
> >    [<c0193334>] (zynq_cpu_clk_setup+0x210/0x26c) from [<c0192944>] (of_clk_init+0x30/0x58)
> >    [<c0192944>] (of_clk_init+0x30/0x58) from [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48)
> >    [<c0189708>] (xilinx_zynq_timer_init+0x40/0x48) from [<c0185d54>] (time_init+0x28/0x30)
> >    [<c0185d54>] (time_init+0x28/0x30) fr m [<c0183650>] (start_kernel+0x1a8/0x2c0)
> >    [<c0183650>] (start_kernel+0x1a8/0x2c0) from [<00008074>] (0x8074)
> >    ---[ end trace 1b75b31a2719ed1e ]---

zynq_cpu_clk_setup:
345 >-------for (i = 0; i < 4; i++) {
346 >------->-------cpuclk->subclks[i] = zynq_cpu_subclk_setup(np, i, clk_621);
347 >------->-------if (WARN_ON(IS_ERR(cpuclk->subclks[i])))
348 >------->------->-------return;
349 >-------}

The third warning is a bit redundant, it is triggered by the same conditions
that triggered the second warning.

	  Josh

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Stephen Boyd]

On 12/19/12 10:36, Josh Cartwright wrote:
> On Wed, Dec 19, 2012 at 10:20:30AM -0800, Stephen Boyd wrote:
>> On 12/19/12 09:26, Josh Cartwright wrote:
>>> On Mon, Dec 17, 2012 at 01:02:15PM -0800, Stephen Boyd wrote:
> [..]
>> Can you show the code at those line numbers? There are quite a few
>> WARN_ONs in that code and it's possible the WARN_ON is the one
>> introduced in this patch.
> It looks like we're not hitting the WARN_ON() you added, but several of
> the other ones.

Ah it seems that zynq is doing different things with the clock names.
The periph clock is this

                                uart_clk: uart_clk {
                                        #clock-cells = <1>;
                                        compatible =
"xlnx,zynq-periph-clock";
                                        clocks = <&iopll &armpll &ddrpll>;
                                        reg = <0x154>;
                                        clock-output-names =
"uart0_ref_clk",
                                                            
"uart1_ref_clk";

and so zynq_periph_clk_setup() wants to register clocks named uart_clk,
uart0_ref_clk, and uart1_ref_clk. But my change causes uart0_ref_clk to
be registered twice because of the way of_init_clk_data() detects the
init.name property from the binding (we use clock-output-names[0] and
only use np->name if there is no clock-output-names).

Perhaps we need to make of_init_clk_data() take an integer argument
indicating which name to use? So of_init_clk_data(np, &init, 0) would
mean use the np->name as the init.name, and of_init_clk_data(np, &init,
1) would mean use the clock-output-names[0] property,
of_init_clk_data(np, &init, 2) would mean use the clock-output-names[1]
property.

Or perhaps we should think of some way to generate unique names from the
bindings that the clock APIs can use internally.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Soren Brinkmann]

Hi Stephen,

I guess Josh is the better person to talk about this, since he created the
patches regarding common clock for mainline, but I tried running your series
and ran into this:

Unable to handle kernel NULL pointer dereference at virtual address 0000002a
pgd = c0004000
[0000002a] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT ARM
Modules linked in:
CPU: 0    Tainted: G        W     (3.7.0-rc3-00025-gc11ffdd-dirty #246)
PC is at __clk_prepare+0x20/0x80
LR is at clk_prepare+0x2c/0x44
pc : [<c031659c>]    lr : [<c0316628>]    psr: a0000153
sp : ee02fdd0  ip : ee02fde8  fp : ee02fde4
r10: 00000000  r9 : 00000000  r8 : c0587884
r7 : 00000000  r6 : c05aab98  r5 : fffffffe  r4 : fffffffe
r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : fffffffe
Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
Control: 18c5387d  Table: 00004059  DAC: 00000015
Process swapper (pid: 1, stack limit = 0xee02e230)
Stack: (0xee02fdd0 to 0xee030000)
fdc0:                                     c05b6a18 fffffffe ee02fdfc ee02fde8
fde0: c0316628 c0316588 ee085400 fffffffe ee02fe24 ee02fe00 c03c1358 c0316608
fe00: c03c1328 ee085410 c05aab98 c05aab98 00000000 c0587884 ee02fe34 ee02fe28
fe20: c026a5c0 c03c1334 ee02fe5c ee02fe38 c0268f50 c026a5a8 c026a7b8 c0313ac0
fe40: ee085410 ee085410 ee085444 c05aab98 ee02fe7c ee02fe60 c02691c0 c0268e18
fe60: c0269150 c05aab98 c0269150 00000000 ee02fea4 ee02fe80 c0267320 c026915c
fe80: ee00948c ee096df0 c021e02c c05aab98 ed9a0bc0 c05ab3a8 ee02feb4 ee02fea8
fea0: c0268a28 c02672d0 ee02fee4 ee02feb8 c0268408 c0268a0c c04b56d1 00000000
fec0: ee02fee4 c05aab98 c0565e60 00000000 0000006f c0587884 ee02ff14 ee02fee8
fee0: c0269990 c0268340 00000000 00000000 c0565e60 00000000 0000006f c0587884
ff00: 00000000 00000000 ee02ff24 ee02ff18 c026a9cc c02698f0 ee02ff3c ee02ff28
ff20: c0565e84 c026a984 ee02e000 ee02ff40 ee02ff74 ee02ff40 c00086f8 c0565e6c
ff40: c04f72e4 00000000 ee02ff74 00000006 00000006 c0571dac c0571d8c 0000006f
ff60: c0587884 00000000 ee02ffac ee02ff78 c03bf9c4 c0008664 00000006 00000006
ff80: c05511a8 00000000 ee02ffac 00000000 c03bf8cc 00000000 00000000 00000000
ffa0: 00000000 ee02ffb0 c000ea98 c03bf8d8 00000000 00000000 00000000 00000000
ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 0158ec21 0eced1ff
[<c031659c>] (__clk_prepare+0x20/0x80) from [<c0316628>] (clk_prepare+0x2c/0x44)
[<c0316628>] (clk_prepare+0x2c/0x44) from [<c03c1358>] (xuartps_probe+0x30/0x1ac
)
[<c03c1358>] (xuartps_probe+0x30/0x1ac) from [<c026a5c0>] (platform_drv_probe+0x
24/0x28)
[<c026a5c0>] (platform_drv_probe+0x24/0x28) from [<c0268f50>] (driver_probe_devi
ce+0x144/0x344)
[<c0268f50>] (driver_probe_device+0x144/0x344) from [<c02691c0>] (__driver_attac
h+0x70/0x94)
[<c02691c0>] (__driver_attach+0x70/0x94) from [<c0267320>] (bus_for_each_dev+0x5
c/0x8c)
[<c0267320>] (bus_for_each_dev+0x5c/0x8c) from [<c0268a28>] (driver_attach+0x28/
0x30)
[<c0268a28>] (driver_attach+0x28/0x30) from [<c0268408>] (bus_add_driver+0xd4/0x
254)
[<c0268408>] (bus_add_driver+0xd4/0x254) from [<c0269990>] (driver_register+0xac
/0x14c)
[<c0269990>] (driver_register+0xac/0x14c) from [<c026a9cc>] (platform_driver_reg
ister+0x54/0x68)
[<c026a9cc>] (platform_driver_register+0x54/0x68) from [<c0565e84>] (xuartps_ini
t+0x24/0x44)
[<c0565e84>] (xuartps_init+0x24/0x44) from [<c00086f8>] (do_one_initcall+0xa0/0x
170)
[<c00086f8>] (do_one_initcall+0xa0/0x170) from [<c03bf9c4>] (kernel_init+0xf8/0x
2ac)
[<c03bf9c4>] (kernel_init+0xf8/0x2ac) from [<c000ea98>] (ret_from_fork+0x14/0x20
)
Code: e8bd4000 e2504000 01a05004 0a000015 (e594302c) 
---[ end trace 1b75b31a2719ed1d ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b


A probably unique thing I do is, I set the status of uart0 to disabled. This way
I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
before.

	Soren

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Stephen Boyd]

On 12/19/12 11:22, Soren Brinkmann wrote:
> Hi Stephen,
>
> I guess Josh is the better person to talk about this, since he created the
> patches regarding common clock for mainline, but I tried running your series
> and ran into this:
>
> Unable to handle kernel NULL pointer dereference at virtual address 0000002a
> pgd = c0004000
> [0000002a] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0    Tainted: G        W     (3.7.0-rc3-00025-gc11ffdd-dirty #246)
> PC is at __clk_prepare+0x20/0x80
> LR is at clk_prepare+0x2c/0x44
> pc : [<c031659c>]    lr : [<c0316628>]    psr: a0000153
> sp : ee02fdd0  ip : ee02fde8  fp : ee02fde4
> r10: 00000000  r9 : 00000000  r8 : c0587884
> r7 : 00000000  r6 : c05aab98  r5 : fffffffe  r4 : fffffffe
> r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : fffffffe
> Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> Control: 18c5387d  Table: 00004059  DAC: 00000015
> Process swapper (pid: 1, stack limit = 0xee02e230)
> Stack: (0xee02fdd0 to 0xee030000)
> fdc0:                                     c05b6a18 fffffffe ee02fdfc ee02fde8
> fde0: c0316628 c0316588 ee085400 fffffffe ee02fe24 ee02fe00 c03c1358 c0316608
> fe00: c03c1328 ee085410 c05aab98 c05aab98 00000000 c0587884 ee02fe34 ee02fe28
> fe20: c026a5c0 c03c1334 ee02fe5c ee02fe38 c0268f50 c026a5a8 c026a7b8 c0313ac0
> fe40: ee085410 ee085410 ee085444 c05aab98 ee02fe7c ee02fe60 c02691c0 c0268e18
> fe60: c0269150 c05aab98 c0269150 00000000 ee02fea4 ee02fe80 c0267320 c026915c
> fe80: ee00948c ee096df0 c021e02c c05aab98 ed9a0bc0 c05ab3a8 ee02feb4 ee02fea8
> fea0: c0268a28 c02672d0 ee02fee4 ee02feb8 c0268408 c0268a0c c04b56d1 00000000
> fec0: ee02fee4 c05aab98 c0565e60 00000000 0000006f c0587884 ee02ff14 ee02fee8
> fee0: c0269990 c0268340 00000000 00000000 c0565e60 00000000 0000006f c0587884
> ff00: 00000000 00000000 ee02ff24 ee02ff18 c026a9cc c02698f0 ee02ff3c ee02ff28
> ff20: c0565e84 c026a984 ee02e000 ee02ff40 ee02ff74 ee02ff40 c00086f8 c0565e6c
> ff40: c04f72e4 00000000 ee02ff74 00000006 00000006 c0571dac c0571d8c 0000006f
> ff60: c0587884 00000000 ee02ffac ee02ff78 c03bf9c4 c0008664 00000006 00000006
> ff80: c05511a8 00000000 ee02ffac 00000000 c03bf8cc 00000000 00000000 00000000
> ffa0: 00000000 ee02ffb0 c000ea98 c03bf8d8 00000000 00000000 00000000 00000000
> ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 0158ec21 0eced1ff
> [<c031659c>] (__clk_prepare+0x20/0x80) from [<c0316628>] (clk_prepare+0x2c/0x44)
> [<c0316628>] (clk_prepare+0x2c/0x44) from [<c03c1358>] (xuartps_probe+0x30/0x1ac
> )
> [<c03c1358>] (xuartps_probe+0x30/0x1ac) from [<c026a5c0>] (platform_drv_probe+0x
> 24/0x28)
> [<c026a5c0>] (platform_drv_probe+0x24/0x28) from [<c0268f50>] (driver_probe_devi
> ce+0x144/0x344)
> [<c0268f50>] (driver_probe_device+0x144/0x344) from [<c02691c0>] (__driver_attac
> h+0x70/0x94)
> [<c02691c0>] (__driver_attach+0x70/0x94) from [<c0267320>] (bus_for_each_dev+0x5
> c/0x8c)
> [<c0267320>] (bus_for_each_dev+0x5c/0x8c) from [<c0268a28>] (driver_attach+0x28/
> 0x30)
> [<c0268a28>] (driver_attach+0x28/0x30) from [<c0268408>] (bus_add_driver+0xd4/0x
> 254)
> [<c0268408>] (bus_add_driver+0xd4/0x254) from [<c0269990>] (driver_register+0xac
> /0x14c)
> [<c0269990>] (driver_register+0xac/0x14c) from [<c026a9cc>] (platform_driver_reg
> ister+0x54/0x68)
> [<c026a9cc>] (platform_driver_register+0x54/0x68) from [<c0565e84>] (xuartps_ini
> t+0x24/0x44)
> [<c0565e84>] (xuartps_init+0x24/0x44) from [<c00086f8>] (do_one_initcall+0xa0/0x
> 170)
> [<c00086f8>] (do_one_initcall+0xa0/0x170) from [<c03bf9c4>] (kernel_init+0xf8/0x
> 2ac)
> [<c03bf9c4>] (kernel_init+0xf8/0x2ac) from [<c000ea98>] (ret_from_fork+0x14/0x20
> )
> Code: e8bd4000 e2504000 01a05004 0a000015 (e594302c) 
> ---[ end trace 1b75b31a2719ed1d ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
>
> A probably unique thing I do is, I set the status of uart0 to disabled. This way
> I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
> before.
>

Thanks for testing. It seems that clocks are failing to register. Please
try this patch.

--->8-----

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 2be22a2..2734715 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
        struct clk *clk;

        clk = of_clk_get(pdev->dev.of_node, 0);
-       if (!clk) {
-               dev_err(&pdev->dev, "no clock specified\n");
-               return -ENODEV;
+       if (IS_ERR(clk)) {
+               dev_err(&pdev->dev, "failed to get clock\n");
+               return PTR_ERR(clk);
        }

        rc = clk_prepare_enable(clk);


-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Josh Cartwright]

On Wed, Dec 19, 2012 at 12:30:21PM -0800, Stephen Boyd wrote:
> On 12/19/12 11:22, Soren Brinkmann wrote:
[..]
> >
> > A probably unique thing I do is, I set the status of uart0 to disabled. This way
> > I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
> > before.
> >
> 
> Thanks for testing. It seems that clocks are failing to register. Please
> try this patch.
> 
> --->8-----
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 2be22a2..2734715 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
>         struct clk *clk;
> 
>         clk = of_clk_get(pdev->dev.of_node, 0);
> -       if (!clk) {
> -               dev_err(&pdev->dev, "no clock specified\n");
> -               return -ENODEV;
> +       if (IS_ERR(clk)) {
> +               dev_err(&pdev->dev, "failed to get clock\n");
> +               return PTR_ERR(clk);
>         }
> 
>         rc = clk_prepare_enable(clk);

Yes, indeed.

As a side note, this is introduced in my patch "serial: xilinx_uartps:
get clock rate info from dts", which is in xilinx/arm-next (and thus in
linux-next), but as far as I can tell, didn't ever make it into the
arm-soc tree.

Michal, did you have plans for pushing this through arm-soc?

Thanks,

  Josh

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Stephen Boyd]

On 12/19/12 12:30, Stephen Boyd wrote:
> On 12/19/12 11:22, Soren Brinkmann wrote:
>> Hi Stephen,
>>
>> I guess Josh is the better person to talk about this, since he created the
>> patches regarding common clock for mainline, but I tried running your series
>> and ran into this:
>>
>> Unable to handle kernel NULL pointer dereference at virtual address 0000002a
>> pgd = c0004000
>> [0000002a] *pgd=00000000
>> Internal error: Oops: 5 [#1] PREEMPT ARM
>> Modules linked in:
>> CPU: 0    Tainted: G        W     (3.7.0-rc3-00025-gc11ffdd-dirty #246)
>> PC is at __clk_prepare+0x20/0x80
>> LR is at clk_prepare+0x2c/0x44
>> pc : [<c031659c>]    lr : [<c0316628>]    psr: a0000153
>> sp : ee02fdd0  ip : ee02fde8  fp : ee02fde4
>> r10: 00000000  r9 : 00000000  r8 : c0587884
>> r7 : 00000000  r6 : c05aab98  r5 : fffffffe  r4 : fffffffe
>> r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : fffffffe
>> Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
>> Control: 18c5387d  Table: 00004059  DAC: 00000015
>> Process swapper (pid: 1, stack limit = 0xee02e230)
>> Stack: (0xee02fdd0 to 0xee030000)
>> fdc0:                                     c05b6a18 fffffffe ee02fdfc ee02fde8
>> fde0: c0316628 c0316588 ee085400 fffffffe ee02fe24 ee02fe00 c03c1358 c0316608
>> fe00: c03c1328 ee085410 c05aab98 c05aab98 00000000 c0587884 ee02fe34 ee02fe28
>> fe20: c026a5c0 c03c1334 ee02fe5c ee02fe38 c0268f50 c026a5a8 c026a7b8 c0313ac0
>> fe40: ee085410 ee085410 ee085444 c05aab98 ee02fe7c ee02fe60 c02691c0 c0268e18
>> fe60: c0269150 c05aab98 c0269150 00000000 ee02fea4 ee02fe80 c0267320 c026915c
>> fe80: ee00948c ee096df0 c021e02c c05aab98 ed9a0bc0 c05ab3a8 ee02feb4 ee02fea8
>> fea0: c0268a28 c02672d0 ee02fee4 ee02feb8 c0268408 c0268a0c c04b56d1 00000000
>> fec0: ee02fee4 c05aab98 c0565e60 00000000 0000006f c0587884 ee02ff14 ee02fee8
>> fee0: c0269990 c0268340 00000000 00000000 c0565e60 00000000 0000006f c0587884
>> ff00: 00000000 00000000 ee02ff24 ee02ff18 c026a9cc c02698f0 ee02ff3c ee02ff28
>> ff20: c0565e84 c026a984 ee02e000 ee02ff40 ee02ff74 ee02ff40 c00086f8 c0565e6c
>> ff40: c04f72e4 00000000 ee02ff74 00000006 00000006 c0571dac c0571d8c 0000006f
>> ff60: c0587884 00000000 ee02ffac ee02ff78 c03bf9c4 c0008664 00000006 00000006
>> ff80: c05511a8 00000000 ee02ffac 00000000 c03bf8cc 00000000 00000000 00000000
>> ffa0: 00000000 ee02ffb0 c000ea98 c03bf8d8 00000000 00000000 00000000 00000000
>> ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 0158ec21 0eced1ff
>> [<c031659c>] (__clk_prepare+0x20/0x80) from [<c0316628>] (clk_prepare+0x2c/0x44)
>> [<c0316628>] (clk_prepare+0x2c/0x44) from [<c03c1358>] (xuartps_probe+0x30/0x1ac
>> )
>> [<c03c1358>] (xuartps_probe+0x30/0x1ac) from [<c026a5c0>] (platform_drv_probe+0x
>> 24/0x28)
>> [<c026a5c0>] (platform_drv_probe+0x24/0x28) from [<c0268f50>] (driver_probe_devi
>> ce+0x144/0x344)
>> [<c0268f50>] (driver_probe_device+0x144/0x344) from [<c02691c0>] (__driver_attac
>> h+0x70/0x94)
>> [<c02691c0>] (__driver_attach+0x70/0x94) from [<c0267320>] (bus_for_each_dev+0x5
>> c/0x8c)
>> [<c0267320>] (bus_for_each_dev+0x5c/0x8c) from [<c0268a28>] (driver_attach+0x28/
>> 0x30)
>> [<c0268a28>] (driver_attach+0x28/0x30) from [<c0268408>] (bus_add_driver+0xd4/0x
>> 254)
>> [<c0268408>] (bus_add_driver+0xd4/0x254) from [<c0269990>] (driver_register+0xac
>> /0x14c)
>> [<c0269990>] (driver_register+0xac/0x14c) from [<c026a9cc>] (platform_driver_reg
>> ister+0x54/0x68)
>> [<c026a9cc>] (platform_driver_register+0x54/0x68) from [<c0565e84>] (xuartps_ini
>> t+0x24/0x44)
>> [<c0565e84>] (xuartps_init+0x24/0x44) from [<c00086f8>] (do_one_initcall+0xa0/0x
>> 170)
>> [<c00086f8>] (do_one_initcall+0xa0/0x170) from [<c03bf9c4>] (kernel_init+0xf8/0x
>> 2ac)
>> [<c03bf9c4>] (kernel_init+0xf8/0x2ac) from [<c000ea98>] (ret_from_fork+0x14/0x20
>> )
>> Code: e8bd4000 e2504000 01a05004 0a000015 (e594302c) 
>> ---[ end trace 1b75b31a2719ed1d ]---
>> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>>
>>
>> A probably unique thing I do is, I set the status of uart0 to disabled. This way
>> I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
>> before.
>>
> Thanks for testing. It seems that clocks are failing to register. Please
> try this patch.
>

Also, why are clock-names an optional property in devicetree? It would
be nice to not have two different APIs for consumers (i.e. drivers) to
get clocks from devicetree bindings. I suggest we do this for your uart
driver so that you can use the regular clock APIs and not the of_* ones.
We should get rid of of_clk_get().

---->8-----

diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index 5914b56..eef5f0c 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -45,6 +45,7 @@
                        reg = <0xE0000000 0x1000>;
                        interrupts = <0 27 4>;
                        clocks = <&uart_clk 0>;
+                       clock-names = "ref";
                };

                uart1: uart@e0001000 {
@@ -52,6 +53,7 @@
                        reg = <0xE0001000 0x1000>;
                        interrupts = <0 50 4>;
                        clocks = <&uart_clk 1>;
+                       clock-names = "ref";
                };

                slcr: slcr@f8000000 {
diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 2be22a2..6868e6b 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -947,10 +947,10 @@ static int xuartps_probe(struct platform_device *pdev)
        struct resource *res, *res2;
        struct clk *clk;

-       clk = of_clk_get(pdev->dev.of_node, 0);
-       if (!clk) {
-               dev_err(&pdev->dev, "no clock specified\n");
-               return -ENODEV;
+       clk = devm_clk_get(&pdev->dev, "ref");
+       if (IS_ERR(clk)) {
+               dev_err(&pdev->dev, "failed to get clock\n");
+               return PTR_ERR(clk);
        }

        rc = clk_prepare_enable(clk);

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Soren Brinkmann]

On Wed, Dec 19, 2012 at 12:30:21PM -0800, Stephen Boyd wrote:
> On 12/19/12 11:22, Soren Brinkmann wrote:
> > Hi Stephen,
> >
> > I guess Josh is the better person to talk about this, since he created the
> > patches regarding common clock for mainline, but I tried running your series
> > and ran into this:
> >
> > Unable to handle kernel NULL pointer dereference at virtual address 0000002a
> > pgd = c0004000
> > [0000002a] *pgd=00000000
> > Internal error: Oops: 5 [#1] PREEMPT ARM
> > Modules linked in:
> > CPU: 0    Tainted: G        W     (3.7.0-rc3-00025-gc11ffdd-dirty #246)
> > PC is at __clk_prepare+0x20/0x80
> > LR is at clk_prepare+0x2c/0x44
> > pc : [<c031659c>]    lr : [<c0316628>]    psr: a0000153
> > sp : ee02fdd0  ip : ee02fde8  fp : ee02fde4
> > r10: 00000000  r9 : 00000000  r8 : c0587884
> > r7 : 00000000  r6 : c05aab98  r5 : fffffffe  r4 : fffffffe
> > r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : fffffffe
> > Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> > Control: 18c5387d  Table: 00004059  DAC: 00000015
> > Process swapper (pid: 1, stack limit = 0xee02e230)
> > Stack: (0xee02fdd0 to 0xee030000)
> > fdc0:                                     c05b6a18 fffffffe ee02fdfc ee02fde8
> > fde0: c0316628 c0316588 ee085400 fffffffe ee02fe24 ee02fe00 c03c1358 c0316608
> > fe00: c03c1328 ee085410 c05aab98 c05aab98 00000000 c0587884 ee02fe34 ee02fe28
> > fe20: c026a5c0 c03c1334 ee02fe5c ee02fe38 c0268f50 c026a5a8 c026a7b8 c0313ac0
> > fe40: ee085410 ee085410 ee085444 c05aab98 ee02fe7c ee02fe60 c02691c0 c0268e18
> > fe60: c0269150 c05aab98 c0269150 00000000 ee02fea4 ee02fe80 c0267320 c026915c
> > fe80: ee00948c ee096df0 c021e02c c05aab98 ed9a0bc0 c05ab3a8 ee02feb4 ee02fea8
> > fea0: c0268a28 c02672d0 ee02fee4 ee02feb8 c0268408 c0268a0c c04b56d1 00000000
> > fec0: ee02fee4 c05aab98 c0565e60 00000000 0000006f c0587884 ee02ff14 ee02fee8
> > fee0: c0269990 c0268340 00000000 00000000 c0565e60 00000000 0000006f c0587884
> > ff00: 00000000 00000000 ee02ff24 ee02ff18 c026a9cc c02698f0 ee02ff3c ee02ff28
> > ff20: c0565e84 c026a984 ee02e000 ee02ff40 ee02ff74 ee02ff40 c00086f8 c0565e6c
> > ff40: c04f72e4 00000000 ee02ff74 00000006 00000006 c0571dac c0571d8c 0000006f
> > ff60: c0587884 00000000 ee02ffac ee02ff78 c03bf9c4 c0008664 00000006 00000006
> > ff80: c05511a8 00000000 ee02ffac 00000000 c03bf8cc 00000000 00000000 00000000
> > ffa0: 00000000 ee02ffb0 c000ea98 c03bf8d8 00000000 00000000 00000000 00000000
> > ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 0158ec21 0eced1ff
> > [<c031659c>] (__clk_prepare+0x20/0x80) from [<c0316628>] (clk_prepare+0x2c/0x44)
> > [<c0316628>] (clk_prepare+0x2c/0x44) from [<c03c1358>] (xuartps_probe+0x30/0x1ac
> > )
> > [<c03c1358>] (xuartps_probe+0x30/0x1ac) from [<c026a5c0>] (platform_drv_probe+0x
> > 24/0x28)
> > [<c026a5c0>] (platform_drv_probe+0x24/0x28) from [<c0268f50>] (driver_probe_devi
> > ce+0x144/0x344)
> > [<c0268f50>] (driver_probe_device+0x144/0x344) from [<c02691c0>] (__driver_attac
> > h+0x70/0x94)
> > [<c02691c0>] (__driver_attach+0x70/0x94) from [<c0267320>] (bus_for_each_dev+0x5
> > c/0x8c)
> > [<c0267320>] (bus_for_each_dev+0x5c/0x8c) from [<c0268a28>] (driver_attach+0x28/
> > 0x30)
> > [<c0268a28>] (driver_attach+0x28/0x30) from [<c0268408>] (bus_add_driver+0xd4/0x
> > 254)
> > [<c0268408>] (bus_add_driver+0xd4/0x254) from [<c0269990>] (driver_register+0xac
> > /0x14c)
> > [<c0269990>] (driver_register+0xac/0x14c) from [<c026a9cc>] (platform_driver_reg
> > ister+0x54/0x68)
> > [<c026a9cc>] (platform_driver_register+0x54/0x68) from [<c0565e84>] (xuartps_ini
> > t+0x24/0x44)
> > [<c0565e84>] (xuartps_init+0x24/0x44) from [<c00086f8>] (do_one_initcall+0xa0/0x
> > 170)
> > [<c00086f8>] (do_one_initcall+0xa0/0x170) from [<c03bf9c4>] (kernel_init+0xf8/0x
> > 2ac)
> > [<c03bf9c4>] (kernel_init+0xf8/0x2ac) from [<c000ea98>] (ret_from_fork+0x14/0x20
> > )
> > Code: e8bd4000 e2504000 01a05004 0a000015 (e594302c) 
> > ---[ end trace 1b75b31a2719ed1d ]---
> > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> >
> >
> > A probably unique thing I do is, I set the status of uart0 to disabled. This way
> > I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
> > before.
> >
> 
> Thanks for testing. It seems that clocks are failing to register. Please
> try this patch.
> 
> --->8-----
> 
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 2be22a2..2734715 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
>         struct clk *clk;
> 
>         clk = of_clk_get(pdev->dev.of_node, 0);
> -       if (!clk) {
> -               dev_err(&pdev->dev, "no clock specified\n");
> -               return -ENODEV;
> +       if (IS_ERR(clk)) {
> +               dev_err(&pdev->dev, "failed to get clock\n");
> +               return PTR_ERR(clk);
>         }
> 
>         rc = clk_prepare_enable(clk);
> 
Sorry, I messed something up. I tried to test this patch and my config didn't
build anymore. I rebuilt after making clean and now I'm seeing the same warnings
Josh already reported (w/o this patch applied).
Sorry for causing confusion.

I then cherry-picked Josh's commit (it's in the Xilinx arm-next tree btw. (sha1:
74cad6042156c30fe64d3226e041c77139ae8bcf)) and also added your proposed patch
from above. These all consistently gain the same results showing the warnings.

And you're right with that some clocks are not registered:
zynq:/debug/clk # lst
   .
   |-orphans
   |-ps_clk
   |---armpll
   |-----cpu_6x4x
   |---ddrpll
   |---iopll
   |-----uart0_ref_clk

	Soren

RE: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Michal Simek]

> -----Original Message-----
> From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> Sent: Wednesday, December 19, 2012 9:54 PM
> To: Stephen Boyd; Michal Simek
> Cc: Soren Brinkmann; Mike Turquette; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Subject: Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()
> 
> On Wed, Dec 19, 2012 at 12:30:21PM -0800, Stephen Boyd wrote:
> > On 12/19/12 11:22, Soren Brinkmann wrote:
> [..]
> > >
> > > A probably unique thing I do is, I set the status of uart0 to
> > > disabled. This way I can reuse my rootfs which does not run getty on
> > > ttyPS1. And this worked fine before.
> > >
> >
> > Thanks for testing. It seems that clocks are failing to register.
> > Please try this patch.
> >
> > --->8-----
> >
> > diff --git a/drivers/tty/serial/xilinx_uartps.c
> > b/drivers/tty/serial/xilinx_uartps.c
> > index 2be22a2..2734715 100644
> > --- a/drivers/tty/serial/xilinx_uartps.c
> > +++ b/drivers/tty/serial/xilinx_uartps.c
> > @@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
> >         struct clk *clk;
> >
> >         clk = of_clk_get(pdev->dev.of_node, 0);
> > -       if (!clk) {
> > -               dev_err(&pdev->dev, "no clock specified\n");
> > -               return -ENODEV;
> > +       if (IS_ERR(clk)) {
> > +               dev_err(&pdev->dev, "failed to get clock\n");
> > +               return PTR_ERR(clk);
> >         }
> >
> >         rc = clk_prepare_enable(clk);
> 
> Yes, indeed.
> 
> As a side note, this is introduced in my patch "serial: xilinx_uartps:
> get clock rate info from dts", which is in xilinx/arm-next (and thus in linux-next),
> but as far as I can tell, didn't ever make it into the arm-soc tree.
> 
> Michal, did you have plans for pushing this through arm-soc?

I have had this patch in my devel branch for a while. 
It is in arm-next tree right now and I will provide path to mainline.

Thanks for reminder,
Michal

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Josh Cartwright]

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

On Fri, Dec 21, 2012 at 03:28:10PM +0000, Michal Simek wrote:
> 
> 
> > -----Original Message-----
> > From: Josh Cartwright [mailto:josh.cartwright@ni.com]
> > Sent: Wednesday, December 19, 2012 9:54 PM
> > To: Stephen Boyd; Michal Simek
> > Cc: Soren Brinkmann; Mike Turquette; linux-kernel@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org
> > Subject: Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()
> > 
> > On Wed, Dec 19, 2012 at 12:30:21PM -0800, Stephen Boyd wrote:
> > > On 12/19/12 11:22, Soren Brinkmann wrote:
> > [..]
> > > >
> > > > A probably unique thing I do is, I set the status of uart0 to
> > > > disabled. This way I can reuse my rootfs which does not run getty on
> > > > ttyPS1. And this worked fine before.
> > > >
> > >
> > > Thanks for testing. It seems that clocks are failing to register.
> > > Please try this patch.
> > >
> > > --->8-----
> > >
> > > diff --git a/drivers/tty/serial/xilinx_uartps.c
> > > b/drivers/tty/serial/xilinx_uartps.c
> > > index 2be22a2..2734715 100644
> > > --- a/drivers/tty/serial/xilinx_uartps.c
> > > +++ b/drivers/tty/serial/xilinx_uartps.c
> > > @@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
> > >         struct clk *clk;
> > >
> > >         clk = of_clk_get(pdev->dev.of_node, 0);
> > > -       if (!clk) {
> > > -               dev_err(&pdev->dev, "no clock specified\n");
> > > -               return -ENODEV;
> > > +       if (IS_ERR(clk)) {
> > > +               dev_err(&pdev->dev, "failed to get clock\n");
> > > +               return PTR_ERR(clk);
> > >         }
> > >
> > >         rc = clk_prepare_enable(clk);
> > 
> > Yes, indeed.
> > 
> > As a side note, this is introduced in my patch "serial: xilinx_uartps:
> > get clock rate info from dts", which is in xilinx/arm-next (and thus in linux-next),
> > but as far as I can tell, didn't ever make it into the arm-soc tree.
> > 
> > Michal, did you have plans for pushing this through arm-soc?
> 
> I have had this patch in my devel branch for a while. 
> It is in arm-next tree right now and I will provide path to mainline.

Will you be rolling in Stephen's suggestions, or should he/I cook up a
patch on top with the fix in place?

It probably makes sense to pull the quoted fix above directly into the
patch before it hits mainline, and we can change the use of of_clk_get
as a patch on top.  Thoughts?

Thanks,

  Josh

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Rob Herring]

On 12/19/2012 02:54 PM, Stephen Boyd wrote:
> On 12/19/12 12:30, Stephen Boyd wrote:
>> On 12/19/12 11:22, Soren Brinkmann wrote:
>>> Hi Stephen,
>>>
>>> I guess Josh is the better person to talk about this, since he created the
>>> patches regarding common clock for mainline, but I tried running your series
>>> and ran into this:
>>>
>>> Unable to handle kernel NULL pointer dereference at virtual address 0000002a
>>> pgd = c0004000
>>> [0000002a] *pgd=00000000
>>> Internal error: Oops: 5 [#1] PREEMPT ARM
>>> Modules linked in:
>>> CPU: 0    Tainted: G        W     (3.7.0-rc3-00025-gc11ffdd-dirty #246)
>>> PC is at __clk_prepare+0x20/0x80
>>> LR is at clk_prepare+0x2c/0x44
>>> pc : [<c031659c>]    lr : [<c0316628>]    psr: a0000153
>>> sp : ee02fdd0  ip : ee02fde8  fp : ee02fde4
>>> r10: 00000000  r9 : 00000000  r8 : c0587884
>>> r7 : 00000000  r6 : c05aab98  r5 : fffffffe  r4 : fffffffe
>>> r3 : 00000000  r2 : 00000000  r1 : 00000000  r0 : fffffffe
>>> Flags: NzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
>>> Control: 18c5387d  Table: 00004059  DAC: 00000015
>>> Process swapper (pid: 1, stack limit = 0xee02e230)
>>> Stack: (0xee02fdd0 to 0xee030000)
>>> fdc0:                                     c05b6a18 fffffffe ee02fdfc ee02fde8
>>> fde0: c0316628 c0316588 ee085400 fffffffe ee02fe24 ee02fe00 c03c1358 c0316608
>>> fe00: c03c1328 ee085410 c05aab98 c05aab98 00000000 c0587884 ee02fe34 ee02fe28
>>> fe20: c026a5c0 c03c1334 ee02fe5c ee02fe38 c0268f50 c026a5a8 c026a7b8 c0313ac0
>>> fe40: ee085410 ee085410 ee085444 c05aab98 ee02fe7c ee02fe60 c02691c0 c0268e18
>>> fe60: c0269150 c05aab98 c0269150 00000000 ee02fea4 ee02fe80 c0267320 c026915c
>>> fe80: ee00948c ee096df0 c021e02c c05aab98 ed9a0bc0 c05ab3a8 ee02feb4 ee02fea8
>>> fea0: c0268a28 c02672d0 ee02fee4 ee02feb8 c0268408 c0268a0c c04b56d1 00000000
>>> fec0: ee02fee4 c05aab98 c0565e60 00000000 0000006f c0587884 ee02ff14 ee02fee8
>>> fee0: c0269990 c0268340 00000000 00000000 c0565e60 00000000 0000006f c0587884
>>> ff00: 00000000 00000000 ee02ff24 ee02ff18 c026a9cc c02698f0 ee02ff3c ee02ff28
>>> ff20: c0565e84 c026a984 ee02e000 ee02ff40 ee02ff74 ee02ff40 c00086f8 c0565e6c
>>> ff40: c04f72e4 00000000 ee02ff74 00000006 00000006 c0571dac c0571d8c 0000006f
>>> ff60: c0587884 00000000 ee02ffac ee02ff78 c03bf9c4 c0008664 00000006 00000006
>>> ff80: c05511a8 00000000 ee02ffac 00000000 c03bf8cc 00000000 00000000 00000000
>>> ffa0: 00000000 ee02ffb0 c000ea98 c03bf8d8 00000000 00000000 00000000 00000000
>>> ffc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> ffe0: 00000000 00000000 00000000 00000000 00000013 00000000 0158ec21 0eced1ff
>>> [<c031659c>] (__clk_prepare+0x20/0x80) from [<c0316628>] (clk_prepare+0x2c/0x44)
>>> [<c0316628>] (clk_prepare+0x2c/0x44) from [<c03c1358>] (xuartps_probe+0x30/0x1ac
>>> )
>>> [<c03c1358>] (xuartps_probe+0x30/0x1ac) from [<c026a5c0>] (platform_drv_probe+0x
>>> 24/0x28)
>>> [<c026a5c0>] (platform_drv_probe+0x24/0x28) from [<c0268f50>] (driver_probe_devi
>>> ce+0x144/0x344)
>>> [<c0268f50>] (driver_probe_device+0x144/0x344) from [<c02691c0>] (__driver_attac
>>> h+0x70/0x94)
>>> [<c02691c0>] (__driver_attach+0x70/0x94) from [<c0267320>] (bus_for_each_dev+0x5
>>> c/0x8c)
>>> [<c0267320>] (bus_for_each_dev+0x5c/0x8c) from [<c0268a28>] (driver_attach+0x28/
>>> 0x30)
>>> [<c0268a28>] (driver_attach+0x28/0x30) from [<c0268408>] (bus_add_driver+0xd4/0x
>>> 254)
>>> [<c0268408>] (bus_add_driver+0xd4/0x254) from [<c0269990>] (driver_register+0xac
>>> /0x14c)
>>> [<c0269990>] (driver_register+0xac/0x14c) from [<c026a9cc>] (platform_driver_reg
>>> ister+0x54/0x68)
>>> [<c026a9cc>] (platform_driver_register+0x54/0x68) from [<c0565e84>] (xuartps_ini
>>> t+0x24/0x44)
>>> [<c0565e84>] (xuartps_init+0x24/0x44) from [<c00086f8>] (do_one_initcall+0xa0/0x
>>> 170)
>>> [<c00086f8>] (do_one_initcall+0xa0/0x170) from [<c03bf9c4>] (kernel_init+0xf8/0x
>>> 2ac)
>>> [<c03bf9c4>] (kernel_init+0xf8/0x2ac) from [<c000ea98>] (ret_from_fork+0x14/0x20
>>> )
>>> Code: e8bd4000 e2504000 01a05004 0a000015 (e594302c) 
>>> ---[ end trace 1b75b31a2719ed1d ]---
>>> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>>>
>>>
>>> A probably unique thing I do is, I set the status of uart0 to disabled. This way
>>> I can reuse my rootfs which does not run getty on ttyPS1. And this worked fine
>>> before.
>>>
>> Thanks for testing. It seems that clocks are failing to register. Please
>> try this patch.
>>
> 
> Also, why are clock-names an optional property in devicetree? It would
> be nice to not have two different APIs for consumers (i.e. drivers) to
> get clocks from devicetree bindings. I suggest we do this for your uart
> driver so that you can use the regular clock APIs and not the of_* ones.
> We should get rid of of_clk_get().

This needs to work w/o names as you are breaking any platform with an
old dtb and this applied. This is a no-no. We've ignored that on ARM as
DT support has been under development, but it is time to start enforcing
that requirement. At least some platforms are using DT in production.

We match by index for irqs, register space, etc., so we should be able
to for clocks as well.

Rob

> 
> ---->8-----
> 
> diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
> index 5914b56..eef5f0c 100644
> --- a/arch/arm/boot/dts/zynq-7000.dtsi
> +++ b/arch/arm/boot/dts/zynq-7000.dtsi
> @@ -45,6 +45,7 @@
>                         reg = <0xE0000000 0x1000>;
>                         interrupts = <0 27 4>;
>                         clocks = <&uart_clk 0>;
> +                       clock-names = "ref";
>                 };
> 
>                 uart1: uart@e0001000 {
> @@ -52,6 +53,7 @@
>                         reg = <0xE0001000 0x1000>;
>                         interrupts = <0 50 4>;
>                         clocks = <&uart_clk 1>;
> +                       clock-names = "ref";
>                 };
> 
>                 slcr: slcr@f8000000 {
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 2be22a2..6868e6b 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -947,10 +947,10 @@ static int xuartps_probe(struct platform_device *pdev)
>         struct resource *res, *res2;
>         struct clk *clk;
> 
> -       clk = of_clk_get(pdev->dev.of_node, 0);
> -       if (!clk) {
> -               dev_err(&pdev->dev, "no clock specified\n");
> -               return -ENODEV;
> +       clk = devm_clk_get(&pdev->dev, "ref");
> +       if (IS_ERR(clk)) {
> +               dev_err(&pdev->dev, "failed to get clock\n");
> +               return PTR_ERR(clk);
>         }
> 
>         rc = clk_prepare_enable(clk);
> 

Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()

[Michal Simek]

2012/12/21 Josh Cartwright <josh.cartwright@ni.com>:
> On Fri, Dec 21, 2012 at 03:28:10PM +0000, Michal Simek wrote:
>>
>>
>> > -----Original Message-----
>> > From: Josh Cartwright [mailto:josh.cartwright@ni.com]
>> > Sent: Wednesday, December 19, 2012 9:54 PM
>> > To: Stephen Boyd; Michal Simek
>> > Cc: Soren Brinkmann; Mike Turquette; linux-kernel@vger.kernel.org; linux-arm-
>> > kernel@lists.infradead.org
>> > Subject: Re: [PATCH 4/4] clk: zynq: Use of_init_clk_data()
>> >
>> > On Wed, Dec 19, 2012 at 12:30:21PM -0800, Stephen Boyd wrote:
>> > > On 12/19/12 11:22, Soren Brinkmann wrote:
>> > [..]
>> > > >
>> > > > A probably unique thing I do is, I set the status of uart0 to
>> > > > disabled. This way I can reuse my rootfs which does not run getty on
>> > > > ttyPS1. And this worked fine before.
>> > > >
>> > >
>> > > Thanks for testing. It seems that clocks are failing to register.
>> > > Please try this patch.
>> > >
>> > > --->8-----
>> > >
>> > > diff --git a/drivers/tty/serial/xilinx_uartps.c
>> > > b/drivers/tty/serial/xilinx_uartps.c
>> > > index 2be22a2..2734715 100644
>> > > --- a/drivers/tty/serial/xilinx_uartps.c
>> > > +++ b/drivers/tty/serial/xilinx_uartps.c
>> > > @@ -948,9 +948,9 @@ static int xuartps_probe(struct platform_device *pdev)
>> > >         struct clk *clk;
>> > >
>> > >         clk = of_clk_get(pdev->dev.of_node, 0);
>> > > -       if (!clk) {
>> > > -               dev_err(&pdev->dev, "no clock specified\n");
>> > > -               return -ENODEV;
>> > > +       if (IS_ERR(clk)) {
>> > > +               dev_err(&pdev->dev, "failed to get clock\n");
>> > > +               return PTR_ERR(clk);
>> > >         }
>> > >
>> > >         rc = clk_prepare_enable(clk);
>> >
>> > Yes, indeed.
>> >
>> > As a side note, this is introduced in my patch "serial: xilinx_uartps:
>> > get clock rate info from dts", which is in xilinx/arm-next (and thus in linux-next),
>> > but as far as I can tell, didn't ever make it into the arm-soc tree.
>> >
>> > Michal, did you have plans for pushing this through arm-soc?
>>
>> I have had this patch in my devel branch for a while.
>> It is in arm-next tree right now and I will provide path to mainline.
>
> Will you be rolling in Stephen's suggestions, or should he/I cook up a
> patch on top with the fix in place?

Here is the patch I have applied.

http://git.xilinx.com/?p=linux-xlnx.git;a=commit;h=a3607ea56f4455c515201ab91d2304a634925f6c

> It probably makes sense to pull the quoted fix above directly into the
> patch before it hits mainline, and we can change the use of of_clk_get
> as a patch on top.  Thoughts?

Stephen: Can you please tell me how do you want to add these patches
to mainline?
Through arm-soc tree? Have you asked for pulling?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian