linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
@ 2018-11-15 19:47 Mark Brown
  0 siblings, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-11-15 19:47 UTC (permalink / raw)
  To: Linus Walleij
  Cc: patches, Charles Keepax, Lee Jones, Mark Brown, linux-kernel

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 1d2f46814d20a55c45ac171739b6885826e0c793 Mon Sep 17 00:00:00 2001
From: Linus Walleij <linus.walleij@linaro.org>
Date: Thu, 15 Nov 2018 09:01:18 +0100
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".

Cc: patches@opensource.cirrus.com
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm/mach-s3c64xx/mach-crag6410-module.c | 17 +++++++++++++++--
 drivers/mfd/wm8994-core.c                    |  9 ---------
 drivers/regulator/wm8994-regulator.c         | 20 ++++++++++++--------
 include/linux/mfd/wm8994/pdata.h             |  3 ---
 4 files changed, 27 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 22bd6525e09c..04a177efd245 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>
@@ -306,14 +305,6 @@ static int wm8994_set_pdata_from_of(struct wm8994 *wm8994)
 
 	pdata->csnaddr_pd = of_property_read_bool(np, "wlf,csnaddr-pd");
 
-	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..d7fec533c403 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,15 @@ 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 |
+					GPIOD_FLAGS_BIT_NONEXCLUSIVE);
+	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 +163,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 b19c370fe81a..f346167c0e00 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;
 };
 
-- 
2.19.1


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 17:03                       ` Charles Keepax
@ 2018-11-20 17:23                         ` Marek Szyprowski
  0 siblings, 0 replies; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 17:23 UTC (permalink / raw)
  To: Charles Keepax, Richard Fitzgerald
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

Hi Charles,

On 2018-11-20 18:03, Charles Keepax wrote:
> On Tue, Nov 20, 2018 at 04:57:16PM +0000, Richard Fitzgerald wrote:
>> On 20/11/18 16:34, Marek Szyprowski wrote:
>>> On 2018-11-20 17:16, Richard Fitzgerald wrote:
>>>> On 20/11/18 15:56, Marek Szyprowski wrote:
>>>>> On 2018-11-20 16:36, Charles Keepax wrote:
>>>>>> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>>>>>>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>>>>>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>>>>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>>>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of
>>>>>>>>>>> GPIO number
>>>>>>>>>> This patch causes following kernel warning on Samsung Exynos4412
>>>>>>>>>> based
>> Sounds like all is ok and working as expected.
>> If this is causing you a problem you'll need to provide more explanation of
>> what problem you have so we can understand.
>>
> The problem looks to be that we shouldn't be using devm for the
> GPIO allocation because the device we want to allocate the memory
> against differs from the one that holds the OF node. Apologies
> for missing that in review of the patch.

No problem. That's why we have linux-next. This way such issues can be
noticed before they cause any real problems.

> I have sent a fix that hopefully should resolve the issue, if you
> could test it on you system that would be awesome.

I will check in a minute.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 16:57                     ` Richard Fitzgerald
  2018-11-20 17:01                       ` Mark Brown
  2018-11-20 17:03                       ` Charles Keepax
@ 2018-11-20 17:12                       ` Marek Szyprowski
  2 siblings, 0 replies; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 17:12 UTC (permalink / raw)
  To: Richard Fitzgerald, Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

Hi Richard,

On 2018-11-20 17:57, Richard Fitzgerald wrote:
> On 20/11/18 16:34, Marek Szyprowski wrote:
>> Hi Richard,
>>
>> On 2018-11-20 17:16, Richard Fitzgerald wrote:
>>> On 20/11/18 15:56, Marek Szyprowski wrote:
>>>> Hi Charles,
>>>>
>>>> On 2018-11-20 16:36, Charles Keepax wrote:
>>>>> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>>>>>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>>>>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>>>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of
>>>>>>>>>> GPIO number
>>>>>>>>> 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
>>>>>> This is really weird, because the error in your log relates to
>>>>>> DBVDD1 which is an independent regulator supplied by a separate
>>>>>> regulator. I am really having some difficulty seeing how the
>>>>>> patch interfers. It is definitely that patch which causes the
>>>>>> issue, like you revert it and things work again?
>>>>> Wait does the board still boot just you have an extra probe defer
>>>>> now? Or does it actually fail?
>>>>
>>>> The board boots fine. The only new thing is the mentioned warning,
>>>> which
>>>> I would
>>>>
>>>> like to have fixed.
>>>>
>>>>
>>>> Best regards
>>>>
>>>
>>> -517 is EPROBE_DEFER. This isn't something  that needs "fixing" unless
>>> the
>>> driver is never able to probe.
>>>
>>> If the wm8994 eventually probes ok after retries it's not a problem,
>>> it's normal kernel behaviour.
>>>
>>> If the wm8994 driver never manages to probe successfully it should
>>> mean that
>>> the driver which supplies DBVDD1 isn't available.
>>
>> Deferred probe was there already. This patch however introduced the
>> warning from gpiolib and I would like to have it fixed somehow. In both
>
> I don't follow what it is you want, are you asking that it shouldn't
> probe
> defer, or that it shouldn't log the reason why it deferred?
>
>> cases (with this patch and before it) the wm8994 driver probes okay -
>> when the required regulators are finally available.
>
> Sounds like all is ok and working as expected.
> If this is causing you a problem you'll need to provide more
> explanation of
> what problem you have so we can understand.


I'm asking for fixing the code (or giving a hint how to fix it) in a way
that gpiolib will not complain. My initial reply [1] had a gpiolib
warning, which is the issue. Deferred probe is the way to trigger it. My
fault that I didn't explain it literally what is the issue.

[1] https://lkml.org/lkml/2018/11/20/997

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 17:01                       ` Mark Brown
@ 2018-11-20 17:10                         ` Richard Fitzgerald
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Fitzgerald @ 2018-11-20 17:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: Marek Szyprowski, Charles Keepax, Linus Walleij, Lee Jones,
	Liam Girdwood, linux-kernel, patches

On 20/11/18 17:01, Mark Brown wrote:
> On Tue, Nov 20, 2018 at 04:57:16PM +0000, Richard Fitzgerald wrote:
>> On 20/11/18 16:34, Marek Szyprowski wrote:
> 
>>> Deferred probe was there already. This patch however introduced the
>>> warning from gpiolib and I would like to have it fixed somehow. In both
> 
>> I don't follow what it is you want, are you asking that it shouldn't probe
>> defer, or that it shouldn't log the reason why it deferred?
> 
> He's complaining that gpiolib and/or the driver's usage of it shouldn't
> be generating a backtrace in normal operation.
> 
Ah, I didn't see that. I seem to have not received the start of this thread and
the subsequent discussion only includes the -517 warnings not a mention of
a backtrace.

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 16:57                     ` Richard Fitzgerald
  2018-11-20 17:01                       ` Mark Brown
@ 2018-11-20 17:03                       ` Charles Keepax
  2018-11-20 17:23                         ` Marek Szyprowski
  2018-11-20 17:12                       ` Marek Szyprowski
  2 siblings, 1 reply; 17+ messages in thread
From: Charles Keepax @ 2018-11-20 17:03 UTC (permalink / raw)
  To: Richard Fitzgerald
  Cc: Marek Szyprowski, Mark Brown, Linus Walleij, Lee Jones,
	Liam Girdwood, linux-kernel, patches

On Tue, Nov 20, 2018 at 04:57:16PM +0000, Richard Fitzgerald wrote:
> On 20/11/18 16:34, Marek Szyprowski wrote:
> >On 2018-11-20 17:16, Richard Fitzgerald wrote:
> >>On 20/11/18 15:56, Marek Szyprowski wrote:
> >>>On 2018-11-20 16:36, Charles Keepax wrote:
> >>>>On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
> >>>>>On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
> >>>>>>On 2018-11-20 15:47, Charles Keepax wrote:
> >>>>>>>On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
> >>>>>>>>On 2018-05-17 18:41, Mark Brown wrote:
> >>>>>>>>>Subject: [PATCH] regulator: wm8994: Pass descriptor instead of
> >>>>>>>>>GPIO number
> >>>>>>>>This patch causes following kernel warning on Samsung Exynos4412
> >>>>>>>>based
> Sounds like all is ok and working as expected.
> If this is causing you a problem you'll need to provide more explanation of
> what problem you have so we can understand.
> 

The problem looks to be that we shouldn't be using devm for the
GPIO allocation because the device we want to allocate the memory
against differs from the one that holds the OF node. Apologies
for missing that in review of the patch.

I have sent a fix that hopefully should resolve the issue, if you
could test it on you system that would be awesome.

Thanks,
Charles

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 16:57                     ` Richard Fitzgerald
@ 2018-11-20 17:01                       ` Mark Brown
  2018-11-20 17:10                         ` Richard Fitzgerald
  2018-11-20 17:03                       ` Charles Keepax
  2018-11-20 17:12                       ` Marek Szyprowski
  2 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2018-11-20 17:01 UTC (permalink / raw)
  To: Richard Fitzgerald
  Cc: Marek Szyprowski, Charles Keepax, Linus Walleij, Lee Jones,
	Liam Girdwood, linux-kernel, patches

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

On Tue, Nov 20, 2018 at 04:57:16PM +0000, Richard Fitzgerald wrote:
> On 20/11/18 16:34, Marek Szyprowski wrote:

> > Deferred probe was there already. This patch however introduced the
> > warning from gpiolib and I would like to have it fixed somehow. In both

> I don't follow what it is you want, are you asking that it shouldn't probe
> defer, or that it shouldn't log the reason why it deferred?

He's complaining that gpiolib and/or the driver's usage of it shouldn't
be generating a backtrace in normal operation.

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

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 16:34                   ` Marek Szyprowski
@ 2018-11-20 16:57                     ` Richard Fitzgerald
  2018-11-20 17:01                       ` Mark Brown
                                         ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Richard Fitzgerald @ 2018-11-20 16:57 UTC (permalink / raw)
  To: Marek Szyprowski, Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

On 20/11/18 16:34, Marek Szyprowski wrote:
> Hi Richard,
> 
> On 2018-11-20 17:16, Richard Fitzgerald wrote:
>> On 20/11/18 15:56, Marek Szyprowski wrote:
>>> Hi Charles,
>>>
>>> On 2018-11-20 16:36, Charles Keepax wrote:
>>>> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>>>>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>>>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of
>>>>>>>>> GPIO number
>>>>>>>> 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
>>>>> This is really weird, because the error in your log relates to
>>>>> DBVDD1 which is an independent regulator supplied by a separate
>>>>> regulator. I am really having some difficulty seeing how the
>>>>> patch interfers. It is definitely that patch which causes the
>>>>> issue, like you revert it and things work again?
>>>> Wait does the board still boot just you have an extra probe defer
>>>> now? Or does it actually fail?
>>>
>>> The board boots fine. The only new thing is the mentioned warning, which
>>> I would
>>>
>>> like to have fixed.
>>>
>>>
>>> Best regards
>>>
>>
>> -517 is EPROBE_DEFER. This isn't something  that needs "fixing" unless
>> the
>> driver is never able to probe.
>>
>> If the wm8994 eventually probes ok after retries it's not a problem,
>> it's normal kernel behaviour.
>>
>> If the wm8994 driver never manages to probe successfully it should
>> mean that
>> the driver which supplies DBVDD1 isn't available.
> 
> Deferred probe was there already. This patch however introduced the
> warning from gpiolib and I would like to have it fixed somehow. In both

I don't follow what it is you want, are you asking that it shouldn't probe
defer, or that it shouldn't log the reason why it deferred?

> cases (with this patch and before it) the wm8994 driver probes okay -
> when the required regulators are finally available.

Sounds like all is ok and working as expected.
If this is causing you a problem you'll need to provide more explanation of
what problem you have so we can understand.

> 
> Best regards
> 


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 16:16                 ` Richard Fitzgerald
@ 2018-11-20 16:34                   ` Marek Szyprowski
  2018-11-20 16:57                     ` Richard Fitzgerald
  0 siblings, 1 reply; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 16:34 UTC (permalink / raw)
  To: Richard Fitzgerald, Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

Hi Richard,

On 2018-11-20 17:16, Richard Fitzgerald wrote:
> On 20/11/18 15:56, Marek Szyprowski wrote:
>> Hi Charles,
>>
>> On 2018-11-20 16:36, Charles Keepax wrote:
>>> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>>>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of
>>>>>>>> GPIO number
>>>>>>> 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
>>>> This is really weird, because the error in your log relates to
>>>> DBVDD1 which is an independent regulator supplied by a separate
>>>> regulator. I am really having some difficulty seeing how the
>>>> patch interfers. It is definitely that patch which causes the
>>>> issue, like you revert it and things work again?
>>> Wait does the board still boot just you have an extra probe defer
>>> now? Or does it actually fail?
>>
>> The board boots fine. The only new thing is the mentioned warning, which
>> I would
>>
>> like to have fixed.
>>
>>
>> Best regards
>>
>
> -517 is EPROBE_DEFER. This isn't something  that needs "fixing" unless
> the
> driver is never able to probe.
>
> If the wm8994 eventually probes ok after retries it's not a problem,
> it's normal kernel behaviour.
>
> If the wm8994 driver never manages to probe successfully it should
> mean that
> the driver which supplies DBVDD1 isn't available.

Deferred probe was there already. This patch however introduced the
warning from gpiolib and I would like to have it fixed somehow. In both
cases (with this patch and before it) the wm8994 driver probes okay -
when the required regulators are finally available.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 15:56               ` Marek Szyprowski
@ 2018-11-20 16:16                 ` Richard Fitzgerald
  2018-11-20 16:34                   ` Marek Szyprowski
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Fitzgerald @ 2018-11-20 16:16 UTC (permalink / raw)
  To: Marek Szyprowski, Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

On 20/11/18 15:56, Marek Szyprowski wrote:
> Hi Charles,
> 
> On 2018-11-20 16:36, Charles Keepax wrote:
>> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
>>>>>> 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
>>> This is really weird, because the error in your log relates to
>>> DBVDD1 which is an independent regulator supplied by a separate
>>> regulator. I am really having some difficulty seeing how the
>>> patch interfers. It is definitely that patch which causes the
>>> issue, like you revert it and things work again?
>> Wait does the board still boot just you have an extra probe defer
>> now? Or does it actually fail?
> 
> The board boots fine. The only new thing is the mentioned warning, which
> I would
> 
> like to have fixed.
> 
> 
> Best regards
> 

-517 is EPROBE_DEFER. This isn't something  that needs "fixing" unless the
driver is never able to probe.

If the wm8994 eventually probes ok after retries it's not a problem,
it's normal kernel behaviour.

If the wm8994 driver never manages to probe successfully it should mean that
the driver which supplies DBVDD1 isn't available.

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 15:36             ` Charles Keepax
@ 2018-11-20 15:56               ` Marek Szyprowski
  2018-11-20 16:16                 ` Richard Fitzgerald
  0 siblings, 1 reply; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 15:56 UTC (permalink / raw)
  To: Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

Hi Charles,

On 2018-11-20 16:36, Charles Keepax wrote:
> On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
>> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
>>> On 2018-11-20 15:47, Charles Keepax wrote:
>>>> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>>>>> On 2018-05-17 18:41, Mark Brown wrote:
>>>>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
>>>>> 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
>> This is really weird, because the error in your log relates to
>> DBVDD1 which is an independent regulator supplied by a separate
>> regulator. I am really having some difficulty seeing how the
>> patch interfers. It is definitely that patch which causes the
>> issue, like you revert it and things work again?
> Wait does the board still boot just you have an extra probe defer
> now? Or does it actually fail?

The board boots fine. The only new thing is the mentioned warning, which
I would

like to have fixed.


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 15:32           ` Charles Keepax
  2018-11-20 15:36             ` Charles Keepax
@ 2018-11-20 15:43             ` Mark Brown
  1 sibling, 0 replies; 17+ messages in thread
From: Mark Brown @ 2018-11-20 15:43 UTC (permalink / raw)
  To: Charles Keepax
  Cc: Marek Szyprowski, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

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

On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:

> > No board file, everything in DT.

> This is really weird, because the error in your log relates to
> DBVDD1 which is an independent regulator supplied by a separate
> regulator. I am really having some difficulty seeing how the
> patch interfers. It is definitely that patch which causes the
> issue, like you revert it and things work again?

It's a warning in the GPIO code, the switch to the gpiod API will have
triggered it.

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

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 15:32           ` Charles Keepax
@ 2018-11-20 15:36             ` Charles Keepax
  2018-11-20 15:56               ` Marek Szyprowski
  2018-11-20 15:43             ` Mark Brown
  1 sibling, 1 reply; 17+ messages in thread
From: Charles Keepax @ 2018-11-20 15:36 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

On Tue, Nov 20, 2018 at 03:32:15PM +0000, Charles Keepax wrote:
> On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
> > On 2018-11-20 15:47, Charles Keepax wrote:
> > > On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
> > >> On 2018-05-17 18:41, Mark Brown wrote:
> > >>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
> > >> 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
> This is really weird, because the error in your log relates to
> DBVDD1 which is an independent regulator supplied by a separate
> regulator. I am really having some difficulty seeing how the
> patch interfers. It is definitely that patch which causes the
> issue, like you revert it and things work again?

Wait does the board still boot just you have an extra probe defer
now? Or does it actually fail?

Thanks,
Charles

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 14:58         ` Marek Szyprowski
@ 2018-11-20 15:32           ` Charles Keepax
  2018-11-20 15:36             ` Charles Keepax
  2018-11-20 15:43             ` Mark Brown
  0 siblings, 2 replies; 17+ messages in thread
From: Charles Keepax @ 2018-11-20 15:32 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

On Tue, Nov 20, 2018 at 03:58:59PM +0100, Marek Szyprowski wrote:
> Hi Charles,
> 
> On 2018-11-20 15:47, Charles Keepax wrote:
> > On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
> >> On 2018-05-17 18:41, Mark Brown wrote:
> >>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
> >> 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
> > How is the wm8994 being registered on this board? I am having
> > difficulty finding a device tree or a board file that relates to
> > the board and includes the wm8994.
> 
> 
> Please check arch/arm/boot/dts/exynos4412-trats2.dts details. The I2C device
> is defined in exynos4412-midas.dtsi, it uses "wlf,wm1811" compatible.
> 

Ok got it, thanks.

> 
> >>> @@ -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),
> >>> +		{ },
> >>>  	},
> >>>  };
> > If its being done through a board file I guess you will need the
> > equivalent of this.
> 
> 
> No board file, everything in DT.
> 

This is really weird, because the error in your log relates to
DBVDD1 which is an independent regulator supplied by a separate
regulator. I am really having some difficulty seeing how the
patch interfers. It is definitely that patch which causes the
issue, like you revert it and things work again?

Thanks,
Charles

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 14:47       ` Charles Keepax
@ 2018-11-20 14:58         ` Marek Szyprowski
  2018-11-20 15:32           ` Charles Keepax
  0 siblings, 1 reply; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 14:58 UTC (permalink / raw)
  To: Charles Keepax
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

Hi Charles,

On 2018-11-20 15:47, Charles Keepax wrote:
> On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
>> On 2018-05-17 18:41, Mark Brown wrote:
>>> Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
>> 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
> How is the wm8994 being registered on this board? I am having
> difficulty finding a device tree or a board file that relates to
> the board and includes the wm8994.


Please check arch/arm/boot/dts/exynos4412-trats2.dts details. The I2C device
is defined in exynos4412-midas.dtsi, it uses "wlf,wm1811" compatible.


>>> @@ -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),
>>> +		{ },
>>>  	},
>>>  };
> If its being done through a board file I guess you will need the
> equivalent of this.


No board file, everything in DT.


Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-11-20 13:43     ` Marek Szyprowski
@ 2018-11-20 14:47       ` Charles Keepax
  2018-11-20 14:58         ` Marek Szyprowski
  0 siblings, 1 reply; 17+ messages in thread
From: Charles Keepax @ 2018-11-20 14:47 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Mark Brown, Linus Walleij, Lee Jones, Liam Girdwood,
	linux-kernel, patches

On Tue, Nov 20, 2018 at 02:43:32PM +0100, Marek Szyprowski wrote:
> On 2018-05-17 18:41, Mark Brown wrote:
> > Subject: [PATCH] regulator: wm8994: Pass descriptor instead of GPIO number
> 
> 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

How is the wm8994 being registered on this board? I am having
difficulty finding a device tree or a board file that relates to
the board and includes the wm8994.

> > @@ -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),
> > +		{ },
> >  	},
> >  };

If its being done through a board file I guess you will need the
equivalent of this.

Thanks,
Charles

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

* Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
       [not found]   ` <CGME20181120134333eucas1p27a19912dddf4b9b34da505e0973c9137@eucas1p2.samsung.com>
@ 2018-11-20 13:43     ` Marek Szyprowski
  2018-11-20 14:47       ` Charles Keepax
  0 siblings, 1 reply; 17+ messages in thread
From: Marek Szyprowski @ 2018-11-20 13:43 UTC (permalink / raw)
  To: Mark Brown, Linus Walleij
  Cc: Charles Keepax, Lee Jones, Liam Girdwood, linux-kernel, patches

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@linaro.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@linaro.org>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Mark Brown <broonie@kernel.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


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

* Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree
  2018-04-22 23:07 [PATCH 13/18 v2] regulator: wm8994: Pass descriptor instead of GPIO number Linus Walleij
@ 2018-05-17 16:41 ` Mark Brown
       [not found]   ` <CGME20181120134333eucas1p27a19912dddf4b9b34da505e0973c9137@eucas1p2.samsung.com>
  0 siblings, 1 reply; 17+ messages in thread
From: Mark Brown @ 2018-05-17 16:41 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Charles Keepax, Lee Jones, Mark Brown, Liam Girdwood, Mark Brown,
	linux-kernel, patches, Charles Keepax, linux-kernel

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@linaro.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@linaro.org>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 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;
 };
 
-- 
2.17.0

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

end of thread, other threads:[~2018-11-20 17:23 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-15 19:47 Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2018-04-22 23:07 [PATCH 13/18 v2] regulator: wm8994: Pass descriptor instead of GPIO number Linus Walleij
2018-05-17 16:41 ` Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree Mark Brown
     [not found]   ` <CGME20181120134333eucas1p27a19912dddf4b9b34da505e0973c9137@eucas1p2.samsung.com>
2018-11-20 13:43     ` Marek Szyprowski
2018-11-20 14:47       ` Charles Keepax
2018-11-20 14:58         ` Marek Szyprowski
2018-11-20 15:32           ` Charles Keepax
2018-11-20 15:36             ` Charles Keepax
2018-11-20 15:56               ` Marek Szyprowski
2018-11-20 16:16                 ` Richard Fitzgerald
2018-11-20 16:34                   ` Marek Szyprowski
2018-11-20 16:57                     ` Richard Fitzgerald
2018-11-20 17:01                       ` Mark Brown
2018-11-20 17:10                         ` Richard Fitzgerald
2018-11-20 17:03                       ` Charles Keepax
2018-11-20 17:23                         ` Marek Szyprowski
2018-11-20 17:12                       ` Marek Szyprowski
2018-11-20 15:43             ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).