On Wed, Aug 20, 2025 at 10:23:44PM +0200, Andrew Lunn wrote:

quoted

+/**
+ * mucse_mbx_get_ack - Read ack from reg
+ * @mbx: pointer to the MBX structure
+ * @reg: register to read
+ *
+ * @return: the ack value
+ **/
+static u16 mucse_mbx_get_ack(struct mucse_mbx_info *mbx, int reg)
+{
+	return (mbx_data_rd32(mbx, reg) >> 16);
+}

quoted

+static int mucse_check_for_ack_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u16 hw_fw_ack;
+
+	hw_fw_ack = mucse_mbx_get_ack(mbx, MBX_FW2PF_COUNTER);

quoted

+int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int size_inwords = size / 4;
+	u32 ctrl_reg;
+	int ret;
+	int i;
+
+	ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+	ret = mucse_obtain_mbx_lock_pf(hw);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < size_inwords; i++)
+		mbx_data_wr32(mbx, MBX_FW_PF_SHM_DATA + i * 4, msg[i]);
+
+	/* flush msg and acks as we are overwriting the message buffer */
+	hw->mbx.fw_ack = mucse_mbx_get_ack(mbx, MBX_FW2PF_COUNTER);

It seems like the ACK is always at MBX_FW2PF_COUNTER. So why pass it
to mucse_mbx_get_ack()? Please look at your other getters and setters.

'mucse_mbx_get_ack' is always at MBX_FW2PF_COUNTER now, just for pf-fw mbx. 
But, in the future, there will be pf-vf mbx with different input.
Should I move 'MBX_FW2PF_COUNTER' to function 'mucse_mbx_get_ack', and
update the function when I add vf relative code in the future?

quoted

+/**
+ * mucse_write_mbx - Write a message to the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_write_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	return mucse_write_mbx_pf(hw, msg, size);
+}

This function does not do anything useful. Why not call
mucse_write_mbx_pf() directly?

Yes, I should call it directly. 

quoted

+/**
+ * mucse_check_for_msg - Check to see if fw sent us mail
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_check_for_msg(struct mucse_hw *hw)
+{
+	return mucse_check_for_msg_pf(hw);
+}
+
+/**
+ * mucse_check_for_ack - Check to see if fw sent us ACK
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+int mucse_check_for_ack(struct mucse_hw *hw)
+{
+	return mucse_check_for_ack_pf(hw);
+}

These as well.

Got it, I will update it.

	Andrew

Thanks for your feedback.