On Sun, Mar 28, 2021 at 04:43:24PM +0200, Fabio Aiuto wrote: > On Sun, Mar 28, 2021 at 02:08:41PM +0200, Greg KH wrote: > > On Sat, Mar 27, 2021 at 03:24:08PM +0100, Fabio Aiuto wrote: > > > fix the following checkpatch warning: > > > > > > ERROR: Macros starting with if should be enclosed by a > > > do - while loop to avoid possible if/else logic defects > > > + #define RT_PRINT_DATA(_Comp, _Level, > > > _TitleString, _HexData, _HexDataLen) \ > > > > > > Signed-off-by: Fabio Aiuto <fabioaiut...@gmail.com> > > > --- > > > drivers/staging/rtl8723bs/include/rtw_debug.h | 28 ++++++++++--------- > > > 1 file changed, 15 insertions(+), 13 deletions(-) > > > > > > diff --git a/drivers/staging/rtl8723bs/include/rtw_debug.h > > > b/drivers/staging/rtl8723bs/include/rtw_debug.h > > > index d1c557818305..b00f8a6c4312 100644 > > > --- a/drivers/staging/rtl8723bs/include/rtw_debug.h > > > +++ b/drivers/staging/rtl8723bs/include/rtw_debug.h > > > @@ -236,19 +236,21 @@ > > > #if defined(_dbgdump) > > > #undef RT_PRINT_DATA > > > #define RT_PRINT_DATA(_Comp, _Level, _TitleString, _HexData, > > > _HexDataLen) \ > > > - if (((_Comp) & GlobalDebugComponents) && (_Level <= > > > GlobalDebugLevel)) \ > > > - { > > > \ > > > - int __i; > > > \ > > > - u8 *ptr = (u8 *)_HexData; > > > \ > > > - _dbgdump("%s", DRIVER_PREFIX); > > > \ > > > - _dbgdump(_TitleString); > > > \ > > > - for (__i = 0; __i < (int)_HexDataLen; __i++) > > > \ > > > - { > > > \ > > > - _dbgdump("%02X%s", ptr[__i], (((__i + 1) % 4) > > > == 0)?" ":" "); \ > > > - if (((__i + 1) % 16) == 0) _dbgdump("\n"); > > > \ > > > - } > > > \ > > > - _dbgdump("\n"); > > > \ > > > - } > > > > This whole nightmare needs to just be deleted and any callers of it need > > to use the in-kernel functions for this instead. > > > > thanks, > > > > greg k-h > > ok, will we get rid of the private debug level and components tracking as > well?
Please do. > Maybe encapsulating the built-in proposed by Joe, keeping the macro alive? No, do not keep it alive. Individual debugging/trace/info macros for drivers do not make sense at all. We have kernel-wide infrastructure for this, it is bold for an individual driver to somehow think that they need a "custom" solution for this instead. Just use the normal apis, that is what they are there for. thanks, greg k-h