Hi Charles, On 2018-11-20 18:01, Charles Keepax wrote: > We need to manage the life time of the enable GPIO against the regulator > device but the OF node lives on the parent MFD device. As such we can't > use the devm functions which assume the same device will be used for > both. > > Reported-by: Marek Szyprowski <m.szyprow...@samsung.com> > Signed-off-by: Charles Keepax <ckee...@opensource.cirrus.com> > --- > > This patch is only build tested so it would be good to get a > tested by from Marek before applying.
Sadly it looks that it is not enough. The issue is still there: wm8994 4-001a: Failed to get supply 'DBVDD1': -517 wm8994 4-001a: Failed to get supplies: -517 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at drivers/gpio/gpiolib.c:2421 regulator_ena_gpio_free+0x70/0xa0 Modules linked in: CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc3-next-20181120-00001-g330f37c7fb0c #5090 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [<c0111a54>] (unwind_backtrace) from [<c010d900>] (show_stack+0x10/0x14) [<c010d900>] (show_stack) from [<c09e5878>] (dump_stack+0x90/0xc8) [<c09e5878>] (dump_stack) from [<c0125998>] (__warn+0xf8/0x124) [<c0125998>] (__warn) from [<c0125a04>] (warn_slowpath_null+0x40/0x48) [<c0125a04>] (warn_slowpath_null) from [<c04a5b68>] (regulator_ena_gpio_free+0x70/0xa0) [<c04a5b68>] (regulator_ena_gpio_free) from [<c04aa03c>] (regulator_unregister+0x134/0x17c) [<c04aa03c>] (regulator_unregister) from [<c054b5f8>] (release_nodes+0x164/0x1dc) [<c054b5f8>] (release_nodes) from [<c054810c>] (device_release_driver_internal+0x18c/0x21c) [<c054810c>] (device_release_driver_internal) from [<c054698c>] (bus_remove_device+0xdc/0x12c) [<c054698c>] (bus_remove_device) from [<c05434bc>] (device_del+0x108/0x2e0) [<c05434bc>] (device_del) from [<c0549c38>] (platform_device_del+0x20/0x8c) [<c0549c38>] (platform_device_del) from [<c0549cb0>] (platform_device_unregister+0xc/0x18) [<c0549cb0>] (platform_device_unregister) from [<c0575120>] (mfd_remove_devices_fn+0x58/0x64) [<c0575120>] (mfd_remove_devices_fn) from [<c05425a8>] (device_for_each_child_reverse+0x34/0x74) [<c05425a8>] (device_for_each_child_reverse) from [<c0574f18>] (mfd_remove_devices+0x20/0x30) [<c0574f18>] (mfd_remove_devices) from [<c0573b1c>] (wm8994_i2c_probe+0x2dc/0x8d0) [<c0573b1c>] (wm8994_i2c_probe) from [<c068bce4>] (i2c_device_probe+0x234/0x2b0) [<c068bce4>] (i2c_device_probe) from [<c0547774>] (really_probe+0x224/0x3f4) [<c0547774>] (really_probe) from [<c0547b98>] (driver_probe_device+0x70/0x1c4) [<c0547b98>] (driver_probe_device) from [<c0547e0c>] (__driver_attach+0x120/0x144) [<c0547e0c>] (__driver_attach) from [<c05459f0>] (bus_for_each_dev+0x58/0x7c) [<c05459f0>] (bus_for_each_dev) from [<c0546b50>] (bus_add_driver+0x174/0x250) [<c0546b50>] (bus_add_driver) from [<c0548db8>] (driver_register+0x7c/0x110) [<c0548db8>] (driver_register) from [<c068ddcc>] (i2c_register_driver+0x38/0xa8) [<c068ddcc>] (i2c_register_driver) from [<c01030fc>] (do_one_initcall+0x80/0x408) [<c01030fc>] (do_one_initcall) from [<c0f011e4>] (kernel_init_freeable+0x3a0/0x4fc) [<c0f011e4>] (kernel_init_freeable) from [<c09fdd6c>] (kernel_init+0x8/0x118) [<c09fdd6c>] (kernel_init) from [<c01010b4>] (ret_from_fork+0x14/0x20) Exception stack(0xef0e3fb0 to 0xef0e3ff8) 3fa0: 00000000 00000000 00000000 00000000 3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 3fe0: 00000000 00000000 00000000 00000000 00000013 00000000 irq event stamp: 176913 hardirqs last enabled at (176921): [<c018b1c0>] console_unlock+0x460/0x6ec hardirqs last disabled at (176940): [<c018ae18>] console_unlock+0xb8/0x6ec softirqs last enabled at (176956): [<c01024e4>] __do_softirq+0x3a4/0x66c softirqs last disabled at (176967): [<c012d618>] irq_exit+0x140/0x168 I will check more tomorrow. > Thanks, > Charles > > drivers/regulator/wm8994-regulator.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/drivers/regulator/wm8994-regulator.c > b/drivers/regulator/wm8994-regulator.c > index d7fec533c403..f2592c30d0ca 100644 > --- a/drivers/regulator/wm8994-regulator.c > +++ b/drivers/regulator/wm8994-regulator.c > @@ -31,6 +31,7 @@ struct wm8994_ldo { > struct wm8994 *wm8994; > struct regulator_consumer_supply supply; > struct regulator_init_data init_data; > + struct gpio_desc *ena_gpiod; > }; > > #define WM8994_LDO1_MAX_SELECTOR 0x7 > @@ -147,14 +148,19 @@ static int wm8994_ldo_probe(struct platform_device > *pdev) > config.regmap = wm8994->regmap; > config.init_data = &ldo->init_data; > > - /* Look up LDO enable GPIO from the parent device node */ > - gpiod = devm_gpiod_get_optional(pdev->dev.parent, > - id ? "wlf,ldo2ena" : "wlf,ldo1ena", > - GPIOD_OUT_LOW | > - GPIOD_FLAGS_BIT_NONEXCLUSIVE); > + /* > + * Look up LDO enable GPIO from the parent device node, we can't > + * use devm because it assumes the device we want to allocate > + * against is the same one that holds the OF node > + */ > + gpiod = gpiod_get_optional(pdev->dev.parent, > + id ? "wlf,ldo2ena" : "wlf,ldo1ena", > + GPIOD_OUT_LOW | > + GPIOD_FLAGS_BIT_NONEXCLUSIVE); > if (IS_ERR(gpiod)) > return PTR_ERR(gpiod); > config.ena_gpiod = gpiod; > + ldo->ena_gpiod = gpiod; > > /* Use default constraints if none set up */ > if (!pdata || !pdata->ldo[id].init_data || wm8994->dev->of_node) { > @@ -184,11 +190,23 @@ static int wm8994_ldo_probe(struct platform_device > *pdev) > return 0; > > err: > + gpiod_put(gpiod); > return ret; > } > > +static int wm8994_ldo_remove(struct platform_device *pdev) > +{ > + struct wm8994_ldo *ldo = platform_get_drvdata(pdev); > + > + if (ldo->ena_gpiod) > + gpiod_put(ldo->ena_gpiod); > + > + return 0; > +} > + > static struct platform_driver wm8994_ldo_driver = { > .probe = wm8994_ldo_probe, > + .remove = wm8994_ldo_remove, > .driver = { > .name = "wm8994-ldo", > }, Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland