2018-11-15 19:49:03

by Mark Brown

[permalink] [raw]
Subject: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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 <[email protected]>
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: [email protected]
Acked-by: Charles Keepax <[email protected]>
Acked-by: Lee Jones <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
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



2018-11-20 15:29:59

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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


2018-11-20 15:36:22

by Charles Keepax

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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

2018-11-20 15:37:40

by Charles Keepax

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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

2018-11-20 16:10:43

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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


2018-11-20 17:03:17

by Mark Brown

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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.


Attachments:
(No filename) (533.00 B)
signature.asc (499.00 B)
Download all attachments

2018-11-20 17:05:38

by Charles Keepax

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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

2018-11-20 17:11:38

by Richard Fitzgerald

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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.

2018-11-20 17:14:47

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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


2018-11-20 17:24:39

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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


2018-11-20 17:53:38

by Mark Brown

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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.


Attachments:
(No filename) (579.00 B)
signature.asc (499.00 B)
Download all attachments

2018-11-20 19:29:16

by Richard Fitzgerald

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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.

2018-11-20 19:32:41

by Marek Szyprowski

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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


2018-11-20 19:41:49

by Richard Fitzgerald

[permalink] [raw]
Subject: Re: Applied "regulator: wm8994: Pass descriptor instead of GPIO number" to the regulator tree

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
>