> > > - 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? > > Mainly for consistency: we have a couple of interfaces that are called from > the > qed driver that are implemented in qed_roce.c. We can either use a 'static > inline' > helper for all of them, or use if(IS_ENABLED()) everywhere. Since this was the > only function that had a helper and that helper was defined incorrectly, I > went > with the second option.
Actually, that's not the case. I think with this exception, all the rest of the prototypes in qed_roce.h aren't really needed, as those functions should only be accessed via the qed_rdma_ops. I'll remove those later [or we can remove them as part of v2]. The genereal qed* preference is to have empty static-inline implementations in case content is compiled-out [Look at iov for example]. > > > -#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] > > That would add even more #ifdefs though. I agree. Although I'm never clear on the guidelines for the tradeoff - How much memory/code is considered too much so that you'd have To ifdef code out instead of 'wasting'? [I obviously don't claim 64 bytes of memory hit that threshold] BTW, are you interested in doing a v2 for this? Or would you prefer if we'd pick it up from here? Thanks, Yuval
