Description of regulators should generally be optional so if there is no DT node for the regulators container then we shouldn't print an error message. Lower the severity of the message to debug level (it might help someone work out what went wrong) and while we're at it say what we were looking for.
Reported-by: Guenter Roeck linux@roeck-us.net Signed-off-by: Mark Brown broonie@kernel.org --- drivers/regulator/of_regulator.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 7a51814abdc5..5a1d4afa4776 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -211,7 +211,8 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev, search = dev->of_node;
if (!search) { - dev_err(dev, "Failed to find regulator container node\n"); + dev_dbg(dev, "Failed to find regulator container node '%s'\n", + desc->regulators_node); return NULL; }
On Wed, Oct 08, 2014 at 11:09:59PM +0100, Mark Brown wrote:
Description of regulators should generally be optional so if there is no DT node for the regulators container then we shouldn't print an error message. Lower the severity of the message to debug level (it might help someone work out what went wrong) and while we're at it say what we were looking for.
Reported-by: Guenter Roeck linux@roeck-us.net Signed-off-by: Mark Brown broonie@kernel.org
Excellent.
Reviewed-by: Guenter Roeck linux@roeck-us.net
Would it also be possible to lower the severity of the "no parameters" message ?
Thanks, Guenter
drivers/regulator/of_regulator.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 7a51814abdc5..5a1d4afa4776 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -211,7 +211,8 @@ struct regulator_init_data *regulator_of_get_init_data(struct device *dev, search = dev->of_node; if (!search) {
dev_err(dev, "Failed to find regulator container node\n");
dev_dbg(dev, "Failed to find regulator container node '%s'\n",desc->regulators_node);2.1.1
On Wed, Oct 08, 2014 at 11:36:01PM +0100, Mark Brown wrote:
On Wed, Oct 08, 2014 at 03:34:03PM -0700, Guenter Roeck wrote:
Would it also be possible to lower the severity of the "no parameters" message ?
Could you be more specific please?
There is a log message "no parameters" for each regulator. This is printed unconditionally from print_constraints().
Looking through the code again, looks like this is on purpose. It is just a bit annoying to get lots of those messages. One of the systems I am dealing with has 17 LTC2978 chips in it, with 8 channels each. That results in 136 times "no parameters" in the boot log. And that is not even a fully populated system; if fully populated, there can be more than 60 of those chips. 500+ lines of similar log messages is really a bit on the high side.
It might help if there was a way to silence the messages, ie to make "print_constraints" optional.
Thanks, Guenter
On Wed, Oct 08, 2014 at 03:59:12PM -0700, Guenter Roeck wrote:
There is a log message "no parameters" for each regulator. This is printed unconditionally from print_constraints().
Looking through the code again, looks like this is on purpose. It is just a bit annoying to get lots of those messages. One of the systems I am dealing with has 17 LTC2978 chips in it, with 8 channels each. That results in 136 times "no parameters" in the boot log. And that is not even a fully populated system; if fully populated, there can be more than 60 of those chips. 500+ lines of similar log messages is really a bit on the high side.
It might help if there was a way to silence the messages, ie to make "print_constraints" optional.
Ah, from the constraints rather than from the DT parsing. I do like having it there since it's enormously helpful in debugging and that is a... specialist number of regulators you have in your system. We can definitely at least add a boot argument or something to suppress them, let me have a think if we want to do that by default.
On Thu, Oct 09, 2014 at 12:45:41AM +0100, Mark Brown wrote:
On Wed, Oct 08, 2014 at 03:59:12PM -0700, Guenter Roeck wrote:
There is a log message "no parameters" for each regulator. This is printed unconditionally from print_constraints().
Looking through the code again, looks like this is on purpose. It is just a bit annoying to get lots of those messages. One of the systems I am dealing with has 17 LTC2978 chips in it, with 8 channels each. That results in 136 times "no parameters" in the boot log. And that is not even a fully populated system; if fully populated, there can be more than 60 of those chips. 500+ lines of similar log messages is really a bit on the high side.
It might help if there was a way to silence the messages, ie to make "print_constraints" optional.
Ah, from the constraints rather than from the DT parsing. I do like having it there since it's enormously helpful in debugging and that is a... specialist number of regulators you have in your system. We can
Yes, this is a pretty large backbone switch. Kind of amazing how many sensors are in those systems.
definitely at least add a boot argument or something to suppress them, let me have a think if we want to do that by default.
It is a nuisance, so I might just disable it in our tree if we don't find some other solution.
Did you notice the problem with debugfs I had mentioned earlier ? With all those regulators, not all of them being used, I end up with many having the same name. This causes issues with debugfs, which is trying to create the same file several times.
Any idea how we could solve this ? The constraints message is annoying, but this one is a real issue.
Thanks, Guenter
On Wed, Oct 08, 2014 at 05:05:55PM -0700, Guenter Roeck wrote:
On Thu, Oct 09, 2014 at 12:45:41AM +0100, Mark Brown wrote:
definitely at least add a boot argument or something to suppress them, let me have a think if we want to do that by default.
It is a nuisance, so I might just disable it in our tree if we don't find some other solution.
We'll do something, just a question of what and what the default is.
Did you notice the problem with debugfs I had mentioned earlier ? With all those regulators, not all of them being used, I end up with many having the same name. This causes issues with debugfs, which is trying to create the same file several times.
Any idea how we could solve this ? The constraints message is annoying, but this one is a real issue.
Shove a dev_name() on the front if we get a collision? I have to say I've never cared, the debugfs isn't that important so it doesn't matter too much if it fails.
On Thu, Oct 09, 2014 at 01:12:13AM +0100, Mark Brown wrote:
On Wed, Oct 08, 2014 at 05:05:55PM -0700, Guenter Roeck wrote:
On Thu, Oct 09, 2014 at 12:45:41AM +0100, Mark Brown wrote:
definitely at least add a boot argument or something to suppress them, let me have a think if we want to do that by default.
It is a nuisance, so I might just disable it in our tree if we don't find some other solution.
We'll do something, just a question of what and what the default is.
Ok. Note that a boot parameter would not work well for our use case, so it would be great if we can find something else.
Did you notice the problem with debugfs I had mentioned earlier ? With all those regulators, not all of them being used, I end up with many having the same name. This causes issues with debugfs, which is trying to create the same file several times.
Any idea how we could solve this ? The constraints message is annoying, but this one is a real issue.
Shove a dev_name() on the front if we get a collision? I have to say I've never cared, the debugfs isn't that important so it doesn't matter too much if it fails.
Sure, but, again, I am getting lots and lots of those error messages. I probably would not care either (and probably not even have noticed) if not for those messages.
Want me to submit a patch with the dev_name solution ?
Thanks, Guenter
On Wed, Oct 08, 2014 at 05:25:31PM -0700, Guenter Roeck wrote:
On Thu, Oct 09, 2014 at 01:12:13AM +0100, Mark Brown wrote:
We'll do something, just a question of what and what the default is.
Ok. Note that a boot parameter would not work well for our use case, so it would be great if we can find something else.
Could you explain why please?
Shove a dev_name() on the front if we get a collision? I have to say I've never cared, the debugfs isn't that important so it doesn't matter too much if it fails.
Sure, but, again, I am getting lots and lots of those error messages. I probably would not care either (and probably not even have noticed) if not for those messages.
Want me to submit a patch with the dev_name solution ?
Yes, please.
On Thu, Oct 09, 2014 at 04:54:40PM +0100, Mark Brown wrote:
On Wed, Oct 08, 2014 at 05:25:31PM -0700, Guenter Roeck wrote:
On Thu, Oct 09, 2014 at 01:12:13AM +0100, Mark Brown wrote:
We'll do something, just a question of what and what the default is.
Ok. Note that a boot parameter would not work well for our use case, so it would be great if we can find something else.
Could you explain why please?
Some of the system are loaded from u-boot. We can technically change the environment, but that would not be persistent. Product requirement is that the default (hard-coded) environment has to be the one that is used. And changing u-boot in those systems is more difficult than getting an audience with the Pope - believe me, we went through that. Unless there is a fatal problem, it simply won't be approved.
On x86 systems, which are booted through grub, we have a similar problem. The boot menu is secured and for all practical purposes untouchable.
All that makes it much simpler to carry a one-line patch to remove the output from the log. I may try to do without it and keep the message, but I am quite sure that someone will complain and we'll have to do it.
Shove a dev_name() on the front if we get a collision? I have to say I've never cared, the debugfs isn't that important so it doesn't matter too much if it fails.
Sure, but, again, I am getting lots and lots of those error messages. I probably would not care either (and probably not even have noticed) if not for those messages.
Want me to submit a patch with the dev_name solution ?
Yes, please.
Ok, will do.
Thanks, Guenter
On Thu, Oct 09, 2014 at 04:54:40PM +0100, Mark Brown wrote:
On Wed, Oct 08, 2014 at 05:25:31PM -0700, Guenter Roeck wrote:
Shove a dev_name() on the front if we get a collision? I have to say I've never cared, the debugfs isn't that important so it doesn't matter too much if it fails.
Sure, but, again, I am getting lots and lots of those error messages. I probably would not care either (and probably not even have noticed) if not for those messages.
Want me to submit a patch with the dev_name solution ?
Yes, please.
Turns out that is already supposed to happen if the function creating the entry knows about the device. I guess this explains why others don't see the problem. We'll have to find out what is going on.
Guenter
linaro-kernel@lists.linaro.org
-
Guenter Roeck -
Mark Brown