[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250903164431.GD361157@horms.kernel.org>
Date: Wed, 3 Sep 2025 17:44:31 +0100
From: Simon Horman <horms@...nel.org>
To: Dong Yibo <dong100@...se.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, corbet@....net,
gur.stavi@...wei.com, maddy@...ux.ibm.com, mpe@...erman.id.au,
danishanwar@...com, lee@...ger.us, gongfan1@...wei.com,
lorenzo@...nel.org, geert+renesas@...der.be,
Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
alexanderduyck@...com, richardcochran@...il.com, kees@...nel.org,
gustavoars@...nel.org, rdunlap@...radead.org,
vadim.fedorenko@...ux.dev, netdev@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH net-next v10 3/5] net: rnpgbe: Add basic mbx ops support
On Wed, Sep 03, 2025 at 10:54:28AM +0800, Dong Yibo wrote:
> Initialize basic mbx function.
>
> Signed-off-by: Dong Yibo <dong100@...se.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));
> +}
...
Powered by blists - more mailing lists