Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Joe Perches]

On Mon, 2020-07-13 at 00:24 +0530, Suraj Upadhyay wrote:
> On Sat, Jul 11, 2020 at 11:16:33AM -0700, Joe Perches wrote:
[]
> > Perhaps change the __drm_printk macro to not
> > dereference the drm argument when NULL.
> > 
> > A trivial but perhaps inefficient way might be
> > used like:
> > 
> > 	drm_<level>(NULL, fmt, ...)
[]
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
> > @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> >  
> >  /* Helper for struct drm_device based logging. */
> >  #define __drm_printk(drm, level, type, fmt, ...)			\
> > -	dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
> > -
> > +	dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt,	\
> > +			  ##__VA_ARGS__)
> >  
> >  #define drm_info(drm, fmt, ...)					\
> >  	__drm_printk((drm), info,, fmt, ##__VA_ARGS__)
> > 
> 
> Hi Joe,
> 	Thanks for your input.
> But I don't think that this change would be a good idea as we are
> supposed to find or make a substitute of WARN_* macros which
> take a `condition` as an argument and check for its truth.
> And I guess passing a NULL to dev_<level> would cause a format warning.
> 
> Also, the WARN_* macros are doing their job fine, and passing a NULL
> value everytime you want to warn about a certain condition at a
> particular line, doesn't seem good to me.
> 
> Thus, I think that WARN_* macros should be untouched.

So do I but the suggestion was not about WARN macros
only about drm_<level> macros and possibly unnecessary
conversions to dev_<level> when a drm_device context
is unavailable.

Also, you don't have to guess, the code is there for
you to inspect.

dev_<level> when a NULL is used as the first argument
emits "(NULL device *)" instead of dev_driver_string(dev)
and dev_name(dev).

See: drivers/base/core.c::__dev_printk()