Re: [dpdk-dev] [PATCH 7/8] net/bnxt: fix HWRM_*() macros and locking

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 8/24/2017 5:29 PM, Ajit Khaparde wrote:
> Obtain the spinlock in HWRM_PREP()
> Eliminate two unnecessary arguments in HWRM_PREP().
> Unlock the spinlock before returning in HWRM_ERROR_CHECK()
> Add new HWRM_UNLOCK() macro
> Update usage of the thre macros.
> 
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

<...>

> -#define HWRM_PREP(req, type, cr, resp) \
> +/*
> + * HWRM_PREP() should be used to prepare *ALL* HWRM commands.  It grabs the
> + * spinlock, and does initial processing.
> + *
> + * HWRM_CHECK_RESULT() returns errors on failure and may not be used.  It
> + * releases the spinlock only if it returns.  If the regular int return codes
> + * are not used by the function, HWRM_CHECK_RESULT() should not be used
> + * directly, rather it should be copied and modified to suit the function.
> + *
> + * HWRM_UNLOCK() must be called after all response processing is completed.
> + */
> +#define HWRM_PREP(x, type) do { \
> +	typeof(x) req = (x); \
> +	rte_spinlock_lock(&bp->hwrm_lock); \
>  	memset(bp->hwrm_cmd_resp_addr, 0, bp->max_resp_len); \
>  	req.req_type = rte_cpu_to_le_16(HWRM_##type); \
> -	req.cmpl_ring = rte_cpu_to_le_16(cr); \
> +	req.cmpl_ring = rte_cpu_to_le_16(-1); \
>  	req.seq_id = rte_cpu_to_le_16(bp->hwrm_cmd_seq++); \
>  	req.target_id = rte_cpu_to_le_16(0xffff); \
> -	req.resp_addr = rte_cpu_to_le_64(bp->hwrm_cmd_resp_dma_addr)
> +	req.resp_addr = rte_cpu_to_le_64(bp->hwrm_cmd_resp_dma_addr); \
> +} while (0)
>  

<...>

> -	HWRM_PREP(req, CFA_L2_SET_RX_MASK, -1, resp);
> +	HWRM_PREP(req, CFA_L2_SET_RX_MASK);

Getting following build error [1] many times with clang (and icc).
I guess this is because using variable name in macro itself.

[1]
.../drivers/net/bnxt/bnxt_hwrm.c:235:12: error: variable 'req' is
uninitialized when used within its own initialization
[-Werror,-Wuninitialized]

        HWRM_PREP(req, CFA_L2_SET_RX_MASK);
        ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~
.../drivers/net/bnxt/bnxt_hwrm.c:186:19: note: expanded from macro
'HWRM_PREP'
        typeof(x) req = (x); \
                  ~~~    ^