On Mon, Dec 05, 2016 at 04:34:08PM -0600, Larry Finger wrote:
> On 12/05/2016 03:34 PM, Dan Carpenter wrote:
> > On Thu, Dec 01, 2016 at 07:48:29PM -0600, Larry Finger wrote:
> > > --- 
> > > wireless-drivers-next.orig/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > > +++ 
> > > wireless-drivers-next/drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.h
> > > @@ -27,6 +27,29 @@
> > > 
> > >  #include "../wifi.h"
> > > 
> > > +#ifdef CONFIG_RTLWIFI_DEBUG
> > > +
> > > +#define BTC_SPRINTF(ptr, ...)    snprintf(ptr, ##__VA_ARGS__)
> > > +#define BTC_TRACE(fmt)                                           \
> > > +do {                                                             \
> > > + struct rtl_priv *rtlpriv = gl_bt_coexist.adapter;       \
> > > + if (!rtlpriv)                                   \
> > > +         break;                                          \
> > > + RT_TRACE_STRING(rtlpriv, COMP_COEX, DBG_LOUD, fmt);     \
> > > +} while (0)
> > > +
> > 
> > This sort of macro is exactly when the rtl drivers spent so long in
> > staging...  Subsystems should not invent their own tracing but in
> > particular these macros are so very very ugly.
> > 
> > It's just super frustrating to even look at this...
> > 
> > There are a lot of staging drivers I feel good about when they leave.
> > The HyperV drivers.  The IIO stuff.  A lot of the media stuff and
> > generally the media tree is getting better and better.  I like comedi
> > and unisys, those are in staging, but they are great and could move out
> > any time as far as I'm concerned.
> > 
> > But this patch just makes me super discouraged.  What are we doing???
> 
> Dan,
> 
> It would not matter to me if these drivers got moved to staging, but there
> are a lot of users whose distros do not build staging drivers that would be
> very unhappy.
> 
> Can you point me to a driver with a better way to conditionally dump a
> debugging string to the logs?

Just use 'dev_dbg()', or 'pr_debug()' if you don't have a device pointer
(hint, all drivers should have that pointer).  That can be turned on or
off by a user dynamically as the kernel runs.  No need to invent fancy
custom macros for things we have already for many many years.

thanks,

greg k-h
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to