On 1/17/21 11:27 PM, Sean Anderson wrote:
On 1/17/21 2:26 AM, Heinrich Schuchardt wrote:
On 1/17/21 1:37 AM, Sean Anderson wrote:
On 1/4/21 2:02 AM, Heinrich Schuchardt wrote:
In drivers we use a family of printing functions including pr_err() and
pr_cont(). CONFIG_LOGLEVEL is used to control which of these lead to
output
via printf().
Our logging functions allow finer grained control of output. So replace
printf() by the matching logging functions. The usage of
CONFIG_LOGLEVEL
remains unchanged.
Signed-off-by: Heinrich Schuchardt <xypron.g...@gmx.de>
---
include/linux/bitops.h | 4 ++-
include/linux/printk.h | 82
+++++++++++++++++++++++-------------------
2 files changed, 48 insertions(+), 38 deletions(-)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 16f28993f5..d2e5ca026e 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -6,7 +6,6 @@
#include <asm/types.h>
#include <asm-generic/bitsperlong.h>
#include <linux/compiler.h>
-#include <linux/kernel.h>
#ifdef __KERNEL__
#define BIT(nr) (1UL << (nr))
@@ -19,6 +18,9 @@
#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE *
sizeof(long))
#endif
+/* kernel.h includes log.h which include bitops.h */
+#include <linux/kernel.h>
+
/*
* Create a contiguous bitmask starting at bit position @l and
ending at
* position @h. For example
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 088513ad29..5e85513853 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -1,6 +1,7 @@
#ifndef __KERNEL_PRINTK__
#define __KERNEL_PRINTK__
+#include <log.h>
#include <stdio.h>
#include <linux/compiler.h>
@@ -28,49 +29,56 @@
0; \
})
-#define __printk(level, fmt, ...) \
-({ \
- level < CONFIG_LOGLEVEL ? printk(fmt, ##__VA_ARGS__) : 0; \
Couldn't we just do
#define __printk(level, fmt, ...) log(LOG_CATEGORY, level, fmt,
##__VA_ARGS__)
Dear Sean,
Thanks for reviewing.
As of today log() does not print anything if CONFIG_LOG is not enabled.
#if CONFIG_IS_ENABLED(LOG)
#define _LOG_MAX_LEVEL CONFIG_VAL(LOG_MAX_LEVEL)
#define log_err(_fmt...) log(LOG_CATEGORY, LOGL_ERR, ##_fmt)
#define log_warning(_fmt...) log(LOG_CATEGORY, LOGL_WARNING, ##_fmt)
#define log_notice(_fmt...) log(LOG_CATEGORY, LOGL_NOTICE, ##_fmt)
#define log_info(_fmt...) log(LOG_CATEGORY, LOGL_INFO, ##_fmt)
#define log_debug(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG, ##_fmt)
#define log_content(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_CONTENT,
##_fmt)
#define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, ##_fmt)
#else
#define _LOG_MAX_LEVEL LOGL_INFO
#define log_err(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
#define log_warning(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
#define log_notice(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
#define log_info(_fmt, ...) printf(_fmt, ##__VA_ARGS__)
#define log_debug(_fmt, ...) debug(_fmt, ##__VA_ARGS__)
#define log_content(_fmt...) log_nop(LOG_CATEGORY, \
LOGL_DEBUG_CONTENT, ##_fmt)
#define log_io(_fmt...) log_nop(LOG_CATEGORY, LOGL_DEBUG_IO,
##_fmt)
#endif
If CONFIG_LOG is not enabled, then log statements are converted to
debug and printf.
Only the ones you cited, not the function log() referenced in your proposal.
A patch by Simon to change this behavior is pending. If it gets
merged, we can do the suggested simplification.
log: Convert log values to printf() if not enabled
https://patchwork.ozlabs.org/project/uboot/patch/20210114033051.483560-5-...@chromium.org/
-})
-
#ifndef pr_fmt
#define pr_fmt(fmt) fmt
#endif
-#define pr_emerg(fmt, ...) \
- __printk(0, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_alert(fmt, ...) \
- __printk(1, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_crit(fmt, ...) \
- __printk(2, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_err(fmt, ...) \
- __printk(3, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warning(fmt, ...) \
- __printk(4, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_warn pr_warning
-#define pr_notice(fmt, ...) \
- __printk(5, pr_fmt(fmt), ##__VA_ARGS__)
-#define pr_info(fmt, ...) \
- __printk(6, pr_fmt(fmt), ##__VA_ARGS__)
-
-#define pr_cont(fmt, ...) \
- printk(fmt, ##__VA_ARGS__)
-
-/* pr_devel() should produce zero code unless DEBUG is defined */
-#ifdef DEBUG
-#define pr_devel(fmt, ...) \
- __printk(7, pr_fmt(fmt), ##__VA_ARGS__)
-#else
-#define pr_devel(fmt, ...) \
- no_printk(pr_fmt(fmt), ##__VA_ARGS__)
-#endif
+#define pr_emerg(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 0 ? log_emerg(fmt, ##__VA_ARGS__) : 0; \
+})
There is also an off-by-one mismatch between the numbers here and the
log level constants. E.g. LOGL_INFO is 6, but pr_info only gets emitted
if CONFIG_LOGLEVEL is >= 7.
The Kconfig for CONFIG_LOGLEVEL description reads:
"All Messages with a loglevel *smaller than* the console loglevel will
be compiled in."
Ok then, perhaps that should be rectified.
The current definition allows to disable log level 0. So you better
don't change it.
Best regards
Heinrich
--Sean
I did not intend to change this definition.
Best regards
Heinrich
--Sean
+#define pr_alert(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 1 ? log_alert(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_crit(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 2 ? log_crit(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_err(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 3 ? log_err(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_warn(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 4 ? log_warning(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_notice(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 5 ? log_notice(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_info(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 6 ? log_info(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_debug(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 7 ? log_debug(fmt, ##__VA_ARGS__) : 0; \
+})
+#define pr_devel(fmt, ...) \
+({ \
+ CONFIG_LOGLEVEL > 7 ? log_debug(fmt, ##__VA_ARGS__) : 0; \
+})
-#ifdef DEBUG
-#define pr_debug(fmt, ...) \
- __printk(7, pr_fmt(fmt), ##__VA_ARGS__)
+#ifdef CONFIG_LOG
+#define pr_cont(fmt, ...) \
+({ \
+ gd->logl_prev < CONFIG_LOGLEVEL ? \
+ log_cont(fmt, ##__VA_ARGS__) : 0; \
+})
#else
-#define pr_debug(fmt, ...) \
- no_printk(pr_fmt(fmt), ##__VA_ARGS__)
+#define pr_cont(fmt, ...) \
+ printk(fmt, ##__VA_ARGS__)
#endif
#define printk_once(fmt, ...) \
--
2.29.2