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

[Dave Gordon <david.s.gordon@intel.com>]

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?

.Dave.

>> +#define DRM_WARN(fmt, ...)                        \
>> +    _DRM_PRINTK(, WARNING, fmt, ##__VA_ARGS__)
>> +
>> +#define DRM_INFO_ONCE(fmt, ...)                        \
>> +    _DRM_PRINTK(_once, INFO, fmt, __VA_ARGS__)
>> +#define DRM_NOTE_ONCE(fmt, ...)                    \
>> +    _DRM_PRINTK(_once, NOTICE, fmt, ##__VA_ARGS__)
>> +#define DRM_WARN_ONCE(fmt, ...)                        \
>> +    _DRM_PRINTK(_once, WARNING, fmt, ##__VA_ARGS__)
>> +
>>   /**
>>    * Error output.
>>    *
>> @@ -187,12 +207,6 @@ void drm_err(const char *format, ...);
>>           drm_err(fmt, ##__VA_ARGS__);                \
>>   })
>>
>> -#define DRM_INFO(fmt, ...)                \
>> -    printk(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
>> -
>> -#define DRM_INFO_ONCE(fmt, ...)                \
>> -    printk_once(KERN_INFO "[" DRM_NAME "] " fmt, ##__VA_ARGS__)
>> -
>>   /**
>>    * Debug output.
>>    *
>>
>
> Otherwise acked by me.
>
> Regards,
> Tvrtko

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx