Re: [PATCH 3/5] staging: r8188eu: (trivial) remove a duplicate debug print

[Fabio M. De Francesco]

On Saturday, August 14, 2021 6:54:40 PM CEST Phillip Potter wrote:
> On Fri, 13 Aug 2021 at 13:42, Fabio M. De Francesco
> 
> <fmdefrancesco@xxxxxxxxx> wrote:
> > On Friday, August 13, 2021 12:05:36 PM CEST Martin Kaiser wrote:
> > > Hi Dan and Phil,
> > > [...] 
> > > > > Just my personal opinion, but I'd be inclined to strip out all 
DBG_88E
> > > > > calls totally. If there are necessary functions being called such as
> > > > > device_may_wakeup() we can always just keep this part and remove the
> > > > > macro call (not checked this function out myself yet). Thanks.
> > > 
> > > I'd agree with you, Phil. Most DBG_88E prints don't say anything useful.
> > > 
> > > This comment from Greg made me drop the DBG_88E removal for now
> > > 
> > > https://lore.kernel.org/linux-staging/20210803201511.29000-1-martin@xxxxxxxxx/T/#m05
> > > d82a 0ca8ed36180ebdc987114b4d892445c52d
> > 
> > Hi Martin,
> > 
> > I think you misunderstood what Greg was trying to convey with the above-
> > mentioned message.
> > 
> > Well, he doesn't like to feed developers with little spoons :-)
> > 
> > I'm pretty sure that, by "Why not use the proper debugging calls instead 
of
> > just deleting them?", he meant you should research, understand, and use 
the
> > proper APIs for printing debug messages.
> > 
> > Please check out pr_debug(), dev_dbg(), netdev_dbg(). Use them 
appropriately,
> > according to the subsystem you're working in and to the different types of
> > arguments they take.
> > 
> > Thanks,
> > 
> > Fabio
> > 
> > > A compromise would be to remove only those DBG_88E prints which are
> > > really not helpful.
> > > 
> > > Best regards,
> > > Martin
> 
> The problem I see is that this driver is so littered with unnecessary
> macro calls, how do we decide which ones to keep? In my mind, the
> better option is to remove them all and then come up with some new
> ones in the vein of netdev_dbg() and friends. I could be wrong of
> course :-) I tried going down the route of keeping/converting some to
> proper calls such as netdev_dbg() and the issue is a lot of the calls
> don't have an obvious value anyway.
> 
> Regards,
> Phil

I think that you'd better remove only the ones that "have no obvious value" 
and convert the others to using netdev_dbg(). Obviously, telling which have no 
value is at the discretion of whoever wants to carry on this work.

Regards,

Fabio