All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them
@ 2016-09-15 11:17 Viresh Kumar
  2016-09-15 11:17 ` [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API Viresh Kumar
  2016-09-15 12:31 ` [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Charles Keepax
  0 siblings, 2 replies; 7+ messages in thread
From: Viresh Kumar @ 2016-09-15 11:17 UTC (permalink / raw)
  To: Lee Jones
  Cc: linaro-kernel, Mark Brown, Krzysztof Kozłowski,
	Viresh Kumar, patches, linux-kernel

The order in which resources were freed in wm8994_device_exit() isn't
correct. The regulators are removed before they are disabled.

Fix it by reordering code a bit, which makes it exact opposite of
wm8994_device_init() as well.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/mfd/wm8994-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 7eec619a6023..1e644aa53a2d 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -604,10 +604,10 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 static void wm8994_device_exit(struct wm8994 *wm8994)
 {
 	pm_runtime_disable(wm8994->dev);
-	mfd_remove_devices(wm8994->dev);
 	wm8994_irq_exit(wm8994);
 	regulator_bulk_disable(wm8994->num_supplies,
 			       wm8994->supplies);
+	mfd_remove_devices(wm8994->dev);
 }
 
 static const struct of_device_id wm8994_of_match[] = {
-- 
2.7.1.410.g6faf27b

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API
  2016-09-15 11:17 [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Viresh Kumar
@ 2016-09-15 11:17 ` Viresh Kumar
  2016-09-15 12:34   ` Charles Keepax
  2016-09-15 14:47   ` Mark Brown
  2016-09-15 12:31 ` [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Charles Keepax
  1 sibling, 2 replies; 7+ messages in thread
From: Viresh Kumar @ 2016-09-15 11:17 UTC (permalink / raw)
  To: Lee Jones
  Cc: linaro-kernel, Mark Brown, Krzysztof Kozłowski,
	Viresh Kumar, patches, linux-kernel

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 resources are freed isn't correct.

The regulators are added as children of wm8994->dev.
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)
 [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
 [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
 [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
 [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
 [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
 [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
 [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
 [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
 [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
 [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
 [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
 [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
 [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
 [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
 [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
 [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
 [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
 [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 ---[ end trace 0919d3d0bc998260 ]---
 ------------[ 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 Tainted: G        W       4.8.0-rc6-00154-g54fe84cbd50b #41
 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
 [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
 [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
 [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
 [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
 [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
 [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
 [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
 [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
 [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
 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)
 [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
 [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
 [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
 [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
 [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
 [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
 [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
 [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
 [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
 [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
 [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
 [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
 [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
 [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
 [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
 [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
 [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
 [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 ---[ end trace 0919d3d0bc998260 ]---
 ------------[ 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 Tainted: G        W       4.8.0-rc6-00154-g54fe84cbd50b #41
 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
 [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
 [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
 [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
 [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
 [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
 [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
 [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
 [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
 [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
 [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
 [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
 [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
 [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
 [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
 [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
 [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
 [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 ---[ end trace 0919d3d0bc998261 ]---
 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
 pc : [<c06e489c>]    lr : [<c02e9954>]    psr: 80000013
 sp : ee879e18  ip : 2e575000  fp : 00000000
 r10: 00000000  r9 : c0406004  r8 : ee46f428
 r7 : ee46f4ac  r6 : ee46f478  r5 : ee46f428  r4 : 00000078
 r3 : ffff0001  r2 : 00000000  r1 : 00000001  r0 : 00000078
 Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
 Control: 10c5387d  Table: 4000406a  DAC: 00000051
 Process swapper/0 (pid: 1, stack limit = 0xee878210)
 Stack: (0xee879e18 to 0xee87a000)
 9e00:                                                       ee216340 c02e9954
 9e20: ee216340 ee12bc00 0000000d eea22220 ee879e58 c0382b78 c0b1505c ee216340
 9e40: 0000000d c0382c1c ee216300 ee216200 0000000d c0406434 ee1e7c00 ee216700
 9e60: eea22220 c0b81c0c c0b81c14 c0b22320 ffffffea 00000000 00000000 c04035d4
 9e80: eea22220 c0b22320 eea22254 c0b2a950 c0b44000 000000d8 00000000 c0403854
 9ea0: 00000000 c0b22320 c04037a8 c0401a74 ee97be74 eea153c0 c0b22320 ee16f480
 9ec0: 00000000 c0402cf0 c08a5174 c0a1d3cc c0a34840 c0b22320 c0a1d3cc c0a34840
 9ee0: c0b44000 c040406c c0b22304 c0a1d3cc c0a34840 c04f20a0 c0a5ff58 c0a1d3cc
 9f00: c0a34840 c01017d0 c0b0ac18 ee879f18 c0b45fe8 c06e3f40 60000000 c0b095a0
 9f20: 00000000 c0b095a0 efffca46 c0716b00 efffca42 c01350dc 0000cccd 00000000
 9f40: c08cdb2c 00000000 00000006 00000006 c0b0957c efffc940 c0a5ff58 00000006
 9f60: c0a34840 c0b44000 c0b44000 000000d8 c0a34848 c0a00dbc 00000006 00000006
 9f80: 00000000 c0a005a0 00000000 c06e07a8 00000000 00000000 00000000 00000000
 9fa0: 00000000 c06e07b0 00000000 c0107978 00000000 00000000 00000000 00000000
 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
 [<c06e489c>] (down_write) from [<c02e9954>] (debugfs_remove_recursive+0x30/0x150)
 [<c02e9954>] (debugfs_remove_recursive) from [<c0382b78>] (_regulator_put+0x24/0xac)
 [<c0382b78>] (_regulator_put) from [<c0382c1c>] (regulator_put+0x1c/0x2c)
 [<c0382c1c>] (regulator_put) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04035d4>] (driver_probe_device+0xec/0x2c0)
 [<c04035d4>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
 ---[ end trace 0919d3d0bc998262 ]---
 Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

 CPU1: stopping
 CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D W       4.8.0-rc6-00154-g54fe84cbd50b #41
 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
 [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
 [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
 [<c032a1c4>] (dump_stack) from [<c010d4f8>] (handle_IPI+0x198/0x1ac)
 [<c010d4f8>] (handle_IPI) from [<c01014e4>] (gic_handle_irq+0x94/0x98)
 [<c01014e4>] (gic_handle_irq) from [<c010ba0c>] (__irq_svc+0x6c/0xa8)
 Exception stack(0xee89ff88 to 0xee89ffd0)
 ff80:                   00000001 00000000 00000000 c0113f40 ee89e000 c0b02454
 ffa0: 00000002 00000000 00000000 c0a65f88 c0b024c0 c0b024b8 00000000 ee89ffd8
 ffc0: c01083b8 c01083bc 60000013 ffffffff
 [<c010ba0c>] (__irq_svc) from [<c01083bc>] (arch_cpu_idle+0x38/0x3c)
 [<c01083bc>] (arch_cpu_idle) from [<c01531f0>] (cpu_startup_entry+0x1cc/0x250)
 [<c01531f0>] (cpu_startup_entry) from [<4010158c>] (0x4010158c)
 ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
 [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
 [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
 [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
 [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
 [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
 [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
 [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
 [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 ---[ end trace 0919d3d0bc998261 ]---
 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
 pc : [<c06e489c>]    lr : [<c02e9954>]    psr: 80000013
 sp : ee879e18  ip : 2e575000  fp : 00000000
 r10: 00000000  r9 : c0406004  r8 : ee46f428
 r7 : ee46f4ac  r6 : ee46f478  r5 : ee46f428  r4 : 00000078
 r3 : ffff0001  r2 : 00000000  r1 : 00000001  r0 : 00000078
 Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
 Control: 10c5387d  Table: 4000406a  DAC: 00000051
 Process swapper/0 (pid: 1, stack limit = 0xee878210)
 Stack: (0xee879e18 to 0xee87a000)
 9e00:                                                       ee216340 c02e9954
 9e20: ee216340 ee12bc00 0000000d eea22220 ee879e58 c0382b78 c0b1505c ee216340
 9e40: 0000000d c0382c1c ee216300 ee216200 0000000d c0406434 ee1e7c00 ee216700
 9e60: eea22220 c0b81c0c c0b81c14 c0b22320 ffffffea 00000000 00000000 c04035d4
 9e80: eea22220 c0b22320 eea22254 c0b2a950 c0b44000 000000d8 00000000 c0403854
 9ea0: 00000000 c0b22320 c04037a8 c0401a74 ee97be74 eea153c0 c0b22320 ee16f480
 9ec0: 00000000 c0402cf0 c08a5174 c0a1d3cc c0a34840 c0b22320 c0a1d3cc c0a34840
 9ee0: c0b44000 c040406c c0b22304 c0a1d3cc c0a34840 c04f20a0 c0a5ff58 c0a1d3cc
 9f00: c0a34840 c01017d0 c0b0ac18 ee879f18 c0b45fe8 c06e3f40 60000000 c0b095a0
 9f20: 00000000 c0b095a0 efffca46 c0716b00 efffca42 c01350dc 0000cccd 00000000
 9f40: c08cdb2c 00000000 00000006 00000006 c0b0957c efffc940 c0a5ff58 00000006
 9f60: c0a34840 c0b44000 c0b44000 000000d8 c0a34848 c0a00dbc 00000006 00000006
 9f80: 00000000 c0a005a0 00000000 c06e07a8 00000000 00000000 00000000 00000000
 9fa0: 00000000 c06e07b0 00000000 c0107978 00000000 00000000 00000000 00000000
 9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
 9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
 [<c06e489c>] (down_write) from [<c02e9954>] (debugfs_remove_recursive+0x30/0x150)
 [<c02e9954>] (debugfs_remove_recursive) from [<c0382b78>] (_regulator_put+0x24/0xac)
 [<c0382b78>] (_regulator_put) from [<c0382c1c>] (regulator_put+0x1c/0x2c)
 [<c0382c1c>] (regulator_put) from [<c0406434>] (release_nodes+0x16c/0x1dc)
 [<c0406434>] (release_nodes) from [<c04035d4>] (driver_probe_device+0xec/0x2c0)
 [<c04035d4>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
 [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
 [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
 [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
 [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
 [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
 [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
 [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
 [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
 Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
 ---[ end trace 0919d3d0bc998262 ]---
 Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

--------x------------------x----------------

Fix the kernel warnings and crashes by moving away from managed
regulator bulk get API by using regulator_bulk_get() and explicitly
calling regulator_put() for all the supplies in exit paths.

Tested on Exynos 5250, dual core ARM A15 machine.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/mfd/wm8994-core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 1e644aa53a2d..a232387b7783 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -394,7 +394,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 		goto err;
 	}
 		
-	ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
+	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);
@@ -405,7 +405,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 				    wm8994->supplies);
 	if (ret != 0) {
 		dev_err(wm8994->dev, "Failed to enable supplies: %d\n", ret);
-		goto err;
+		goto err_regulator_put;
 	}
 
 	ret = wm8994_reg_read(wm8994, WM8994_SOFTWARE_RESET);
@@ -596,6 +596,9 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 err_enable:
 	regulator_bulk_disable(wm8994->num_supplies,
 			       wm8994->supplies);
+err_regulator_put:
+	for (i = wm8994->num_supplies - 1; i >= 0; i--)
+		regulator_put(wm8994->supplies[i].consumer);
 err:
 	mfd_remove_devices(wm8994->dev);
 	return ret;
@@ -603,10 +606,13 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 
 static void wm8994_device_exit(struct wm8994 *wm8994)
 {
+	int i;
 	pm_runtime_disable(wm8994->dev);
 	wm8994_irq_exit(wm8994);
 	regulator_bulk_disable(wm8994->num_supplies,
 			       wm8994->supplies);
+	for (i = wm8994->num_supplies - 1; i >= 0; i--)
+		regulator_put(wm8994->supplies[i].consumer);
 	mfd_remove_devices(wm8994->dev);
 }
 
-- 
2.7.1.410.g6faf27b

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them
  2016-09-15 11:17 [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Viresh Kumar
  2016-09-15 11:17 ` [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API Viresh Kumar
@ 2016-09-15 12:31 ` Charles Keepax
  1 sibling, 0 replies; 7+ messages in thread
From: Charles Keepax @ 2016-09-15 12:31 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lee Jones, linaro-kernel, Mark Brown, Krzysztof Kozłowski,
	patches, linux-kernel

On Thu, Sep 15, 2016 at 04:47:00PM +0530, Viresh Kumar wrote:
> The order in which resources were freed in wm8994_device_exit() isn't
> correct. The regulators are removed before they are disabled.
> 
> Fix it by reordering code a bit, which makes it exact opposite of
> wm8994_device_init() as well.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API
  2016-09-15 11:17 ` [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API Viresh Kumar
@ 2016-09-15 12:34   ` Charles Keepax
  2016-09-15 14:47   ` Mark Brown
  1 sibling, 0 replies; 7+ messages in thread
From: Charles Keepax @ 2016-09-15 12:34 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lee Jones, linaro-kernel, Mark Brown, Krzysztof Kozłowski,
	patches, linux-kernel

On Thu, Sep 15, 2016 at 04:47:01PM +0530, Viresh Kumar wrote:
> 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 resources are freed isn't correct.
> 
> The regulators are added as children of wm8994->dev.
> 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)
>  [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
>  [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
>  [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
>  [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
>  [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
>  [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
>  [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
>  [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
>  [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
>  [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
>  [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
>  [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
>  [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
>  [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
>  [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
>  [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
>  [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
>  [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  ---[ end trace 0919d3d0bc998260 ]---
>  ------------[ 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 Tainted: G        W       4.8.0-rc6-00154-g54fe84cbd50b #41
>  Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>  [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
>  [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
>  [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
>  [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
>  [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
>  [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
>  [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
>  [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
>  [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
>  [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
>  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)
>  [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
>  [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
>  [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
>  [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
>  [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
>  [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
>  [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
>  [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
>  [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
>  [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
>  [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
>  [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
>  [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
>  [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
>  [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
>  [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
>  [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
>  [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  ---[ end trace 0919d3d0bc998260 ]---
>  ------------[ 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 Tainted: G        W       4.8.0-rc6-00154-g54fe84cbd50b #41
>  Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>  [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
>  [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
>  [<c032a1c4>] (dump_stack) from [<c011a98c>] (__warn+0xe8/0x100)
>  [<c011a98c>] (__warn) from [<c011aa54>] (warn_slowpath_null+0x20/0x28)
>  [<c011aa54>] (warn_slowpath_null) from [<c0384a0c>] (regulator_unregister+0xc8/0xd0)
>  [<c0384a0c>] (regulator_unregister) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04039c4>] (__device_release_driver+0x8c/0x110)
>  [<c04039c4>] (__device_release_driver) from [<c0403a64>] (device_release_driver+0x1c/0x28)
>  [<c0403a64>] (device_release_driver) from [<c0402b24>] (bus_remove_device+0xd8/0x104)
>  [<c0402b24>] (bus_remove_device) from [<c03ffcd8>] (device_del+0x10c/0x218)
>  [<c03ffcd8>] (device_del) from [<c0404e4c>] (platform_device_del+0x1c/0x88)
>  [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
>  [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
>  [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
>  [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
>  [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
>  [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
>  [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
>  [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  ---[ end trace 0919d3d0bc998261 ]---
>  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
>  pc : [<c06e489c>]    lr : [<c02e9954>]    psr: 80000013
>  sp : ee879e18  ip : 2e575000  fp : 00000000
>  r10: 00000000  r9 : c0406004  r8 : ee46f428
>  r7 : ee46f4ac  r6 : ee46f478  r5 : ee46f428  r4 : 00000078
>  r3 : ffff0001  r2 : 00000000  r1 : 00000001  r0 : 00000078
>  Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
>  Control: 10c5387d  Table: 4000406a  DAC: 00000051
>  Process swapper/0 (pid: 1, stack limit = 0xee878210)
>  Stack: (0xee879e18 to 0xee87a000)
>  9e00:                                                       ee216340 c02e9954
>  9e20: ee216340 ee12bc00 0000000d eea22220 ee879e58 c0382b78 c0b1505c ee216340
>  9e40: 0000000d c0382c1c ee216300 ee216200 0000000d c0406434 ee1e7c00 ee216700
>  9e60: eea22220 c0b81c0c c0b81c14 c0b22320 ffffffea 00000000 00000000 c04035d4
>  9e80: eea22220 c0b22320 eea22254 c0b2a950 c0b44000 000000d8 00000000 c0403854
>  9ea0: 00000000 c0b22320 c04037a8 c0401a74 ee97be74 eea153c0 c0b22320 ee16f480
>  9ec0: 00000000 c0402cf0 c08a5174 c0a1d3cc c0a34840 c0b22320 c0a1d3cc c0a34840
>  9ee0: c0b44000 c040406c c0b22304 c0a1d3cc c0a34840 c04f20a0 c0a5ff58 c0a1d3cc
>  9f00: c0a34840 c01017d0 c0b0ac18 ee879f18 c0b45fe8 c06e3f40 60000000 c0b095a0
>  9f20: 00000000 c0b095a0 efffca46 c0716b00 efffca42 c01350dc 0000cccd 00000000
>  9f40: c08cdb2c 00000000 00000006 00000006 c0b0957c efffc940 c0a5ff58 00000006
>  9f60: c0a34840 c0b44000 c0b44000 000000d8 c0a34848 c0a00dbc 00000006 00000006
>  9f80: 00000000 c0a005a0 00000000 c06e07a8 00000000 00000000 00000000 00000000
>  9fa0: 00000000 c06e07b0 00000000 c0107978 00000000 00000000 00000000 00000000
>  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
>  [<c06e489c>] (down_write) from [<c02e9954>] (debugfs_remove_recursive+0x30/0x150)
>  [<c02e9954>] (debugfs_remove_recursive) from [<c0382b78>] (_regulator_put+0x24/0xac)
>  [<c0382b78>] (_regulator_put) from [<c0382c1c>] (regulator_put+0x1c/0x2c)
>  [<c0382c1c>] (regulator_put) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04035d4>] (driver_probe_device+0xec/0x2c0)
>  [<c04035d4>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
>  ---[ end trace 0919d3d0bc998262 ]---
>  Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
>  CPU1: stopping
>  CPU: 1 PID: 0 Comm: swapper/1 Tainted: G      D W       4.8.0-rc6-00154-g54fe84cbd50b #41
>  Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
>  [<c010e24c>] (unwind_backtrace) from [<c010af38>] (show_stack+0x10/0x14)
>  [<c010af38>] (show_stack) from [<c032a1c4>] (dump_stack+0x88/0x9c)
>  [<c032a1c4>] (dump_stack) from [<c010d4f8>] (handle_IPI+0x198/0x1ac)
>  [<c010d4f8>] (handle_IPI) from [<c01014e4>] (gic_handle_irq+0x94/0x98)
>  [<c01014e4>] (gic_handle_irq) from [<c010ba0c>] (__irq_svc+0x6c/0xa8)
>  Exception stack(0xee89ff88 to 0xee89ffd0)
>  ff80:                   00000001 00000000 00000000 c0113f40 ee89e000 c0b02454
>  ffa0: 00000002 00000000 00000000 c0a65f88 c0b024c0 c0b024b8 00000000 ee89ffd8
>  ffc0: c01083b8 c01083bc 60000013 ffffffff
>  [<c010ba0c>] (__irq_svc) from [<c01083bc>] (arch_cpu_idle+0x38/0x3c)
>  [<c01083bc>] (arch_cpu_idle) from [<c01531f0>] (cpu_startup_entry+0x1cc/0x250)
>  [<c01531f0>] (cpu_startup_entry) from [<4010158c>] (0x4010158c)
>  ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>  [<c0404e4c>] (platform_device_del) from [<c0404ec4>] (platform_device_unregister+0xc/0x20)
>  [<c0404ec4>] (platform_device_unregister) from [<c0428bc0>] (mfd_remove_devices_fn+0x5c/0x64)
>  [<c0428bc0>] (mfd_remove_devices_fn) from [<c03ff9d8>] (device_for_each_child_reverse+0x4c/0x78)
>  [<c03ff9d8>] (device_for_each_child_reverse) from [<c04288c4>] (mfd_remove_devices+0x20/0x30)
>  [<c04288c4>] (mfd_remove_devices) from [<c042758c>] (wm8994_device_init+0x2ac/0x7f0)
>  [<c042758c>] (wm8994_device_init) from [<c04f14a8>] (i2c_device_probe+0x178/0x1fc)
>  [<c04f14a8>] (i2c_device_probe) from [<c04036fc>] (driver_probe_device+0x214/0x2c0)
>  [<c04036fc>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  ---[ end trace 0919d3d0bc998261 ]---
>  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
>  pc : [<c06e489c>]    lr : [<c02e9954>]    psr: 80000013
>  sp : ee879e18  ip : 2e575000  fp : 00000000
>  r10: 00000000  r9 : c0406004  r8 : ee46f428
>  r7 : ee46f4ac  r6 : ee46f478  r5 : ee46f428  r4 : 00000078
>  r3 : ffff0001  r2 : 00000000  r1 : 00000001  r0 : 00000078
>  Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
>  Control: 10c5387d  Table: 4000406a  DAC: 00000051
>  Process swapper/0 (pid: 1, stack limit = 0xee878210)
>  Stack: (0xee879e18 to 0xee87a000)
>  9e00:                                                       ee216340 c02e9954
>  9e20: ee216340 ee12bc00 0000000d eea22220 ee879e58 c0382b78 c0b1505c ee216340
>  9e40: 0000000d c0382c1c ee216300 ee216200 0000000d c0406434 ee1e7c00 ee216700
>  9e60: eea22220 c0b81c0c c0b81c14 c0b22320 ffffffea 00000000 00000000 c04035d4
>  9e80: eea22220 c0b22320 eea22254 c0b2a950 c0b44000 000000d8 00000000 c0403854
>  9ea0: 00000000 c0b22320 c04037a8 c0401a74 ee97be74 eea153c0 c0b22320 ee16f480
>  9ec0: 00000000 c0402cf0 c08a5174 c0a1d3cc c0a34840 c0b22320 c0a1d3cc c0a34840
>  9ee0: c0b44000 c040406c c0b22304 c0a1d3cc c0a34840 c04f20a0 c0a5ff58 c0a1d3cc
>  9f00: c0a34840 c01017d0 c0b0ac18 ee879f18 c0b45fe8 c06e3f40 60000000 c0b095a0
>  9f20: 00000000 c0b095a0 efffca46 c0716b00 efffca42 c01350dc 0000cccd 00000000
>  9f40: c08cdb2c 00000000 00000006 00000006 c0b0957c efffc940 c0a5ff58 00000006
>  9f60: c0a34840 c0b44000 c0b44000 000000d8 c0a34848 c0a00dbc 00000006 00000006
>  9f80: 00000000 c0a005a0 00000000 c06e07a8 00000000 00000000 00000000 00000000
>  9fa0: 00000000 c06e07b0 00000000 c0107978 00000000 00000000 00000000 00000000
>  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 ffffffff ffffffff
>  [<c06e489c>] (down_write) from [<c02e9954>] (debugfs_remove_recursive+0x30/0x150)
>  [<c02e9954>] (debugfs_remove_recursive) from [<c0382b78>] (_regulator_put+0x24/0xac)
>  [<c0382b78>] (_regulator_put) from [<c0382c1c>] (regulator_put+0x1c/0x2c)
>  [<c0382c1c>] (regulator_put) from [<c0406434>] (release_nodes+0x16c/0x1dc)
>  [<c0406434>] (release_nodes) from [<c04035d4>] (driver_probe_device+0xec/0x2c0)
>  [<c04035d4>] (driver_probe_device) from [<c0403854>] (__driver_attach+0xac/0xb0)
>  [<c0403854>] (__driver_attach) from [<c0401a74>] (bus_for_each_dev+0x68/0x9c)
>  [<c0401a74>] (bus_for_each_dev) from [<c0402cf0>] (bus_add_driver+0x1a0/0x218)
>  [<c0402cf0>] (bus_add_driver) from [<c040406c>] (driver_register+0x78/0xf8)
>  [<c040406c>] (driver_register) from [<c04f20a0>] (i2c_register_driver+0x34/0x84)
>  [<c04f20a0>] (i2c_register_driver) from [<c01017d0>] (do_one_initcall+0x40/0x170)
>  [<c01017d0>] (do_one_initcall) from [<c0a00dbc>] (kernel_init_freeable+0x15c/0x1fc)
>  [<c0a00dbc>] (kernel_init_freeable) from [<c06e07b0>] (kernel_init+0x8/0x114)
>  [<c06e07b0>] (kernel_init) from [<c0107978>] (ret_from_fork+0x14/0x3c)
>  Code: e1a04000 f590f000 e3a03001 e34f3fff (e1902f9f)
>  ---[ end trace 0919d3d0bc998262 ]---
>  Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> 
> --------x------------------x----------------

Feels like we could get away with a little less of the dump in
the commit message.

> 
> Fix the kernel warnings and crashes by moving away from managed
> regulator bulk get API by using regulator_bulk_get() and explicitly
> calling regulator_put() for all the supplies in exit paths.
> 
> Tested on Exynos 5250, dual core ARM A15 machine.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/mfd/wm8994-core.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
> index 1e644aa53a2d..a232387b7783 100644
> --- a/drivers/mfd/wm8994-core.c
> +++ b/drivers/mfd/wm8994-core.c
> @@ -394,7 +394,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
>  		goto err;
>  	}
>  		
> -	ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> +	ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
>  				 wm8994->supplies);

Would be nice to stick a comment in here just to say we don't
want to use devres as some of the supplies will be provided by
the MFDs children. Just incase someone is going through the code
later and decides it would be good to update this to use devres.

Otherwise all looks good to me.

Thanks,
Charles

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API
  2016-09-15 11:17 ` [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API Viresh Kumar
  2016-09-15 12:34   ` Charles Keepax
@ 2016-09-15 14:47   ` Mark Brown
  2016-09-15 15:08     ` Viresh Kumar
  1 sibling, 1 reply; 7+ messages in thread
From: Mark Brown @ 2016-09-15 14:47 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lee Jones, linaro-kernel, Krzysztof Kozłowski, patches,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1023 bytes --]

On Thu, Sep 15, 2016 at 04:47:01PM +0530, Viresh Kumar wrote:

> Fix the kernel warnings and crashes by moving away from managed
> regulator bulk get API by using regulator_bulk_get() and explicitly
> calling regulator_put() for all the supplies in exit paths.

Moving away from regulator bulk get to regulator bulk get?

> -	ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> +	ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
>  				 wm8994->supplies);

> +err_regulator_put:
> +	for (i = wm8994->num_supplies - 1; i >= 0; i--)
> +		regulator_put(wm8994->supplies[i].consumer);

Why are you unwinding regulator_bulk_get() with an open coded
regulator_bulk_put()?

Also please think hard before including complete backtraces in upstream
reports, they are very large and contain almost no useful information
relative to their size so often obscure the relevant content in your
message. If part of the backtrace is usefully illustrative then it's
usually better to pull out the relevant sections.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API
  2016-09-15 14:47   ` Mark Brown
@ 2016-09-15 15:08     ` Viresh Kumar
  2016-09-15 15:47       ` Mark Brown
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2016-09-15 15:08 UTC (permalink / raw)
  To: Mark Brown
  Cc: Lee Jones, linaro-kernel, Krzysztof Kozłowski, patches,
	linux-kernel

On 15-09-16, 15:47, Mark Brown wrote:
> On Thu, Sep 15, 2016 at 04:47:01PM +0530, Viresh Kumar wrote:
> 
> > Fix the kernel warnings and crashes by moving away from managed
> > regulator bulk get API by using regulator_bulk_get() and explicitly
> > calling regulator_put() for all the supplies in exit paths.
> 
> Moving away from regulator bulk get to regulator bulk get?

Moving away from "managed" regulator bulk get to regulator bulk get?
Isn't this how we call the devm_* APIs as?

> > -	ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> > +	ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> >  				 wm8994->supplies);
> 
> > +err_regulator_put:
> > +	for (i = wm8994->num_supplies - 1; i >= 0; i--)
> > +		regulator_put(wm8994->supplies[i].consumer);
> 
> Why are you unwinding regulator_bulk_get() with an open coded
> regulator_bulk_put()?

Because there is no regulator_bulk_put(). Should I add it?

> Also please think hard before including complete backtraces in upstream
> reports, they are very large and contain almost no useful information
> relative to their size so often obscure the relevant content in your
> message. If part of the backtrace is usefully illustrative then it's
> usually better to pull out the relevant sections.

Sure, will add only relevant bits in the next version.

-- 
viresh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API
  2016-09-15 15:08     ` Viresh Kumar
@ 2016-09-15 15:47       ` Mark Brown
  0 siblings, 0 replies; 7+ messages in thread
From: Mark Brown @ 2016-09-15 15:47 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Lee Jones, linaro-kernel, Krzysztof Kozłowski, patches,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On Thu, Sep 15, 2016 at 08:38:44PM +0530, Viresh Kumar wrote:
> On 15-09-16, 15:47, Mark Brown wrote:
> > On Thu, Sep 15, 2016 at 04:47:01PM +0530, Viresh Kumar wrote:

> > > Fix the kernel warnings and crashes by moving away from managed
> > > regulator bulk get API by using regulator_bulk_get() and explicitly
> > > calling regulator_put() for all the supplies in exit paths.

> > Moving away from regulator bulk get to regulator bulk get?

> Moving away from "managed" regulator bulk get to regulator bulk get?
> Isn't this how we call the devm_* APIs as?

That's really unclearly worded (especially since one uses the function
name and the other doesn't).

> > > -	ret = devm_regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> > > +	ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
> > >  				 wm8994->supplies);

> > > +err_regulator_put:
> > > +	for (i = wm8994->num_supplies - 1; i >= 0; i--)
> > > +		regulator_put(wm8994->supplies[i].consumer);

> > Why are you unwinding regulator_bulk_get() with an open coded
> > regulator_bulk_put()?

> Because there is no regulator_bulk_put(). Should I add it?

I mean regulator_bulk_free() there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-09-15 15:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-15 11:17 [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Viresh Kumar
2016-09-15 11:17 ` [PATCH 2/2] mfd: wm8994-core: Don't use managed regulator bulk get API Viresh Kumar
2016-09-15 12:34   ` Charles Keepax
2016-09-15 14:47   ` Mark Brown
2016-09-15 15:08     ` Viresh Kumar
2016-09-15 15:47       ` Mark Brown
2016-09-15 12:31 ` [PATCH 1/2] mfd: wm8994-core: disable regulators before removing them Charles Keepax

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.