Re: [PATCH 7/7] mailbox: goog-mba: Introduce the goog-mba mailbox driver

[Doug Anderson]

Hi,

On Sat, Sep 5, 2026 at 6:20 PM Jassi Brar <jassisinghbrar@xxxxxxxxx> wrote:
>
> Hi Douglas,
>
> On Tue, Jul 14, 2026 at 5:24 PM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
> ....
> > +
> > +/**
> > + * goog_mba_handle_tx_interrupt() - Handle interrupt that remote Acked our msg.
> > + * @goog_mbox: The mailbox info.
> > + */
> > +static void goog_mba_handle_tx_interrupt(struct goog_mbox_info *goog_mbox)
> > +{
> > +       unsigned int reqs_completed;
> > +       int i;
> > +
> > +       /*
> > +        * ACK interrupt needs to be cleared before reading CLIENT_OUTSTANDING_MSG.
> > +        * Then if a "race" happens and another message gets Acked after we clear
> > +        * but before we read CLIENT_OUTSTANDING_MSG then the worst that will
> > +        * happen is we'll get a followup interrupt that will show 0 reqs_completed.
> > +        */
> > +       writel(CLIENT_IRQ_STATUS_ACK_INT, goog_mbox->iomem + CLIENT_IRQ_STATUS_OFFSET);
> > +
> > +       if (goog_mbox->queue_mode) {
> > +               u32 outstanding_msgs;
> > +
> > +               outstanding_msgs = readl(goog_mbox->iomem + CLIENT_OUTSTANDING_MSG);
> > +               spin_lock(&goog_mbox->lock);
> > +
> > +               if (goog_mbox->outstanding_msgs >= outstanding_msgs) {
> > +                       reqs_completed = goog_mbox->outstanding_msgs - outstanding_msgs;
> > +               } else {
> > +                       /*
> > +                        * The hardware's track of outstanding messages should always
> > +                        * be less than or equal to the number of messages we queued.
> > +                        * If it thinks there are more messages outstanding than we
> > +                        * queued, something is wrong. Assume nothing was completed.
> > +                        */
> > +                       dev_warn_ratelimited(goog_mbox->mba->dev,
> > +                                            "%pOFP: unexpected outstanding msgs: %u -> %u\n",
> > +                                            goog_mbox->np, goog_mbox->outstanding_msgs,
> > +                                            outstanding_msgs);
> > +                       reqs_completed = 0;
> > +               }
> > +               goog_mbox->outstanding_msgs = outstanding_msgs;
> > +               spin_unlock(&goog_mbox->lock);
> > +
> > +               trace_goog_mba_process_q_txdone(goog_mbox, reqs_completed, outstanding_msgs);
> > +       } else {
> > +               reqs_completed = 1;
> > +               trace_goog_mba_process_nq_txdone(goog_mbox);
> > +       }
> > +
> > +       for (i = 0; i < reqs_completed; i++)
> > +               mbox_chan_txdone(&goog_mbox->chan, 0);
>
> The patchset doesn't say much about the clients but if the platform
> works like this, I have a strong feeling we can do without introducing
> mbox_controller.has_queue.
> Just use the msg_data[] ringbuffer -- we can avoid changing the core
> internals and you will still get ACK for each message.
> What are the clients going to be like?

I don't think we can get rid of `mbox_controller.has_queue`.
Specifically, the queue mode allows the mailbox controller to handle
multiple outstanding transactions simultaneously to reduce latency.
Imagine host (Linux) interrupt latency is 100us and we want to send 3
mailbox messages. Let's say queuing a mailbox message takes 10us.
We'll say that the remote interrupt latency is 50us. Timing would look
something like this:

0.000000: Client sends msg #1 and mbox controller initiates the xfer
0.000010: Client sends msg #2 and mbox controller initiates the xfer
0.000020: Client sends msg #3 and mbox controller initiates the xfer
0.000050: Remote gets IRQ, sees 3 messages, and ACKs them all.
0.000150: ACK IRQ arrives; mbox controller sends 3 txdone

...so all 3 messages are sent / ACKed in 150us.

Without letting the mailbox controller queue, we'd end up more like this:

0.000000: Client sends msg #1 and mbox controller initiates the xfer
0.000050: Remote gets IRQ, sees msg #1, ACKs it.
0.000150: ACK IRQ arrives; mbox controller sends txdone
0.000150: mbox core notes txdone and sends msg #2
0.000200: Remote gets IRQ, sees msg #2, ACKs it.
0.000300: ACK IRQ arrives; mbox controller sends txdone
0.000300: mbox core notes txdone and sends msg #3
0.000350: Remote gets IRQ, sees msg #3, ACKs it.
0.000450: ACK IRQ arrives; mbox controller sends txdone

Hopefully that clears up what the "has_queue" is about and why we need it?

-Doug