Re: [PATCH 6/7] dt-bindings: mailbox: goog-mba: Add goog-mba mailbox bindings

From: Doug Anderson

Date: Thu Jul 16 2026 - 12:37:32 EST


Hi,

On Wed, Jul 15, 2026 at 10:47 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>
> > I don't suppose I can change your mind here? If we take
> > "google,lga-mailbox-array" as the generic, then going foward a few
> > generations we end up with:
> >
> > properties:
> > compatible:
> > oneOf:
> > - const: google,lga-mailbox-array
> > - items:
> > - enum:
> > - google,next-mailbox-array
> > - google,nextnext-mailbox-array
> > - google,another-mailbox-array
> > - const: google,lga-mailbox-array
>
> This is the expected appriach.
>
> >
> > If we keep "google,mailbox-array" as the generic, then going forward a
> > few generations we end up with this, which seems nicer / less
> > confusing:
> >
> > properties:
> > compatible:
> > items:
> > - enum:
> > - google,lga-mailbox-array
> > - google,next-mailbox-array
> > - google,nextnext-mailbox-array
> > - google,another-mailbox-array
> > - const: google,mailbox-array
>
> It is the discouraged approach. I already have a few real examples for
> Qualcomm when people added such generic mailbox and after some time it
> turned out not generic. So people wanted to add an another generic one...

OK, I'll change it to use "google,lga-mailbox-array" as the generic.


> >>> + reg:
> >>> + minItems: 1
> >>> + items:
> >>> + - description: Host registers (not accessible to client)
> >>> + - description: Global registers (not present on newer IP blocks)
> >>
> >> You have only one SoC. One SoC has only one IP block, no?
> >
> > Nope! There are several instances of the MBA IP block per SoC. I tried
> > to explain the situation exhaustively, but there's always the tradeoff
> > between explaining thoroughly and providing too much text.
>
> That's ok, the "newer" is confusing.
>
> >
> > To answer this specific question concretely, there is one MBA per
> > remote processor. Looking at the downstream DTS, I see at least these
> > MBA instances:
> > * AOC (Always On Compute)
> > * GSA (Google Security Anchor)
> > * GDMC (Google Debug Monitor Core)
> > * CPM (Central Power Manager)
> >
> > Each of these 4 MBA instances has its own "global" registers.
> >
> > To provide more context, each MBA instance can support communication
> > beyond just the AP (Apps Processor). For instance, the AOC's MBA
> > instance could be used to talk between the AOC and AP and also between
> > the AOC and CPM. Let's take this as an example. In this case:
> >
> > * The AOC is the "host" of this MBA.
> > * The AP is a "client" of this MBA.
> > * The CPM is another "client" of this MBA.
> >
> > The AOC is the only one with access to the "host" registers.
> >
> > Everyone (AOC, AP, CPM in this case) has access to the read-only
> > "global" registers describing the MBA instance.
> >
> > The clients have access to several banks of client registers, one per
> > mailbox they can access. The host (AOC in this case) also has access
> > to the client register spaces since that's where the shared message
> > memory is located.
> >
> > The overall MBA instance is best identified by the address of the host
> > registers, even if the client (the AP in this case) can't access those
> > registers.
> >
> > On newer versions of the IP block the "global" register bank was
> > removed and the read-only registers that were part of it were simply
> > copied to each client instance.
> >
> > Does that clarify?
>
> Yeah, just s/on newer/on all/ ?

No. Although this driver only supports the "laguna" version of the
mailbox array, I tried to look forward to what was coming in the
future. On "laguna", the "global" register bank is needed. On SoCs
past "laguna" (AKA "newer" ones) there is no global register bank.
This is why the global register bank needs to be optional.

Yes, I could add some validation to tie it to a specific SoC version,
but that doesn't really buy anything. It's just as easy to say that if
the global register bank is defined in the device tree that it exists.
If the global register bank isn't defined, the info must be in the
per-client banks.

If you insist, I can make the "global" register space non-optional for
now and we can re-litigate when official support for newer SoCs is
proposed.


> >>> +patternProperties:
> >>> + "^mailbox@[0-9a-f]+$":
> >>> + type: object
> >>> + description:
> >>> + Each sub-node is a single-channel mailbox.
> >>
> >> This does not look like correct representation. You have one mailbox
> >> controller with multiple mailboxes, not multiple mailbox controllers of
> >> single channel boxes.
> >
> > I spent quite a bit of time debating this when rewriting the driver.
> > While we could certainly hack things into the existing "mailbox with a
> > bunch of channels", IMO it would be a worse representation of the
> > hardware.
> >
> > I discussed this in the wall of text in this patch series, but
> > re-hashing it here:
> >
> > Each "mailbox" in the mailbox array is more like a full-fledged
> > mailbox than a channel within a mailbox. Each (single-channel)
> > mailbox:
> > * Has its own client register space.
>
> That's nothing special yet. Many providers of multiple resources have
> these resources in dedicated registers. Arguing this, each GPIO in a
> GPIO controller as well has its own register space so is basically a
> GPIO controller on its own.
>
> > * Has its own interrupt.
>
> Just like GPIOs...

Sure, having a giant list of interrupts and register offsets isn't the
end of the world.


> > * Can have a different amount of memory for messages.
> > * Can have its own conventions for communication.

I notice you didn't respond to the above bullets. How would I deal
with different mailboxes in the array having different communication
conventions? Different mailboxes in the array might have different
"payload" sizes. Some mailboxes in the array might use "queue" mode
conventions and some might not. Do I need to devise some complex
scheme for describing which mailboxes in the array use which
convention?


> Well, this could be. But you still have one child per channel (cells=0)
> and all children address space is in parent's space, so that's clear
> indication. It's one controller with multiple, although some different,
> channels.

Sure, it's definitely one IP block and I'm not arguing against that.


> > If we tried to represent the mailbox array as a single mailbox with a
> > bunch of channels, each instance would have a different number of
> > "reg" entries and a different number of interrupts. We would also need
>
> No, you would have only one device node. Very clean solution instead of
> 100 children for each individual mailbox.

There are definitely not anywhere near 100 children. At most a given
instance of an MBA IP block could have 32 mailboxes. In practice, most
have fewer.


> It's the same with clocks (TI) - you do not get device node per clock,
> even if TI did it 10 years ago. You do not get here device node per channel.

Sure, the clock history is one thing to look at here. IMO, this is not
the right comparison, though. Trying to describe all of the complexity
of a clock tree in device tree is an exercise in futility. It's just
too complex to get it right, and there really are hundreds of clocks.
The mailbox array, on the other hand, is a much simpler beast.

I think regulators and PMICs are a better comparison here. When we
describe a PMIC in the device tree, do we have a single PMIC node and
then have clients refer to a "regulator ID" to index into which
specific regulator in the PMIC they want? No, we don't. We use
sub-nodes for each specific regulator. This gives us a nice place in
the device tree to put information about each individual regulator,
since each regulator in a PMIC is different.

Certainly if we had a mailbox controller with a bunch of identical
channels then describing them as sub-nodes wouldn't make sense.
Existing in-tree mailbox controllers have a bunch of homogenous
channels, and thus the existing solution of using a channel ID works
well. Here, the sub-nodes buy us something and provide for a cleaner
solution.

I don't understand the downside of my current proposal. It represents
the hardware better (both the HW designers' intentions and the reality
of its structure), avoids adding complexity to the device tree, and
feels clean.


-Doug