From patchwork Fri Sep 16 03:27:00 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 76348 Delivered-To: patch@linaro.org Received: by 10.140.106.72 with SMTP id d66csp278236qgf; Thu, 15 Sep 2016 20:27:35 -0700 (PDT) X-Received: by 10.67.16.76 with SMTP id fu12mr5894087pad.171.1473996455334; Thu, 15 Sep 2016 20:27:35 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c73si9749967pfc.116.2016.09.15.20.27.35; Thu, 15 Sep 2016 20:27:35 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757511AbcIPD1U (ORCPT + 27 others); Thu, 15 Sep 2016 23:27:20 -0400 Received: from mail-pa0-f47.google.com ([209.85.220.47]:34812 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757425AbcIPD1R (ORCPT ); Thu, 15 Sep 2016 23:27:17 -0400 Received: by mail-pa0-f47.google.com with SMTP id wk8so21819622pab.1 for ; Thu, 15 Sep 2016 20:27:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=QfPt0o+TL8xVXtOJOo/RP8VnbMiZVyeKKiIfi4PP1q4=; b=PA6SrwHVpnQnozjDe/NQWGacz6f4+Ejj4l6LXI7aC2XIGbfW0l/HKWZd1GJnb64j4y f3oDAXmLoeP+D/Vo+oLE1B557TZDltb/p72xtmz9iFz735d+a8G2yxQ+dXt07ItaqAML N2sThHk65y7YITnwuozirqgxvCvQ4ruF+72XA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=QfPt0o+TL8xVXtOJOo/RP8VnbMiZVyeKKiIfi4PP1q4=; b=GfDpN9oSB/CXpJhU3942Az2JS17R4FAWgjrLaclYYFTDpDaeTo7bjs4Wn0mcGmdua9 4NdMCCUmnq8/ji3b2nOhrYda4HOzaa+nQ5EDy8xGnCnF1fNCVW7+Ebwwq2dBQE2mgXGM Qy9C275NykfDkkXpSqIm5RVEMRGs2ySsp11NbDD7vWKg8cg6wNCEVMJQhDXEwnxd90+2 ToQ0EyEh5pjLU+9AWI65Y5BcCuCzAxY+0UA4tnC9/7jeH6Jg91tom2rv1UsgrdL0Aj1Q 7AZ0Ei7KfJRpy32H/Ymhyye3wg9Z6+RkQFbpejHWIIe4HIBFF6gR3ZV++yq5kqQp75kM kgGg== X-Gm-Message-State: AE9vXwPhjlaDpoUr76tKwNq8udVx6hgMzoWGLOz96bazs2/PZF+WgO37I3n3Px+H1T2U/CqR X-Received: by 10.66.81.201 with SMTP id c9mr19698683pay.14.1473996435694; Thu, 15 Sep 2016 20:27:15 -0700 (PDT) Received: from localhost ([122.172.39.60]) by smtp.gmail.com with ESMTPSA id g10sm48013849pfc.57.2016.09.15.20.27.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 15 Sep 2016 20:27:15 -0700 (PDT) From: Viresh Kumar To: Lee Jones Cc: linaro-kernel@lists.linaro.org, Mark Brown , =?UTF-8?q?Krzysztof=20Koz=C5=82owski?= , Charles Keepax , Viresh Kumar , patches@opensource.wolfsonmicro.com, linux-kernel@vger.kernel.org Subject: [PATCH V2 3/3] mfd: wm8994-core: Don't use managed regulator bulk get API Date: Fri, 16 Sep 2016 08:57:00 +0530 Message-Id: <00005c8c0b1294412c543604cb537622d6257416.1473996370.git.viresh.kumar@linaro.org> X-Mailer: git-send-email 2.7.1.410.g6faf27b In-Reply-To: <7d68a9e5dd338081342dd8b06310c2f3e61fccc0.1473996370.git.viresh.kumar@linaro.org> References: <7d68a9e5dd338081342dd8b06310c2f3e61fccc0.1473996370.git.viresh.kumar@linaro.org> In-Reply-To: <7d68a9e5dd338081342dd8b06310c2f3e61fccc0.1473996370.git.viresh.kumar@linaro.org> References: <7d68a9e5dd338081342dd8b06310c2f3e61fccc0.1473996370.git.viresh.kumar@linaro.org> Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The kernel WARNs and then crashes today if wm8994_device_init() fails after calling devm_regulator_bulk_get(). That happens because there are multiple devices involved here and the order in which managed resources are freed isn't correct. The regulators are added as children of wm8994->dev. Whereas, devm_regulator_bulk_get() receives wm8994->dev as the device, though it gets the same regulators which were added as children of wm8994->dev earlier. During failures, the children are removed first and the core eventually calls regulator_unregister() for them. As regulator_put() was never done for them (opposite of devm_regulator_bulk_get()), the kernel WARNs at WARN_ON(rdev->open_count); And eventually it crashes from debugfs_remove_recursive(). --------x------------------x---------------- wm8994 3-001a: Device is not a WM8994, ID is 0 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at /mnt/ssd/all/work/repos/devel/linux/drivers/regulator/core.c:4072 regulator_unregister+0xc8/0xd0 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.8.0-rc6-00154-g54fe84cbd50b #41 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0x88/0x9c) [] (dump_stack) from [] (__warn+0xe8/0x100) [] (__warn) from [] (warn_slowpath_null+0x20/0x28) [] (warn_slowpath_null) from [] (regulator_unregister+0xc8/0xd0) [] (regulator_unregister) from [] (release_nodes+0x16c/0x1dc) [] (release_nodes) from [] (__device_release_driver+0x8c/0x110) [] (__device_release_driver) from [] (device_release_driver+0x1c/0x28) [] (device_release_driver) from [] (bus_remove_device+0xd8/0x104) [] (bus_remove_device) from [] (device_del+0x10c/0x218) [] (device_del) from [] (platform_device_del+0x1c/0x88) [] (platform_device_del) from [] (platform_device_unregister+0xc/0x20) [] (platform_device_unregister) from [] (mfd_remove_devices_fn+0x5c/0x64) [] (mfd_remove_devices_fn) from [] (device_for_each_child_reverse+0x4c/0x78) [] (device_for_each_child_reverse) from [] (mfd_remove_devices+0x20/0x30) [] (mfd_remove_devices) from [] (wm8994_device_init+0x2ac/0x7f0) [] (wm8994_device_init) from [] (i2c_device_probe+0x178/0x1fc) [] (i2c_device_probe) from [] (driver_probe_device+0x214/0x2c0) [] (driver_probe_device) from [] (__driver_attach+0xac/0xb0) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x9c) [] (bus_for_each_dev) from [] (bus_add_driver+0x1a0/0x218) [] (bus_add_driver) from [] (driver_register+0x78/0xf8) [] (driver_register) from [] (i2c_register_driver+0x34/0x84) [] (i2c_register_driver) from [] (do_one_initcall+0x40/0x170) [] (do_one_initcall) from [] (kernel_init_freeable+0x15c/0x1fc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) ---[ end trace 0919d3d0bc998260 ]--- [snip..] Unable to handle kernel NULL pointer dereference at virtual address 00000078 pgd = c0004000 [00000078] *pgd=00000000 Internal error: Oops: 5 [#1] PREEMPT SMP ARM Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Tainted: G W 4.8.0-rc6-00154-g54fe84cbd50b #41 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) task: ee874000 task.stack: ee878000 PC is at down_write+0x14/0x54 LR is at debugfs_remove_recursive+0x30/0x150 [snip..] [] (down_write) from [] (debugfs_remove_recursive+0x30/0x150) [] (debugfs_remove_recursive) from [] (_regulator_put+0x24/0xac) [] (_regulator_put) from [] (regulator_put+0x1c/0x2c) [] (regulator_put) from [] (release_nodes+0x16c/0x1dc) [] (release_nodes) from [] (driver_probe_device+0xec/0x2c0) [] (driver_probe_device) from [] (__driver_attach+0xac/0xb0) [] (__driver_attach) from [] (bus_for_each_dev+0x68/0x9c) [] (bus_for_each_dev) from [] (bus_add_driver+0x1a0/0x218) [] (bus_add_driver) from [] (driver_register+0x78/0xf8) [] (driver_register) from [] (i2c_register_driver+0x34/0x84) [] (i2c_register_driver) from [] (do_one_initcall+0x40/0x170) [] (do_one_initcall) from [] (kernel_init_freeable+0x15c/0x1fc) [] (kernel_init_freeable) from [] (kernel_init+0x8/0x114) [] (kernel_init) from [] (ret_from_fork+0x14/0x3c) Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f) ---[ end trace 0919d3d0bc998262 ]--- --------x------------------x---------------- Fix the kernel warnings and crashes by using regulator_bulk_get() instead of devm_regulator_bulk_get() and explicitly freeing the supplies in exit paths. Tested on Exynos 5250, dual core ARM A15 machine. Signed-off-by: Viresh Kumar --- V1->V2: - Use regulator_bulk_free() instead of open coding it. - Shorter backtrace - Reworded the last paragraph to make it more clear - Added a comment in code --- drivers/mfd/wm8994-core.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- 2.7.1.410.g6faf27b diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c index 95e6bc55adbb..953d0790ffd5 100644 --- a/drivers/mfd/wm8994-core.c +++ b/drivers/mfd/wm8994-core.c @@ -393,8 +393,13 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq) BUG(); goto err; } - - ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies, + + /* + * Can't use devres helper here as some of the supplies are provided by + * wm8994->dev's children (regulators) and those regulators are + * unregistered by the devres core before the supplies are freed. + */ + ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies, wm8994->supplies); if (ret != 0) { dev_err(wm8994->dev, "Failed to get supplies: %d\n", ret); @@ -404,7 +409,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq) ret = regulator_bulk_enable(wm8994->num_supplies, wm8994->supplies); if (ret != 0) { dev_err(wm8994->dev, "Failed to enable supplies: %d\n", ret); - goto err; + goto err_regulator_free; } ret = wm8994_reg_read(wm8994, WM8994_SOFTWARE_RESET); @@ -595,6 +600,8 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq) err_enable: regulator_bulk_disable(wm8994->num_supplies, wm8994->supplies); +err_regulator_free: + regulator_bulk_free(wm8994->num_supplies, wm8994->supplies); err: mfd_remove_devices(wm8994->dev); return ret; @@ -605,6 +612,7 @@ static void wm8994_device_exit(struct wm8994 *wm8994) pm_runtime_disable(wm8994->dev); wm8994_irq_exit(wm8994); regulator_bulk_disable(wm8994->num_supplies, wm8994->supplies); + regulator_bulk_free(wm8994->num_supplies, wm8994->supplies); mfd_remove_devices(wm8994->dev); }