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.schulz@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 ada@thorsis.com Fixes: 6afdb1585112 ("dm: core: migrate debug() messages to use dm_warn") Signed-off-by: Quentin Schulz quentin.schulz@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");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