[PATCH 4/5] log: convert pr_*() to logging

[Heinrich Schuchardt]

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.

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."

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
>>
>