forked from mirrors/linux
		
	clkdev: Always allocate a struct clk and call __clk_get() w/ CCF
of_clk_get_by_clkspec() returns a struct clk pointer but it
doesn't create a new handle for the consumers when we're using
the common clock framework. Instead it just returns whatever the
clk provider hands out. When the consumers go to call clk_put()
we get an Oops.
Unable to handle kernel paging request at virtual address 00200200
pgd = c0004000
[00200200] *pgd=00000000
Internal error: Oops: 805 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.19.0-rc1-00104-ga251361a-dirty #992
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
task: ee00b000 ti: ee088000 task.ti: ee088000
PC is at __clk_put+0x24/0xd0
LR is at clk_prepare_lock+0xc/0xec
pc : [<c03eef38>]    lr : [<c03ec1f4>]    psr: 20000153
sp : ee089de8  ip : 00000000  fp : 00000000
r10: ee02f480  r9 : 00000001  r8 : 00000000
r7 : ee031cc0  r6 : ee089e08  r5 : 00000000  r4 : ee02f480
r3 : 00100100  r2 : 00200200  r1 : 0000091e  r0 : 00000001
Flags: nzCv  IRQs on  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
Control: 10c5387d  Table: 4000404a  DAC: 00000015
Process swapper/0 (pid: 1, stack limit = 0xee088238)
Stack: (0xee089de8 to 0xee08a000)
9de0:                   ee7c8f14 c03f0ec8 ee089e08 00000000 c0718dc8 00000001
9e00: 00000000 c04ee0f0 ee7e0844 00000001 00000181 c04edb58 ee2bd320 00000000
9e20: 00000000 c011dc5c ee16a1e0 00000000 00000000 c0718dc8 ee16a1e0 ee2bd1e0
9e40: c0641740 ee16a1e0 00000000 ee2bd320 c0718dc8 ee1d3e10 ee1d3e10 00000000
9e60: c0769a88 00000000 c0718dc8 00000000 00000000 c02c3124 c02c310c ee1d3e10
9e80: c07b4eec 00000000 c0769a88 c02c1d0c ee1d3e10 c0769a88 ee1d3e44 00000000
9ea0: c07091dc c02c1eb8 00000000 c0769a88 c02c1e2c c02c0544 ee005478 ee1676c0
9ec0: c0769a88 ee3a4e80 c0760ce8 c02c150c c0669b90 c0769a88 c0746cd8 c0769a88
9ee0: c0746cd8 ee2bc4c0 c0778c00 c02c24e0 00000000 c0746cd8 c0746cd8 c07091f0
9f00: 00000000 c0008944 c04f405c 00000025 ee00b000 60000153 c074ab00 00000000
9f20: 00000000 c074ab90 60000153 00000000 ef7fca5d c050860c 000000b6 c0036b88
9f40: c065ecc4 c06bc728 00000006 00000006 c074ab30 ef7fca40 c0739bdc 00000006
9f60: c0718dbc c0778c00 000000b6 c0718dc8 c06ed598 c06edd64 00000006 00000006
9f80: c06ed598 c003b438 00000000 c04e64f4 00000000 00000000 00000000 00000000
9fa0: 00000000 c04e64fc 00000000 c000e838 00000000 00000000 00000000 00000000
9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 c0c0c0c0 c0c0c0c0
[<c03eef38>] (__clk_put) from [<c03f0ec8>] (of_clk_set_defaults+0xe0/0x2c0)
[<c03f0ec8>] (of_clk_set_defaults) from [<c02c3124>] (platform_drv_probe+0x18/0xa4)
[<c02c3124>] (platform_drv_probe) from [<c02c1d0c>] (driver_probe_device+0x10c/0x22c)
[<c02c1d0c>] (driver_probe_device) from [<c02c1eb8>] (__driver_attach+0x8c/0x90)
[<c02c1eb8>] (__driver_attach) from [<c02c0544>] (bus_for_each_dev+0x54/0x88)
[<c02c0544>] (bus_for_each_dev) from [<c02c150c>] (bus_add_driver+0xd4/0x1d0)
[<c02c150c>] (bus_add_driver) from [<c02c24e0>] (driver_register+0x78/0xf4)
[<c02c24e0>] (driver_register) from [<c07091f0>] (fimc_md_init+0x14/0x30)
[<c07091f0>] (fimc_md_init) from [<c0008944>] (do_one_initcall+0x80/0x1d0)
[<c0008944>] (do_one_initcall) from [<c06edd64>] (kernel_init_freeable+0x108/0x1d4)
[<c06edd64>] (kernel_init_freeable) from [<c04e64fc>] (kernel_init+0x8/0xec)
[<c04e64fc>] (kernel_init) from [<c000e838>] (ret_from_fork+0x14/0x3c)
Code: ebfff4ae e5943014 e5942018 e3530000 (e5823000)
Let's create a per-user handle here so that clk_put() can
properly unlink it and free the handle. Now that we allocate a
clk structure here we need to free it if __clk_get() fails so
bury the __clk_get() call in __of_clk_get_from_provider(). We
need to handle the same problem in clk_get_sys() so export
__clk_free_clk() to clkdev.c and do the same thing, except let's
use a union to make this code #ifdef free.
This fixes the above crash, properly calls __clk_get() when
of_clk_get_from_provider() is called, and cleans up the clk
structure on the error path of clk_get_sys().
Fixes: 035a61c314 "clk: Make clk API return per-user struct clk instances"
Reported-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Reported-by: Alban Browaeys <alban.browaeys@gmail.com>
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Tested-by: Alban Browaeys <prahal@yahoo.com>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Michael Turquette <mturquette@linaro.org>
			
			
This commit is contained in:
		
							parent
							
								
									5469d4f22e
								
							
						
					
					
						commit
						73e0e496af
					
				
					 3 changed files with 59 additions and 37 deletions
				
			
		|  | @ -2382,7 +2382,7 @@ struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id, | |||
| 	return clk; | ||||
| } | ||||
| 
 | ||||
| static void __clk_free_clk(struct clk *clk) | ||||
| void __clk_free_clk(struct clk *clk) | ||||
| { | ||||
| 	clk_prepare_lock(); | ||||
| 	hlist_del(&clk->child_node); | ||||
|  | @ -2894,7 +2894,8 @@ void of_clk_del_provider(struct device_node *np) | |||
| } | ||||
| EXPORT_SYMBOL_GPL(of_clk_del_provider); | ||||
| 
 | ||||
| struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec) | ||||
| struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec, | ||||
| 				       const char *dev_id, const char *con_id) | ||||
| { | ||||
| 	struct of_clk_provider *provider; | ||||
| 	struct clk *clk = ERR_PTR(-EPROBE_DEFER); | ||||
|  | @ -2903,8 +2904,17 @@ struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec) | |||
| 	list_for_each_entry(provider, &of_clk_providers, link) { | ||||
| 		if (provider->node == clkspec->np) | ||||
| 			clk = provider->get(clkspec, provider->data); | ||||
| 		if (!IS_ERR(clk)) | ||||
| 		if (!IS_ERR(clk)) { | ||||
| 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id, | ||||
| 					       con_id); | ||||
| 
 | ||||
| 			if (!IS_ERR(clk) && !__clk_get(clk)) { | ||||
| 				__clk_free_clk(clk); | ||||
| 				clk = ERR_PTR(-ENOENT); | ||||
| 			} | ||||
| 
 | ||||
| 			break; | ||||
| 		} | ||||
| 	} | ||||
| 
 | ||||
| 	return clk; | ||||
|  | @ -2915,7 +2925,7 @@ struct clk *of_clk_get_from_provider(struct of_phandle_args *clkspec) | |||
| 	struct clk *clk; | ||||
| 
 | ||||
| 	mutex_lock(&of_clk_mutex); | ||||
| 	clk = __of_clk_get_from_provider(clkspec); | ||||
| 	clk = __of_clk_get_from_provider(clkspec, NULL, __func__); | ||||
| 	mutex_unlock(&of_clk_mutex); | ||||
| 
 | ||||
| 	return clk; | ||||
|  |  | |||
|  | @ -13,10 +13,27 @@ struct clk_hw; | |||
| 
 | ||||
| #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) | ||||
| struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec); | ||||
| struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec); | ||||
| struct clk *__of_clk_get_from_provider(struct of_phandle_args *clkspec, | ||||
| 				       const char *dev_id, const char *con_id); | ||||
| void of_clk_lock(void); | ||||
| void of_clk_unlock(void); | ||||
| #endif | ||||
| 
 | ||||
| #ifdef CONFIG_COMMON_CLK | ||||
| struct clk *__clk_create_clk(struct clk_hw *hw, const char *dev_id, | ||||
| 			     const char *con_id); | ||||
| void __clk_free_clk(struct clk *clk); | ||||
| #else | ||||
| /* All these casts to avoid ifdefs in clkdev... */ | ||||
| static inline struct clk * | ||||
| __clk_create_clk(struct clk_hw *hw, const char *dev_id, const char *con_id) | ||||
| { | ||||
| 	return (struct clk *)hw; | ||||
| } | ||||
| static inline void __clk_free_clk(struct clk *clk) { } | ||||
| static struct clk_hw *__clk_get_hw(struct clk *clk) | ||||
| { | ||||
| 	return (struct clk_hw *)clk; | ||||
| } | ||||
| 
 | ||||
| #endif | ||||
|  |  | |||
|  | @ -29,6 +29,20 @@ static DEFINE_MUTEX(clocks_mutex); | |||
| 
 | ||||
| #if defined(CONFIG_OF) && defined(CONFIG_COMMON_CLK) | ||||
| 
 | ||||
| static struct clk *__of_clk_get_by_clkspec(struct of_phandle_args *clkspec, | ||||
| 					 const char *dev_id, const char *con_id) | ||||
| { | ||||
| 	struct clk *clk; | ||||
| 
 | ||||
| 	if (!clkspec) | ||||
| 		return ERR_PTR(-EINVAL); | ||||
| 
 | ||||
| 	of_clk_lock(); | ||||
| 	clk = __of_clk_get_from_provider(clkspec, dev_id, con_id); | ||||
| 	of_clk_unlock(); | ||||
| 	return clk; | ||||
| } | ||||
| 
 | ||||
| /**
 | ||||
|  * of_clk_get_by_clkspec() - Lookup a clock form a clock provider | ||||
|  * @clkspec: pointer to a clock specifier data structure | ||||
|  | @ -39,22 +53,11 @@ static DEFINE_MUTEX(clocks_mutex); | |||
|  */ | ||||
| struct clk *of_clk_get_by_clkspec(struct of_phandle_args *clkspec) | ||||
| { | ||||
| 	struct clk *clk; | ||||
| 
 | ||||
| 	if (!clkspec) | ||||
| 		return ERR_PTR(-EINVAL); | ||||
| 
 | ||||
| 	of_clk_lock(); | ||||
| 	clk = __of_clk_get_from_provider(clkspec); | ||||
| 
 | ||||
| 	if (!IS_ERR(clk) && !__clk_get(clk)) | ||||
| 		clk = ERR_PTR(-ENOENT); | ||||
| 
 | ||||
| 	of_clk_unlock(); | ||||
| 	return clk; | ||||
| 	return __of_clk_get_by_clkspec(clkspec, NULL, __func__); | ||||
| } | ||||
| 
 | ||||
| static struct clk *__of_clk_get(struct device_node *np, int index) | ||||
| static struct clk *__of_clk_get(struct device_node *np, int index, | ||||
| 			       const char *dev_id, const char *con_id) | ||||
| { | ||||
| 	struct of_phandle_args clkspec; | ||||
| 	struct clk *clk; | ||||
|  | @ -68,7 +71,7 @@ static struct clk *__of_clk_get(struct device_node *np, int index) | |||
| 	if (rc) | ||||
| 		return ERR_PTR(rc); | ||||
| 
 | ||||
| 	clk = of_clk_get_by_clkspec(&clkspec); | ||||
| 	clk = __of_clk_get_by_clkspec(&clkspec, dev_id, con_id); | ||||
| 	of_node_put(clkspec.np); | ||||
| 
 | ||||
| 	return clk; | ||||
|  | @ -76,12 +79,7 @@ static struct clk *__of_clk_get(struct device_node *np, int index) | |||
| 
 | ||||
| struct clk *of_clk_get(struct device_node *np, int index) | ||||
| { | ||||
| 	struct clk *clk = __of_clk_get(np, index); | ||||
| 
 | ||||
| 	if (!IS_ERR(clk)) | ||||
| 		clk = __clk_create_clk(__clk_get_hw(clk), np->full_name, NULL); | ||||
| 
 | ||||
| 	return clk; | ||||
| 	return __of_clk_get(np, index, np->full_name, NULL); | ||||
| } | ||||
| EXPORT_SYMBOL(of_clk_get); | ||||
| 
 | ||||
|  | @ -102,12 +100,10 @@ static struct clk *__of_clk_get_by_name(struct device_node *np, | |||
| 		 */ | ||||
| 		if (name) | ||||
| 			index = of_property_match_string(np, "clock-names", name); | ||||
| 		clk = __of_clk_get(np, index); | ||||
| 		clk = __of_clk_get(np, index, dev_id, name); | ||||
| 		if (!IS_ERR(clk)) { | ||||
| 			clk = __clk_create_clk(__clk_get_hw(clk), dev_id, name); | ||||
| 			break; | ||||
| 		} | ||||
| 		else if (name && index >= 0) { | ||||
| 		} else if (name && index >= 0) { | ||||
| 			if (PTR_ERR(clk) != -EPROBE_DEFER) | ||||
| 				pr_err("ERROR: could not get clock %s:%s(%i)\n", | ||||
| 					np->full_name, name ? name : "", index); | ||||
|  | @ -209,17 +205,16 @@ struct clk *clk_get_sys(const char *dev_id, const char *con_id) | |||
| 	if (!cl) | ||||
| 		goto out; | ||||
| 
 | ||||
| 	if (!__clk_get(cl->clk)) { | ||||
| 	clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id); | ||||
| 	if (IS_ERR(clk)) | ||||
| 		goto out; | ||||
| 
 | ||||
| 	if (!__clk_get(clk)) { | ||||
| 		__clk_free_clk(clk); | ||||
| 		cl = NULL; | ||||
| 		goto out; | ||||
| 	} | ||||
| 
 | ||||
| #if defined(CONFIG_COMMON_CLK) | ||||
| 	clk = __clk_create_clk(__clk_get_hw(cl->clk), dev_id, con_id); | ||||
| #else | ||||
| 	clk = cl->clk; | ||||
| #endif | ||||
| 
 | ||||
| out: | ||||
| 	mutex_unlock(&clocks_mutex); | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue
	
	 Stephen Boyd
						Stephen Boyd