On Wed, Sep 03, 2025 at 10:54:28AM +0800, Dong Yibo wrote:
> Initialize basic mbx function.
> 
> Signed-off-by: Dong Yibo <dong...@mucse.com>

...

> +/**
> + * mucse_mbx_inc_pf_req - Increase req
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_inc_pf_req read pf_req from hw, then write
> + * new value back after increase
> + **/
> +static void mucse_mbx_inc_pf_req(struct mucse_hw *hw)
> +{
> +     struct mucse_mbx_info *mbx = &hw->mbx;
> +     u16 req;
> +     u32 v;
> +
> +     v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> +     req = (v & GENMASK_U32(15, 0));

nit1: Unnecessary parentheses
nit2: I would have used FIELD_GET here in conjunction with something like.

      #define MBX_PF2FW_COUNTER_MASK GENMASK_U32(15, 0)

> +     req++;
> +     v &= GENMASK_U32(31, 16);
> +     v |= req;

      And using FIELD_PREP is probably more succinct here.

      Likewise in the following function

> +     mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
> +     hw->mbx.stats.msgs_tx++;
> +}
> +
> +/**
> + * mucse_mbx_inc_pf_ack - Increase ack
> + * @hw: pointer to the HW structure
> + *
> + * mucse_mbx_inc_pf_ack read pf_ack from hw, then write
> + * new value back after increase
> + **/
> +static void mucse_mbx_inc_pf_ack(struct mucse_hw *hw)
> +{
> +     struct mucse_mbx_info *mbx = &hw->mbx;
> +     u16 ack;
> +     u32 v;
> +
> +     v = mbx_data_rd32(mbx, MBX_PF2FW_COUNTER);
> +     ack = (v >> 16) & GENMASK_U32(15, 0);
> +     ack++;
> +     v &= GENMASK_U32(15, 0);
> +     v |= (ack << 16);
> +     mbx_data_wr32(mbx, MBX_PF2FW_COUNTER, v);
> +     hw->mbx.stats.msgs_rx++;
> +}

...

> +/**
> + * mucse_mbx_reset - Reset mbx info, sync info from regs
> + * @hw: pointer to the HW structure
> + *
> + * This function reset all mbx variables to default.
> + **/
> +static void mucse_mbx_reset(struct mucse_hw *hw)
> +{
> +     struct mucse_mbx_info *mbx = &hw->mbx;
> +     u32 v;
> +
> +     v = mbx_data_rd32(mbx, MBX_FW2PF_COUNTER);
> +     hw->mbx.fw_req = v & GENMASK_U32(15, 0);
> +     hw->mbx.fw_ack = (v >> 16) & GENMASK_U32(15, 0);

I'd use FIELD_GET here too.

> +     mbx_ctrl_wr32(mbx, PF2FW_MBOX_CTRL(mbx), 0);
> +     mbx_ctrl_wr32(mbx, FW_PF_MBOX_MASK(mbx), GENMASK_U32(31, 16));
> +}

...

Reply via email to