From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Anderson Date: Sun, 17 Jan 2021 17:27:55 -0500 Subject: [PATCH 4/5] log: convert pr_*() to logging In-Reply-To: <8ff2f664-dbb0-04c7-a605-0839e0632ce1@gmx.de> References: <20210104070256.260002-1-xypron.glpk@gmx.de> <20210104070256.260002-5-xypron.glpk@gmx.de> <8ff2f664-dbb0-04c7-a605-0839e0632ce1@gmx.de> Message-ID: <9c739e51-29f5-d6d3-9ecf-1f918f41e844@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>> --- >>> 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 >>> #include >>> #include >>> -#include >>> >>> #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 >>> + >>> /* >>> * 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 >>> #include >>> #include >>> >>> @@ -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. > > 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-sjg at 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. --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 >>> >> >