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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

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

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