Hi Simon,
On 10/27/24 6:16 PM, Simon Glass wrote:
Hi Quentin,
On Tue, 15 Oct 2024 at 16:32, Quentin Schulz <foss+ub...@0leil.net> wrote:
From: Quentin Schulz <quentin.sch...@cherry.de>
People complained that enabling (SPL_)DM_WARN was now totally unusable
due to the amount of messages printed on the console.
Let's downgrade the log level of some messages that are clearly not on
the error path.
Note that there's one pr_debug in there, because it is followed by
pr_cont so it made sense to reuse the same family of functions.
Reported-by: Alexander Dahl <a...@thorsis.com>
Fixes: 6afdb1585112 ("dm: core: migrate debug() messages to use dm_warn")
Signed-off-by: Quentin Schulz <quentin.sch...@cherry.de>
---
Note that I am not entirely sure about the "not found" and "not large
enough" changes there.
Another note, %#x isn't handled by tinyprintf so it just prints "x"
instead of the value.
Finally, I don't know how one can enable LOG_DEBUG level without
enabling DEBUG which enables assert() so I just tested that by removing
the #define DEBUG in include/log.h :)
---
drivers/core/of_access.c | 36 ++++++++++++-------------
drivers/core/of_addr.c | 26 +++++++++---------
drivers/core/of_extra.c | 6 ++---
drivers/core/ofnode.c | 68 ++++++++++++++++++++++++------------------------
4 files changed, 68 insertions(+), 68 deletions(-)
This is an improvement, so:
Reviewed-by: Simon Glass <s...@chromium.org>
For -EOVERFLOW I think warning would be better, but in that case you
need to print a prefix, since otherwise the warning will have nothing
before it. So it is a little tricky. I can imagine some support in the
log module to help with this, although I haven't thought about it too
hard.
Agreed. We could also simply print the propname in the dm_warn() call
and keep the one at the top of the function to use dm_debug() instead.
Also, we should really remove __func__ when adding logging, since
people can enable the option if they want function names to be stored.
But then we'll have a bunch of functions printing one of their
parameters and "(not found)" for example. A bit odd to me. But on the
theoretical level, I agree, enabling the appropriate symbol would
already add that line, it's just that I see limit use of those messages
if it's not set.
Thanks for the review!
Cheers,
Quentin
