Re: [Intel-gfx] [PATCH 1/3] drm: extra printk() wrapper macros

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel