Re: [dpdk-dev] [PATCH v8 2/6] raw/ntb: add intel ntb support

[Wu, Jingjing]

Few minor comments.

> +static int
> +intel_ntb_dev_init(struct rte_rawdev *dev)
> +{
> +     struct ntb_hw *hw = dev->dev_private;
> +     uint8_t reg_val, bar;
> +     int ret, i;
> +
> +     if (hw == NULL) {
> +             NTB_LOG(ERR, "Invalid device.");
> +             return -EINVAL;
> +     }
> +
> +     ret = rte_pci_read_config(hw->pci_dev, &reg_val,
> +                               sizeof(reg_val), XEON_PPD_OFFSET);
> +     if (ret < 0) {
> +             NTB_LOG(ERR, "Cannot get NTB PPD (PCIe port definition).");
> +             return -EIO;
> +     }
> +
> +     /* Check connection topo type. Only support B2B. */
> +     switch (reg_val & XEON_PPD_CONN_MASK) {
> +     case XEON_PPD_CONN_B2B:
> +             NTB_LOG(INFO, "Topo B2B (back to back) is using.");
> +             break;
> +     case XEON_PPD_CONN_TRANSPARENT:
> +     case XEON_PPD_CONN_RP:
> +             NTB_LOG(ERR, "Not supported conn topo. Please use B2B.");
> +             return -EINVAL;

Do We need "default:" ?
> +     }
> +
> +     /* Check device type. */
> +     if (reg_val & XEON_PPD_DEV_DSD) {
> +             NTB_LOG(INFO, "DSD, Downstream Device.");
> +             hw->topo = NTB_TOPO_B2B_DSD;
> +     } else {
> +             NTB_LOG(INFO, "USD, Upstream device.");
> +             hw->topo = NTB_TOPO_B2B_USD;
> +     }
> +
> +     /* Check if bar4 is split. Do not support split bar. */
> +     if (reg_val & XEON_PPD_SPLIT_BAR_MASK) {
> +             NTB_LOG(ERR, "Do not support split bar.");
> +             return -EINVAL;
> +     }
> +
> +     hw->hw_addr = (char *)hw->pci_dev->mem_resource[0].addr;
> +
> +     hw->mw_cnt = XEON_MW_COUNT;
> +     hw->db_cnt = XEON_DB_COUNT;
> +     hw->spad_cnt = XEON_SPAD_COUNT;
> +
> +     hw->mw_size = rte_zmalloc("uint64_t",
> +                               hw->mw_cnt * sizeof(uint64_t), 0);
> +     for (i = 0; i < hw->mw_cnt; i++) {
> +             bar = intel_ntb_bar[i];
> +             hw->mw_size[i] = hw->pci_dev->mem_resource[bar].len;
> +     }
> +
> +     /* Reserve the last 2 spad registers for users. */
> +     for (i = 0; i < NTB_SPAD_USER_MAX_NUM; i++)
> +             hw->spad_user_list[i] = hw->spad_cnt;
> +     hw->spad_user_list[0] = hw->spad_cnt - 2;
> +     hw->spad_user_list[1] = hw->spad_cnt - 1;

How about:
hw->spad_user_list[0] = hw->spad_cnt - 2;
hw->spad_user_list[1] = hw->spad_cnt - 1;
for (i = 2; i < NTB_SPAD_USER_MAX_NUM; i++)
    hw->spad_user_list[i] = hw->spad_cnt;