* [PATCH v4 0/3] mailbox queue length check
@ 2026-02-03 22:58 Tanmay Shah
2026-02-03 22:58 ` [PATCH v4 1/3] mailbox: check mailbox queue is full or not Tanmay Shah
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tanmay Shah @ 2026-02-03 22:58 UTC (permalink / raw)
To: jassisinghbrar, andersson, mathieu.poirier
Cc: linux-kernel, linux-remoteproc, Tanmay Shah
Provide mechanism to check mailbox queue length before posting new
message to the mailbox queue. Use this mechanims in xlnx r5 remoteproc
driver.
Changes in v4:
- %s/msg_slot_ro/tx_slots_avail_ro/
- do not use msg_count to calculate tx_slots_avail_ro instead modify
it directly
- separate xlnx remoteproc driver patch in to two patches
Changes in v3:
- move msg_slot_ro to mbox_client instead of mbox_chan
- allocate separate mailbox client to the "tx" mbox channel
- modify rest of the patch to use msg_slot_ro from mbox_client
Changes in v2:
- Split patchset in two patches for two different subsystems.
- Change design and introduce new field per channel to track mbox
queue length.
- use this new field in xlnx remoteproc driver to prevent sending new
messages if no slots available in the mbox tx channel queue.
Tanmay Shah (3):
mailbox: check mailbox queue is full or not
remoteproc: xlnx: assign mbox client per mbox channel
remoteproc: xlnx: do not send new mbox notification
drivers/mailbox/mailbox.c | 3 +++
drivers/remoteproc/xlnx_r5_remoteproc.c | 26 +++++++++++++++++++------
include/linux/mailbox_client.h | 2 ++
3 files changed, 25 insertions(+), 6 deletions(-)
base-commit: fa3e2d1dc10864887674c484372a9ccc6a01eb18
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v4 1/3] mailbox: check mailbox queue is full or not 2026-02-03 22:58 [PATCH v4 0/3] mailbox queue length check Tanmay Shah @ 2026-02-03 22:58 ` Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 3/3] remoteproc: xlnx: do not send new mbox notification Tanmay Shah 2 siblings, 0 replies; 6+ messages in thread From: Tanmay Shah @ 2026-02-03 22:58 UTC (permalink / raw) To: jassisinghbrar, andersson, mathieu.poirier Cc: linux-kernel, linux-remoteproc, Tanmay Shah 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. Introduce variable per channel to track available tx msg slots. Clients can check this variable and decide not to send new message if it is 0. This will help avoid false positive warning from mailbox framework "Try increasing MBOX_TX_QUEUE_LEN". Signed-off-by: Tanmay Shah <tanmay.shah@amd.com> --- v4: - %s/msg_slot_ro/tx_slots_avail_ro/ - do not use msg_count to calculate tx_slots_avail_ro instead modify it directly v3: - move msg_slot_ro to mbox_client instead of mbox_chan - modify rest of the patch to use msg_slot_ro from mbox_client v2: - change design and introduce msg_slot_ro field for each channel instead of API. Clients can use this variable directly. - remove mbox_queue_full API and use msg_slot_ro to know if single slot is available in msg queue or not. drivers/mailbox/mailbox.c | 3 +++ include/linux/mailbox_client.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c index 2acc6ec229a4..8228e6a9ef97 100644 --- a/drivers/mailbox/mailbox.c +++ b/drivers/mailbox/mailbox.c @@ -36,6 +36,7 @@ static int add_to_rbuf(struct mbox_chan *chan, void *mssg) idx = chan->msg_free; chan->msg_data[idx] = mssg; chan->msg_count++; + chan->cl->tx_slots_avail_ro--; if (idx == MBOX_TX_QUEUE_LEN - 1) chan->msg_free = 0; @@ -71,6 +72,7 @@ static void msg_submit(struct mbox_chan *chan) if (!err) { chan->active_req = data; chan->msg_count--; + chan->cl->tx_slots_avail_ro++; } } @@ -321,6 +323,7 @@ static int __mbox_bind_client(struct mbox_chan *chan, struct mbox_client *cl) chan->msg_count = 0; chan->active_req = NULL; chan->cl = cl; + chan->cl->tx_slots_avail_ro = MBOX_TX_QUEUE_LEN; init_completion(&chan->tx_complete); if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h index c6eea9afb943..e867f3d5c89b 100644 --- a/include/linux/mailbox_client.h +++ b/include/linux/mailbox_client.h @@ -17,6 +17,7 @@ struct mbox_chan; * @dev: The client device * @tx_block: If the mbox_send_message should block until data is * transmitted. + * @tx_slots_avail_ro: msg slots remaining for this client's channel. * @tx_tout: Max block period in ms before TX is assumed failure * @knows_txdone: If the client could run the TX state machine. Usually * if the client receives some ACK packet for transmission. @@ -29,6 +30,7 @@ struct mbox_chan; struct mbox_client { struct device *dev; bool tx_block; + unsigned int tx_slots_avail_ro; unsigned long tx_tout; bool knows_txdone; -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel 2026-02-03 22:58 [PATCH v4 0/3] mailbox queue length check Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 1/3] mailbox: check mailbox queue is full or not Tanmay Shah @ 2026-02-03 22:58 ` Tanmay Shah 2026-02-09 23:27 ` Jassi Brar 2026-02-03 22:58 ` [PATCH v4 3/3] remoteproc: xlnx: do not send new mbox notification Tanmay Shah 2 siblings, 1 reply; 6+ messages in thread From: Tanmay Shah @ 2026-02-03 22:58 UTC (permalink / raw) To: jassisinghbrar, andersson, mathieu.poirier Cc: linux-kernel, linux-remoteproc, Tanmay Shah Sharing mbox client data structure between "tx" and "rx" channels can lead to data corruption. Instead each channel should have its own mbox client data structure. Signed-off-by: Tanmay Shah <tanmay.shah@amd.com> --- changes in v4: - separate remoteproc driver patch in to two patches drivers/remoteproc/xlnx_r5_remoteproc.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c index bd619a6c42aa..109831c5815c 100644 --- a/drivers/remoteproc/xlnx_r5_remoteproc.c +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c @@ -74,7 +74,8 @@ struct zynqmp_sram_bank { * @tx_mc_buf: to copy data to mailbox tx channel * @r5_core: this mailbox's corresponding r5_core pointer * @mbox_work: schedule work after receiving data from mailbox - * @mbox_cl: mailbox client + * @mbox_tx_cl: tx channel mailbox client + * @mbox_rx_cl: rx channel mailbox client * @tx_chan: mailbox tx channel * @rx_chan: mailbox rx channel */ @@ -83,7 +84,8 @@ struct mbox_info { unsigned char tx_mc_buf[MBOX_CLIENT_BUF_MAX]; struct zynqmp_r5_core *r5_core; struct work_struct mbox_work; - struct mbox_client mbox_cl; + struct mbox_client mbox_tx_cl; + struct mbox_client mbox_rx_cl; struct mbox_chan *tx_chan; struct mbox_chan *rx_chan; }; @@ -230,7 +232,7 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg) struct mbox_info *ipi; size_t len; - ipi = container_of(cl, struct mbox_info, mbox_cl); + ipi = container_of(cl, struct mbox_info, mbox_rx_cl); /* copy data from ipi buffer to r5_core */ ipi_msg = (struct zynqmp_ipi_message *)msg; @@ -269,8 +271,8 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) if (!ipi) return NULL; - mbox_cl = &ipi->mbox_cl; - mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; + mbox_cl = &ipi->mbox_tx_cl; + mbox_cl->rx_callback = NULL; mbox_cl->tx_block = false; mbox_cl->knows_txdone = false; mbox_cl->tx_done = NULL; @@ -285,6 +287,13 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) return NULL; } + mbox_cl = &ipi->mbox_rx_cl; + mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; + mbox_cl->tx_block = false; + mbox_cl->knows_txdone = false; + mbox_cl->tx_done = NULL; + mbox_cl->dev = cdev; + ipi->rx_chan = mbox_request_channel_byname(mbox_cl, "rx"); if (IS_ERR(ipi->rx_chan)) { mbox_free_channel(ipi->tx_chan); -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel 2026-02-03 22:58 ` [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel Tanmay Shah @ 2026-02-09 23:27 ` Jassi Brar 2026-02-10 17:12 ` Shah, Tanmay 0 siblings, 1 reply; 6+ messages in thread From: Jassi Brar @ 2026-02-09 23:27 UTC (permalink / raw) To: Tanmay Shah; +Cc: andersson, mathieu.poirier, linux-kernel, linux-remoteproc On Tue, Feb 3, 2026 at 4:59 PM Tanmay Shah <tanmay.shah@amd.com> wrote: > > Sharing mbox client data structure between "tx" and "rx" channels > can lead to data corruption. Instead each channel should have its own > mbox client data structure. > > Signed-off-by: Tanmay Shah <tanmay.shah@amd.com> > --- > > changes in v4: > - separate remoteproc driver patch in to two patches > > drivers/remoteproc/xlnx_r5_remoteproc.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c > index bd619a6c42aa..109831c5815c 100644 > --- a/drivers/remoteproc/xlnx_r5_remoteproc.c > +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c > @@ -74,7 +74,8 @@ struct zynqmp_sram_bank { > * @tx_mc_buf: to copy data to mailbox tx channel > * @r5_core: this mailbox's corresponding r5_core pointer > * @mbox_work: schedule work after receiving data from mailbox > - * @mbox_cl: mailbox client > + * @mbox_tx_cl: tx channel mailbox client > + * @mbox_rx_cl: rx channel mailbox client > * @tx_chan: mailbox tx channel > * @rx_chan: mailbox rx channel > */ > @@ -83,7 +84,8 @@ struct mbox_info { > unsigned char tx_mc_buf[MBOX_CLIENT_BUF_MAX]; > struct zynqmp_r5_core *r5_core; > struct work_struct mbox_work; > - struct mbox_client mbox_cl; > + struct mbox_client mbox_tx_cl; > + struct mbox_client mbox_rx_cl; > struct mbox_chan *tx_chan; > struct mbox_chan *rx_chan; > }; > @@ -230,7 +232,7 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg) > struct mbox_info *ipi; > size_t len; > > - ipi = container_of(cl, struct mbox_info, mbox_cl); > + ipi = container_of(cl, struct mbox_info, mbox_rx_cl); > > /* copy data from ipi buffer to r5_core */ > ipi_msg = (struct zynqmp_ipi_message *)msg; > @@ -269,8 +271,8 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) > if (!ipi) > return NULL; > > - mbox_cl = &ipi->mbox_cl; > - mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; > + mbox_cl = &ipi->mbox_tx_cl; > + mbox_cl->rx_callback = NULL; > mbox_cl->tx_block = false; > mbox_cl->knows_txdone = false; > mbox_cl->tx_done = NULL; > @@ -285,6 +287,13 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) > return NULL; > } > > + mbox_cl = &ipi->mbox_rx_cl; > + mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; > + mbox_cl->tx_block = false; > + mbox_cl->knows_txdone = false; > + mbox_cl->tx_done = NULL; > + mbox_cl->dev = cdev; > + hmm... this looks bad. Sorry. I think all we need is to export unsigned int mbox_chan_tx_slots_available(struct mbox_chan *chan) { return MBOX_TX_QUEUE_LEN - chan->msg_count} from mailbox.c, of course with protection of the channel lock. That should work for you, right? Regards, Jassi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel 2026-02-09 23:27 ` Jassi Brar @ 2026-02-10 17:12 ` Shah, Tanmay 0 siblings, 0 replies; 6+ messages in thread From: Shah, Tanmay @ 2026-02-10 17:12 UTC (permalink / raw) To: Jassi Brar, Tanmay Shah Cc: andersson, mathieu.poirier, linux-kernel, linux-remoteproc On 2/9/2026 5:27 PM, Jassi Brar wrote: > On Tue, Feb 3, 2026 at 4:59 PM Tanmay Shah <tanmay.shah@amd.com> wrote: >> >> Sharing mbox client data structure between "tx" and "rx" channels >> can lead to data corruption. Instead each channel should have its own >> mbox client data structure. >> >> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com> >> --- >> >> changes in v4: >> - separate remoteproc driver patch in to two patches >> >> drivers/remoteproc/xlnx_r5_remoteproc.c | 19 ++++++++++++++----- >> 1 file changed, 14 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c >> index bd619a6c42aa..109831c5815c 100644 >> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c >> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c >> @@ -74,7 +74,8 @@ struct zynqmp_sram_bank { >> * @tx_mc_buf: to copy data to mailbox tx channel >> * @r5_core: this mailbox's corresponding r5_core pointer >> * @mbox_work: schedule work after receiving data from mailbox >> - * @mbox_cl: mailbox client >> + * @mbox_tx_cl: tx channel mailbox client >> + * @mbox_rx_cl: rx channel mailbox client >> * @tx_chan: mailbox tx channel >> * @rx_chan: mailbox rx channel >> */ >> @@ -83,7 +84,8 @@ struct mbox_info { >> unsigned char tx_mc_buf[MBOX_CLIENT_BUF_MAX]; >> struct zynqmp_r5_core *r5_core; >> struct work_struct mbox_work; >> - struct mbox_client mbox_cl; >> + struct mbox_client mbox_tx_cl; >> + struct mbox_client mbox_rx_cl; >> struct mbox_chan *tx_chan; >> struct mbox_chan *rx_chan; >> }; >> @@ -230,7 +232,7 @@ static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *msg) >> struct mbox_info *ipi; >> size_t len; >> >> - ipi = container_of(cl, struct mbox_info, mbox_cl); >> + ipi = container_of(cl, struct mbox_info, mbox_rx_cl); >> >> /* copy data from ipi buffer to r5_core */ >> ipi_msg = (struct zynqmp_ipi_message *)msg; >> @@ -269,8 +271,8 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) >> if (!ipi) >> return NULL; >> >> - mbox_cl = &ipi->mbox_cl; >> - mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; >> + mbox_cl = &ipi->mbox_tx_cl; >> + mbox_cl->rx_callback = NULL; >> mbox_cl->tx_block = false; >> mbox_cl->knows_txdone = false; >> mbox_cl->tx_done = NULL; >> @@ -285,6 +287,13 @@ static struct mbox_info *zynqmp_r5_setup_mbox(struct device *cdev) >> return NULL; >> } >> >> + mbox_cl = &ipi->mbox_rx_cl; >> + mbox_cl->rx_callback = zynqmp_r5_mb_rx_cb; >> + mbox_cl->tx_block = false; >> + mbox_cl->knows_txdone = false; >> + mbox_cl->tx_done = NULL; >> + mbox_cl->dev = cdev; >> + > hmm... this looks bad. Sorry. > I think all we need is to export > unsigned int mbox_chan_tx_slots_available(struct mbox_chan *chan) { > return MBOX_TX_QUEUE_LEN - chan->msg_count} > from mailbox.c, of course with protection of the channel lock. > That should work for you, right? > Hi Jassi, Yes. Similar design was proposed in v1 [1]. That patch had some other problems, but the design was similar. I see your new patch, that should work for me. I will test the patch and provide tested-by tag. Thanks, Tanmay [1] https://lore.kernel.org/all/20250925185043.3013388-1-tanmay.shah@amd.com/#t > Regards, > Jassi ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 3/3] remoteproc: xlnx: do not send new mbox notification 2026-02-03 22:58 [PATCH v4 0/3] mailbox queue length check Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 1/3] mailbox: check mailbox queue is full or not Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel Tanmay Shah @ 2026-02-03 22:58 ` Tanmay Shah 2 siblings, 0 replies; 6+ messages in thread From: Tanmay Shah @ 2026-02-03 22:58 UTC (permalink / raw) To: jassisinghbrar, andersson, mathieu.poirier Cc: linux-kernel, linux-remoteproc, Tanmay Shah Only write new message to the tx mbox queue if slot is available. If queue is full, then do not send new mbox notification. Signed-off-by: Tanmay Shah <tanmay.shah@amd.com> --- drivers/remoteproc/xlnx_r5_remoteproc.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c index 109831c5815c..46c20b603bc7 100644 --- a/drivers/remoteproc/xlnx_r5_remoteproc.c +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c @@ -9,6 +9,7 @@ #include <linux/firmware/xlnx-zynqmp.h> #include <linux/kernel.h> #include <linux/mailbox_client.h> +#include <linux/mailbox_controller.h> #include <linux/mailbox/zynqmp-ipi-message.h> #include <linux/module.h> #include <linux/of_address.h> @@ -341,7 +342,11 @@ static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid) int ret; ipi = r5_core->ipi; - if (!ipi) + if (!ipi || !ipi->tx_chan) + return; + + /* Do not need new kick as already many kicks are pending. */ + if (ipi->tx_chan->cl->tx_slots_avail_ro == 0) return; mb_msg = (struct zynqmp_ipi_message *)ipi->tx_mc_buf; -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-02-10 17:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-02-03 22:58 [PATCH v4 0/3] mailbox queue length check Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 1/3] mailbox: check mailbox queue is full or not Tanmay Shah 2026-02-03 22:58 ` [PATCH v4 2/3] remoteproc: xlnx: assign mbox client per mbox channel Tanmay Shah 2026-02-09 23:27 ` Jassi Brar 2026-02-10 17:12 ` Shah, Tanmay 2026-02-03 22:58 ` [PATCH v4 3/3] remoteproc: xlnx: do not send new mbox notification Tanmay Shah
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®