Initialize basic mbx function.

Signed-off-by: Dong Yibo <dong100@mucse.com>
---
 drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
 .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
 6 files changed, 520 insertions(+), 1 deletion(-)
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
 create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h

diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
index 42c359f459d9..5fc878ada4b1 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
+++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
@@ -6,4 +6,5 @@
 
 obj-$(CONFIG_MGBE) += rnpgbe.o
 rnpgbe-objs := rnpgbe_main.o\
-	       rnpgbe_chip.o
+	       rnpgbe_chip.o\
+	       rnpgbe_mbx.o
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
index 0dd3d3cb2a4d..05830bb73d3e 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
@@ -5,6 +5,7 @@
 #define _RNPGBE_H
 
 #include <linux/types.h>
+#include <linux/mutex.h>
 
 extern const struct rnpgbe_info rnpgbe_n500_info;
 extern const struct rnpgbe_info rnpgbe_n210_info;
@@ -40,7 +41,43 @@ struct mucse_mac_info {
 	void *back;
 };
 
+struct mucse_hw;
+
+struct mucse_mbx_operations {
+	void (*init_params)(struct mucse_hw *hw);
+	int (*read)(struct mucse_hw *hw, u32 *msg,
+		    u16 size);
+	int (*write)(struct mucse_hw *hw, u32 *msg,
+		     u16 size);
+	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
+			   u16 size);
+	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
+			    u16 size);
+	int (*check_for_msg)(struct mucse_hw *hw);
+	int (*check_for_ack)(struct mucse_hw *hw);
+	void (*configure)(struct mucse_hw *hw, int num_vec,
+			  bool enable);
+};
+
+struct mucse_mbx_stats {
+	u32 msgs_tx;
+	u32 msgs_rx;
+	u32 acks;
+	u32 reqs;
+	u32 rsts;
+};
+
 struct mucse_mbx_info {
+	const struct mucse_mbx_operations *ops;
+	struct mucse_mbx_stats stats;
+	u32 timeout;
+	u32 usec_delay;
+	u16 size;
+	u16 fw_req;
+	u16 fw_ack;
+	/* lock for only one use mbx */
+	struct mutex lock;
+	bool irq_enabled;
 	/* fw <--> pf mbx */
 	u32 fw_pf_shm_base;
 	u32 pf2fw_mbox_ctrl;
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
index 20ec67c9391e..16d0a76114b5 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
@@ -1,8 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright(c) 2020 - 2025 Mucse Corporation. */
 
+#include <linux/string.h>
+
 #include "rnpgbe.h"
 #include "rnpgbe_hw.h"
+#include "rnpgbe_mbx.h"
 
 /**
  * rnpgbe_init_common - Setup common attribute
@@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
 
 	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
 	mac->back = hw;
+
+	hw->mbx.ops = &mucse_mbx_ops_generic;
 }
 
 /**
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
index fc57258537cf..aee037e3219d 100644
--- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
@@ -7,6 +7,8 @@
 #define RNPGBE_RING_BASE 0x1000
 #define RNPGBE_MAC_BASE 0x20000
 #define RNPGBE_ETH_BASE 0x10000
+/**************** DMA Registers ****************************/
+#define RNPGBE_DMA_DUMY 0x000c
 /**************** CHIP Resource ****************************/
 #define RNPGBE_MAX_QUEUES 8
 #endif /* _RNPGBE_HW_H */
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
new file mode 100644
index 000000000000..1195cf945ad1
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2022 - 2025 Mucse Corporation. */
+
+#include <linux/pci.h>
+#include <linux/errno.h>
+#include <linux/delay.h>
+#include <linux/iopoll.h>
+
+#include "rnpgbe.h"
+#include "rnpgbe_mbx.h"
+#include "rnpgbe_hw.h"
+
+/**
+ * mucse_read_mbx - Reads a message from 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_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	/* limit read size */
+	min(size, mbx->size);
+	return mbx->ops->read(hw, msg, size);
+}
+
+/**
+ * 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)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->write(hw, msg, size);
+}
+
+/**
+ * mucse_mbx_get_req - Read req from reg
+ * @hw: pointer to the HW structure
+ * @reg: register to read
+ *
+ * @return: the req value
+ **/
+static u16 mucse_mbx_get_req(struct mucse_hw *hw, int reg)
+{
+	return mbx_rd32(hw, reg) & GENMASK(15, 0);
+}
+
+/**
+ * mucse_mbx_get_ack - Read ack from reg
+ * @hw: pointer to the HW structure
+ * @reg: register to read
+ *
+ * @return: the ack value
+ **/
+static u16 mucse_mbx_get_ack(struct mucse_hw *hw, int reg)
+{
+	return (mbx_rd32(hw, reg) >> 16);
+}
+
+/**
+ * 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;
+	u32 reg, v;
+	u16 req;
+
+	reg = PF2FW_COUNTER(mbx);
+	v = mbx_rd32(hw, reg);
+	req = (v & GENMASK(15, 0));
+	req++;
+	v &= GENMASK(31, 16);
+	v |= req;
+	mbx_wr32(hw, reg, 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;
+	u32 reg, v;
+	u16 ack;
+
+	reg = PF2FW_COUNTER(mbx);
+	v = mbx_rd32(hw, reg);
+	ack = (v >> 16) & GENMASK(15, 0);
+	ack++;
+	v &= GENMASK(15, 0);
+	v |= (ack << 16);
+	mbx_wr32(hw, reg, v);
+	hw->mbx.stats.msgs_rx++;
+}
+
+/**
+ * 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)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->check_for_msg(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)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	return mbx->ops->check_for_ack(hw);
+}
+
+/**
+ * mucse_poll_for_msg - Wait for message notification
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_poll_for_msg(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int countdown = mbx->timeout;
+	int val;
+
+	return read_poll_timeout(mbx->ops->check_for_msg,
+				 val, val == 0, mbx->usec_delay,
+				 countdown * mbx->usec_delay,
+				 false, hw);
+}
+
+/**
+ * mucse_poll_for_ack - Wait for message acknowledgment
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if it successfully received a message acknowledgment
+ **/
+static int mucse_poll_for_ack(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int countdown = mbx->timeout;
+	int val;
+
+	return read_poll_timeout(mbx->ops->check_for_ack,
+				 val, val == 0, mbx->usec_delay,
+				 countdown * mbx->usec_delay,
+				 false, hw);
+}
+
+/**
+ * mucse_read_posted_mbx - Wait for message notification and receive message
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 if it successfully received a message notification and
+ * copied it into the receive buffer.
+ **/
+static int mucse_read_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int ret;
+
+	ret = mucse_poll_for_msg(hw);
+	if (ret)
+		return ret;
+
+	return mbx->ops->read(hw, msg, size);
+}
+
+/**
+ * mucse_write_posted_mbx - Write a message to the mailbox, wait for ack
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * @return: 0 if it successfully copied message into the buffer and
+ * received an ack to that message within delay * timeout period
+ **/
+static int mucse_write_posted_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int ret;
+
+	ret = mbx->ops->write(hw, msg, size);
+	if (ret)
+		return ret;
+	return mucse_poll_for_ack(hw);
+}
+
+/**
+ * mucse_check_for_msg_pf - Check to see if the fw has sent mail
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if the fw has set the Status bit or else
+ * -EIO
+ **/
+static int mucse_check_for_msg_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u16 hw_req_count = 0;
+
+	hw_req_count = mucse_mbx_get_req(hw, FW2PF_COUNTER(mbx));
+	/* chip's register is reset to 0 when rc send reset
+	 * mbx command. This causes 'hw_req_count != hw->mbx.fw_req'
+	 * be TRUE before fw really reply. Driver must wait fw reset
+	 * done reply before using chip, we must check no-zero.
+	 **/
+	if (hw_req_count != 0 && hw_req_count != hw->mbx.fw_req) {
+		hw->mbx.stats.reqs++;
+		return 0;
+	}
+
+	return -EIO;
+}
+
+/**
+ * mucse_check_for_ack_pf - Check to see if the VF has ACKed
+ * @hw: pointer to the HW structure
+ *
+ * @return: 0 if the fw has set the Status bit or else
+ * -EIO
+ **/
+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(hw, FW2PF_COUNTER(mbx));
+	/* chip's register is reset to 0 when rc send reset
+	 * mbx command. This causes 'hw_fw_ack != hw->mbx.fw_ack'
+	 * be TRUE before fw really reply. Driver must wait fw reset
+	 * done reply before using chip, we must check no-zero.
+	 **/
+	if (hw_fw_ack != 0 && hw_fw_ack != hw->mbx.fw_ack) {
+		hw->mbx.stats.acks++;
+		return 0;
+	}
+
+	return -EIO;
+}
+
+/**
+ * mucse_obtain_mbx_lock_pf - Obtain mailbox lock
+ * @hw: pointer to the HW structure
+ *
+ * This function maybe used in an irq handler.
+ *
+ * @return: 0 if we obtained the mailbox lock
+ **/
+static int mucse_obtain_mbx_lock_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	int try_cnt = 5000;
+	u32 reg;
+
+	reg = PF2FW_MBOX_CTRL(mbx);
+	while (try_cnt-- > 0) {
+		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 -EIO;
+}
+
+/**
+ * mucse_write_mbx_pf - Place a message in the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * This function maybe used in an irq handler.
+ *
+ * @return: 0 if it successfully copied message into the buffer
+ **/
+static int mucse_write_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 data_reg, ctrl_reg;
+	int ret;
+	u16 i;
+
+	data_reg = FW_PF_SHM_DATA(mbx);
+	ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+	ret = mucse_obtain_mbx_lock_pf(hw);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < size; i++)
+		mbx_wr32(hw, data_reg + i * 4, msg[i]);
+
+	/* flush msg and acks as we are overwriting the message buffer */
+	hw->mbx.fw_ack = mucse_mbx_get_ack(hw, FW2PF_COUNTER(mbx));
+	mucse_mbx_inc_pf_req(hw);
+	mbx_wr32(hw, ctrl_reg, MBOX_CTRL_REQ);
+
+	return 0;
+}
+
+/**
+ * mucse_read_mbx_pf - Read a message from the mailbox
+ * @hw: pointer to the HW structure
+ * @msg: the message buffer
+ * @size: length of buffer
+ *
+ * This function copies a message from the mailbox buffer to the caller's
+ * memory buffer. The presumption is that the caller knows that there was
+ * a message due to a fw request so no polling for message is needed.
+ *
+ * @return: 0 on success, negative on failure
+ **/
+static int mucse_read_mbx_pf(struct mucse_hw *hw, u32 *msg, u16 size)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 data_reg, ctrl_reg;
+	int ret;
+	u32 i;
+
+	data_reg = FW_PF_SHM_DATA(mbx);
+	ctrl_reg = PF2FW_MBOX_CTRL(mbx);
+
+	ret = mucse_obtain_mbx_lock_pf(hw);
+	if (ret)
+		return ret;
+	for (i = 0; i < size; i++)
+		msg[i] = mbx_rd32(hw, data_reg + 4 * i);
+	/* Hw need write data_reg at last */
+	mbx_wr32(hw, data_reg, 0);
+	hw->mbx.fw_req = mucse_mbx_get_req(hw, FW2PF_COUNTER(mbx));
+	mucse_mbx_inc_pf_ack(hw);
+	mbx_wr32(hw, ctrl_reg, 0);
+
+	return 0;
+}
+
+/**
+ * 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;
+	int v;
+
+	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
+	hw->mbx.fw_req = v & GENMASK(15, 0);
+	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
+	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
+}
+
+/**
+ * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
+ * @hw: pointer to the HW structure
+ * @nr_vec: vector number for mbx
+ * @enable: TRUE for enable, FALSE for disable
+ *
+ * This function configure mbx to use interrupt nr_vec.
+ **/
+static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
+				   bool enable)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+	u32 v;
+
+	if (enable) {
+		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
+		hw->mbx.fw_req = v & GENMASK(15, 0);
+		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
+		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
+		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
+	} else {
+		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
+		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
+		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
+	}
+}
+
+/**
+ * mucse_init_mbx_params_pf - Set initial values for pf mailbox
+ * @hw: pointer to the HW structure
+ *
+ * Initializes the hw->mbx struct to correct values for pf mailbox
+ */
+static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
+{
+	struct mucse_mbx_info *mbx = &hw->mbx;
+
+	mbx->usec_delay = 100;
+	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
+	mbx->stats.msgs_tx = 0;
+	mbx->stats.msgs_rx = 0;
+	mbx->stats.reqs = 0;
+	mbx->stats.acks = 0;
+	mbx->stats.rsts = 0;
+	mbx->size = MUCSE_MAILBOX_WORDS;
+	mutex_init(&mbx->lock);
+	mucse_mbx_reset(hw);
+}
+
+const struct mucse_mbx_operations mucse_mbx_ops_generic = {
+	.init_params = mucse_init_mbx_params_pf,
+	.read = mucse_read_mbx_pf,
+	.write = mucse_write_mbx_pf,
+	.read_posted = mucse_read_posted_mbx,
+	.write_posted = mucse_write_posted_mbx,
+	.check_for_msg = mucse_check_for_msg_pf,
+	.check_for_ack = mucse_check_for_ack_pf,
+	.configure = mucse_mbx_configure_pf,
+};
diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
new file mode 100644
index 000000000000..2d850a11c369
--- /dev/null
+++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2020 - 2025 Mucse Corporation. */
+
+#ifndef _RNPGBE_MBX_H
+#define _RNPGBE_MBX_H
+
+#include "rnpgbe.h"
+
+#define MUCSE_MAILBOX_WORDS 14
+#define MUCSE_FW_MAILBOX_WORDS MUCSE_MAILBOX_WORDS
+#define FW_PF_SHM(mbx) ((mbx)->fw_pf_shm_base)
+#define FW2PF_COUNTER(mbx) (FW_PF_SHM(mbx) + 0)
+#define PF2FW_COUNTER(mbx) (FW_PF_SHM(mbx) + 4)
+#define FW_PF_SHM_DATA(mbx) (FW_PF_SHM(mbx) + 8)
+#define FW2PF_MBOX_VEC(mbx) ((mbx)->fw2pf_mbox_vec)
+#define PF2FW_MBOX_CTRL(mbx) ((mbx)->pf2fw_mbox_ctrl)
+#define FW_PF_MBOX_MASK(mbx) ((mbx)->fw_pf_mbox_mask)
+#define MBOX_CTRL_REQ BIT(0)
+#define MBOX_PF_HOLD BIT(3)
+#define MBOX_IRQ_EN 0
+#define MBOX_IRQ_DISABLE 1
+#define mbx_rd32(hw, reg) readl((hw)->hw_addr + (reg))
+#define mbx_wr32(hw, reg, val) writel((val), (hw)->hw_addr + (reg))
+
+extern const struct mucse_mbx_operations mucse_mbx_ops_generic;
+
+int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_write_mbx(struct mucse_hw *hw, u32 *msg, u16 size);
+int mucse_check_for_msg(struct mucse_hw *hw);
+int mucse_check_for_ack(struct mucse_hw *hw);
+#endif /* _RNPGBE_MBX_H */
-- 
2.25.1

> +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);
> +	return mbx->ops->read(hw, msg, size);

As well as the obvious bug pointed out by others, isn't this condition
actually indicating a bug somewhere else? If size is bigger than
mbx->size, the caller is broken. You probably want a dev_err() here,
and return -EINVAL, so you get a hint something else is broken
somewhere.

	Andrew

On Fri, Aug 15, 2025 at 01:55:42AM +0200, Andrew Lunn wrote:
> > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	/* limit read size */
> > +	min(size, mbx->size);
> > +	return mbx->ops->read(hw, msg, size);
> 
> As well as the obvious bug pointed out by others, isn't this condition
> actually indicating a bug somewhere else? If size is bigger than
> mbx->size, the caller is broken. You probably want a dev_err() here,
> and return -EINVAL, so you get a hint something else is broken
> somewhere.
> 
> 	Andrew
> 

Ok, the caller is broken when size is bigger than mbx->size. I will use
dev_err here in v5 since I had sent v4 before this mail.
By the way, how long should I wait before sending the new version? If it
is too frequent, it might cause reviewers to check old versions and miss
feedback, link what happened with this mail. And if it is too long, it
is easy to miss the 'open window'....

Thanks for your feedback.

> By the way, how long should I wait before sending the new version? If it
> is too frequent, it might cause reviewers to check old versions and miss
> feedback, link what happened with this mail. And if it is too long, it
> is easy to miss the 'open window'....

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Says at least 24 hours.

But i have been away from emails for a few days, so i was slower than
usual.

Most patches get reviewed in 3 work days. So i would probably not wait
much longer than that. But also wait for any discussion about a patch
to come to an end.

Also, different subsystems work are different speed. 3 days would be
way to fast for USB for example, that would be more like 2 weeks.

	Andrew

On 8/12/2025 3:09 PM, Dong Yibo wrote:
> Initialize basic mbx function.
> 
> Signed-off-by: Dong Yibo <dong100@mucse.com>
> ---
>  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
>  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
>  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
>  6 files changed, 520 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
>  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> 
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> index 42c359f459d9..5fc878ada4b1 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> @@ -6,4 +6,5 @@
>  
>  obj-$(CONFIG_MGBE) += rnpgbe.o
>  rnpgbe-objs := rnpgbe_main.o\
> -	       rnpgbe_chip.o
> +	       rnpgbe_chip.o\
> +	       rnpgbe_mbx.o
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> index 0dd3d3cb2a4d..05830bb73d3e 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> @@ -5,6 +5,7 @@
>  #define _RNPGBE_H
>  
>  #include <linux/types.h>
> +#include <linux/mutex.h>
>  
>  extern const struct rnpgbe_info rnpgbe_n500_info;
>  extern const struct rnpgbe_info rnpgbe_n210_info;
> @@ -40,7 +41,43 @@ struct mucse_mac_info {
>  	void *back;
>  };
>  
> +struct mucse_hw;
> +
> +struct mucse_mbx_operations {
> +	void (*init_params)(struct mucse_hw *hw);
> +	int (*read)(struct mucse_hw *hw, u32 *msg,
> +		    u16 size);
> +	int (*write)(struct mucse_hw *hw, u32 *msg,
> +		     u16 size);
> +	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> +			   u16 size);
> +	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> +			    u16 size);
> +	int (*check_for_msg)(struct mucse_hw *hw);
> +	int (*check_for_ack)(struct mucse_hw *hw);
> +	void (*configure)(struct mucse_hw *hw, int num_vec,
> +			  bool enable);
> +};
> +
> +struct mucse_mbx_stats {
> +	u32 msgs_tx;
> +	u32 msgs_rx;
> +	u32 acks;
> +	u32 reqs;
> +	u32 rsts;
> +};
> +
>  struct mucse_mbx_info {
> +	const struct mucse_mbx_operations *ops;
> +	struct mucse_mbx_stats stats;
> +	u32 timeout;
> +	u32 usec_delay;
> +	u16 size;
> +	u16 fw_req;
> +	u16 fw_ack;
> +	/* lock for only one use mbx */
> +	struct mutex lock;
> +	bool irq_enabled;
>  	/* fw <--> pf mbx */
>  	u32 fw_pf_shm_base;
>  	u32 pf2fw_mbox_ctrl;
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> index 20ec67c9391e..16d0a76114b5 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> @@ -1,8 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
>  
> +#include <linux/string.h>
> +
>  #include "rnpgbe.h"
>  #include "rnpgbe_hw.h"
> +#include "rnpgbe_mbx.h"
>  
>  /**
>   * rnpgbe_init_common - Setup common attribute
> @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
>  
>  	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
>  	mac->back = hw;
> +
> +	hw->mbx.ops = &mucse_mbx_ops_generic;
>  }
>  
>  /**
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> index fc57258537cf..aee037e3219d 100644
> --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> @@ -7,6 +7,8 @@
>  #define RNPGBE_RING_BASE 0x1000
>  #define RNPGBE_MAC_BASE 0x20000
>  #define RNPGBE_ETH_BASE 0x10000
> +/**************** DMA Registers ****************************/
> +#define RNPGBE_DMA_DUMY 0x000c
>  /**************** CHIP Resource ****************************/
>  #define RNPGBE_MAX_QUEUES 8
>  #endif /* _RNPGBE_HW_H */
> diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> new file mode 100644
> index 000000000000..1195cf945ad1
> --- /dev/null
> +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> +
> +#include <linux/pci.h>
> +#include <linux/errno.h>
> +#include <linux/delay.h>
> +#include <linux/iopoll.h>
> +
> +#include "rnpgbe.h"
> +#include "rnpgbe_mbx.h"
> +#include "rnpgbe_hw.h"
> +
> +/**
> + * mucse_read_mbx - Reads a message from 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_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);
> +	return mbx->ops->read(hw, msg, size);
> +}

What's the purpose of min() here if you are anyways passing size to read()?

The min() call needs to be assigned to size, e.g.: size = min(size,
mbx->size);

> +
> +/**
> + * 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
> + **/

> +
> +/**
> + * 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;
> +	int v;
> +

Variable 'v' should be declared as u32 to match the register read.

> +	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> +	hw->mbx.fw_req = v & GENMASK(15, 0);
> +	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> +	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> +}
> +
> +/**
> + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> + * @hw: pointer to the HW structure
> + * @nr_vec: vector number for mbx
> + * @enable: TRUE for enable, FALSE for disable
> + *
> + * This function configure mbx to use interrupt nr_vec.
> + **/
> +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> +				   bool enable)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +	u32 v;
> +
> +	if (enable) {
> +		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> +		hw->mbx.fw_req = v & GENMASK(15, 0);
> +		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> +	} else {
> +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> +		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> +	}
> +}
> +
> +/**
> + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> + * @hw: pointer to the HW structure
> + *
> + * Initializes the hw->mbx struct to correct values for pf mailbox
> + */
> +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	mbx->usec_delay = 100;
> +	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;

Use appropriate constants like USEC_PER_SEC instead of hardcoded values.

> +	mbx->stats.msgs_tx = 0;
> +	mbx->stats.msgs_rx = 0;


-- 
Thanks and Regards,
Md Danish Anwar

On Tue, Aug 12, 2025 at 10:00:57PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Initialize basic mbx function.
> > 
> > Signed-off-by: Dong Yibo <dong100@mucse.com>
> > ---
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
> >  6 files changed, 520 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 42c359f459d9..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -6,4 +6,5 @@
> >  
> >  obj-$(CONFIG_MGBE) += rnpgbe.o
> >  rnpgbe-objs := rnpgbe_main.o\
> > -	       rnpgbe_chip.o
> > +	       rnpgbe_chip.o\
> > +	       rnpgbe_mbx.o
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 0dd3d3cb2a4d..05830bb73d3e 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -5,6 +5,7 @@
> >  #define _RNPGBE_H
> >  
> >  #include <linux/types.h>
> > +#include <linux/mutex.h>
> >  
> >  extern const struct rnpgbe_info rnpgbe_n500_info;
> >  extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -40,7 +41,43 @@ struct mucse_mac_info {
> >  	void *back;
> >  };
> >  
> > +struct mucse_hw;
> > +
> > +struct mucse_mbx_operations {
> > +	void (*init_params)(struct mucse_hw *hw);
> > +	int (*read)(struct mucse_hw *hw, u32 *msg,
> > +		    u16 size);
> > +	int (*write)(struct mucse_hw *hw, u32 *msg,
> > +		     u16 size);
> > +	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> > +			   u16 size);
> > +	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> > +			    u16 size);
> > +	int (*check_for_msg)(struct mucse_hw *hw);
> > +	int (*check_for_ack)(struct mucse_hw *hw);
> > +	void (*configure)(struct mucse_hw *hw, int num_vec,
> > +			  bool enable);
> > +};
> > +
> > +struct mucse_mbx_stats {
> > +	u32 msgs_tx;
> > +	u32 msgs_rx;
> > +	u32 acks;
> > +	u32 reqs;
> > +	u32 rsts;
> > +};
> > +
> >  struct mucse_mbx_info {
> > +	const struct mucse_mbx_operations *ops;
> > +	struct mucse_mbx_stats stats;
> > +	u32 timeout;
> > +	u32 usec_delay;
> > +	u16 size;
> > +	u16 fw_req;
> > +	u16 fw_ack;
> > +	/* lock for only one use mbx */
> > +	struct mutex lock;
> > +	bool irq_enabled;
> >  	/* fw <--> pf mbx */
> >  	u32 fw_pf_shm_base;
> >  	u32 pf2fw_mbox_ctrl;
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 20ec67c9391e..16d0a76114b5 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -1,8 +1,11 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >  
> > +#include <linux/string.h>
> > +
> >  #include "rnpgbe.h"
> >  #include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
> >  
> >  /**
> >   * rnpgbe_init_common - Setup common attribute
> > @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
> >  
> >  	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> >  	mac->back = hw;
> > +
> > +	hw->mbx.ops = &mucse_mbx_ops_generic;
> >  }
> >  
> >  /**
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index fc57258537cf..aee037e3219d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -7,6 +7,8 @@
> >  #define RNPGBE_RING_BASE 0x1000
> >  #define RNPGBE_MAC_BASE 0x20000
> >  #define RNPGBE_ETH_BASE 0x10000
> > +/**************** DMA Registers ****************************/
> > +#define RNPGBE_DMA_DUMY 0x000c
> >  /**************** CHIP Resource ****************************/
> >  #define RNPGBE_MAX_QUEUES 8
> >  #endif /* _RNPGBE_HW_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > new file mode 100644
> > index 000000000000..1195cf945ad1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/pci.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_mbx.h"
> > +#include "rnpgbe_hw.h"
> > +
> > +/**
> > + * mucse_read_mbx - Reads a message from 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_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	/* limit read size */
> > +	min(size, mbx->size);
> > +	return mbx->ops->read(hw, msg, size);
> > +}
> 
> What's the purpose of min() here if you are anyways passing size to read()?
> 
> The min() call needs to be assigned to size, e.g.: size = min(size,
> mbx->size);
> 

Yes, I will fix this error.

> > +
> > +/**
> > + * 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
> > + **/
> 
> > +
> > +/**
> > + * 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;
> > +	int v;
> > +
> 
> Variable 'v' should be declared as u32 to match the register read.
> 

Got it, I will fix it.

> > +	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +	hw->mbx.fw_req = v & GENMASK(15, 0);
> > +	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +}
> > +
> > +/**
> > + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> > + * @hw: pointer to the HW structure
> > + * @nr_vec: vector number for mbx
> > + * @enable: TRUE for enable, FALSE for disable
> > + *
> > + * This function configure mbx to use interrupt nr_vec.
> > + **/
> > +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> > +				   bool enable)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +	u32 v;
> > +
> > +	if (enable) {
> > +		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +		hw->mbx.fw_req = v & GENMASK(15, 0);
> > +		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +	} else {
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> > +	}
> > +}
> > +
> > +/**
> > + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> > + * @hw: pointer to the HW structure
> > + *
> > + * Initializes the hw->mbx struct to correct values for pf mailbox
> > + */
> > +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	mbx->usec_delay = 100;
> > +	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
> 
> Use appropriate constants like USEC_PER_SEC instead of hardcoded values.
> 

Ok, I will update it.

> > +	mbx->stats.msgs_tx = 0;
> > +	mbx->stats.msgs_rx = 0;
> 
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

> + * mucse_read_mbx - Reads a message from 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_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> +{
> +	struct mucse_mbx_info *mbx = &hw->mbx;
> +
> +	/* limit read size */
> +	min(size, mbx->size);

you have to store the result of min() as it's lost now