> #define MAX_VF_NUM (8) > + hw->max_vfs = 7; ??? > } > > /** > @@ -117,6 +119,7 @@ static void rnpgbe_get_invariants_n210(struct mucse_hw *hw) > /* update hw feature */ > hw->feature_flags |= M_HW_FEATURE_EEE; > hw->usecstocount = 62; > + hw->max_vfs_noari = 7; ??? > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size, > + enum MBX_ID mbx_id) > +{ > + struct mucse_mbx_info *mbx = &hw->mbx; > + > + /* limit read to size of mailbox */ > + if (size > mbx->size) > + size = mbx->size; > + > + if (!mbx->ops.read) > + return -EIO; How would that happen? > + > + return mbx->ops.read(hw, msg, size, mbx_id); > +int mucse_write_mbx(struct mucse_hw *hw, u32 *msg, u16 size, > + enum MBX_ID mbx_id) > +{ > + struct mucse_mbx_info *mbx = &hw->mbx; > + > + if (size > mbx->size) > + return -EINVAL; > + > + if (!mbx->ops.write) > + return -EIO; How would either of these two conditions happen. > +static u16 mucse_mbx_get_req(struct mucse_hw *hw, int reg) > +{ > + /* force memory barrier */ > + mb(); > + return ioread32(hw->hw_addr + reg) & GENMASK(15, 0); I'm no expert on memory barriers, but what are you trying to achieve here? Probably the most used pattern of an mb() is to flush out writes to hardware before doing a special write which triggers the hardware to do something. That is not what is happening here. > +static void mucse_mbx_inc_pf_req(struct mucse_hw *hw, > + enum MBX_ID mbx_id) > +{ > + struct mucse_mbx_info *mbx = &hw->mbx; > + u32 reg, v; > + u16 req; > + > + reg = (mbx_id == MBX_FW) ? PF2FW_COUNTER(mbx) : > + PF2VF_COUNTER(mbx, mbx_id); > + v = mbx_rd32(hw, reg); > + req = (v & GENMASK(15, 0)); > + req++; > + v &= GENMASK(31, 16); > + v |= req; > + /* force before write to hw */ > + mb(); > + mbx_wr32(hw, reg, v); > + /* update stats */ > + hw->mbx.stats.msgs_tx++; What are you forcing? As i said, i'm no expert on memory barriers, but to me, it looks like whoever wrote this code also does not understand memory barriers. > +static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw, enum MBX_ID mbx_id) > +{ > + struct mucse_mbx_info *mbx = &hw->mbx; > + int try_cnt = 5000, ret; > + u32 reg; > + > + reg = (mbx_id == MBX_FW) ? PF2FW_MBOX_CTRL(mbx) : > + PF2VF_MBOX_CTRL(mbx, mbx_id); > + while (try_cnt-- > 0) { > + /* Take ownership of the buffer */ > + mbx_wr32(hw, reg, MBOX_PF_HOLD); > + /* force write back before check */ > + wmb(); > + if (mbx_rd32(hw, reg) & MBOX_PF_HOLD) > + return 0; > + udelay(100); > + } > + return ret; I've not compiled this, but isn't ret uninitialized here? I would also expect it to return -ETIMEDOUT? Andrew