Re: [PATCH] staging: rtl8723bs: cleanup macros within include/rtw_debug.h

[Phillip Potter]

> > So I'm in the process of stripping out _dbgdump entirely as per Greg
> > K-H's suggestion - am I to understand raw printk is frowned upon though,
> > even with the correct KERN_x level specified?
> 
> Yes.  Ideally in drivers everything would use dev_dbg() and dev_err() or
> whatever.  But it's perhaps tricky to convert everything in a single
> patch so changing _dbgdump() to "#define pr_debug" as an intermediate
> step is probably fine.
> 
> Look at how people do pr_fmt():
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> 
> You could do a patch that does a mass replacement of DBG_871X with
> pr_debug().  Again, I haven't really looked at this code so you'll have
> to double check and consider what is the best way to break up the
> patches.
> 

That sounds great, I'll take a look, thanks.

> > One query I have is that individual patches I'm working on for this file are
> > generating an awful lot of checkpatch warnings themselves due to the
> > nature of the existing violations on the relevant lines. Is it
> > considered acceptable for me to still submit these, providing I do so in
> > a series which cleans up the other violations in separate patches?
> 
> It's tricky to know how to break up patches.  Probably the simplest
> advice is to only clean up a single type of checkpatch warning at a
> time.  But fix all the instances of that warning in a file.  Don't
> change anything else even if it is tempting.  Do that in the next patch.
> 
> The actuall rules are slightly more complicated and nuanced than that,
> but if you just fix one type at a time then that's okay.
> 
> One thing is that your patches should not introduce new checkpatch
> warnings.  So if you have two statements in an if statement and you
> delete one, then that means you have to delete he curly braces as well.
> 
> regards,
> dan carpenter
> 

Thanks again for the feedback. I will work on something over the next
few days.

Regards,
Phil