* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
@ 2016-12-05 14:13 Alexandre Bailon
2016-12-06 9:33 ` Sekhar Nori
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Bailon @ 2016-12-05 14:13 UTC (permalink / raw)
To: linux-arm-kernel
Everytime the usb20 phy is enabled, there is a
"sleeping function called from invalid context" BUG.
clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
before to invoke the callback usb20_phy_clk_enable().
usb20_phy_clk_enable() 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 | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c
index b010e5f..f62f9c2 100644
--- a/arch/arm/mach-davinci/usb-da8xx.c
+++ b/arch/arm/mach-davinci/usb-da8xx.c
@@ -22,6 +22,8 @@
#define DA8XX_USB0_BASE 0x01e00000
#define DA8XX_USB1_BASE 0x01e25000
+static struct clk *usb20_clk;
+
static struct platform_device da8xx_usb_phy = {
.name = "da8xx-usb-phy",
.id = -1,
@@ -158,24 +160,16 @@ int __init da8xx_register_usb_refclkin(int rate)
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)) {
- 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);
return;
}
@@ -197,8 +191,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)
@@ -285,11 +278,19 @@ static struct clk_lookup usb20_phy_clk_lookup =
int __init da8xx_register_usb20_phy_clk(bool use_usb_refclkin)
{
struct clk *parent;
- int ret = 0;
+ int ret;
+
+ usb20_clk = clk_get(&da8xx_usb20_dev.dev, "usb20");
+ ret = PTR_ERR_OR_ZERO(usb20_clk);
+ if (ret)
+ return ret;
parent = clk_get(NULL, use_usb_refclkin ? "usb_refclkin" : "pll0_aux");
- if (IS_ERR(parent))
- return PTR_ERR(parent);
+ ret = PTR_ERR_OR_ZERO(parent);
+ if (ret) {
+ clk_put(usb20_clk);
+ return ret;
+ }
usb20_phy_clk.parent = parent;
ret = clk_register(&usb20_phy_clk);
--
2.7.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
2016-12-05 14:13 [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context Alexandre Bailon
@ 2016-12-06 9:33 ` Sekhar Nori
2016-12-06 9:51 ` Alexandre Bailon
0 siblings, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2016-12-06 9:33 UTC (permalink / raw)
To: linux-arm-kernel
On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
> Everytime the usb20 phy is enabled, there is a
> "sleeping function called from invalid context" BUG.
>
> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
> before to invoke the callback usb20_phy_clk_enable().
> usb20_phy_clk_enable() 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>
This will still cause the recursive locking problem reported by David.
Not sure what the point of sending this version was.
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
2016-12-06 9:33 ` Sekhar Nori
@ 2016-12-06 9:51 ` Alexandre Bailon
2016-12-06 9:57 ` Sekhar Nori
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Bailon @ 2016-12-06 9:51 UTC (permalink / raw)
To: linux-arm-kernel
On 12/06/2016 10:33 AM, Sekhar Nori wrote:
> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>> Everytime the usb20 phy is enabled, there is a
>> "sleeping function called from invalid context" BUG.
>>
>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>> before to invoke the callback usb20_phy_clk_enable().
>> usb20_phy_clk_enable() 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>
>
> This will still cause the recursive locking problem reported by David.
> Not sure what the point of sending this version was.
>
> Thanks,
> Sekhar
>
What am I supposed to do ?
Thanks,
Alexandre
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
2016-12-06 9:51 ` Alexandre Bailon
@ 2016-12-06 9:57 ` Sekhar Nori
2016-12-06 18:56 ` David Lechner
0 siblings, 1 reply; 6+ messages in thread
From: Sekhar Nori @ 2016-12-06 9:57 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 06 December 2016 03:21 PM, Alexandre Bailon wrote:
> On 12/06/2016 10:33 AM, Sekhar Nori wrote:
>> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>>> Everytime the usb20 phy is enabled, there is a
>>> "sleeping function called from invalid context" BUG.
>>>
>>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>>> before to invoke the callback usb20_phy_clk_enable().
>>> usb20_phy_clk_enable() 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>
>>
>> This will still cause the recursive locking problem reported by David.
>> Not sure what the point of sending this version was.
>>
>> Thanks,
>> Sekhar
>>
> What am I supposed to do ?
That needs to be resolved between you and David. Perhaps convert the fix
sent by David into a proper patch and base this patch on that. Or wait
for David to send it himself and let him also make the modifications
needed in this patch.
David ?
Thanks,
Sekhar
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
2016-12-06 9:57 ` Sekhar Nori
@ 2016-12-06 18:56 ` David Lechner
2016-12-07 9:07 ` Alexandre Bailon
0 siblings, 1 reply; 6+ messages in thread
From: David Lechner @ 2016-12-06 18:56 UTC (permalink / raw)
To: linux-arm-kernel
On 12/06/2016 03:57 AM, Sekhar Nori wrote:
> On Tuesday 06 December 2016 03:21 PM, Alexandre Bailon wrote:
>> On 12/06/2016 10:33 AM, Sekhar Nori wrote:
>>> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>>>> Everytime the usb20 phy is enabled, there is a
>>>> "sleeping function called from invalid context" BUG.
>>>>
>>>> clk_enable() from arch/arm/mach-davinci/clock.c uses spin_lock_irqsave()
>>>> before to invoke the callback usb20_phy_clk_enable().
>>>> usb20_phy_clk_enable() 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>
>>>
>>> This will still cause the recursive locking problem reported by David.
>>> Not sure what the point of sending this version was.
>>>
>>> Thanks,
>>> Sekhar
>>>
>
>> What am I supposed to do ?
>
> That needs to be resolved between you and David. Perhaps convert the fix
> sent by David into a proper patch and base this patch on that. Or wait
> for David to send it himself and let him also make the modifications
> needed in this patch.
>
> David ?
>
> Thanks,
> Sekhar
>
Alexandre, I was hoping that you would just squash my patch with your
patch and take Sekhar's suggestion about a separate patch to make the
private __clk_enable() public as davinci_clk_enable() when you re-submit.
You can add "Suggested-By: David Lechner <david@lechnology.com>" to the
commit message if you would like to give me some credit for my ideas.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context
2016-12-06 18:56 ` David Lechner
@ 2016-12-07 9:07 ` Alexandre Bailon
0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Bailon @ 2016-12-07 9:07 UTC (permalink / raw)
To: linux-arm-kernel
On 12/06/2016 07:56 PM, David Lechner wrote:
> On 12/06/2016 03:57 AM, Sekhar Nori wrote:
>> On Tuesday 06 December 2016 03:21 PM, Alexandre Bailon wrote:
>>> On 12/06/2016 10:33 AM, Sekhar Nori wrote:
>>>> On Monday 05 December 2016 07:43 PM, Alexandre Bailon wrote:
>>>>> Everytime the usb20 phy is enabled, there is a
>>>>> "sleeping function called from invalid context" BUG.
>>>>>
>>>>> clk_enable() from arch/arm/mach-davinci/clock.c uses
>>>>> spin_lock_irqsave()
>>>>> before to invoke the callback usb20_phy_clk_enable().
>>>>> usb20_phy_clk_enable() 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>
>>>>
>>>> This will still cause the recursive locking problem reported by David.
>>>> Not sure what the point of sending this version was.
>>>>
>>>> Thanks,
>>>> Sekhar
>>>>
>>
>>> What am I supposed to do ?
>>
>> That needs to be resolved between you and David. Perhaps convert the fix
>> sent by David into a proper patch and base this patch on that. Or wait
>> for David to send it himself and let him also make the modifications
>> needed in this patch.
>>
>> David ?
>>
>> Thanks,
>> Sekhar
>>
>
> Alexandre, I was hoping that you would just squash my patch with your
> patch and take Sekhar's suggestion about a separate patch to make the
> private __clk_enable() public as davinci_clk_enable() when you re-submit.
OK.
>
> You can add "Suggested-By: David Lechner <david@lechnology.com>" to the
> commit message if you would like to give me some credit for my ideas.
That was my concern. I will do that.
Thanks,
Alexandre
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-07 9:07 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-05 14:13 [PATCH v5] ARM: davinci: da8xx: Fix sleeping function called from invalid context Alexandre Bailon
2016-12-06 9:33 ` Sekhar Nori
2016-12-06 9:51 ` Alexandre Bailon
2016-12-06 9:57 ` Sekhar Nori
2016-12-06 18:56 ` David Lechner
2016-12-07 9:07 ` Alexandre Bailon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox