On Mon, Dec 05, 2016 at 04:34:08PM -0600, Larry Finger wrote:

On 12/05/2016 03:34 PM, Dan Carpenter wrote:

quoted

On Thu, Dec 01, 2016 at 07:48:29PM -0600, Larry Finger wrote:

quoted

--- 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