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

[Jassi Brar]

Hi Doug,

On Sun, Sep 20, 2026 at 4:39 PM Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
 >
> > > Imagine we have three clocks: They are: "clk_i2c1", "clk_i2c2", and
> > > "clk_spi". The "clk_i2c1" needs to be turned on when we're going to
> > > start an I2C transfer on I2C bus 1. "clk_i2c2" is the same but for the
> > > I2C bus 2. "clk_spi" needs to be turned on when we want to start a a
> > > SPI transfer.
> > >
> > clk_prepare() is allowed to sleep, so any path, that includes the
> > call, can not be expected to have low or even deterministic latency.
>
> Sure. Essentially anything using a mailbox as a transport mechanism
> needs to be able to sleep, at least if it cares about "txdone". Even
> without a guaranteed latency, though, that doesn't mean we shouldn't
> try to reduce the latency.
>
>
> > > In the i2c/spi drivers, we've got "clk_prepare(clk)" calls to turn on
> > > the clocks. The three clocks are operated independently by their
> > > respective users. An I2C or SPI transfer may take place at any time.
> > > The I2C and SPI drivers need to know for sure when the clock has
> > > finished enabling because as soon as the clk_prepare() calls return
> > > they will start transferring.
> > >
> > > All three clocks are backed by a single clock driver. We'll call it
> > > the "clk_mailbox" driver. The "clk_mailbox" driver takes the request
> > > to turn on the clock and encodes it as a mailbox message to a remote
> > > processor. Let's imagine that the mailbox message looks like a 2-word
> > > transfer. The first word contains a 0 or a 1 for enable/disable and
> > > the second word contains the ID of the clock: 0 for "clk_i2c1", 1 for
> > > "clk_i2c2", and 2 for "clk_i3c3".
> > >
> > > Now, imagine that we want to turn on all three clocks simultaneously.
> >
> > Why? (not that it can't be done) I2C and SPI clients run independent
> > of each other.
>
> Right, the whole "independent" aspect is what I'm talking about. I'm
> not saying that we're _trying_ to do all three transfers at once, but
> just in the natural state of things there will be lots of clock calls
> going on at the system all independently of each other. That means
> it's likely there will be times when several clocks are enabled
> simultaneously.
>
>
> > > The "clk_mailbox" driver will see 3 calls to turn on the clocks and it
> > > needs to convert those to messages to send the remote processor. It
> > > will then call mbox_send_message() to queue those messages with the
> > > mailbox controller. In other words, we'll see these three calls happen
> > > nearly simultaneously:
> > >
> > > mbox_send_message(chan, &{0x1, 0x0});
> > > mbox_send_message(chan, &{0x1, 0x1});
> > > mbox_send_message(chan, &{0x1, 0x2});
> > >
> > > The LGA mailbox controller knows "txdone". That is, when the remote
> > > processor "acks" a message, the LGA mailbox controller can tell (and
> > > get an interrupt). This "ack" signals that the clock has finished
> > > enabling. This means that the "clk_mailbox" cannot run the state
> > > machine and it can't call "txdone" itself.
> > >
> > > Without my patches, when the above 3 mbox_send_message() calls are
> > > made, the first one will go straight to the LGA mailbox controller and
> > > the second two will be queued up. Once the "txdone" for the first
> > > message arrives, we'll queue the second message. Once the "txdone" for
> > > the second message arrives, we'll queue the third message. This means
> > > that the messages are not being processed simultaneously. The third
> > > clk_prepare() call will be processed much more slowly since it has to
> > > wait in line. If we had 10 clocks enabling at the same time, the 10th
> > > clock could have a pretty significant wait.
> > >
> > > With my patches, the LGA mailbox controller is given the second and
> > > third message even though the first message isn't done yet. The LGA
> > > mailbox controller can queue the second and third messages even though
> > > the txdone for the first message hasn't arrived yet. If things are
> > > fast enough, all three messages can be queued up before the remote
> > > processor has even started processing the first one. The remote
> > > processor can process all three messages concurrently and acknowledge
> > > all three at once. The LGA mailbox controller can get a single
> > > interrupt representing all three "txdone" ACKs and call "txdone" for
> > > all three messages simultaneously.
> > >
> > If your remote (host) can actually act on multiple requests parallely,
> > you need a way to map ACKs back onto the requests. Currently you
> > don't.
> > Looking at the goog_mba_handle_tx_interrupt() implementation, consider
> > the situation when 5 requests are submitted in the h/w fifo and 3 are
> > reported ACKed by the interrupt after some time.
> > You assume the three done are the first three -- which implies that
> > the remote handles requests in the order they arrive i.e, serially.
> > Otherwise, say, request-1 may be wrongly completed if the ACKs were
> > for requests 2, 3 & 4.
> > So it seems mostly an illusion of parallelism - remote is acting on
> > requests (or atleast sending ACKs) serially.
>
> It's more than an illusion because all of the slow paths are
> parallelized. For the remote processor, I believe that turning on a
> clock is trivially easy: just set a bit. The slow parts are the
> interrupt arriving on the remote processor and the interrupt arriving
> on the Linux processor. Those _are_ parallelized. During a single
> interrupt, we can see and process multiple mailbox messges (or
> multiple mailbox ACKs). I can paste the example I gave earlier. Just
> as you say, interrupts are processed and acted on serially. ...but
> because we can parallelize the interrupt handling we still get the win
> because interrupt latency is much worse than any of the actual work
> that needs to be done.
>
> 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
>
> The numbers here for interrupt latency are made up for my example and
> I haven't personally measured them, but I think it's not completely
> absurd to say that interrupt latency (on both the Linux and remote
> sides) dominates the communication path.
>
Yes, the numbers do look biased. It takes 50us for remote to get the
irq and act upon it before ACKing but it takes 100us for that ACK to
get back.
And the benefit will be hard to achieve - it involves three unrelated
clk_prepare() requests done within 50us often enough. When the stars
align you save 300us on a clk_prepare()

It feels you are trying to optimize a non-issue. clk_prepare() is
expected to be slow and anyways shouldn't be frequent enough from all
devices to give noticeable benefit.

If you do have some real numbers and think it is worth it on your
platform, then maybe expose each doorbell/shm-slot as a generic
channel. clk_mailbox will request a generic channel, do the request
and free it. The same effect but without inventing a new api. I can
share a draft if you want, but I suggest let's not make things
complicated without proven benefit.

> > We can keep the core
> > unchanged and the driver much simpler if we simply use the buffering
> > in the core.
>
> FWIW, the driver will need to contain the queuing complexity
> regardless. This is because the remote processor expects queuing. Even
> if we don't queue at the Linux level, the driver still needs to know
> if the remote side uses a "queuing protocol." This is because the
> remote side expects the shared memory to be partitioned into chunks
> and that we must move onto the next chunk between messages.
>
My first concern is to avoid implanting a yet another path of sending
messages and second is to keep drivers simple if they can.

Regards,
Jassi