[PATCH RESEND 0/4] Minor WM8994 MFD/codec fixes

[PATCH RESEND 0/4] Minor WM8994 MFD/codec fixes

[Marek Szyprowski]

Hi!

This is a resend of the minor WM8994 MFD/codec driver fixes posted in
last days of the February 2020:

https://lore.kernel.org/patchwork/project/lkml/list/?series=431296
https://lore.kernel.org/patchwork/project/lkml/list/?series=431721

I hope this time the patches will find their way to mainline.

Best regards,
Marek Szyprowski


Patch summary:

Marek Szyprowski (4):
  mfd: wm8994: Fix driver operation if loaded as modules
  mfd: wm8994: Fix unbalanced calls to regulator_bulk_disable()
  mfd: wm8994: Silence warning about supplies during deferred probe
  ASoC: wm8994: Silence warnings during deferred probe

 drivers/mfd/wm8994-core.c | 8 +++++++-
 sound/soc/codecs/wm8994.c | 3 ++-
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH 1/4] mfd: wm8994: Fix driver operation if loaded as modules

[Marek Szyprowski]

WM8994 chip has built-in regulators, which might be used for chip
operation. They are controlled by a separate wm8994-regulator driver,
which should be loaded before this driver calls regulator_get(), because
that driver also provides consumer-supply mapping for the them. If that
driver is not yet loaded, regulator core substitute them with dummy
regulator, what breaks chip operation, because the built-in regulators are
never enabled. Fix this by annotating this driver with MODULE_SOFTDEP()
"pre" dependency to "wm8994_regulator" module.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/wm8994-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 1e9fe7d92597..737dede4a95c 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -690,3 +690,4 @@ module_i2c_driver(wm8994_i2c_driver);
 MODULE_DESCRIPTION("Core support for the WM8994 audio CODEC");
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_SOFTDEP("pre: wm8994_regulator");
-- 
2.17.1

[PATCH 2/4] mfd: wm8994: Fix unbalanced calls to regulator_bulk_disable()

[Marek Szyprowski]

When runtime PM is enabled, regulators are being controlled by the
driver's suspend and resume callbacks. They are also unconditionally
enabled at driver's probe(), and disabled in remove() functions. Add
more calls to runtime PM framework to ensure that the device's runtime
PM state matches the regulators state:
1. at the end of probe() function: set runtime PM state to active, so
there will be no spurious call to resume();
2. in remove(), ensure that resume() is called before disabling runtime PM
management and unconditionally disabling the regulators.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 drivers/mfd/wm8994-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 737dede4a95c..69d973ec42bf 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -584,6 +584,7 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 		goto err_irq;
 	}
 
+	pm_runtime_set_active(wm8994->dev);
 	pm_runtime_enable(wm8994->dev);
 	pm_runtime_idle(wm8994->dev);
 
@@ -603,7 +604,9 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 
 static void wm8994_device_exit(struct wm8994 *wm8994)
 {
+	pm_runtime_get_sync(wm8994->dev);
 	pm_runtime_disable(wm8994->dev);
+	pm_runtime_put_noidle(wm8994->dev);
 	wm8994_irq_exit(wm8994);
 	regulator_bulk_disable(wm8994->num_supplies, wm8994->supplies);
 	regulator_bulk_free(wm8994->num_supplies, wm8994->supplies);
-- 
2.17.1

[PATCH 3/4] mfd: wm8994: Silence warning about supplies during deferred probe

[Marek Szyprowski]

Don't confuse user with meaningless warning about the failure in getting
supplies in case of deferred probe.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/mfd/wm8994-core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mfd/wm8994-core.c b/drivers/mfd/wm8994-core.c
index 69d973ec42bf..3b2b93c5bbcb 100644
--- a/drivers/mfd/wm8994-core.c
+++ b/drivers/mfd/wm8994-core.c
@@ -393,7 +393,9 @@ static int wm8994_device_init(struct wm8994 *wm8994, int irq)
 	ret = regulator_bulk_get(wm8994->dev, wm8994->num_supplies,
 				 wm8994->supplies);
 	if (ret != 0) {
-		dev_err(wm8994->dev, "Failed to get supplies: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(wm8994->dev, "Failed to get supplies: %d\n",
+				ret);
 		goto err;
 	}
 
-- 
2.17.1

[PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Marek Szyprowski]

Don't confuse user with meaningless warning about the failure in getting
clocks in case of deferred probe.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 sound/soc/codecs/wm8994.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c
index 55d0b9be6ff0..7426df1f806c 100644
--- a/sound/soc/codecs/wm8994.c
+++ b/sound/soc/codecs/wm8994.c
@@ -4593,7 +4593,8 @@ static int wm8994_probe(struct platform_device *pdev)
 	ret = devm_clk_bulk_get_optional(pdev->dev.parent, ARRAY_SIZE(wm8994->mclk),
 					 wm8994->mclk);
 	if (ret < 0) {
-		dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);
 		return ret;
 	}
 
-- 
2.17.1

Re: [PATCH 3/4] mfd: wm8994: Silence warning about supplies during deferred probe

[Charles Keepax]

On Mon, Apr 27, 2020 at 09:48:31AM +0200, Marek Szyprowski wrote:
> Don't confuse user with meaningless warning about the failure in getting
> supplies in case of deferred probe.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Charles Keepax]

On Mon, Apr 27, 2020 at 09:48:32AM +0200, Marek Szyprowski wrote:
> Don't confuse user with meaningless warning about the failure in getting
> clocks in case of deferred probe.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Mark Brown]

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

On Mon, Apr 27, 2020 at 09:48:32AM +0200, Marek Szyprowski wrote:
> Don't confuse user with meaningless warning about the failure in getting
> clocks in case of deferred probe.

>  	if (ret < 0) {
> -		dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);

This completely eliminates the diagnostics which means that if the clock
isn't there the user is a bit stuck trying to work out what's missing.
There should still be a diagnostic.

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

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Lee Jones]

On Mon, 27 Apr 2020, Mark Brown wrote:

> On Mon, Apr 27, 2020 at 09:48:32AM +0200, Marek Szyprowski wrote:
> > Don't confuse user with meaningless warning about the failure in getting
> > clocks in case of deferred probe.
> 
> >  	if (ret < 0) {
> > -		dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(&pdev->dev, "Failed to get clocks: %d\n", ret);
> 
> This completely eliminates the diagnostics which means that if the clock
> isn't there the user is a bit stuck trying to work out what's missing.
> There should still be a diagnostic.

The driver won't defer forever though.  The final pass should fail
with a different error.  At which point the error will be released to
the system log, no?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/4] mfd: wm8994: Fix driver operation if loaded as modules

[Lee Jones]

On Mon, 27 Apr 2020, Marek Szyprowski wrote:

> WM8994 chip has built-in regulators, which might be used for chip
> operation. They are controlled by a separate wm8994-regulator driver,
> which should be loaded before this driver calls regulator_get(), because
> that driver also provides consumer-supply mapping for the them. If that
> driver is not yet loaded, regulator core substitute them with dummy
> regulator, what breaks chip operation, because the built-in regulators are
> never enabled. Fix this by annotating this driver with MODULE_SOFTDEP()
> "pre" dependency to "wm8994_regulator" module.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  drivers/mfd/wm8994-core.c | 1 +
>  1 file changed, 1 insertion(+)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] mfd: wm8994: Fix unbalanced calls to regulator_bulk_disable()

[Lee Jones]

On Mon, 27 Apr 2020, Marek Szyprowski wrote:

> When runtime PM is enabled, regulators are being controlled by the
> driver's suspend and resume callbacks. They are also unconditionally
> enabled at driver's probe(), and disabled in remove() functions. Add
> more calls to runtime PM framework to ensure that the device's runtime
> PM state matches the regulators state:
> 1. at the end of probe() function: set runtime PM state to active, so
> there will be no spurious call to resume();
> 2. in remove(), ensure that resume() is called before disabling runtime PM
> management and unconditionally disabling the regulators.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  drivers/mfd/wm8994-core.c | 3 +++
>  1 file changed, 3 insertions(+)

For my own reference (apply this as-is to your sign-off block):

  Acked-for-MFD-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Mark Brown]

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

On Tue, Apr 28, 2020 at 11:36:38AM +0100, Lee Jones wrote:
> On Mon, 27 Apr 2020, Mark Brown wrote:

> > This completely eliminates the diagnostics which means that if the clock
> > isn't there the user is a bit stuck trying to work out what's missing.
> > There should still be a diagnostic.

> The driver won't defer forever though.  The final pass should fail
> with a different error.  At which point the error will be released to
> the system log, no?

One of the really common cases is that someone forgot to build the
driver for the dependency so it'll just defer forever waiting for
something that never loads.

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

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Lee Jones]

On Tue, 28 Apr 2020, Mark Brown wrote:

> On Tue, Apr 28, 2020 at 11:36:38AM +0100, Lee Jones wrote:
> > On Mon, 27 Apr 2020, Mark Brown wrote:
> 
> > > This completely eliminates the diagnostics which means that if the clock
> > > isn't there the user is a bit stuck trying to work out what's missing.
> > > There should still be a diagnostic.
> 
> > The driver won't defer forever though.  The final pass should fail
> > with a different error.  At which point the error will be released to
> > the system log, no?
> 
> One of the really common cases is that someone forgot to build the
> driver for the dependency so it'll just defer forever waiting for
> something that never loads.

Need to find another way to identify these failures.  There are 10's
if not 100's of cases of silently returning if -EPROBE_DEFER is
caught.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 4/4] ASoC: wm8994: Silence warnings during deferred probe

[Mark Brown]

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

On Wed, Apr 29, 2020 at 08:15:53AM +0100, Lee Jones wrote:
> On Tue, 28 Apr 2020, Mark Brown wrote:

> > One of the really common cases is that someone forgot to build the
> > driver for the dependency so it'll just defer forever waiting for
> > something that never loads.

> Need to find another way to identify these failures.  There are 10's
> if not 100's of cases of silently returning if -EPROBE_DEFER is
> caught.

Or someone could go through and improve the diagnostics on those cases.

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

Re: [PATCH RESEND 0/4] Minor WM8994 MFD/codec fixes

[Marek Szyprowski]

Hi All,

On 27.04.2020 09:48, Marek Szyprowski wrote:
> This is a resend of the minor WM8994 MFD/codec driver fixes posted in
> last days of the February 2020:
>
> https://lore.kernel.org/patchwork/project/lkml/list/?series=431296
> https://lore.kernel.org/patchwork/project/lkml/list/?series=431721
>
> I hope this time the patches will find their way to mainline.

Gentle ping. Lee: could you queue at least the first 2 patches if you 
consider the latter 2 a bit controversial?

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

Re: [PATCH 1/4] mfd: wm8994: Fix driver operation if loaded as modules

[Lee Jones]

On Mon, 27 Apr 2020, Marek Szyprowski wrote:

> WM8994 chip has built-in regulators, which might be used for chip
> operation. They are controlled by a separate wm8994-regulator driver,
> which should be loaded before this driver calls regulator_get(), because
> that driver also provides consumer-supply mapping for the them. If that
> driver is not yet loaded, regulator core substitute them with dummy
> regulator, what breaks chip operation, because the built-in regulators are
> never enabled. Fix this by annotating this driver with MODULE_SOFTDEP()
> "pre" dependency to "wm8994_regulator" module.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  drivers/mfd/wm8994-core.c | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 2/4] mfd: wm8994: Fix unbalanced calls to regulator_bulk_disable()

[Lee Jones]

On Mon, 27 Apr 2020, Marek Szyprowski wrote:

> When runtime PM is enabled, regulators are being controlled by the
> driver's suspend and resume callbacks. They are also unconditionally
> enabled at driver's probe(), and disabled in remove() functions. Add
> more calls to runtime PM framework to ensure that the device's runtime
> PM state matches the regulators state:
> 1. at the end of probe() function: set runtime PM state to active, so
> there will be no spurious call to resume();
> 2. in remove(), ensure that resume() is called before disabling runtime PM
> management and unconditionally disabling the regulators.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> ---
>  drivers/mfd/wm8994-core.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/4] mfd: wm8994: Silence warning about supplies during deferred probe

[Lee Jones]

On Mon, 27 Apr 2020, Marek Szyprowski wrote:

> Don't confuse user with meaningless warning about the failure in getting
> supplies in case of deferred probe.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/mfd/wm8994-core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog