Re: [dpdk-dev] [EXT] Re: [PATCH 3/4] net/qede: add infrastructure for debug data collection

[Rasesh Mody <rmody@marvell.com>]

>From: Jerin Jacob <jerinjacobk@gmail.com>
>Sent: Sunday, June 28, 2020 5:30 AM
>
>----------------------------------------------------------------------
>On Sun, Jun 28, 2020 at 11:29 AM Rasesh Mody <rmody@marvell.com> wrote:
>>
>> The patch adds QEDE PMD ops and APIs to calculate the size and collect
>> the debug dump for various firmware components. The patch adds new
>> files qede_debug.[ch] that has all the firmware debug data collection
>> infrastructure changes.
>>
>> Signed-off-by: Rasesh Mody <rmody@marvell.com>
>> Signed-off-by: Igor Russkikh <irusskikh@marvell.com>
>> ---
>>  drivers/net/qede/Makefile        |    4 +-
>>  drivers/net/qede/base/bcm_osal.h |    5 +-
>>  drivers/net/qede/qede_debug.c    | 8119
>++++++++++++++++++++++++++++++
>>  drivers/net/qede/qede_debug.h    |  759 +++
>>  drivers/net/qede/qede_if.h       |   45 +
>>  drivers/net/qede/qede_main.c     |   39 +-
>>  6 files changed, 8967 insertions(+), 4 deletions(-)  create mode
>> 100644 drivers/net/qede/qede_debug.c  create mode 100644
>> drivers/net/qede/qede_debug.h
>
>Please fix the following checkpatch issues.
>
>
>CHECK:MACRO_ARG_REUSE: Macro argument reuse 'field' - possible side-
>effects?
>#494: FILE: drivers/net/qede/qede_debug.c:339:
>+#define FIELD_BIT_MASK(type, field) \
>+ (((1 << FIELD_BIT_SIZE(type, field)) - 1) << \
>+FIELD_DWORD_SHIFT(type, field))
>
>CHECK:MACRO_ARG_REUSE: Macro argument reuse 'var' - possible side-
>effects?
>#498: FILE: drivers/net/qede/qede_debug.c:343:
>+#define SET_VAR_FIELD(var, type, field, val) \  do { \
>+var[FIELD_DWORD_OFFSET(type, field)] &= \  (~FIELD_BIT_MASK(type,
>+field)); \  var[FIELD_DWORD_OFFSET(type, field)] |= \
>+ (val) << FIELD_DWORD_SHIFT(type, field); \  } while (0)
>
>CHECK:MACRO_ARG_REUSE: Macro argument reuse 'field' - possible side-
>effects?
>#498: FILE: drivers/net/qede/qede_debug.c:343:
>+#define SET_VAR_FIELD(var, type, field, val) \  do { \
>+var[FIELD_DWORD_OFFSET(type, field)] &= \  (~FIELD_BIT_MASK(type,
>+field)); \  var[FIELD_DWORD_OFFSET(type, field)] |= \
>+ (val) << FIELD_DWORD_SHIFT(type, field); \  } while (0)
>
>CHECK:MACRO_ARG_REUSE: Macro argument reuse 'block' - possible side-
>effects?
>#518: FILE: drivers/net/qede/qede_debug.c:363:
>+#define NUM_DBG_LINES(block) \
>+ ((block)->num_of_dbg_bus_lines + NUM_EXTRA_DBG_LINES(block))

Please ignore MACRO_ARG_REUSE checks as we do not see any side effects.

>
>WARNING:TYPO_SPELLING: 'DIDNT' may be misspelled - perhaps 'DIDN'T'?
>#5814: FILE: drivers/net/qede/qede_debug.c:5659:
>+ /* DBG_STATUS_DATA_DIDNT_TRIGGER */

This seems to be a false positive, however it can be renamed.

>
>ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
>#6344: FILE: drivers/net/qede/qede_debug.c:6189:
>+ for (; reg_id > curr_reg_id;
>[...]
>+     1);

Will address in v2 series.