From patchwork Mon Dec 5 03:44:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Lechner X-Patchwork-Id: 9460261 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4EE0E6022E for ; Mon, 5 Dec 2016 03:47:23 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 483AB26212 for ; Mon, 5 Dec 2016 03:47:23 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3A74D26224; Mon, 5 Dec 2016 03:47:23 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 5793026212 for ; Mon, 5 Dec 2016 03:47:22 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1cDkDd-0004BQ-TO; Mon, 05 Dec 2016 03:45:33 +0000 Received: from vern.gendns.com ([206.190.152.46]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1cDkDX-0002r2-0N for linux-arm-kernel@lists.infradead.org; Mon, 05 Dec 2016 03:45:30 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lechnology.com; s=default; h=Content-Transfer-Encoding:Content-Type: In-Reply-To:MIME-Version:Date:Message-ID:From:Cc:References:To:Subject:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help: List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=eNplQJutEspyPMuCjCPwSOQ/DyJoxvbntxQSqtFw5DU=; b=ahOXW7rNtmHdfWnwLYEq5tDM1p lAmutsyFeT3gWnJQ0Lfz0YiTjv6WmynIy6Qf2rQSqIW5bN1ncD4XIqv7dGUmeygmumVagaR1QzI5u wDrEf4flyDFlxXXqHExU32461sGkh+0mOeywjjJZsoT2QDcvnMLV+YTZLojy3kv82AKQB8gEpBM/M 44SrdcbZLNSHgioVR3COJLKTqphkI4+eQo0+9X+OOOsPetM6IQR3Q71pmKM4dFsHjMbVPluGfP+sa ME4Q2PZxQJpKqjBMC6sG09mAbuYHM1m5ie1WIA2aqnhAOmwJcaZ+ogD49ba8amp8i2x53OJ6V2+Rr hgTbuqwQ==; Received: from 108-198-5-147.lightspeed.okcbok.sbcglobal.net ([108.198.5.147]:47658 helo=[192.168.0.113]) by vern.gendns.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.87) (envelope-from ) id 1cDkCT-001JhF-8C; Sun, 04 Dec 2016 22:44:21 -0500 Subject: Re: [PATCH v4] ARM: davinci: da8xx: Fix sleeping function called from invalid context To: Alexandre Bailon References: <1480690380-13216-1-git-send-email-abailon@baylibre.com> From: David Lechner Message-ID: <907a3c22-534f-80ce-daff-be84dd5e5cf8@lechnology.com> Date: Sun, 4 Dec 2016 21:44:22 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <1480690380-13216-1-git-send-email-abailon@baylibre.com> X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - lists.infradead.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20161204_194527_232095_A5FCAA5E X-CRM114-Status: GOOD ( 21.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: ahaslam@baylibre.com, arnd@arndb.de, khilman@baylibre.com, nsekhar@ti.com, ptitiano@baylibre.com, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 12/02/2016 08:53 AM, 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 > --- > arch/arm/mach-davinci/usb-da8xx.c | 28 +++++++++++++++------------- > 1 file changed, 15 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx.c > index b010e5f..704f506 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,21 +160,14 @@ 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); Need to remove clk_put() here. > @@ -197,8 +192,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 +279,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); > I have just tried this patch with a bunch of kernel hacking options enabled. I am getting the message show at the end of this email. We still have the problem of nested spin locks due to nested calls to clk_enable(). So, really, we need to use __clk_enable() and __clk_disable() from arch/arm/mach-davinci/clock.c in usb20_phy_clk_enable() above. Applying the following patch on top of your patch fixed the recursive lock message. --- * Turn on the USB 2.0 PHY, but just the PLL, and not OTG. The USB 1.1 @@ -192,7 +186,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(usb20_clk); + __clk_disable(usb20_clk); } static void usb20_phy_clk_disable(struct clk *clk) --- ============================================= [ INFO: possible recursive locking detected ] 4.9.0-rc8-dlech-ev3-lms2012+ #22 Not tainted --------------------------------------------- swapper/1 is trying to acquire lock: ( clockfw_lock ){......} , at: [] clk_enable+0x24/0x50 k: ( clockfw_lock ){......} , at: [] clk_enable+0x24/0x50 ebug this: Possible unsafe locking scenario: CPU0 ---- lock( clockfw_lock ); lock( clockfw_lock ); May be due to missing lock nesting notation 4 locks held by swapper/1: #0: ( &dev->mutex ){......} , at: [] __driver_attach+0x68/0xb4 #1: ( &dev->mutex ){......} , at: [] __driver_attach+0x78/0xb4 #2: ( &phy->mutex ){+.+...} , at: [] phy_power_on+0x5c/0xe4 #3: ( clockfw_lock ){......} , at: [] clk_enable+0x24/0x50 CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc8-dlech-ev3-lms2012+ #22 Hardware name: Generic DA850/OMAP-L138/AM18x Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:c0e998a4 r6:c0820750 r5:c0820750 r4:c3838000 [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (__lock_acquire+0x10f4/0x167c) [] (__lock_acquire) from [] (lock_acquire+0x78/0x98) r10:00000000 r9:c06a5ed4 r8:00000000 r7:00000001 r6:60000093 r5:00000000 r4:ffffe000 [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x60/0x74) r7:0000003b r6:c0014884 r5:80000093 r4:c06b09a0 [] (_raw_spin_lock_irqsave) from [] (clk_enable+0x24/0x50) r6:c06f69f4 r5:0001ef02 r4:c06b3398 [] (clk_enable) from [] (usb20_phy_clk_enable+0x24/0xb8) r5:0001ef02 r4:c06f69f0 [] (usb20_phy_clk_enable) from [] (__clk_enable+0x74/0x7c) r9:c06a5ed4 r8:00000000 r7:0000003b r6:c3a6ca00 r5:80000013 r4:c06b7ee8 [] (__clk_enable) from [] (__clk_enable+0x24/0x7c) r4:c06b8648 [] (__clk_enable) from [] (clk_enable+0x30/0x50) r4:c06b8648 [] (clk_enable) from [] (da8xx_usb11_phy_power_on+0x30/0x80) r5:c3a54050 r4:c06b8648 [] (da8xx_usb11_phy_power_on) from [] (phy_power_on+0x80/0xe4) r5:c3a6c800 r4:fffffdf4 [] (phy_power_on) from [] (ohci_da8xx_enable+0x4c/0x80) r7:0000003b r6:c3af6000 r5:c3af6000 r4:00000000 [] (ohci_da8xx_enable) from [] (ohci_da8xx_reset+0x1c/0xd8) r5:00000000 r4:c3af6000 [] (ohci_da8xx_reset) from [] (usb_add_hcd+0x314/0x834) r7:0000003b r6:ffffffed r5:00000000 r4:c3af6000 [] (usb_add_hcd) from [] (ohci_da8xx_probe+0x14c/0x21c) r9:c06a5ed4 r8:00000000 r7:c38bd010 r6:c38bd000 r5:c3af6000 r4:c38af900 [] (ohci_da8xx_probe) from [] (platform_drv_probe+0x40/0x8c) r7:c06e610c r6:c06e610c r5:c38bd010 r4:c0321ec0 [] (platform_drv_probe) from [] (driver_probe_device+0x138/0x2a4) r7:c06e610c r6:c0e9db10 r5:00000000 r4:c38bd010 [] (driver_probe_device) from [] (__driver_attach+0x90/0xb4) r9:c06a5ed4 r8:c06f66e0 r7:c06e1250 r6:c06e610c r5:c38bd044 r4:c38bd010 [] (__driver_attach) from [] (bus_for_each_dev+0x74/0x98) r7:c06e1250 r6:c02bd5a4 r5:c06e610c r4:00000000 [] (bus_for_each_dev) from [] (driver_attach+0x20/0x28) r6:c39a2300 r5:00000000 r4:c06e610c [] (driver_attach) from [] (bus_add_driver+0xd4/0x1f0) [] (bus_add_driver) from [] (driver_register+0xa4/0xe8) r7:c0698850 r6:00000000 r5:00000000 r4:c06e610c [] (driver_register) from [] (__platform_driver_register+0x38/0x4c) r5:00000000 r4:c06acce4 [] (__platform_driver_register) from [] (ohci_da8xx_init+0x64/0x90) [] (ohci_da8xx_init) from [] (do_one_initcall+0xb0/0x168) r5:c068d9bc r4:ffffe000 [] (do_one_initcall) from [] (kernel_init_freeable+0x110/0x1cc) r9:c06a5ed4 r8:c06f66e0 r7:c0698850 r6:000000c1 r5:c06f66e0 r4:00000007 [] (kernel_init_freeable) from [] (kernel_init+0x10/0xf8) r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c04d37d4 r4:00000000 [] (kernel_init) from [] (ret_from_fork+0x14/0x28) r5:c04d37d4 r4:00000000 BUG: spinlock lockup suspected on CPU#0, swapper/1 lock: clockfw_lock+0x0/0x20, .magic: dead4ead, .owner: swapper/1, .owner_cpu: 0 CPU: 0 PID: 1 Comm: swapper Not tainted 4.9.0-rc8-dlech-ev3-lms2012+ #22 Hardware name: Generic DA850/OMAP-L138/AM18x Backtrace: [] (dump_backtrace) from [] (show_stack+0x18/0x1c) r7:00000000 r6:04605000 r5:c06b09a0 r4:c3838000 [] (show_stack) from [] (dump_stack+0x20/0x28) [] (dump_stack) from [] (spin_dump+0x80/0x94) [] (spin_dump) from [] (do_raw_spin_lock+0xdc/0x11c) r5:00000000 r4:c06b09a0 [] (do_raw_spin_lock) from [] (_raw_spin_lock_irqsave+0x68/0x74) r10:00000000 r9:c06a5ed4 r8:00000000 r7:0000003b r6:c0014884 r5:80000093 r4:c06b09a0 r3:c3838000 [] (_raw_spin_lock_irqsave) from [] (clk_enable+0x24/0x50) r6:c06f69f4 r5:0001ef02 r4:c06b3398 [] (clk_enable) from [] (usb20_phy_clk_enable+0x24/0xb8) r5:0001ef02 r4:c06f69f0 [] (usb20_phy_clk_enable) from [] (__clk_enable+0x74/0x7c) r9:c06a5ed4 r8:00000000 r7:0000003b r6:c3a6ca00 r5:80000013 r4:c06b7ee8 [] (__clk_enable) from [] (__clk_enable+0x24/0x7c) r4:c06b8648 [] (__clk_enable) from [] (clk_enable+0x30/0x50) r4:c06b8648 [] (clk_enable) from [] (da8xx_usb11_phy_power_on+0x30/0x80) r5:c3a54050 r4:c06b8648 [] (da8xx_usb11_phy_power_on) from [] (phy_power_on+0x80/0xe4) r5:c3a6c800 r4:fffffdf4 [] (phy_power_on) from [] (ohci_da8xx_enable+0x4c/0x80) r7:0000003b r6:c3af6000 r5:c3af6000 r4:00000000 [] (ohci_da8xx_enable) from [] (ohci_da8xx_reset+0x1c/0xd8) r5:00000000 r4:c3af6000 [] (ohci_da8xx_reset) from [] (usb_add_hcd+0x314/0x834) r7:0000003b r6:ffffffed r5:00000000 r4:c3af6000 [] (usb_add_hcd) from [] (ohci_da8xx_probe+0x14c/0x21c) r9:c06a5ed4 r8:00000000 r7:c38bd010 r6:c38bd000 r5:c3af6000 r4:c38af900 [] (ohci_da8xx_probe) from [] (platform_drv_probe+0x40/0x8c) r7:c06e610c r6:c06e610c r5:c38bd010 r4:c0321ec0 [] (platform_drv_probe) from [] (driver_probe_device+0x138/0x2a4) r7:c06e610c r6:c0e9db10 r5:00000000 r4:c38bd010 [] (driver_probe_device) from [] (__driver_attach+0x90/0xb4) r9:c06a5ed4 r8:c06f66e0 r7:c06e1250 r6:c06e610c r5:c38bd044 r4:c38bd010 [] (__driver_attach) from [] (bus_for_each_dev+0x74/0x98) r7:c06e1250 r6:c02bd5a4 r5:c06e610c r4:00000000 [] (bus_for_each_dev) from [] (driver_attach+0x20/0x28) r6:c39a2300 r5:00000000 r4:c06e610c [] (driver_attach) from [] (bus_add_driver+0xd4/0x1f0) [] (bus_add_driver) from [] (driver_register+0xa4/0xe8) r7:c0698850 r6:00000000 r5:00000000 r4:c06e610c [] (driver_register) from [] (__platform_driver_register+0x38/0x4c) r5:00000000 r4:c06acce4 [] (__platform_driver_register) from [] (ohci_da8xx_init+0x64/0x90) [] (ohci_da8xx_init) from [] (do_one_initcall+0xb0/0x168) r5:c068d9bc r4:ffffe000 [] (do_one_initcall) from [] (kernel_init_freeable+0x110/0x1cc) r9:c06a5ed4 r8:c06f66e0 r7:c0698850 r6:000000c1 r5:c06f66e0 r4:00000007 [] (kernel_init_freeable) from [] (kernel_init+0x10/0xf8) r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:c04d37d4 r4:00000000 [] (kernel_init) from [] (ret_from_fork+0x14/0x28) r5:c04d37d4 r4:00000000 diff --git a/arch/arm/mach-davinci/clock.c b/arch/arm/mach-davinci/clock.c index df42c93..4fba579 100644 --- a/arch/arm/mach-davinci/clock.c +++ b/arch/arm/mach-davinci/clock.c @@ -31,7 +31,7 @@ static LIST_HEAD(clocks); static DEFINE_MUTEX(clocks_mutex); static DEFINE_SPINLOCK(clockfw_lock); -static void __clk_enable(struct clk *clk) +void __clk_enable(struct clk *clk) { if (clk->parent) __clk_enable(clk->parent); @@ -44,7 +44,7 @@ static void __clk_enable(struct clk *clk) } } -static void __clk_disable(struct clk *clk) +void __clk_disable(struct clk *clk) { if (WARN_ON(clk->usecount == 0)) return; diff --git a/arch/arm/mach-davinci/clock.h b/arch/arm/mach-davinci/clock.h index e2a5437..8493242 100644 --- a/arch/arm/mach-davinci/clock.h +++ b/arch/arm/mach-davinci/clock.h @@ -136,6 +136,9 @@ int davinci_clk_reset(struct clk *clk, bool reset); extern struct platform_device davinci_wdt_device; extern void davinci_watchdog_reset(struct platform_device *); +void __clk_enable(struct clk *clk); +void __clk_disable(struct clk *clk); + #endif #endif diff --git a/arch/arm/mach-davinci/usb-da8xx.c b/arch/arm/mach-davinci/usb-da8xx .c index 896d014..80ba0df 100644 --- a/arch/arm/mach-davinci/usb-da8xx.c +++ b/arch/arm/mach-davinci/usb-da8xx.c @@ -160,19 +160,13 @@ int __init da8xx_register_usb_refclkin(int rate) static void usb20_phy_clk_enable(struct clk *clk) { - int err; u32 val; u32 timeout = 500000; /* 500 msec */ val = readl(DA8XX_SYSCFG0_VIRT(DA8XX_CFGCHIP2_REG)); /* The USB 2.O PLL requires that the USB 2.O PSC is enabled as well. */ - err = clk_enable(usb20_clk); - if (err) { - pr_err("failed to enable usb20 clk: %d\n", err); - clk_put(usb20_clk); - return; - } + __clk_enable(usb20_clk); /*