[PATCH 2/2 RESEND] mailbox: Make mbox_send_message() return error code when tx fails

[PATCH 2/2 RESEND] mailbox: Make mbox_send_message() return error code when tx fails

[Joonwon Kang]

Previously, when the mailbox controller failed transmitting message, the
error code was only passed to the client's tx done handler and not to
mbox_send_message(). For this reason, the function could return a false
success. This commit resolves the issue by introducing the tx status and
checking it before mbox_send_message() returns.

Cc: stable@vger.kernel.org
Signed-off-by: Joonwon Kang <joonwonkang@google.com>
---
 drivers/mailbox/mailbox.c          | 17 +++++++++++++----
 include/linux/mailbox_controller.h |  2 ++
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 0afe3ae3bfdc..05808ecff774 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -23,7 +23,8 @@
 static LIST_HEAD(mbox_cons);
 static DEFINE_MUTEX(con_mutex);
 
-static int add_to_rbuf(struct mbox_chan *chan, void *mssg, struct completion *tx_complete)
+static int add_to_rbuf(struct mbox_chan *chan, void *mssg, struct completion *tx_complete,
+		       int *tx_status)
 {
 	int idx;
 
@@ -36,6 +37,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg, struct completion *tx
 	idx = chan->msg_free;
 	chan->msg_data[idx].data = mssg;
 	chan->msg_data[idx].tx_complete = tx_complete;
+	chan->msg_data[idx].tx_status = tx_status;
 	chan->msg_count++;
 
 	if (idx == MBOX_TX_QUEUE_LEN - 1)
@@ -87,12 +89,14 @@ static void tx_tick(struct mbox_chan *chan, int r)
 	int idx;
 	void *mssg = NULL;
 	struct completion *tx_complete = NULL;
+	int *tx_status = NULL;
 
 	scoped_guard(spinlock_irqsave, &chan->lock) {
 		idx = chan->active_req;
 		if (idx >= 0) {
 			mssg = chan->msg_data[idx].data;
 			tx_complete = chan->msg_data[idx].tx_complete;
+			tx_status = chan->msg_data[idx].tx_status;
 			chan->active_req = -1;
 		}
 	}
@@ -107,8 +111,10 @@ static void tx_tick(struct mbox_chan *chan, int r)
 	if (chan->cl->tx_done)
 		chan->cl->tx_done(chan->cl, mssg, r);
 
-	if (r != -ETIME && chan->cl->tx_block)
+	if (r != -ETIME && chan->cl->tx_block) {
+		*tx_status = r;
 		complete(tx_complete);
+	}
 }
 
 static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
@@ -253,15 +259,16 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
 {
 	int t;
 	struct completion tx_complete;
+	int tx_status = 0;
 
 	if (!chan || !chan->cl)
 		return -EINVAL;
 
 	if (chan->cl->tx_block) {
 		init_completion(&tx_complete);
-		t = add_to_rbuf(chan, mssg, &tx_complete);
+		t = add_to_rbuf(chan, mssg, &tx_complete, &tx_status);
 	} else {
-		t = add_to_rbuf(chan, mssg, NULL);
+		t = add_to_rbuf(chan, mssg, NULL, NULL);
 	}
 
 	if (t < 0) {
@@ -284,6 +291,8 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
 		if (ret == 0) {
 			t = -ETIME;
 			tx_tick(chan, t);
+		} else if (tx_status < 0) {
+			t = tx_status;
 		}
 	}
 
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 67e08a440f5f..6929774d3129 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -109,10 +109,12 @@ struct mbox_controller {
  * struct mbox_message - Internal representation of a mailbox message
  * @data:		Data packet
  * @tx_complete:	Pointer to the transmission completion
+ * @tx_status:		Pointer to the transmission status
  */
 struct mbox_message {
 	void *data;
 	struct completion *tx_complete;
+	int *tx_status;
 };
 
 /**
-- 
2.52.0.239.gd5f0c6e74e-goog

Re: [PATCH 2/2 RESEND] mailbox: Make mbox_send_message() return error code when tx fails

[Jassi Brar]

On Tue, Dec 16, 2025 at 2:44 AM Joonwon Kang <joonwonkang@google.com> wrote:
>
> Previously, when the mailbox controller failed transmitting message, the
> error code was only passed to the client's tx done handler and not to
> mbox_send_message(). For this reason, the function could return a false
> success. This commit resolves the issue by introducing the tx status and
> checking it before mbox_send_message() returns.
>
A client submitted the message, and that client gets the actual
status. mbox_send_message does not (can not) tell if the message was
successfully sent or not. For example, consider non-blocking mode when
mbox_send_message() immediately returns after simply placing the
message in the fifo. It returns 0, but still the message transmission
may fail when its turn comes. So I think it is fine as is.
Thanks,
Jassi

Re: [PATCH 2/2 RESEND] mailbox: Make mbox_send_message() return error

[Joonwon Kang]

> > Previously, when the mailbox controller failed transmitting message, the
> > error code was only passed to the client's tx done handler and not to
> > mbox_send_message(). For this reason, the function could return a false
> > success. This commit resolves the issue by introducing the tx status and
> > checking it before mbox_send_message() returns.
> >
> A client submitted the message, and that client gets the actual
> status. mbox_send_message does not (can not) tell if the message was
> successfully sent or not. For example, consider non-blocking mode when
> mbox_send_message() immediately returns after simply placing the
> message in the fifo. It returns 0, but still the message transmission
> may fail when its turn comes. So I think it is fine as is.

When it comes to non-blocking send function, the common expectation is that
users of the function do not expect the final send result in its return value
of the non-blocking function. And it makes sense for users to register a
callback to collect the final send result. It is a common practice for non-
blocking function.

For blocking send function, however, it will be quite unexpected behavior if
the blocking send function returns no error code but it actually has failed.
Also, it will be uncommon if users of the blocking send function have to
register additional callback just to collect the final send result. If that is
the case, the blocking function requires of users both blocking and callback,
which is redundant.

Overall, I think it will be better if we return error code in blocking mode if
it has actually failed transmiting the request. How do you think?