2019-01-21 18:39:43

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 0/1] pwm-regulator: reduce noise on EPROBE_DEFER

pwm-regulator can be a bit noisy during boot.

One example are the 32-bit Amlogic SoCs when using
multi_v7_defconfig (which has CONFIG_REGULATOR_PWM=y and
CONFIG_PWM_MESON=m). During boot the pwm-regulator is probed multiple
times - the first five tries end with EPROBE_DEFER. This is expected
because the pwm-meson driver is only loaded once rootfs is ready (as
it's compiled as kernel module).

Other drivers typically suppress an error message for EPROBE_DEFER
cases, so let's do the same for the pwm-regulator driver.

Example boot log where multiple "pwm-regulator regulator-vcck: Failed
to get PWM: -517" messages are logged (Odroid-C1, v5.0-rc3): [0]


[0] https://storage.kernelci.org/mainline/master/v5.0-rc3/arm/multi_v7_defconfig/lab-baylibre/boot-meson8b-odroidc1.html


Martin Blumenstingl (1):
regulator: pwm: suppress EPROBE_DEFER error message

drivers/regulator/pwm-regulator.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

--
2.20.1



2019-01-21 18:39:56

by Martin Blumenstingl

[permalink] [raw]
Subject: [PATCH 1/1] regulator: pwm: suppress EPROBE_DEFER error message

Suppress the "Failed to get PWM" error output if the actual error code
is EPROBE_DEFER. This makes the behavior of the pwm-regulator driver
consistent with what most other drivers do (which is: print all errors
except EPROBE_DEFER).

An example where this cleans up the kernel log are the 32-bit Amlogic
Meson boards:
multi_v7_defconfig has CONFIG_REGULATOR_PWM=y and CONFIG_PWM_MESON=m.
When booting such a board (for example the Meson8b Odroid-C1) the
following message is printed five times during boot:
pwm-regulator regulator-vcck: Failed to get PWM: -517
On the sixth call the pwm-meson driver is finally loaded (as rootfs
becomes ready) and the "VCCK" pwm-regulator comes up fine.

Fixes: aa66cc6630a408 ("regulator: pwm-regulator: get voltage and duty table from dts")
Signed-off-by: Martin Blumenstingl <[email protected]>
---
drivers/regulator/pwm-regulator.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
index 3f53f9134b32..7789d181ae67 100644
--- a/drivers/regulator/pwm-regulator.c
+++ b/drivers/regulator/pwm-regulator.c
@@ -357,7 +357,9 @@ static int pwm_regulator_probe(struct platform_device *pdev)
drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
if (IS_ERR(drvdata->pwm)) {
ret = PTR_ERR(drvdata->pwm);
- dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
+ if (ret != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
+
return ret;
}

--
2.20.1


2019-01-21 19:20:36

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 0/1] pwm-regulator: reduce noise on EPROBE_DEFER

On Mon, Jan 21, 2019 at 07:37:22PM +0100, Martin Blumenstingl wrote:
> pwm-regulator can be a bit noisy during boot.
>
> One example are the 32-bit Amlogic SoCs when using
> multi_v7_defconfig (which has CONFIG_REGULATOR_PWM=y and

Please don't send cover letters for single patches, if there is anything
that needs saying put it in the changelog of the patch or after the ---
if it's administrative stuff. This reduces mail volume and ensures that
any important information is recorded in the changelog rather than being
lost.


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

2019-01-21 20:57:19

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH 0/1] pwm-regulator: reduce noise on EPROBE_DEFER

Hi Mark,

On Mon, Jan 21, 2019 at 8:18 PM Mark Brown <[email protected]> wrote:
>
> On Mon, Jan 21, 2019 at 07:37:22PM +0100, Martin Blumenstingl wrote:
> > pwm-regulator can be a bit noisy during boot.
> >
> > One example are the 32-bit Amlogic SoCs when using
> > multi_v7_defconfig (which has CONFIG_REGULATOR_PWM=y and
>
> Please don't send cover letters for single patches, if there is anything
> that needs saying put it in the changelog of the patch or after the ---
> if it's administrative stuff. This reduces mail volume and ensures that
> any important information is recorded in the changelog rather than being
> lost.
noted - sorry for the noise.

the patch description already contains all relevant information, so I
won't re-send the patch.
only the KernelCI link is not included because I'm not sure how long
it's valid (instead the description includes all relevant information
which also can be seen in the KernelCI link)


Regards
Martin

2019-01-23 15:29:43

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH 1/1] regulator: pwm: suppress EPROBE_DEFER error message

Hi Martin,

On Tue, 22 Jan 2019 at 00:07, Martin Blumenstingl
<[email protected]> wrote:
>
> Suppress the "Failed to get PWM" error output if the actual error code
> is EPROBE_DEFER. This makes the behavior of the pwm-regulator driver
> consistent with what most other drivers do (which is: print all errors
> except EPROBE_DEFER).
>
> An example where this cleans up the kernel log are the 32-bit Amlogic
> Meson boards:
> multi_v7_defconfig has CONFIG_REGULATOR_PWM=y and CONFIG_PWM_MESON=m.
> When booting such a board (for example the Meson8b Odroid-C1) the
> following message is printed five times during boot:
> pwm-regulator regulator-vcck: Failed to get PWM: -517
> On the sixth call the pwm-meson driver is finally loaded (as rootfs
> becomes ready) and the "VCCK" pwm-regulator comes up fine.
>
> Fixes: aa66cc6630a408 ("regulator: pwm-regulator: get voltage and duty table from dts")
> Signed-off-by: Martin Blumenstingl <[email protected]>
> ---
> drivers/regulator/pwm-regulator.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/regulator/pwm-regulator.c b/drivers/regulator/pwm-regulator.c
> index 3f53f9134b32..7789d181ae67 100644
> --- a/drivers/regulator/pwm-regulator.c
> +++ b/drivers/regulator/pwm-regulator.c
> @@ -357,7 +357,9 @@ static int pwm_regulator_probe(struct platform_device *pdev)
> drvdata->pwm = devm_pwm_get(&pdev->dev, NULL);
> if (IS_ERR(drvdata->pwm)) {
> ret = PTR_ERR(drvdata->pwm);
> - dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Failed to get PWM: %d\n", ret);
> +
> return ret;
> }
>
> --
> 2.20.1
>
Please add my. tested on Odroid c1+

Tested-by: Anand Moon <[email protected]>

Best Regards
-Anand

>
> _______________________________________________
> linux-amlogic mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-amlogic

2019-01-23 16:06:26

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/1] regulator: pwm: suppress EPROBE_DEFER error message

On Mon, Jan 21, 2019 at 07:37:23PM +0100, Martin Blumenstingl wrote:
> Suppress the "Failed to get PWM" error output if the actual error code
> is EPROBE_DEFER. This makes the behavior of the pwm-regulator driver
> consistent with what most other drivers do (which is: print all errors
> except EPROBE_DEFER).
>
> An example where this cleans up the kernel log are the 32-bit Amlogic
> Meson boards:
> multi_v7_defconfig has CONFIG_REGULATOR_PWM=y and CONFIG_PWM_MESON=m.

This also cleans up the kernel log in the case where you've not got a
driver enabled that you need (or it's not loading for some reason) which
isn't super helpful when you're trying to figure out why the driver
won't probe. There's not even anything at debug level, that would
probably be fine.

The ideal thing here would be to work on setting up the dependency
information based on DT and using that to try to sort initialization
order so we try things in an order that minimizes the number of failed
tries.


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

2019-01-23 16:17:39

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH 1/1] regulator: pwm: suppress EPROBE_DEFER error message



On 2019-01-23 16:04, Mark Brown wrote:
> On Mon, Jan 21, 2019 at 07:37:23PM +0100, Martin Blumenstingl wrote:
>> Suppress the "Failed to get PWM" error output if the actual error code
>> is EPROBE_DEFER. This makes the behavior of the pwm-regulator driver
>> consistent with what most other drivers do (which is: print all errors
>> except EPROBE_DEFER).
>>
>> An example where this cleans up the kernel log are the 32-bit Amlogic
>> Meson boards:
>> multi_v7_defconfig has CONFIG_REGULATOR_PWM=y and CONFIG_PWM_MESON=m.
>
> This also cleans up the kernel log in the case where you've not got a
> driver enabled that you need (or it's not loading for some reason)
> which
> isn't super helpful when you're trying to figure out why the driver
> won't probe. There's not even anything at debug level, that would
> probably be fine.
>
> The ideal thing here would be to work on setting up the dependency
> information based on DT and using that to try to sort initialization
> order so we try things in an order that minimizes the number of failed
> tries.

Hmm, would it be useful to have a devfs file which holds the last probe
code?

2019-01-23 17:31:21

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/1] regulator: pwm: suppress EPROBE_DEFER error message

On Wed, Jan 23, 2019 at 04:14:49PM +0000, Ben Dooks wrote:
> On 2019-01-23 16:04, Mark Brown wrote:

> > This also cleans up the kernel log in the case where you've not got a
> > driver enabled that you need (or it's not loading for some reason) which
> > isn't super helpful when you're trying to figure out why the driver
> > won't probe. There's not even anything at debug level, that would
> > probably be fine.

> > The ideal thing here would be to work on setting up the dependency
> > information based on DT and using that to try to sort initialization
> > order so we try things in an order that minimizes the number of failed
> > tries.

> Hmm, would it be useful to have a devfs file which holds the last probe
> code?

You really also want the bit that says why it's deferring - "failed to
get clock X" or whatever.


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