> + > +static int > +intel_ntb_dev_init(const struct rte_rawdev *dev) { > + struct ntb_hw *hw = dev->dev_private; > + uint8_t bar; > + int ret, i; > + > + if (hw == NULL) { > + NTB_LOG(ERR, "Invalid device."); > + return -EINVAL; > + } > + > hw->hw_addr = (char *)hw->pci_dev->mem_resource[0].addr; > > + if (is_gen3_ntb(hw)) { > + ret = intel_ntb3_check_ppd(hw); > + } else if (is_gen4_ntb(hw)) { > + /* PPD is in MMIO but not config space for NTB Gen4 */ > + ret = intel_ntb4_check_ppd(hw); > + if (ret) > + return ret; Above two lines are not necessary. > + } else { > + return -ENOTSUP; > + } > + > + if (ret) > + return ret; > + > hw->mw_cnt = XEON_MW_COUNT; > hw->db_cnt = XEON_DB_COUNT; > hw->spad_cnt = XEON_SPAD_COUNT; > @@ -149,15 +219,28 @@ intel_ntb_mw_set_trans(const struct rte_rawdev > *dev, int mw_idx, > rte_write64(base, xlat_addr); > rte_write64(limit, limit_addr); > > - /* Setup the external point so that remote can access. */ > - xlat_off = XEON_EMBAR1_OFFSET + 8 * mw_idx; > - xlat_addr = hw->hw_addr + xlat_off; > - limit_off = XEON_EMBAR1XLMT_OFFSET + mw_idx * > XEON_BAR_INTERVAL_OFFSET; > - limit_addr = hw->hw_addr + limit_off; > - base = rte_read64(xlat_addr); > - base &= ~0xf; > - limit = base + size; > - rte_write64(limit, limit_addr); > + if (is_gen3_ntb(hw)) { > + /* Setup the external point so that remote can access. */ > + xlat_off = XEON_EMBAR1_OFFSET + 8 * mw_idx; > + xlat_addr = hw->hw_addr + xlat_off; > + limit_off = XEON_EMBAR1XLMT_OFFSET + > + mw_idx * XEON_BAR_INTERVAL_OFFSET; > + limit_addr = hw->hw_addr + limit_off; > + base = rte_read64(xlat_addr); > + base &= ~0xf; > + limit = base + size; > + rte_write64(limit, limit_addr); > + } else if (is_gen4_ntb(hw)) { Can we use a variable in struct to indicate it's gen4 or gen3 after init instead of check it every time?
> + /* Set translate base address index register */ > + xlat_off = XEON_GEN4_IM1XBASEIDX_OFFSET + > + mw_idx * XEON_GEN4_XBASEIDX_INTERVAL; > + xlat_addr = hw->hw_addr + xlat_off; > + rte_write16(rte_log2_u64(size), xlat_addr); > + } else { > + rte_write64(base, limit_addr); > + rte_write64(0, xlat_addr); > + return -ENOTSUP; > + } Is the else branch necessary? As if neither gen3 or gen4, the init would fail. Would be better to print an ERR instead of just return NO support. > > return 0; > }