On 9/29/25 9:45 AM, Mathieu Poirier wrote:
On Sun, Sep 28, 2025 at 03:56:41PM +0800, Peng Fan wrote:
Hi,

On Fri, Sep 26, 2025 at 10:40:09AM -0500, Tanmay Shah wrote:
---
drivers/mailbox/mailbox.c               | 24 ++++++++++++++++++++++++
drivers/remoteproc/xlnx_r5_remoteproc.c |  4 ++++
include/linux/mailbox_client.h          |  1 +

The mailbox and remoteproc should be separated.


Mailbox framework is introducing new API. I wanted the use case to be in the
same patch-set, otherwise we might see unused API warning.

I mean two patches in one patchset.


Agreed

Okay.


Hence, both in the same patch makes more sense. If maintainers prefer, I will
separate them.

3 files changed, 29 insertions(+)

diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 5cd8ae222073..7afdb2c9006d 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -217,6 +217,30 @@ bool mbox_client_peek_data(struct mbox_chan *chan)
}
EXPORT_SYMBOL_GPL(mbox_client_peek_data);

+/**
+ * mbox_queue_full - check if mailbox queue is full or not
+ * @chan: Mailbox channel assigned to this client.
+ *
+ * Clients can choose not to send new msg if mbox queue is full.
+ *
+ * Return: true if queue is full else false. < 0 for error
+ */
+int mbox_queue_full(struct mbox_chan *chan)
+{
+       unsigned long flags;
+       int res;
+
+       if (!chan)
+               return -EINVAL;
+
+       spin_lock_irqsave(&chan->lock, flags);

Use scoped_guard.

Other APIs use spin_lock_irqsave. Probably scoped_guard should be introduced
in a different patch for all APIs in the mailbox.

Your code base seems not up to date.


Agreed


Okay. I was referring to different branch when I referred other API in the mailbox framework.

Sure, I will use scoped_guard.



+       res = (chan->msg_count == (MBOX_TX_QUEUE_LEN - 1));

Please have a look at this condition again - the implementation of
addr_to_rbuf() [1] is checking for space differently.

[1]. https://elixir.bootlin.com/linux/v6.17/source/drivers/mailbox/mailbox.c#L32

+       spin_unlock_irqrestore(&chan->lock, flags);
+
+       return res;
+}
+EXPORT_SYMBOL_GPL(mbox_queue_full);

add_to_rbuf is able to return ENOBUFS when call mbox_send_message.
Does checking mbox_send_message return value works for you?


That is the problem. mbox_send_message uses add_to_rbuf and fails. But during
failure, it prints warning message:

dev_err(chan->mbox->dev, "Try increasing MBOX_TX_QUEUE_LEN\n");

In some cases there are lot of such messages on terminal. Functionally
nothing is wrong and everything is working but user keeps getting false
positive warning about increasing mbox tx queue length. That is why we need
API to check if mbox queue length is full or not before doing
mbox_send_message. Not all clients need to use it, but some cane make use of
it.

I think check whether mbox_send_message returns -ENOBUFS or not should
work for you. If the "Try increasing MBOX_TX_QUEUE_LEN" message
bothers you, it could be update to dev_dbg per my understanding.


This new API is trying to avoid calling mbox_send_message(), no checking if it
succeeded or not.  Moving dev_err() nto dev_dbg() is also the wrong approach.


Correct.

Regards,
Peng



+
/**
   * mbox_send_message -        For client to submit a message to be
   *                            sent to the remote.

Regards
Peng



Reply via email to