On 12/07/16 14:28, Dave Gordon wrote: > On 12/07/16 10:06, Tvrtko Ursulin wrote: >> >> On 11/07/16 19:01, Dave Gordon wrote: >>> We had only DRM_INFO() and DRM_ERROR(), whereas the underlying printk() >>> provides several other useful intermediate levels such as NOTICE and >>> WARNING. So this patch fills out the set by providing both regular and >>> once-only macros for each of the levels INFO, NOTICE, and WARNING, using >>> a common underlying macro that does all the token-pasting. >>> >>> DRM_ERROR is unchanged, as it's not just a printk wrapper. >>> >>> Signed-off-by: Dave Gordon <david.s.gordon at intel.com> >>> --- >>> include/drm/drmP.h | 26 ++++++++++++++++++++------ >>> 1 file changed, 20 insertions(+), 6 deletions(-) >>> >>> diff --git a/include/drm/drmP.h b/include/drm/drmP.h >>> index cf918e3e..82648b1 100644 >>> --- a/include/drm/drmP.h >>> +++ b/include/drm/drmP.h >>> @@ -162,6 +162,26 @@ void drm_err(const char *format, ...); >>> /** \name Macros to make printk easier */ >>> /*@{*/ >>> >>> +#define _DRM_PRINTK(once, level, fmt, ...) \ >>> + do { \ >>> + printk##once(KERN_##level "[" DRM_NAME "] " fmt, \ >>> + ##__VA_ARGS__); \ >>> + } while (0) >>> + >>> +#define DRM_INFO(fmt, ...) \ >>> + _DRM_PRINTK(, INFO, fmt, ##__VA_ARGS__) >>> +#define DRM_NOTE(fmt, ...) \ >>> + _DRM_PRINTK(, NOTICE, fmt, ##__VA_ARGS__) >> >> To me DRM_NOTICE would be better to keep consistent with kernel naming >> for the equivalent log level. > > Maybe, but then we'd probably want DRM_WARNING() as well, and the names > get cumbersome, especially when you want to tag "_ONCE" on the end as > well. I liked the consistency of {INFO,NOTE,WARN} all being four letters ;) > > Any comments from dri-devel on INFO/NOTE/WARN vs INFO/NOTICE/WARNING? > Or any other suggestions?
Luckily kernel offers us precedent to avoid the DRM_WARNING verbosity and establish the only exception where log level symbolic name does not match the printk helper name. :) #define pr_emerg(fmt, ...) \ printk(KERN_EMERG pr_fmt(fmt), ##__VA_ARGS__) #define pr_alert(fmt, ...) \ printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__) #define pr_crit(fmt, ...) \ printk(KERN_CRIT pr_fmt(fmt), ##__VA_ARGS__) #define pr_err(fmt, ...) \ printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) #define pr_warning(fmt, ...) \ printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__) #define pr_warn pr_warning #define pr_notice(fmt, ...) \ printk(KERN_NOTICE pr_fmt(fmt), ##__VA_ARGS__) #define pr_info(fmt, ...) \ printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__) And short form is indeed more popular: $ grep pr_warn\( drivers/ -r | wc -l 1935 $ grep pr_warning\( drivers/ -r | wc -l 141 Regards, Tvrtko