On Tue, Sep 30, 2025 at 11:52 AM Tanmay Shah <[email protected]> wrote: > > Hi Jassi, > > Please find my comments below. > > On 9/30/25 9:11 AM, Jassi Brar wrote: > > On Thu, Sep 25, 2025 at 1:51 PM Tanmay Shah <[email protected]> wrote: > >> > >> Sometimes clients need to know if mailbox queue is full or not before > >> posting new message via mailbox. If mailbox queue is full clients can > >> choose not to post new message. This doesn't mean current queue length > >> should be increased, but clients may want to wait till previous Tx is > >> done. This API can help avoid false positive warning from mailbox > >> framework "Try increasing MBOX_TX_QUEUE_LEN". > >> > >> Signed-off-by: Tanmay Shah <[email protected]> > >> --- > >> drivers/mailbox/mailbox.c | 24 ++++++++++++++++++++++++ > >> drivers/remoteproc/xlnx_r5_remoteproc.c | 4 ++++ > >> include/linux/mailbox_client.h | 1 + > >> 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); > >> + res = (chan->msg_count == (MBOX_TX_QUEUE_LEN - 1)); > >> > > 1) If we really need this, it should be > > res = (chan->msg_count == MBOX_TX_QUEUE_LEN); > > > > Ack here. > > > 2) I am thinking instead, introduce a > > struct mbox_client.msg_slots_ro; > > Which is a read-only field for the client, denoting how many message > > slots are currently available. > > The mailbox api will always adjust it when msg_count changes... > > chan->cl->msg_slots_ro = MBOX_TX_QUEUE_LEN - chan->msg_count; > > > > It's not possible to make msg_slots_ro true Read-Only. Nothing prevents > clients to write to that variable as far as I know. > Correct, nothing prevents a client from changing 'msg_slots_ro', just like nothing prevents it from setting tx_done or rx_callback to 0xdeadbabe. The '_ro' suffix is to tell the client developer to not mess with it. I am slightly more inclined towards this approach because it avoids adding another convenience api and is more immediately available without needing a spinlock.
-jassi
