diff mbox

ARM: davinci: da8xx: Fix sleeping function called from invalid context

Message ID 1480350566-26225-1-git-send-email-abailon@baylibre.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandre Bailon Nov. 28, 2016, 4:29 p.m. UTC
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
usb20_phy_clk_enable(), called with the irq disabled uses
clk_get() and clk_enable_prepapre() which may sleep.
Move clk_get() to da8xx_register_usb20_phy_clk() and
replace clk_prepare_enable() by clk_enable().

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
---
 arch/arm/mach-davinci/usb-da8xx.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

Comments

Sekhar Nori Nov. 29, 2016, 10:48 a.m. UTC | #1
On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
> Everytime the usb20 phy is enabled, there is a
> "sleeping function called from invalid context" BUG.

Who calls PHY clk_enable() from non-preemptible context? Can you provide
the call stack?

> usb20_phy_clk_enable(), called with the irq disabled uses
> clk_get() and clk_enable_prepapre() which may sleep.
> Move clk_get() to da8xx_register_usb20_phy_clk() and
> replace clk_prepare_enable() by clk_enable().
> 
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  arch/arm/mach-davinci/usb-da8xx.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
> index b010e5f..c9b5cd4 100644
> --- a/arch/arm/mach-davinci/usb-da8xx.c
> +++ b/arch/arm/mach-davinci/usb-da8xx.c
> @@ -156,23 +156,23 @@ int __init da8xx_register_usb_refclkin(int rate)
>  	return 0;
>  }
>  
> +static struct clk *usb20_clk;
> +
>  static void usb20_phy_clk_enable(struct clk *clk)
>  {
> -	struct clk *usb20_clk;
>  	int err;
>  	u32 val;
>  	u32 timeout = 500000; /* 500 msec */
>  
>  	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>  
> -	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
> -	if (IS_ERR(usb20_clk)) {
> +	if (!usb20_clk || IS_ERR(usb20_clk)) {

NULL is a valid clock handle. There is no way clock enable of
usb20_phy_clk can be invoked if its not registered. So, you can assume
that usb20_clk is valid if you get here.

>  		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
>  		return;
>  	}
>  
>  	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
> -	err = clk_prepare_enable(usb20_clk);
> +	err = clk_enable(usb20_clk);
>  	if (err) {
>  		pr_err("failed to enable usb20 clk: %d\n", err);
>  		clk_put(usb20_clk);
> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>  
>  	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>  done:
> -	clk_disable_unprepare(usb20_clk);
> -	clk_put(usb20_clk);
> +	clk_disable(usb20_clk);


I noticed that we are missing clk_disable(usb20_clk) in
usb20_phy_clk_disable(). It will now be easier to do that after this
patch. Can you add that in a separate patch?

>  }
>  
>  static void usb20_phy_clk_disable(struct clk *clk)
> @@ -287,6 +286,10 @@ int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
>  	struct clk *parent;
>  	int ret = 0;
>  
> +	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
> +	if (IS_ERR(usb20_clk))
> +		return PTR_ERR(parent);
> +
>  	parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
>  	if (IS_ERR(parent))
>  		return PTR_ERR(parent);

clk_put(usb20_clk) should be called here on failure path.

Thanks,
Sekhar
Alexandre Bailon Nov. 29, 2016, 11:16 a.m. UTC | #2
On 11/29/2016 11:48 AM, Sekhar Nori wrote:
> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>> Everytime the usb20 phy is enabled, there is a
>> "sleeping function called from invalid context" BUG.
> 
> Who calls PHY clk_enable() from non-preemptible context? Can you provide
> the call stack?
Actually, clk_enable() is called from preemptible context (from phy
driver) but it disables interrupts before to call the clk_enable()
callback.
I attached the call stack that is probably more understandable than
my explanation.
> 
>> usb20_phy_clk_enable(), called with the irq disabled uses
>> clk_get() and clk_enable_prepapre() which may sleep.
>> Move clk_get() to da8xx_register_usb20_phy_clk() and
>> replace clk_prepare_enable() by clk_enable().
>>
>> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
>> ---
>>  arch/arm/mach-davinci/usb-da8xx.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
>> index b010e5f..c9b5cd4 100644
>> --- a/arch/arm/mach-davinci/usb-da8xx.c
>> +++ b/arch/arm/mach-davinci/usb-da8xx.c
>> @@ -156,23 +156,23 @@ int __init da8xx_register_usb_refclkin(int rate)
>>  	return 0;
>>  }
>>  
>> +static struct clk *usb20_clk;
>> +
>>  static void usb20_phy_clk_enable(struct clk *clk)
>>  {
>> -	struct clk *usb20_clk;
>>  	int err;
>>  	u32 val;
>>  	u32 timeout = 500000; /* 500 msec */
>>  
>>  	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
>>  
>> -	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
>> -	if (IS_ERR(usb20_clk)) {
>> +	if (!usb20_clk || IS_ERR(usb20_clk)) {
> 
> NULL is a valid clock handle. There is no way clock enable of
> usb20_phy_clk can be invoked if its not registered. So, you can assume
> that usb20_clk is valid if you get here.
OK.
> 
>>  		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
>>  		return;
>>  	}
>>  
>>  	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
>> -	err = clk_prepare_enable(usb20_clk);
>> +	err = clk_enable(usb20_clk);
>>  	if (err) {
>>  		pr_err("failed to enable usb20 clk: %d\n", err);
>>  		clk_put(usb20_clk);
>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>  
>>  	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>  done:
>> -	clk_disable_unprepare(usb20_clk);
>> -	clk_put(usb20_clk);
>> +	clk_disable(usb20_clk);
> 
> 
> I noticed that we are missing clk_disable(usb20_clk) in
> usb20_phy_clk_disable(). It will now be easier to do that after this
> patch. Can you add that in a separate patch?
> 
I don't think we need it.
What we don't see in this patch is that usb20_clk is enabled and,
it is disabled right after the PHY PLL is ready in usb20_phy_clk_enable().
>>  }
>>  
>>  static void usb20_phy_clk_disable(struct clk *clk)
>> @@ -287,6 +286,10 @@ int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
>>  	struct clk *parent;
>>  	int ret = 0;
>>  
>> +	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
>> +	if (IS_ERR(usb20_clk))
>> +		return PTR_ERR(parent);
>> +
>>  	parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
>>  	if (IS_ERR(parent))
>>  		return PTR_ERR(parent);
> 
> clk_put(usb20_clk) should be called here on failure path.
I will fix it.
> 
> Thanks,
> Sekhar
> 
Thanks,
Alexandre
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:97
in_atomic(): 1, irqs_disabled(): 128, pid: 1053, name: udevd
Preemption disabled at:[<c0017220>] clk_enable+0x40/0x94
CPU: 0 PID: 1053 Comm: udevd Not tainted 4.9.0-rc5-08315-gc26ef3f-dirty #90
Hardware name: Generic DA850/OMAP-L138/AM18x
Backtrace: 
[<c000d968>] (dump_backtrace) from [<c000dcf8>] (show_stack+0x20/0x24)
 r6:c0531e04 r5:c0017220
 r4:00000000 r3:00000000

[<c000dcd8>] (show_stack) from [<c025b398>] (dump_stack+0x20/0x28)
[<c025b378>] (dump_stack) from [<c00432e4>] (___might_sleep+0x140/0x1d8)
[<c00431a4>] (___might_sleep) from [<c00433e8>] (__might_sleep+0x6c/0xac)
 r5:00000061 r4:00000000

[<c004337c>] (__might_sleep) from [<c049487c>] (mutex_lock+0x28/0x4c)
 r7:c06592cc r6:0000ef32
 r5:c052b8a8 r4:c06592cc

[<c0494854>] (mutex_lock) from [<c02abc2c>] (clk_get_sys+0x2c/0x118)
 r4:c0676cbc r3:c06231f8

[<c02abc00>] (clk_get_sys) from [<c02abd48>] (clk_get+0x30/0x34)
 r10:4ea11003 r9:00000000
 r8:bf1fb620 r7:fee00000
 r6:0000ef32 r5:80000013
 r4:c0676cbc
[<c02abd18>] (clk_get) from [<c0018504>] (usb20_phy_clk_enable+0x2c/0x140)
[<c00184d8>] (usb20_phy_clk_enable) from [<c00171bc>] (__clk_enable+0x60/0x84)
 r7:fee00000 r6:c7bdbd80
 r5:80000013 r4:c0623388

[<c001715c>] (__clk_enable) from [<c0017228>] (clk_enable+0x48/0x94)
 r4:c0623388
[<c00171e0>] (clk_enable) from [<bf000238>] (da8xx_usb20_phy_power_on+0x38/0x88 [phy_da8xx_usb])
 r5:c7ba0fd0 r4:c0623388

[<bf000200>] (da8xx_usb20_phy_power_on [phy_da8xx_usb]) from [<c0284348>] (phy_power_on+0x9c/0xec)
 r5:c7bdbc00 r4:fffffdf4

[<c02842ac>] (phy_power_on) from [<bf1fa4dc>] (da8xx_musb_init+0xe4/0x1dc [da8xx])
 r6:c71fee50 r5:00000000
 r4:c7a3a010 r3:00000081

[<bf1fa3f8>] (da8xx_musb_init [da8xx]) from [<bf1deba4>] (musb_probe+0x1b4/0xc2c [musb_hdrc])
 r10:c7164540 r8:bf1ed3b8
 r7:bf1ee420 r6:bf1fae00
 r5:fee00000 r4:c7a3a010
[<bf1de9f0>] (musb_probe [musb_hdrc]) from [<c02f02e8>] (platform_drv_probe+0x48/0x98)
 r10:0000000c r9:00000000
 r8:bf1ed3b8 r7:00000000
 r6:c70d5410 r5:bf1ed3b8
 r4:bf1de9f0
[<c02f02a0>] (platform_drv_probe) from [<c02ee114>] (driver_probe_device+0x24c/0x448)
 r6:c0673938 r5:c06dce14
 r4:c70d5410 r3:c02f02a0

[<c02edec8>] (driver_probe_device) from [<c02ee694>] (__device_attach_driver+0xc0/0x128)
 r10:00000000 r8:c799d010
 r7:c710bb40 r6:c70d5410
 r5:bf1ed3b8 r4:00000001
[<c02ee5d4>] (__device_attach_driver) from [<c02ec104>] (bus_for_each_drv+0x70/0x98)
 r7:00000000 r6:00000000
 r5:c02ee5d4 r4:c710bb40

[<c02ec094>] (bus_for_each_drv) from [<c02eddbc>] (__device_attach+0xac/0x138)
 r6:00000001 r5:c70d5444
 r4:c70d5410
[<c02edd10>] (__device_attach) from [<c02ee718>] (device_initial_probe+0x1c/0x20)
 r7:00000000 r6:c70d5410
 r5:c065dff8 r4:c70d5410

[<c02ee6fc>] (device_initial_probe) from [<c02ed164>] (bus_probe_device+0x94/0x9c)
[<c02ed0d0>] (bus_probe_device) from [<c02eaf70>] (device_add+0x31c/0x570)
 r6:c06dcdf0 r5:c70d5418
 r4:c70d5410 r3:00000001

[<c02eac54>] (device_add) from [<c02f0504>] (platform_device_add+0x130/0x264)
 r10:00000000 r9:00000000
 r8:ffffffff r7:00000001
 r6:c70d5410 r5:c70d5400
 r4:00000002
[<c02f03d4>] (platform_device_add) from [<c02f0dbc>] (platform_device_register_full+0xec/0x118)
 r7:c710bc10 r6:c71fee90
 r5:c70d5400 r4:c710bc50

[<c02f0cd0>] (platform_device_register_full) from [<bf1facac>] (da8xx_probe+0x1a8/0x284 [da8xx])
 r5:c71fee50 r4:c799d010

[<bf1fab04>] (da8xx_probe [da8xx]) from [<c02f02e8>] (platform_drv_probe+0x48/0x98)
 r10:0000000b r9:bf1fb4a8
 r8:bf1fb360 r7:00000000
 r6:c799d010 r5:bf1fb360
 r4:bf1fab04
[<c02f02a0>] (platform_drv_probe) from [<c02ee114>] (driver_probe_device+0x24c/0x448)
 r6:c0673938 r5:c06dce14
 r4:c799d010 r3:c02f02a0

[<c02edec8>] (driver_probe_device) from [<c02ee40c>] (__driver_attach+0xfc/0x128)
 r10:c8ae54a4 r8:00000000
 r7:c0673860 r6:bf1fb360
 r5:c799d010 r4:c799d044
[<c02ee310>] (__driver_attach) from [<c02ec194>] (bus_for_each_dev+0x68/0x98)
 r6:00000000 r5:c02ee310
 r4:bf1fb360 r3:00000000

[<c02ec12c>] (bus_for_each_dev) from [<c02ed7b8>] (driver_attach+0x28/0x30)
 r6:c065dff8 r5:c723b420
 r4:bf1fb360
[<c02ed790>] (driver_attach) from [<c02ed43c>] (bus_add_driver+0x18c/0x268)
[<c02ed2b0>] (bus_add_driver) from [<c02ef0d0>] (driver_register+0x88/0x104)
 r8:bf1fd000 r7:c71fedc0
 r6:00000000 r5:00000001
 r4:bf1fb360
[<c02ef048>] (driver_register) from [<c02f0058>] (__platform_driver_register+0x40/0x54)
 r5:00000001 r4:bf1fb460

[<c02f0018>] (__platform_driver_register) from [<bf1fd018>] (da8xx_driver_init+0x18/0x24 [da8xx])
[<bf1fd000>] (da8xx_driver_init [da8xx]) from [<c0009af0>] (do_one_initcall+0x50/0x188)
[<c0009aa0>] (do_one_initcall) from [<c0085a2c>] (do_init_module+0x6c/0x1e8)
 r8:bf1fb460 r7:c71fedc0
 r6:c7164240 r5:00000001
 r4:bf1fb460
[<c00859c0>] (do_init_module) from [<c008795c>] (load_module+0x1ce8/0x2344)
 r6:00000001 r5:00000001
 r4:c710bf34
[<c0085c74>] (load_module) from [<c00881f4>] (SyS_finit_module+0xb4/0xe0)
 r10:00000000 r9:c710a000
 r8:c000aaa4 r7:00000000
 r6:7fffffff r5:0004f8c0
 r4:00000000
[<c0088140>] (SyS_finit_module) from [<c000a900>] (ret_fast_syscall+0x0/0x38)
 r7:0000017b r6:00000000
 r5:0004f8c0 r4:00020000
Sekhar Nori Nov. 29, 2016, 12:34 p.m. UTC | #3
On Tuesday 29 November 2016 04:46 PM, Alexandre Bailon wrote:
> On 11/29/2016 11:48 AM, Sekhar Nori wrote:
>> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>>> Everytime the usb20 phy is enabled, there is a
>>> "sleeping function called from invalid context" BUG.
>>
>> Who calls PHY clk_enable() from non-preemptible context? Can you provide
>> the call stack?
> Actually, clk_enable() is called from preemptible context (from phy
> driver) but it disables interrupts before to call the clk_enable()
> callback.
> I attached the call stack that is probably more understandable than
> my explanation.

Thanks! So this happens due to spin_lock_irqsave() in clk_enable() in
arch/arm/mach-davinci/clock.c. Can you add reference to this in your
commit description.

Also +David since this issue should have shown up since the time this
code was added.

>>>  		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
>>>  		return;
>>>  	}
>>>  
>>>  	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
>>> -	err = clk_prepare_enable(usb20_clk);
>>> +	err = clk_enable(usb20_clk);
>>>  	if (err) {
>>>  		pr_err("failed to enable usb20 clk: %d\n", err);
>>>  		clk_put(usb20_clk);
>>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>>  
>>>  	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>>  done:
>>> -	clk_disable_unprepare(usb20_clk);
>>> -	clk_put(usb20_clk);
>>> +	clk_disable(usb20_clk);
>>
>>
>> I noticed that we are missing clk_disable(usb20_clk) in
>> usb20_phy_clk_disable(). It will now be easier to do that after this
>> patch. Can you add that in a separate patch?
>>
> I don't think we need it.
> What we don't see in this patch is that usb20_clk is enabled and,
> it is disabled right after the PHY PLL is ready in usb20_phy_clk_enable().

Agreed.

Thanks,
Sekhar
David Lechner Nov. 29, 2016, 4:22 p.m. UTC | #4
On 11/29/2016 06:34 AM, Sekhar Nori wrote:
> On Tuesday 29 November 2016 04:46 PM, Alexandre Bailon wrote:
>> On 11/29/2016 11:48 AM, Sekhar Nori wrote:
>>> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>>>> Everytime the usb20 phy is enabled, there is a
>>>> "sleeping function called from invalid context" BUG.
>>>
>>> Who calls PHY clk_enable() from non-preemptible context? Can you provide
>>> the call stack?
>> Actually, clk_enable() is called from preemptible context (from phy
>> driver) but it disables interrupts before to call the clk_enable()
>> callback.
>> I attached the call stack that is probably more understandable than
>> my explanation.
>
> Thanks! So this happens due to spin_lock_irqsave() in clk_enable() in
> arch/arm/mach-davinci/clock.c. Can you add reference to this in your
> commit description.
>
> Also +David since this issue should have shown up since the time this
> code was added.

I'm not trying to pass the blame, but it was actually Axel that added 
the clk_get() code, so it will be good if he knows about the issue too. :-)

I don't think I have been building kernels with any of the kernel 
hacking options turned on, so I will start doing this to make sure we 
catch bugs like this.

>
>>>>  		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
>>>>  		return;
>>>>  	}
>>>>
>>>>  	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
>>>> -	err = clk_prepare_enable(usb20_clk);
>>>> +	err = clk_enable(usb20_clk);
>>>>  	if (err) {
>>>>  		pr_err("failed to enable usb20 clk: %d\n", err);
>>>>  		clk_put(usb20_clk);
>>>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>>>
>>>>  	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>>>  done:
>>>> -	clk_disable_unprepare(usb20_clk);
>>>> -	clk_put(usb20_clk);
>>>> +	clk_disable(usb20_clk);
>>>
>>>
>>> I noticed that we are missing clk_disable(usb20_clk) in
>>> usb20_phy_clk_disable(). It will now be easier to do that after this
>>> patch. Can you add that in a separate patch?
>>>
>> I don't think we need it.
>> What we don't see in this patch is that usb20_clk is enabled and,
>> it is disabled right after the PHY PLL is ready in usb20_phy_clk_enable().
>
> Agreed.
>
> Thanks,
> Sekhar
>
Axel Haslam Nov. 29, 2016, 4:36 p.m. UTC | #5
On Tue, Nov 29, 2016 at 5:22 PM, David Lechner <david@lechnology.com> wrote:
> On 11/29/2016 06:34 AM, Sekhar Nori wrote:
>>
>> On Tuesday 29 November 2016 04:46 PM, Alexandre Bailon wrote:
>>>
>>> On 11/29/2016 11:48 AM, Sekhar Nori wrote:
>>>>
>>>> On Monday 28 November 2016 09:59 PM, Alexandre Bailon wrote:
>>>>>
>>>>> Everytime the usb20 phy is enabled, there is a
>>>>> "sleeping function called from invalid context" BUG.
>>>>
>>>>
>>>> Who calls PHY clk_enable() from non-preemptible context? Can you provide
>>>> the call stack?
>>>
>>> Actually, clk_enable() is called from preemptible context (from phy
>>> driver) but it disables interrupts before to call the clk_enable()
>>> callback.
>>> I attached the call stack that is probably more understandable than
>>> my explanation.
>>
>>
>> Thanks! So this happens due to spin_lock_irqsave() in clk_enable() in
>> arch/arm/mach-davinci/clock.c. Can you add reference to this in your
>> commit description.
>>
>> Also +David since this issue should have shown up since the time this
>> code was added.
>
>
> I'm not trying to pass the blame, but it was actually Axel that added the
> clk_get() code, so it will be good if he knows about the issue too. :-)
>
> I don't think I have been building kernels with any of the kernel hacking
> options turned on, so I will start doing this to make sure we catch bugs
> like this.
>

Yes, sorry i missed this. i did not have that option enabled, and did
not catch this, thanks Alex!


>>
>>>>>                 pr_err("could not get usb20 clk: %ld\n",
>>>>> PTR_ERR(usb20_clk));
>>>>>                 return;
>>>>>         }
>>>>>
>>>>>         /* The USB 2.O PLL requires that the USB 2.O PSC is enabled as
>>>>> well. */
>>>>> -       err = clk_prepare_enable(usb20_clk);
>>>>> +       err = clk_enable(usb20_clk);
>>>>>         if (err) {
>>>>>                 pr_err("failed to enable usb20 clk: %d\n", err);
>>>>>                 clk_put(usb20_clk);
>>>>> @@ -197,8 +197,7 @@ static void usb20_phy_clk_enable(struct clk *clk)
>>>>>
>>>>>         pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
>>>>>  done:
>>>>> -       clk_disable_unprepare(usb20_clk);
>>>>> -       clk_put(usb20_clk);
>>>>> +       clk_disable(usb20_clk);
>>>>
>>>>
>>>>
>>>> I noticed that we are missing clk_disable(usb20_clk) in
>>>> usb20_phy_clk_disable(). It will now be easier to do that after this
>>>> patch. Can you add that in a separate patch?
>>>>
>>> I don't think we need it.
>>> What we don't see in this patch is that usb20_clk is enabled and,
>>> it is disabled right after the PHY PLL is ready in
>>> usb20_phy_clk_enable().
>>
>>
>> Agreed.
>>
>> Thanks,
>> Sekhar
>>
>
diff mbox

Patch

diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..c9b5cd4 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -156,23 +156,23 @@  int __init da8xx_register_usb_refclkin(int rate)
 	return 0;
 }
 
+static struct clk *usb20_clk;
+
 static void usb20_phy_clk_enable(struct clk *clk)
 {
-	struct clk *usb20_clk;
 	int err;
 	u32 val;
 	u32 timeout = 500000; /* 500 msec */
 
 	val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG));
 
-	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
-	if (IS_ERR(usb20_clk)) {
+	if (!usb20_clk || IS_ERR(usb20_clk)) {
 		pr_err("could not get usb20 clk: %ld\n", PTR_ERR(usb20_clk));
 		return;
 	}
 
 	/* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */
-	err = clk_prepare_enable(usb20_clk);
+	err = clk_enable(usb20_clk);
 	if (err) {
 		pr_err("failed to enable usb20 clk: %d\n", err);
 		clk_put(usb20_clk);
@@ -197,8 +197,7 @@  static void usb20_phy_clk_enable(struct clk *clk)
 
 	pr_err("Timeout waiting for USB 2.0 PHY clock good\n");
 done:
-	clk_disable_unprepare(usb20_clk);
-	clk_put(usb20_clk);
+	clk_disable(usb20_clk);
 }
 
 static void usb20_phy_clk_disable(struct clk *clk)
@@ -287,6 +286,10 @@  int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
 	struct clk *parent;
 	int ret = 0;
 
+	usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+	if (IS_ERR(usb20_clk))
+		return PTR_ERR(parent);
+
 	parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
 	if (IS_ERR(parent))
 		return PTR_ERR(parent);