Hi Sean, On Thu, 4 Feb 2021 at 18:28, Sean Anderson <sean...@gmail.com> wrote: > > On 1/20/21 10:10 PM, Simon Glass wrote: > > At present if logging not enabled, log_info() becomes a nop. But we want > > log output at the 'info' level to be akin to printf(). Update the macro to > > pass the output straight to printf() in this case. > > > > This mimics the behaviour for the log_...() macros like log_debug() and > > log_info(), so we can drop the special case for these. > > > > Add new tests to cover this case. > > Signed-off-by: Simon Glass <s...@chromium.org> > > --- > > > > Changes in v2: > > - Update commit message and cover letter to mention log_...() macros > > - Add a test for !CONFIG_LOG > > - Update log() to (effectively) call debug() for log_level == LOGL_DEBUG > > > > doc/develop/logging.rst | 6 ++++-- > > include/log.h | 33 ++++++++++++++------------------- > > test/log/Makefile | 1 + > > test/log/nolog_ndebug.c | 38 ++++++++++++++++++++++++++++++++++++++ > > test/log/nolog_test.c | 3 +++ > > 5 files changed, 60 insertions(+), 21 deletions(-) > > create mode 100644 test/log/nolog_ndebug.c > > > > diff --git a/doc/develop/logging.rst b/doc/develop/logging.rst > > index b6c6b45049f..55cef42664d 100644 > > --- a/doc/develop/logging.rst > > +++ b/doc/develop/logging.rst > > @@ -54,6 +54,10 @@ If CONFIG_LOG is not set, then no logging will be > > available. > > The above have SPL and TPL versions also, e.g. CONFIG_SPL_LOG_MAX_LEVEL > > and > > CONFIG_TPL_LOG_MAX_LEVEL. > > > > +If logging is disabled, the default behaviour is to output any message at > > +level LOGL_INFO and below. If logging is disabled and DEBUG is defined (at > > +the very top of a C file) then any message at LOGL_DEBUG will be written. > > + > > Temporary logging within a single file > > -------------------------------------- > > > > @@ -293,8 +297,6 @@ More logging destinations: > > > > Convert debug() statements in the code to log() statements > > > > -Support making printf() emit log statements at L_INFO level > > - > > Convert error() statements in the code to log() statements > > > > Figure out what to do with BUG(), BUG_ON() and warn_non_spl() > > diff --git a/include/log.h b/include/log.h > > index 6ef891d4d2d..748e34d5a26 100644 > > --- a/include/log.h > > +++ b/include/log.h > > @@ -156,6 +156,10 @@ static inline int _log_nop(enum log_category_t cat, > > enum log_level_t level, > > */ > > #if CONFIG_IS_ENABLED(LOG) > > #define _LOG_MAX_LEVEL CONFIG_VAL(LOG_MAX_LEVEL) > > +#else > > +#define _LOG_MAX_LEVEL LOGL_INFO > > +#endif > > + > > #define log_emer(_fmt...) log(LOG_CATEGORY, LOGL_EMERG, ##_fmt) > > #define log_alert(_fmt...) log(LOG_CATEGORY, LOGL_ALERT, ##_fmt) > > #define log_crit(_fmt...) log(LOG_CATEGORY, LOGL_CRIT, ##_fmt) > > @@ -167,41 +171,32 @@ static inline int _log_nop(enum log_category_t cat, > > enum log_level_t level, > > #define log_content(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_CONTENT, > > ##_fmt) > > #define log_io(_fmt...) log(LOG_CATEGORY, LOGL_DEBUG_IO, > > ##_fmt) > > #define log_cont(_fmt...) log(LOGC_CONT, LOGL_CONT, ##_fmt) > > -#else > > -#define _LOG_MAX_LEVEL LOGL_INFO > > -#define log_emerg(_fmt, ...) printf(_fmt, ##__VA_ARGS__) > > -#define log_alert(_fmt, ...) printf(_fmt, ##__VA_ARGS__) > > -#define log_crit(_fmt, ...) printf(_fmt, ##__VA_ARGS__) > > -#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_cont(_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_IS_ENABLED(LOG) > > #ifdef LOG_DEBUG > > #define _LOG_DEBUG LOGL_FORCE_DEBUG > > #else > > #define _LOG_DEBUG 0 > > #endif > > > > +#if CONFIG_IS_ENABLED(LOG) > > + > > /* Emit a log record if the level is less that the maximum */ > > #define log(_cat, _level, _fmt, _args...) ({ \ > > int _l = _level; \ > > - if (CONFIG_IS_ENABLED(LOG) && \ > > - (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL)) \ > > + if (_LOG_DEBUG != 0 || _l <= _LOG_MAX_LEVEL) \ > > _log((enum log_category_t)(_cat), \ > > (enum log_level_t)(_l | _LOG_DEBUG), __FILE__, \ > > __LINE__, __func__, \ > > pr_fmt(_fmt), ##_args); \ > > }) > > #else > > -#define log(_cat, _level, _fmt, _args...) > > +/* Note: _LOG_DEBUG != 0 avoids a warning with clang */ > > +#define log(_cat, _level, _fmt, _args...) ({ \ > > + int _l = _level; \ > > + if (_LOG_DEBUG != 0 || _l <= LOGL_INFO || \ > > Should this be < CONFIG_LOGLEVEL?
Well it could be, but in fact that value predates the log system and I've been wondering how to remove it, to reconcile the pr_...() stuff and the log subsystem. CONFIG_LOGLEVEL should really go away in favour of CONFIG_LOG_MAX_LEVEL I think. Regards, Simon