Re: [PATCH net-next v12 3/5] net: rnpgbe: Add basic mbx ops support

quoted

+	if (fw_req != 0 && fw_req != hw->mbx.fw_req) {
+		hw->mbx.stats.reqs++;
+		return 0;
+	}
+
+	return -EIO;

Only a suggestion: Might it be clearer to flip the cases and handle the if
as error case and continue with the success case?

if (fw_req == 0 || fw_req == hw->mbx.fw_req)
	return -EIO;

hw->mbx.stats.reqs++;
return 0;

That would by the usual pattern in the kernel. It is good to follow
usual patterns.

quoted

+static void mucse_mbx_inc_pf_req(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u16 req;
+	u32 val;
+
+	val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
+	req = FIELD_GET(GENMASK_U32(15, 0), val);

Why not assign the values in the declaration like done with mbx?

Reverse Christmas tree.

	struct mucse_mbx_info *mbx = &hw->mbx;
	u32 val = mbx_data_rd32(mbx, MUCSE_MBX_PF2FW_CNT);
	u16 req = FIELD_GET(GENMASK_U32(15, 0), val);

This is not reverse christmas tree. Sometimes you need to put
assignments into the body of the function.

	Andrew