Search Linux Wireless

Re: [PATCH 10/14] rtlwifi: Add BTC_TRACE_STRING to new btcoex

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux