Hi Alex,
On 10/17/24 4:56 PM, Alexander Dahl wrote:
Hello Quentin,
Am Tue, Oct 15, 2024 at 04:32:14PM +0200 schrieb Quentin Schulz:
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(-)
diff --git a/drivers/core/of_access.c b/drivers/core/of_access.c
index
d05be273e7bbb68c3ad82ef4c1c036ae7f68ae61..77acd76626257b6da95a27d107052ff8800c2b67
100644
--- a/drivers/core/of_access.c
+++ b/drivers/core/of_access.c
@@ -490,17 +490,17 @@ int of_read_u8(const struct device_node *np, const char
*propname, u8 *outp)
{
const u8 *val;
- dm_warn("%s: %s: ", __func__, propname);
+ log_debug("%s: %s: ", __func__, propname);
Printing __func__ when using log_* functions, is redundant, isn't it?
You can enabling printing the function name through the logging
framework, right?
Only if LOGF_FUNC symbol is enabled, if I understood correctly. Not an
excuse but:
$ git grep -o "log.*__func__" | wc -l
202
So there are a "few" other places doing that. Will let Simon decide on
that one, no personal opinion.
if (!np)
return -EINVAL;
val = of_find_property_value_of_size(np, propname, sizeof(*outp));
if (IS_ERR(val)) {
- dm_warn("(not found)\n");
+ log_debug("(not found)\n");
return PTR_ERR(val);
What about using log_msg_ret() instead in these cases?
log_msg_ret will log with LOGL_ERR and not LOGL_DEBUG if
LOG_ERROR_RETURN symbol is enabled, otherwise it'll simply not be
printed. It's a change of behavior/expectation here.
If we go this route we should at least make this a bit more useful by
adding the propname to the error message since it would be printed with
log_debug() at the beginning of the function, and the log level wouldn't
match. Also, this means that enabling debug log level but not enabling
LOG_ERROR_RETURN would basically print the first log_debug() and nothing
else in case it fails. A choice to be made but it's a bit more complex
than the one above.
Cheers,
Quentin
