One general comment:

Think about to use rte_read32() and rte_write32() when reading and writing 
registers. Or you can define a Macro or inline function for NTB driver to touch 
registers. Then you can omit lots of "(char *)hw->pci_dev->mem_resource[0].addr 
" and "*((volatile uint32_t *)" like things. It will make the code much easier 
to read.


> +static void *
> +intel_ntb_get_peer_mw_addr(struct rte_rawdev *dev, int mw_idx)
> +{
> +     struct ntb_hw *hw = dev->dev_private;
> +     uint8_t bar;
> +
> +     if (hw == NULL) {
> +             NTB_LOG(ERR, "Invalid device.");
> +             return 0;
> +     }
> +
> +     if (mw_idx < 0 || mw_idx > hw->mw_cnt) {
mw_idx >= hw->mw_cnt?

[...]


> +static int
> +intel_ntb_mw_set_trans(struct rte_rawdev *dev, int mw_idx,
> +                    uint64_t addr, uint64_t size)
> +{
> +     struct ntb_hw *hw = dev->dev_private;
> +     void *xlat_addr, *limit_addr;
> +     uint64_t xlat_off, limit_off;
> +     uint64_t base, limit;
> +     uint8_t bar;
> +
> +     if (hw == NULL) {
> +             NTB_LOG(ERR, "Invalid device.");
> +             return -EINVAL;
> +     }
> +
> +     if (mw_idx < 0 || mw_idx > hw->mw_cnt) {
Same as above.

[...]

> +static uint32_t
> +intel_ntb_spad_read(struct rte_rawdev *dev, int spad, bool peer)
> +{
> +     struct ntb_hw *hw = dev->dev_private;
> +     uint32_t spad_v, reg_off;
> +     void *reg_addr;
> +
> +     if (spad < 0 || spad >= hw->spad_cnt) {
> +             NTB_LOG(ERR, "Invalid spad reg index.");
> +             return 0;
> +     }
> +
> +     /* When peer is true, read peer spad reg */
> +     if (peer)
> +             reg_off = XEON_B2B_SPAD_OFFSET;
> +     else
> +             reg_off = XEON_IM_SPAD_OFFSET;

How about one line if check is simple?
reg_off = peer ? XEON_B2B_SPAD_OFFSET : XEON_IM_SPAD_OFFSET;

Thanks
Jingjing

Reply via email to