DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5900720827
Message-ID: <de73f12cdfbd52995c44b74df0d94142493de4e9.camel@perches.com>
Subject: Re: [RFC v5 08/13] rtw88: debug files
From:   Joe Perches <joe@perches.com>
To:     Tony Chuang <yhchuang@realtek.com>,
        "kvalo@codeaurora.org" <kvalo@codeaurora.org>,
        "sgruszka@redhat.com" <sgruszka@redhat.com>
Cc:     "Larry.Finger@lwfinger.net" <Larry.Finger@lwfinger.net>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        Pkshih <pkshih@realtek.com>, Andy Huang <tehuang@realtek.com>,
        "johannes@sipsolutions.net" <johannes@sipsolutions.net>
Date:   Sun, 04 Nov 2018 23:11:54 -0800
In-Reply-To: <F7CD281DE3E379468C6D07993EA72F84D173E96A@RTITMBSVM01.realtek.com.tw>
References: <1540980747-9297-1-git-send-email-yhchuang@realtek.com>
         <1540980747-9297-9-git-send-email-yhchuang@realtek.com>
         <d6f81b74ede58f9a7cd3ce01cd8366d5df50d1de.camel@perches.com>
         <F7CD281DE3E379468C6D07993EA72F84D173E96A@RTITMBSVM01.realtek.com.tw>
Content-Type: text/plain; charset="ISO-8859-1"
User-Agent: Evolution 3.30.1-1build1 
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On Mon, 2018-11-05 at 06:13 +0000, Tony Chuang wrote:
> > -----Original Message-----
> > From: Joe Perches [mailto:joe@perches.com]
> > Sent: Sunday, November 04, 2018 12:29 PM
> > To: Tony Chuang; kvalo@codeaurora.org; sgruszka@redhat.com
> > Cc: Larry.Finger@lwfinger.net; linux-wireless@vger.kernel.org; Pkshih; Andy
> > Huang; johannes@sipsolutions.net
> > Subject: Re: [RFC v5 08/13] rtw88: debug files
> > 
> > On Wed, 2018-10-31 at 18:12 +0800, yhchuang@realtek.com wrote
> > > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> > > debug files for Realtek 802.11ac wireless network chips
> > []
> > > diff --git a/drivers/net/wireless/realtek/rtw88/debug.h
> > b/drivers/net/wireless/realtek/rtw88/debug.h
> > []
> > > +#ifdef CONFIG_RTW88_DEBUG
> > []
> > > +#else
> > > +
> > > +static inline void rtw_dbg(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> > > +static inline void rtw_info(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> > > +static inline void rtw_warn(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> > > +static inline void rtw_err(struct rtw_dev *rtwdev, const char *fmt, ...) {}
> > 
> > I still think it's _very, very bad_, and extremely
> > unusual to turn off _all_ logging, even for error
> > conditions, based on a debug config flag.
> > 
[]
> So I think we should move rtw_[err/warn/info] out of the flag.
> Should we need to send it in RFC v6 or in the next patch series?

Up to you.

I think you should look at all the rtw_err/rtw_warn/rtw_info uses
as many seem to be more like they should use rtw_dbg instead.

Also, I think the rtw_err/rtw_warn/rtw_info function calls aren't
particularly useful and these should be simple macros or static
inlines and not use %pV at all.

#define rtw_err(rtw_dev, fmt, ...)	\
	dev_err((rtw_dev)->dev, fmt, ##__VA_ARGS__)

etc