Clients may want to send interrupt only without any useful message
involved. Since the current mailbox framework does not allow NULL
message sending(although it allows receiving it), the clients should
allocate a dummy message buffer and pretend sending it. Besides, if
the mailbox controller calls `mbox_chan_txdone()` when the client
drivers happen to send NULL message anyway, it will result in unexpected
results by making the tx status messed up. This commit lifts the
limitation and allows the clients to send interrupt only without any
message buffer allocated.

Signed-off-by: Joonwon Kang <joonwonkang@google.com>
---
 drivers/mailbox/mailbox.c          | 20 ++++++++++++--------
 drivers/mailbox/pcc.c              |  6 +++---
 drivers/mailbox/tegra-hsp.c        |  2 +-
 include/linux/mailbox_controller.h |  2 +-
 4 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 2acc6ec229a4..0007a023112c 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -52,7 +52,7 @@ static void msg_submit(struct mbox_chan *chan)
 	int err = -EBUSY;
 
 	scoped_guard(spinlock_irqsave, &chan->lock) {
-		if (!chan->msg_count || chan->active_req)
+		if (!chan->msg_count || chan->active_req >= 0)
 			break;
 
 		count = chan->msg_count;
@@ -69,7 +69,7 @@ static void msg_submit(struct mbox_chan *chan)
 		/* Try to submit a message to the MBOX controller */
 		err = chan->mbox->ops->send_data(chan, data);
 		if (!err) {
-			chan->active_req = data;
+			chan->active_req = idx;
 			chan->msg_count--;
 		}
 	}
@@ -83,17 +83,21 @@ static void msg_submit(struct mbox_chan *chan)
 
 static void tx_tick(struct mbox_chan *chan, int r)
 {
+	int idx;
 	void *mssg;
 
 	scoped_guard(spinlock_irqsave, &chan->lock) {
-		mssg = chan->active_req;
-		chan->active_req = NULL;
+		idx = chan->active_req;
+		if (idx >= 0) {
+			mssg = chan->msg_data[idx];
+			chan->active_req = -1;
+		}
 	}
 
 	/* Submit next message */
 	msg_submit(chan);
 
-	if (!mssg)
+	if (idx < 0)
 		return;
 
 	/* Notify the client */
@@ -114,7 +118,7 @@ static enum hrtimer_restart txdone_hrtimer(struct hrtimer *hrtimer)
 	for (i = 0; i < mbox->num_chans; i++) {
 		struct mbox_chan *chan = &mbox->chans[i];
 
-		if (chan->active_req && chan->cl) {
+		if (chan->active_req >= 0 && chan->cl) {
 			txdone = chan->mbox->ops->last_tx_done(chan);
 			if (txdone)
 				tx_tick(chan, 0);
@@ -319,7 +323,7 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl)
 	scoped_guard(spinlock_irqsave, &chan->lock) {
 		chan->msg_free = 0;
 		chan->msg_count = 0;
-		chan->active_req = NULL;
+		chan->active_req = -1;
 		chan->cl = cl;
 		init_completion(&chan->tx_complete);
 
@@ -477,7 +481,7 @@ void mbox_free_channel(struct mbox_chan *chan)
 	/* The queued TX requests are simply aborted, no callbacks are made */
 	scoped_guard(spinlock_irqsave, &chan->lock) {
 		chan->cl = NULL;
-		chan->active_req = NULL;
+		chan->active_req = -1;
 		if (chan->txdone_method == TXDONE_BY_ACK)
 			chan->txdone_method = TXDONE_BY_POLL;
 	}
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 0a00719b2482..7eca06d5ecf6 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -333,7 +333,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
-	struct pcc_header *pcc_header = chan->active_req;
+	struct pcc_header *pcc_header = NULL;
 	void *handle = NULL;
 
 	pchan = chan->con_priv;
@@ -362,8 +362,8 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	if (pchan->chan.rx_alloc)
 		handle = write_response(pchan);
 
-	if (chan->active_req) {
-		pcc_header = chan->active_req;
+	if (chan->active_req >= 0) {
+		pcc_header = chan->msg_data[chan->active_req];
 		if (pcc_header->flags & PCC_CMD_COMPLETION_NOTIFY)
 			mbox_chan_txdone(chan, 0);
 	}
diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
index ed9a0bb2bcd8..de7494ce0a9f 100644
--- a/drivers/mailbox/tegra-hsp.c
+++ b/drivers/mailbox/tegra-hsp.c
@@ -497,7 +497,7 @@ static int tegra_hsp_mailbox_flush(struct mbox_chan *chan,
 			mbox_chan_txdone(chan, 0);
 
 			/* Wait until channel is empty */
-			if (chan->active_req != NULL)
+			if (chan->active_req >= 0)
 				continue;
 
 			return 0;
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
index 80a427c7ca29..230d2669861d 100644
--- a/include/linux/mailbox_controller.h
+++ b/include/linux/mailbox_controller.h
@@ -123,7 +123,7 @@ struct mbox_chan {
 	unsigned txdone_method;
 	struct mbox_client *cl;
 	struct completion tx_complete;
-	void *active_req;
+	int active_req;
 	unsigned msg_count, msg_free;
 	void *msg_data[MBOX_TX_QUEUE_LEN];
 	spinlock_t lock; /* Serialise access to the channel */
-- 
2.52.0.487.g5c8c507ade-goog

On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
>
> Clients may want to send interrupt only without any useful message
> involved. Since the current mailbox framework does not allow NULL
> message sending(although it allows receiving it), the clients should
> allocate a dummy message buffer and pretend sending it. Besides, if
> the mailbox controller calls `mbox_chan_txdone()` when the client
> drivers happen to send NULL message anyway, it will result in unexpected
> results by making the tx status messed up. This commit lifts the
> limitation and allows the clients to send interrupt only without any
> message buffer allocated.
>
Interrupts without data messages are called 'doorbells' and we already
support them.
thanks

On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> >
> > Clients may want to send interrupt only without any useful message
> > involved. Since the current mailbox framework does not allow NULL
> > message sending(although it allows receiving it), the clients should
> > allocate a dummy message buffer and pretend sending it. Besides, if
> > the mailbox controller calls `mbox_chan_txdone()` when the client
> > drivers happen to send NULL message anyway, it will result in unexpected
> > results by making the tx status messed up. This commit lifts the
> > limitation and allows the clients to send interrupt only without any
> > message buffer allocated.
> >
> Interrupts without data messages are called 'doorbells' and we already
> support them.
> thanks

I am not sure if it is already supported. Let me draw two cases which imply
that it is not supported. If the cases make sense, could you reconsider the
patch? If it is supported in another branch, could you refer me to that
branch? I am currently referring to the `for-next` branch of your mailbox
repo.

Case 1: Send API never returns.

```
mbox_client->tx_block = true;

mbox_chan = mbox_request_channel(mbox_client, index);

/*
 * This send function never returns. It is because the mailbox framework
 * will **not** call `complete(&mbox_chan->tx_complete)` when the sent
 * message is NULL. Refer to `tx_tick()` implementation.
 */
ret = mbox_send_message(mbox_chan, NULL);
```

Case 2: TX status is messed up when NULL and non-NULL messages are sent.

```
/* Thread 1: Send NULL message to the remote. */
ret = mbox_send_message(mbox_chan, NULL);

/*
 * Let's say that the mailbox controller has not received an ACK interrupt
 * from the remote. In this case, the mailbox framework should **not** try
 * picking up the next message from `mbox_chan->msg_data[]` and sending it.
 */

/*
 * Thread 2: However, the mailbox framework will think that there is no
 * pending message in `mbox_chan->msg_data[]`, because `mbox_chan->active_req`
 * is NULL, and thus try sending the message of `msg_buf`. Now,
 * `mbox_chan->active_req` will point to `msg_buf`.
 */
ret = mbox_send_message(mbox_chan, &msg_buf);

/*
 * Let's say that the mailbox controller now has received an ACK interrupt
 * for the NULL message sent by Thread 1 and not received it for Thread 2 yet.
 * Then, it will call in chain `mbox_chan_txdone(mbox_chan, 0)` -->
 * `tx_tick(mbox_chan, 0)`, and the last function will mark as if the message
 * sent by Thread 2 is done by assigning `mbox_chan->active_req` with NULL.
 * Now, the TX status is messed up.
 */
```

Thanks.

On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@google.com> wrote:
>
> On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > >
> > > Clients may want to send interrupt only without any useful message
> > > involved. Since the current mailbox framework does not allow NULL
> > > message sending(although it allows receiving it), the clients should
> > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > drivers happen to send NULL message anyway, it will result in unexpected
> > > results by making the tx status messed up. This commit lifts the
> > > limitation and allows the clients to send interrupt only without any
> > > message buffer allocated.
> > >
> > Interrupts without data messages are called 'doorbells' and we already
> > support them.
> > thanks
>
> I am not sure if it is already supported. Let me draw two cases which imply
> that it is not supported. If the cases make sense, could you reconsider the
> patch? If it is supported in another branch, could you refer me to that
> branch? I am currently referring to the `for-next` branch of your mailbox
> repo.
>
I believe you are talking about some hypothetical situation?
Otherwise, which controller is that?
A controller driver is supposed to either expect data or not, but not both.

thanks
-jassi

On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@google.com> wrote:
> >
> > On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > > >
> > > > Clients may want to send interrupt only without any useful message
> > > > involved. Since the current mailbox framework does not allow NULL
> > > > message sending(although it allows receiving it), the clients should
> > > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > > drivers happen to send NULL message anyway, it will result in unexpected
> > > > results by making the tx status messed up. This commit lifts the
> > > > limitation and allows the clients to send interrupt only without any
> > > > message buffer allocated.
> > > >
> > > Interrupts without data messages are called 'doorbells' and we already
> > > support them.
> > > thanks
> >
> > I am not sure if it is already supported. Let me draw two cases which imply
> > that it is not supported. If the cases make sense, could you reconsider the
> > patch? If it is supported in another branch, could you refer me to that
> > branch? I am currently referring to the `for-next` branch of your mailbox
> > repo.
> >
> I believe you are talking about some hypothetical situation?
> Otherwise, which controller is that?
> A controller driver is supposed to either expect data or not, but not both.

I did not notice this controller's expectation since I could not find this info
in the API doc. So, now I believe that Case 2 could be seen as a hypothetical
situation. However, what about Case 1? If the message to send is NULL,
`chan->cl->tx_done(chan->cl, mssg, r)` and `complete(&chan->tx_complete)` will
**never** be called from `tx_tick()`. It also means that there is no way for a
client to know that the sending is really done or not. Even though a controller
driver(I mean any typical controller, not a specific controller) calls
`mbox_chan_txdone()` after receiving the corresponding ACK interrupt from the
remote to the previously sent NULL message, the client will be blocking not
knowing the sending is done.

Thanks.

On Mon, Dec 8, 2025 at 12:51 AM Joonwon Kang <joonwonkang@google.com> wrote:
>
> On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > >
> > > On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > > > >
> > > > > Clients may want to send interrupt only without any useful message
> > > > > involved. Since the current mailbox framework does not allow NULL
> > > > > message sending(although it allows receiving it), the clients should
> > > > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > > > drivers happen to send NULL message anyway, it will result in unexpected
> > > > > results by making the tx status messed up. This commit lifts the
> > > > > limitation and allows the clients to send interrupt only without any
> > > > > message buffer allocated.
> > > > >
> > > > Interrupts without data messages are called 'doorbells' and we already
> > > > support them.
> > > > thanks
> > >
> > > I am not sure if it is already supported. Let me draw two cases which imply
> > > that it is not supported. If the cases make sense, could you reconsider the
> > > patch? If it is supported in another branch, could you refer me to that
> > > branch? I am currently referring to the `for-next` branch of your mailbox
> > > repo.
> > >
> > I believe you are talking about some hypothetical situation?
> > Otherwise, which controller is that?
> > A controller driver is supposed to either expect data or not, but not both.
>
> I did not notice this controller's expectation since I could not find this info
> in the API doc. So, now I believe that Case 2 could be seen as a hypothetical
> situation. However, what about Case 1? If the message to send is NULL,
> `chan->cl->tx_done(chan->cl, mssg, r)` and `complete(&chan->tx_complete)` will
> **never** be called from `tx_tick()`. It also means that there is no way for a
> client to know that the sending is really done or not. Even though a controller
> driver(I mean any typical controller, not a specific controller) calls
> `mbox_chan_txdone()` after receiving the corresponding ACK interrupt from the
> remote to the previously sent NULL message, the client will be blocking not
> knowing the sending is done.
>
For non-data channels (doorbells), the driver may expect the doorbell
info at runtime
via 'mssg'. If it doesn't, the clients send '0' or any arbitrary
non-null value which remains
unused by anyone. Maybe the api can define some MAILBOX_NULL_DATA integer that
can be used instead because the controller driver anyway doesn't need it.
If/when the doorbell is rung is upto the controller - it may be just
setting a bit or observing
a status bit for the remote or whatever. The driver must say tx_done()
appropriately.

Again, it will help if we talk about the controller you have in mind exactly.

Cheers!

> On Mon, Dec 8, 2025 at 12:51 AM Joonwon Kang <joonwonkang@google.com> wrote:
> >
> > On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > > >
> > > > On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > > > > >
> > > > > > Clients may want to send interrupt only without any useful message
> > > > > > involved. Since the current mailbox framework does not allow NULL
> > > > > > message sending(although it allows receiving it), the clients should
> > > > > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > > > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > > > > drivers happen to send NULL message anyway, it will result in unexpected
> > > > > > results by making the tx status messed up. This commit lifts the
> > > > > > limitation and allows the clients to send interrupt only without any
> > > > > > message buffer allocated.
> > > > > >
> > > > > Interrupts without data messages are called 'doorbells' and we already
> > > > > support them.
> > > > > thanks
> > > >
> > > > I am not sure if it is already supported. Let me draw two cases which imply
> > > > that it is not supported. If the cases make sense, could you reconsider the
> > > > patch? If it is supported in another branch, could you refer me to that
> > > > branch? I am currently referring to the `for-next` branch of your mailbox
> > > > repo.
> > > >
> > > I believe you are talking about some hypothetical situation?
> > > Otherwise, which controller is that?
> > > A controller driver is supposed to either expect data or not, but not both.
> >
> > I did not notice this controller's expectation since I could not find this info
> > in the API doc. So, now I believe that Case 2 could be seen as a hypothetical
> > situation. However, what about Case 1? If the message to send is NULL,
> > `chan->cl->tx_done(chan->cl, mssg, r)` and `complete(&chan->tx_complete)` will
> > **never** be called from `tx_tick()`. It also means that there is no way for a
> > client to know that the sending is really done or not. Even though a controller
> > driver(I mean any typical controller, not a specific controller) calls
> > `mbox_chan_txdone()` after receiving the corresponding ACK interrupt from the
> > remote to the previously sent NULL message, the client will be blocking not
> > knowing the sending is done.
> >
> For non-data channels (doorbells), the driver may expect the doorbell
> info at runtime
> via 'mssg'. If it doesn't, the clients send '0' or any arbitrary
> non-null value which remains
> unused by anyone.

By "the driver", I believe that you meant the "controller" driver. If so, yes
the controller driver may or, more importantly, may **not** expect data via
`mssg` as you clarified earlier: "A controller driver is supposed to either
expect data or not, but not both". Also, I think it is the controller driver's
responsibility to check if the `mssg` is NULL or not when it needs it, and the
mailbox "framework" should treat the data/message just as blackbox. In other
words, the framework should not enforce that any `mssg` to be sent should be
non-NULL. Currently, clients should send the unused data even when no data is
expected or should unconditionally believe that the tx is done and not use
blocking mode. This limitation could be lifted by this patch.

> Maybe the api can define some MAILBOX_NULL_DATA integer that
> can be used instead because the controller driver anyway doesn't need it.

I guess you suggested an alternative here. If we add such integer to the client
API, however, it will be a big change to the clients since now they should be
aware of the additional contract on the new integer when sending a doorbell:
they should go with `mbox_send_message(.*, MAILBOX_NULL_DATA)`. Instead, it
will be better if clients could just call `mbox_send_message(.*, NULL)` without
having that additional API requirement.

Or if you meant instead to add the new integer but hide it from the clients,
I think I can upload another patch for that, but it may be equivalent to the
solution in this patch. Could you help me understand the benefit your
alternative has over this patch, if you meant this direction?

> If/when the doorbell is rung is upto the controller - it may be just
> setting a bit or observing
> a status bit for the remote or whatever. The driver must say tx_done()
> appropriately.

If you meant here that the doorbell sending status is assumed to be checked
only by polling, it does not sound generic enough to me. If a device supports
ACK "interrupt" for the doorbell, I think that the mailbox framework should
support it since the mailbox API already claims to support both IRQ and polling
for tx done: TXDONE_BY_IRQ and TXDONE_BY_POLL.

> Again, it will help if we talk about the controller you have in mind exactly.

Sorry that I am working on company private mailbox controller drivers which
have not been upstreamed. What I can say is that the mailbox devices support
TXDONE_BY_IRQ, not TXDONE_BY_POLL, and is to send doorbell. Since NULL message
sending is currently not supported on the framework, we may have to enforce
clients to send unused non-NULL data, which is not practically possible since
the clients are out of our control, or ignore the ACK interrupt entirely and
enforce clients not to use blocking mode, which is also not practically
possible and not good for system stability. I believe the case of TXDONE_BY_IRQ
+ doorbell is generic enough, not exceptional, and should be supported on the
current mailbox framework. If `mbox_send_message(.*, NULL)` just works out, the
problems will be resolved.

Thanks!

On Mon, 08 Dec 2025, Joonwon Kang wrote:

> On Wed, Dec 3, 2025 at 11:57 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > >
> > > On Tue, Nov 25, 2025 at 11:00 PM Joonwon Kang <joonwonkang@google.com> wrote:
> > > > >
> > > > > Clients may want to send interrupt only without any useful message
> > > > > involved. Since the current mailbox framework does not allow NULL
> > > > > message sending(although it allows receiving it), the clients should
> > > > > allocate a dummy message buffer and pretend sending it. Besides, if
> > > > > the mailbox controller calls `mbox_chan_txdone()` when the client
> > > > > drivers happen to send NULL message anyway, it will result in unexpected
> > > > > results by making the tx status messed up. This commit lifts the
> > > > > limitation and allows the clients to send interrupt only without any
> > > > > message buffer allocated.
> > > > >
> > > > Interrupts without data messages are called 'doorbells' and we already
> > > > support them.
> > > > thanks
> > >
> > > I am not sure if it is already supported. Let me draw two cases which imply
> > > that it is not supported. If the cases make sense, could you reconsider the
> > > patch? If it is supported in another branch, could you refer me to that
> > > branch? I am currently referring to the `for-next` branch of your mailbox
> > > repo.
> > >
> > I believe you are talking about some hypothetical situation?
> > Otherwise, which controller is that?
> > A controller driver is supposed to either expect data or not, but not both.
> 
> I did not notice this controller's expectation since I could not find this info
> in the API doc. So, now I believe that Case 2 could be seen as a hypothetical
> situation. However, what about Case 1? If the message to send is NULL,
> `chan->cl->tx_done(chan->cl, mssg, r)` and `complete(&chan->tx_complete)` will
> **never** be called from `tx_tick()`. It also means that there is no way for a
> client to know that the sending is really done or not. Even though a controller
> driver(I mean any typical controller, not a specific controller) calls
> `mbox_chan_txdone()` after receiving the corresponding ACK interrupt from the
> remote to the previously sent NULL message, the client will be blocking not
> knowing the sending is done.

It's been a long time since I had anything to do with the Mailbox
subsystem, but as a drive-by comment, I wonder if my mailbox-test driver
could be expanded to test and prove out the concept of doorbells.

-- 
Lee Jones [李琼斯]