[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <66d3b74c-0152-b17b-89d4-a21d33445b25@samsung.com>
Date: Tue, 20 Nov 2018 14:43:32 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Mark Brown <broonie@...nel.org>,
Linus Walleij <linus.walleij@...aro.org>
Cc: Charles Keepax <ckeepax@...nsource.cirrus.com>,
Lee Jones <lee.jones@...aro.org>,
Liam Girdwood <lgirdwood@...il.com>,
linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: Applied
"regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator
tree
Hi All,
On 2018-05-17 18:41, Mark Brown wrote:
> The patch
>
> regulator: wm8994: Pass descriptor instead of GPIO number
>
> has been applied to the regulator tree at
>
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
>
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.
>
> Thanks,
> Mark
>
> >From 3c6b38d45fa51c7c51c5e2347fc1a6bef6a46525 Mon Sep 17 00:00:00 2001
> From: Linus Walleij <linus.walleij@...aro.org>
> Date: Mon, 14 May 2018 10:06:34 +0200
> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
>
> Instead of passing a global GPIO number for the enable GPIO, pass
> a descriptor looked up from the device tree node or the board file
> decriptor table for the regulator.
>
> There is a single board file passing the GPIOs for LDO1 and LDO2
> through platform data, so augment this to pass descriptors
> associated with the i2c device as well.
>
> The special GPIO enable DT property for the enable GPIO is
> nonstandard but this was accomodated in
> commit 6a537d48461deacc57c07ed86d9915e5aa4b3539
> "gpio: of: Support regulator nonstandard GPIO properties".
>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> Acked-by: Charles Keepax <ckeepax@...nsource.cirrus.com>
> Acked-by: Lee Jones <lee.jones@...aro.org>
> Signed-off-by: Mark Brown <broonie@...nel.org>
This patch causes following kernel warning on Samsung Exynos4412 based
Trats2 board:
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
release_nodes+0x178/0x1fc
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.20.0-rc3-next-20181120-00009-g6917effde9ad #1085
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[<c01125fc>] (unwind_backtrace) from [<c010e140>] (show_stack+0x10/0x14)
[<c010e140>] (show_stack) from [<c0a4aee0>] (dump_stack+0x98/0xc4)
[<c0a4aee0>] (dump_stack) from [<c0127078>] (__warn+0x10c/0x124)
[<c0127078>] (__warn) from [<c01271a4>] (warn_slowpath_null+0x40/0x48)
[<c01271a4>] (warn_slowpath_null) from [<c0583700>]
(release_nodes+0x178/0x1fc)
[<c0583700>] (release_nodes) from [<c057ed8c>] (really_probe+0xe8/0x400)
[<c057ed8c>] (really_probe) from [<c057f260>]
(driver_probe_device+0x78/0x1b8)
[<c057f260>] (driver_probe_device) from [<c057f4c8>]
(__driver_attach+0x128/0x144)
[<c057f4c8>] (__driver_attach) from [<c057cdd8>]
(bus_for_each_dev+0x68/0xb4)
[<c057cdd8>] (bus_for_each_dev) from [<c057e0d8>]
(bus_add_driver+0x1a8/0x268)
[<c057e0d8>] (bus_add_driver) from [<c0580504>] (driver_register+0x78/0x10c)
[<c0580504>] (driver_register) from [<c06d0540>]
(i2c_register_driver+0x3c/0xa8)
[<c06d0540>] (i2c_register_driver) from [<c0103154>]
(do_one_initcall+0x8c/0x410)
[<c0103154>] (do_one_initcall) from [<c0f0124c>]
(kernel_init_freeable+0x3c8/0x4d0)
[<c0f0124c>] (kernel_init_freeable) from [<c0a64df4>]
(kernel_init+0x8/0x10c)
[<c0a64df4>] (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: 177215
hardirqs last enabled at (177223): [<c0192d64>] console_unlock+0x4ac/0x698
hardirqs last disabled at (177242): [<c0192984>] console_unlock+0xcc/0x698
softirqs last enabled at (177260): [<c0102618>] __do_softirq+0x4f0/0x5e0
softirqs last disabled at (177251): [<c012f21c>] irq_exit+0x160/0x16c
---[ end trace ed3dd4223b822796 ]---
> ---
> arch/arm/mach-s3c64xx/mach-crag6410-module.c | 17 +++++++++++++++--
> drivers/mfd/wm8994-core.c | 9 ---------
> drivers/regulator/wm8994-regulator.c | 19 +++++++++++--------
> include/linux/mfd/wm8994/pdata.h | 3 ---
> 4 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410-module.c b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
> index 5aa472892465..76c4855a03bc 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410-module.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410-module.c
> @@ -194,8 +194,8 @@ static struct wm8994_pdata wm8994_pdata = {
> 0x3, /* IRQ out, active high, CMOS */
> },
> .ldo = {
> - { .enable = S3C64XX_GPN(6), .init_data = &wm8994_ldo1, },
> - { .enable = S3C64XX_GPN(4), .init_data = &wm8994_ldo2, },
> + { .init_data = &wm8994_ldo1, },
> + { .init_data = &wm8994_ldo2, },
> },
> };
>
> @@ -203,6 +203,18 @@ static const struct i2c_board_info wm1277_devs[] = {
> { I2C_BOARD_INFO("wm8958", 0x1a), /* WM8958 is the superset */
> .platform_data = &wm8994_pdata,
> .irq = GLENFARCLAS_PMIC_IRQ_BASE + WM831X_IRQ_GPIO_2,
> + .dev_name = "wm8958",
> + },
> +};
> +
> +static struct gpiod_lookup_table wm8994_gpiod_table = {
> + .dev_id = "i2c-wm8958", /* I2C device name */
> + .table = {
> + GPIO_LOOKUP("GPION", 6,
> + "wlf,ldo1ena", GPIO_ACTIVE_HIGH),
> + GPIO_LOOKUP("GPION", 4,
> + "wlf,ldo2ena", GPIO_ACTIVE_HIGH),
> + { },
> },
> };
>
> @@ -381,6 +393,7 @@ static int wlf_gf_module_probe(struct i2c_client *i2c,
>
> gpiod_add_lookup_table(&wm5102_reva_gpiod_table);
> gpiod_add_lookup_table(&wm5102_gpiod_table);
> + gpiod_add_lookup_table(&wm8994_gpiod_table);
>
> if (i < ARRAY_SIZE(gf_mods)) {
> dev_info(&i2c->dev, "%s revision %d\n",
> diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
> index 953d0790ffd5..c409464231f6 100644
> --- a/drivers/mfd/wm8994-core.c
> +++ b/drivers/mfd/wm8994-core.c
> @@ -21,7 +21,6 @@
> #include <linux/mfd/core.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
> -#include <linux/of_gpio.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> @@ -302,14 +301,6 @@ static int wm8994_set_pdata_from_of(struct wm8994 *wm8994)
> if (of_find_property(np, "wlf,ldoena-always-driven", NULL))
> pdata->lineout2fb = true;
>
> - pdata->ldo[0].enable = of_get_named_gpio(np, "wlf,ldo1ena", 0);
> - if (pdata->ldo[0].enable < 0)
> - pdata->ldo[0].enable = 0;
> -
> - pdata->ldo[1].enable = of_get_named_gpio(np, "wlf,ldo2ena", 0);
> - if (pdata->ldo[1].enable < 0)
> - pdata->ldo[1].enable = 0;
> -
> return 0;
> }
> #else
> diff --git a/drivers/regulator/wm8994-regulator.c b/drivers/regulator/wm8994-regulator.c
> index 7a4ce6df4f22..d3a5f48119c2 100644
> --- a/drivers/regulator/wm8994-regulator.c
> +++ b/drivers/regulator/wm8994-regulator.c
> @@ -19,7 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/regulator/driver.h>
> #include <linux/regulator/machine.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
> #include <linux/slab.h>
>
> #include <linux/mfd/wm8994/core.h>
> @@ -129,6 +129,7 @@ static int wm8994_ldo_probe(struct platform_device *pdev)
> int id = pdev->id % ARRAY_SIZE(pdata->ldo);
> struct regulator_config config = { };
> struct wm8994_ldo *ldo;
> + struct gpio_desc *gpiod;
> int ret;
>
> dev_dbg(&pdev->dev, "Probing LDO%d\n", id + 1);
> @@ -145,12 +146,14 @@ static int wm8994_ldo_probe(struct platform_device *pdev)
> config.driver_data = ldo;
> config.regmap = wm8994->regmap;
> config.init_data = &ldo->init_data;
> - if (pdata) {
> - config.ena_gpio = pdata->ldo[id].enable;
> - } else if (wm8994->dev->of_node) {
> - config.ena_gpio = wm8994->pdata.ldo[id].enable;
> - config.ena_gpio_initialized = true;
> - }
> +
> + /* 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);
> + if (IS_ERR(gpiod))
> + return PTR_ERR(gpiod);
> + config.ena_gpiod = gpiod;
>
> /* Use default constraints if none set up */
> if (!pdata || !pdata->ldo[id].init_data || wm8994->dev->of_node) {
> @@ -159,7 +162,7 @@ static int wm8994_ldo_probe(struct platform_device *pdev)
>
> ldo->init_data = wm8994_ldo_default[id];
> ldo->init_data.consumer_supplies = &ldo->supply;
> - if (!config.ena_gpio)
> + if (!gpiod)
> ldo->init_data.constraints.valid_ops_mask = 0;
> } else {
> ldo->init_data = *pdata->ldo[id].init_data;
> diff --git a/include/linux/mfd/wm8994/pdata.h b/include/linux/mfd/wm8994/pdata.h
> index 90c60524a496..fca67bd194e2 100644
> --- a/include/linux/mfd/wm8994/pdata.h
> +++ b/include/linux/mfd/wm8994/pdata.h
> @@ -20,9 +20,6 @@
> #define WM8994_NUM_AIF 3
>
> struct wm8994_ldo_pdata {
> - /** GPIOs to enable regulator, 0 or less if not available */
> - int enable;
> -
> const struct regulator_init_data *init_data;
> };
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists