| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c426f174-dc45-3a25-4c7e-018972c2ceef@samsung.com>
Date: Tue, 20 Nov 2018 18:25:38 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>, broonie@...nel.org
Cc: linus.walleij@...aro.org, lgirdwood@...il.com,
linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH] regulator: wm8994: Don't use devres for enable GPIOs
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.szyprowski@...sung.com>
> Signed-off-by: Charles Keepax <ckeepax@...nsource.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
Powered by blists - more mailing lists