> config INFINIBAND_QEDR
> - tristate "QLogic qede RoCE sources [debug]"
> + bool "QLogic qede RoCE sources [debug]"
Given that the qedr submission is going to turn this back into a tristate,
are you certain this is a good thing [from compilation coverage perspective]?
> - if (cond)
> + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) && cond)
> qed_rdma_dpm_bar(p_hwfn, p_ptt);
Why not simply fix the qed_roce.h empty implementation?
> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> /* Roce CNQ each requires: 1 status block + 1 CNQ. We divide the
> * status blocks equally between L2 / RoCE but with consideration as
> * to how many l2 queues / cnqs we have
> */
> - if (p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
> + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR) &&
> + p_hwfn->hw_info.personality == QED_PCI_ETH_ROCE) {
> num_features++;
>
> feat_num[QED_RDMA_CNQ] =
> min_t(u32, RESC_NUM(p_hwfn, QED_SB) /
> num_features,
> RESC_NUM(p_hwfn, QED_RDMA_CNQ_RAM));
> }
> -#endif
Is there any non-cosmetic gain here?
I would gain that having the comment under the #ifdef is more meaningful
than having the check in the actual condition.
> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> + if (!IS_ENABLED(CONFIG_INFINIBAND_QEDR))
> + return 0;
> +
> num_l2_queues = 0;
> for_each_hwfn(cdev, i)
> num_l2_queues += FEAT_NUM(&cdev->hwfns[i],
> QED_PF_L2_QUE); @@ -738,7 +736,6 @@ static int
> qed_slowpath_setup_int(struct qed_dev *cdev,
> DP_VERBOSE(cdev, QED_MSG_RDMA, "roce_msix_cnt=%d
> roce_msix_base=%d\n",
> cdev->int_params.rdma_msix_cnt,
> cdev->int_params.rdma_msix_base);
> -#endif
While I don't mind, you could have argued is that we're not
removing enough, not too much.
I.e., perhaps the rdma_msix_* fields should also have been
ifdef-ed instead. [in which case this solution would not have worked]
> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> - params->rdma_pf_params.num_qps = QED_ROCE_QPS;
> - params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
> - /* divide by 3 the MRs to avoid MF ILT overflow */
> - params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
> - params->rdma_pf_params.gl_pi = QED_ROCE_PROTOCOL_INDEX;
> -#endif
> + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR)) {
> + params->rdma_pf_params.num_qps = QED_ROCE_QPS;
> + params->rdma_pf_params.min_dpis = QED_ROCE_DPIS;
> + /* divide by 3 the MRs to avoid MF ILT overflow */
> + params->rdma_pf_params.num_mrs = RDMA_MAX_TIDS;
> + params->rdma_pf_params.gl_pi =
> QED_ROCE_PROTOCOL_INDEX;
> + }
Likewise
> -#if IS_ENABLED(CONFIG_INFINIBAND_QEDR)
> - case PROTOCOLID_ROCE:
> - qed_async_roce_event(p_hwfn, p_eqe);
> - return 0;
> -#endif
> case PROTOCOLID_COMMON:
> return qed_sriov_eqe_event(p_hwfn,
> p_eqe->opcode,
> p_eqe->echo, &p_eqe->data);
> + case PROTOCOLID_ROCE:
> + if (IS_ENABLED(CONFIG_INFINIBAND_QEDR)) {
> + qed_async_roce_event(p_hwfn, p_eqe);
> + return 0;
> + }
> + /* fallthrough */
Not sure whether it helps readability; It might give the false
Impression that it's possible to receive an async roce event if
roce is compiled-out, which isn't the case.
