[PATCH 4/5] log: convert pr_*() to logging
Sean Anderson
seanga2 at gmail.com
Sun Jan 17 23:27:55 CET 2021
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.glpk at 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.
>
> 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@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
>>>
>>
>
More information about the U-Boot
mailing list