[PATCH 4/5] log: convert pr_*() to logging
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon Jan 18 00:13:55 CET 2021
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.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.
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-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.
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
>>>>
>>>
>>
>
More information about the U-Boot
mailing list