All of lore.kernel.org
 help / color / mirror / Atom feed
From: Phillip Potter <phil@philpotter.co.uk>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: gregkh@linuxfoundation.org, Larry.Finger@lwfinger.net,
	straube.linux@gmail.com, martin@kaiser.cx,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/7] staging: r8188eu: convert DBG_88E calls in core/rtw_iol.c
Date: Mon, 10 Jan 2022 20:56:02 +0000	[thread overview]
Message-ID: <YdydYlpVzHoLy2Wh@equinox> (raw)
In-Reply-To: <20220110100843.GC1951@kadam>

On Mon, Jan 10, 2022 at 01:08:43PM +0300, Dan Carpenter wrote:
> On Sun, Jan 09, 2022 at 09:54:23PM +0000, Phillip Potter wrote:
> > Convert the DBG_88E macro calls in core/rtw_iol.c to use pr_debug
> > or netdev_dbg appropriately, as their information may be useful to
> > observers, and this gets the driver closer to the point of being
> > able to remove DBG_88E itself.
> > 
> > Some calls are at points in the call chain where use of dev_dbg or
> > netdev_dbg isn't possible due to lack of device pointer, so plain
> > pr_debug is appropriate here.
> > 
> > Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
> > ---
> >  drivers/staging/r8188eu/core/rtw_iol.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/r8188eu/core/rtw_iol.c b/drivers/staging/r8188eu/core/rtw_iol.c
> > index 7e78b47c1284..923da2a9f6ae 100644
> > --- a/drivers/staging/r8188eu/core/rtw_iol.c
> > +++ b/drivers/staging/r8188eu/core/rtw_iol.c
> > @@ -12,13 +12,15 @@ struct xmit_frame	*rtw_IOL_accquire_xmit_frame(struct adapter  *adapter)
> >  
> >  	xmit_frame = rtw_alloc_xmitframe(pxmitpriv);
> >  	if (!xmit_frame) {
> > -		DBG_88E("%s rtw_alloc_xmitframe return null\n", __func__);
> > +		netdev_dbg(adapter->pnetdev,
> > +			   "rtw_alloc_xmitframe return null\n");
> 
> You're going to have to send this anyway because of the compile issue.
> 
> I feel like you are not being aggressive enough in the debug messages
> that you delete.  For example, this one should definitely be deleted.
> Don't print an error message for alloc failures.
> 
> It would be easier to Ack a mass delete of these messages.
> 
> regards,
> dan carpenter
> 

Dear Dan,

Thank you for your feedback. I already sent a V2 series to fix the empty case
label I left in core/rtw_mlme_ext.c, sounds like a V3 is needed though 
based on this feedback - admittedly I have tried to be conservative and
basically only removed commented DBG_88E calls or calls which just print the
function name/line number so far.

I get what you're saying about deleting them all just being easier,
but I've already converted several in previous series that have
made it in. It would make sense to delete these converted calls as well
if going for the total deletion approach. Also, I do worry some of the
info could be useful. I'd appreciate your thoughts on this.

I am happy to delete it all by all means, just want to make sure majority
would be happy with that approach, as opposed to a refinement of this
approach and being more judicious with deletion of more DBG_88E calls.

Thanks again,
Phil

  reply	other threads:[~2022-01-10 20:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-09 21:54 [PATCH 0/7] staging: r8188eu: further per-file DBG_88E cleanups Phillip Potter
2022-01-09 21:54 ` [PATCH 1/7] staging: r8188eu: convert DBG_88E calls in core/rtw_ieee80211.c Phillip Potter
2022-01-09 21:54 ` [PATCH 2/7] staging: r8188eu: convert DBG_88E calls in core/rtw_xmit.c Phillip Potter
2022-01-09 21:54 ` [PATCH 3/7] staging: r8188eu: convert DBG_88E calls in core/rtw_iol.c Phillip Potter
2022-01-10 10:08   ` Dan Carpenter
2022-01-10 20:56     ` Phillip Potter [this message]
2022-01-11  5:06       ` Dan Carpenter
2022-01-14  0:42         ` Phillip Potter
2022-01-09 21:54 ` [PATCH 4/7] staging: r8188eu: convert DBG_88E calls in core/rtw_mlme.c Phillip Potter
2022-01-09 21:54 ` [PATCH 5/7] staging: r8188eu: convert DBG_88E calls in core/rtw_ioctl_set.c Phillip Potter
2022-01-09 21:54 ` [PATCH 6/7] staging: r8188eu: convert DBG_88E calls in core/rtw_mlme_ext.c Phillip Potter
2022-01-10  1:35   ` kernel test robot
2022-01-10  1:35     ` kernel test robot
2022-01-10  6:15   ` kernel test robot
2022-01-10  6:15     ` kernel test robot
2022-01-09 21:54 ` [PATCH 7/7] staging: r8188eu: convert DBG_88E calls in core/rtw_sta_mgt.c Phillip Potter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YdydYlpVzHoLy2Wh@equinox \
    --to=phil@philpotter.co.uk \
    --cc=Larry.Finger@lwfinger.net \
    --cc=dan.carpenter@oracle.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=martin@kaiser.cx \
    --cc=straube.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.