[PATCH 0/7] mailbox: Improve the mbox core then introduce the goog-mba driver

[PATCH 0/7] mailbox: Improve the mbox core then introduce the goog-mba driver

[Douglas Anderson]

The goal of this series is to land support for the goog-mba (MailBox
Array) IP block that's present in Pixel 10 phones.

As can be seen in the device-tree bindings for the goog-mba IP block,
the mailbox IP block in Pixel 10 phones is fairly sophisticated.
Notably:
* It has hardware features that support queuing, meaning that more
  than one mailbox message can be pending at a time.
* The "channels" in a given mailbox array aren't homogeneous. Each
  "channel" in the mailbox array can have a different amount of memory
  for messages. Really, the "channels" in a mailbox are considered to
  be full single-channel mailboxes and a grouping of mailboxes is
  considered a "mailbox array" (hence the IP block being named "mba")

In order to cleanly support some of the sophisticated goog-mba
features, improvements are made to the mailbox core. Specifically,
support for mailbox controllers that can queue is added and also
support for mailbox drivers that have more than one sub-node is added.

This is a fairly big rewrite from the downstream MBA driver shipping
on Pixel 10 phones, which awkwardly makes due without the improvements
to the mailbox core. It has been lightly tested both by porting it to
an experimental downstream tree based on 7.1 and also by running it
directly upstream against a stripped down Pixel 10 device tree.


Douglas Anderson (7):
  dt-bindings: mailbox: Don't require #mbox-cells to be 1
  mailbox: Allow #mbox-cells = <0> without specifying a custom xlate
  mailbox: Find a matching mailbox by fwnode rather than device
  mailbox: Simplify circular queue math with mod arithmetic
  mailbox: Add support for mailbox controllers that can queue
  dt-bindings: mailbox: goog-mba: Add goog-mba mailbox bindings
  mailbox: goog-mba: Introduce the goog-mba mailbox driver

 .../bindings/mailbox/google,mba.yaml          | 216 +++++++
 .../devicetree/bindings/mailbox/mailbox.txt   |   6 +-
 MAINTAINERS                                   |   8 +
 drivers/mailbox/Kconfig                       |   8 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/goog-mba-priv.h               | 108 ++++
 drivers/mailbox/goog-mba-trace.h              | 183 ++++++
 drivers/mailbox/goog-mba.c                    | 567 ++++++++++++++++++
 drivers/mailbox/mailbox.c                     |  84 ++-
 include/linux/mailbox/goog-mba-message.h      |  38 ++
 include/linux/mailbox_controller.h            |  15 +-
 11 files changed, 1209 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/google,mba.yaml
 create mode 100644 drivers/mailbox/goog-mba-priv.h
 create mode 100644 drivers/mailbox/goog-mba-trace.h
 create mode 100644 drivers/mailbox/goog-mba.c
 create mode 100644 include/linux/mailbox/goog-mba-message.h

-- 
2.55.0.141.g00534a21ce-goog

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

[Douglas Anderson]

Introduce bindings for the MailBox Array IP block present in Laguna
SoCs (AKA "lga", AKA "Google Tensor G5").

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 .../bindings/mailbox/google,mba.yaml          | 216 ++++++++++++++++++
 1 file changed, 216 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/google,mba.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/google,mba.yaml b/Documentation/devicetree/bindings/mailbox/google,mba.yaml
new file mode 100644
index 000000000000..6c4505a369e2
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/google,mba.yaml
@@ -0,0 +1,216 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2025 Google LLC
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mailbox/google,mba.yaml#
+$schema: http://devicetree.org/meta-schemas/base.yaml#
+
+title: Google MailBox Array
+
+maintainers:
+  - Douglas Anderson <dianders@chromium.org>
+
+description: |
+  The Google MailBox Array (MBA) is an IP block in Google-designed SoCs
+  starting in Laguna (AKA "lga", AKA Google Tensor G5). In a typical SoC
+  that includes this IP block, there are a number of instances of the MBA
+  controller with each instance having slightly different hardware
+  parameters and intended for communication with a different remote
+  processor.
+
+  An MBA instance has a "host" that is defined as the processor "providing"
+  a "service". This is typically not the main Application Processor (AP) but
+  is instead some specialized co-processor in the SoC like the Central Power
+  Manager (CPM). A processor (like the AP) talking to the "host" of the MBA
+  is a "client" of the MBA. A given MBA instance only ever has one host, but
+  it may have several clients. For instance, the CPM (an MBA "host") may need
+  to send/receive mailbox messages not just from the AP but from other
+  processors in the SoC and each of these other processors can be "clients"
+  of the same MBA.
+
+  The "host" of an MBA instance has full access to everything in the MBA
+  instance. It can access its own private set of "host" MBA registers, the
+  "global" MBA registers (if they exist), and all of the "client" MBA
+  registers.
+
+  A "client" of an MBA instance has access to the "global" MBA registers (if
+  they exist) and one or more sets of "client" MBA registers.
+
+  These bindings are focused on describing the MBA from the point of view of
+  a single client.
+
+  As per above, a client may have access to several sets of MBA "client"
+  registers. Each set of "client" registers represents a logical mailbox
+  "channel". However, because each channel may have different configuration
+  parameters and a mailbox "channel" in typical usage means one of a number
+  of identical channels, each channel in a Google MailBox Array is typically
+  referred to as a full "mailbox" and the whole collection of mailboxes as
+  the "mailbox array".
+
+  Mailboxes in an MBA instance have these features:
+  * 1 to 256 32-bit words of shared memory.
+  * The ability for the client to ring the main doorbell of the host and be
+    notified when the host Acks the doorbell.
+  * The ability for the host to ring the main doorbell of the client and be
+    notified when the client Acks the doorbell.
+
+  Some mailboxes may also have the ability to have counted doorbells. This
+  means that the receiver of the doorbell can tell how many times it rung.
+  This is intended for implementing "queued" mailboxes. See below.
+
+  The MBA hardware doesn't have any specific directionality. That is to
+  say, both the host and the client have full read and write access to
+  their shared memory. All mailbox instances have doorbells going both from
+  the client to the host as well as the host to the client.
+
+  The mailboxes can only be used for communication if the host and client
+  both agree on conventions. These conventions are described in the
+  device tree as they describe how the remote firmware is expecting to
+  communicate.
+
+  Current known in-use conventions:
+  1. An RX mailbox with payloads that are of a well-defined size.
+     On mailboxes of this type, the host is the only one to write shared
+     memory. After placing a fixed-size message in shared memory, it rings
+     the main doorbell of the client. The client reads the message and Acks
+     the doorbell.
+  2. A TX mailbox with payloads that could vary in size.
+     On mailboxes of this type, the mailbox client is the only one to write
+     shared memory. The client always writes a payload to the start of shared
+     memory and rings the main host doorbell. The client then looks for the
+     host to Ack the doorbell. The clients of the mailbox have ways to know
+     the size of any given message.
+  3. A half-duplex TX/RX mailbox. This is a mailbox that can switch between
+     convention #1 and #2 above. Since both sides write data to the start of
+     shared memory, the two sides must have some convention to know whose
+     turn it is to send a message.
+  4. A "queued" RX mailbox with a payload of a well-defined size.
+     This type of mailbox is only possible if the MBA instance can count
+     doorbells. On mailboxes of this type, the host is the only one to write
+     shared memory. When the client doorbell rings, the client reads a
+     fixed-size from the next "slot" in shared memory and then updates its
+     internal state. The shared memory is treated as a circular queue.
+  5. A "queued" TX mailbox with a payload of a well-defined size.
+     This type of mailbox is only possible if the MBA instance can count
+     doorbells. On mailboxes of this type, the mailbox client is the only one
+     to write shared memory. The shared memory is treated as a circular queue.
+     The client writes a fixed-sized payload to the next "slot" in the shared
+     memory (where the slot size is determined by the client's first transfer),
+     updates its internal state, and rings the host doorbell. The client can
+     keep writing more messages as long as the circular queue isn't full. The
+     client gets an interrupt when the host Acks a doorbell and can tell how
+     many doorbells still haven't been Acked.
+
+  Conventions will be supported with a small number of properties specified
+  for each mailbox.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - google,lga-mailbox-array
+      - const: google,mailbox-array
+
+  reg:
+    minItems: 1
+    items:
+      - description: Host registers (not accessible to client)
+      - description: Global registers (not present on newer IP blocks)
+
+  ranges: true
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+patternProperties:
+  "^mailbox@[0-9a-f]+$":
+    type: object
+    description:
+      Each sub-node is a single-channel mailbox.
+
+    properties:
+      reg:
+        maxItems: 1
+
+      interrupts:
+        maxItems: 1
+
+      "#mbox-cells":
+        const: 0
+
+      google,rx-payload-words:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        maximum: 256
+        default: 0
+        description:
+          The number of 32-bit words in each mailbox message from the remote
+          processor. May be 0 for doorbell-only. If not specified this is
+          assumed to be 0.
+
+      google,mba-queue-mode:
+        type: boolean
+        description:
+          The remote processor is expecting the shared memory to be treated
+          as a circular queue and that there may be several outstanding
+          messages at once. Only usable on instances with counted doorbell
+          interrupts.
+
+    required:
+      - reg
+      - interrupts
+      - "#mbox-cells"
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - ranges
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      cpm_ap_ns_mba: mailbox-array@5240000 {
+        compatible = "google,lga-mailbox-array", "google,mailbox-array";
+        reg = <0x0 0x05240000 0x0 0x00010000>,
+              <0x0 0x05250000 0x0 0x00010000>;
+        ranges = <0x0 0x0 0x05260000 0x00020000>;
+
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        cpm_ap_ns_req_mba_client_0: mailbox@0 {
+          reg = <0x0000 0x1000>;
+          interrupts = <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH 0>;
+
+          #mbox-cells = <0>;
+
+          google,mba-queue-mode;
+        };
+
+        cpm_ap_ns_resp_mba_client_1: mailbox@1000 {
+          reg = <0x1000 0x1000>;
+          interrupts = <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH 0>;
+
+          #mbox-cells = <0>;
+
+          google,rx-payload-words = <4>;
+          google,mba-queue-mode;
+        };
+      };
+    };
+
+...
-- 
2.55.0.141.g00534a21ce-goog

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

[Krzysztof Kozlowski]

On 15/07/2026 00:21, Douglas Anderson wrote:
> Introduce bindings for the MailBox Array IP block present in Laguna
> SoCs (AKA "lga", AKA "Google Tensor G5").
> 
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
>  .../bindings/mailbox/google,mba.yaml          | 216 ++++++++++++++++++

Filename must match compatible.

>  1 file changed, 216 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/google,mba.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/google,mba.yaml b/Documentation/devicetree/bindings/mailbox/google,mba.yaml
> new file mode 100644
> index 000000000000..6c4505a369e2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/google,mba.yaml
> @@ -0,0 +1,216 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2025 Google LLC
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/google,mba.yaml#
> +$schema: http://devicetree.org/meta-schemas/base.yaml#
> +
> +title: Google MailBox Array
> +
> +maintainers:
> +  - Douglas Anderson <dianders@chromium.org>
> +
> +description: |
> +  The Google MailBox Array (MBA) is an IP block in Google-designed SoCs
> +  starting in Laguna (AKA "lga", AKA Google Tensor G5). In a typical SoC
> +  that includes this IP block, there are a number of instances of the MBA
> +  controller with each instance having slightly different hardware
> +  parameters and intended for communication with a different remote
> +  processor.
> +
> +  An MBA instance has a "host" that is defined as the processor "providing"
> +  a "service". This is typically not the main Application Processor (AP) but
> +  is instead some specialized co-processor in the SoC like the Central Power
> +  Manager (CPM). A processor (like the AP) talking to the "host" of the MBA
> +  is a "client" of the MBA. A given MBA instance only ever has one host, but
> +  it may have several clients. For instance, the CPM (an MBA "host") may need
> +  to send/receive mailbox messages not just from the AP but from other
> +  processors in the SoC and each of these other processors can be "clients"
> +  of the same MBA.
> +
> +  The "host" of an MBA instance has full access to everything in the MBA
> +  instance. It can access its own private set of "host" MBA registers, the
> +  "global" MBA registers (if they exist), and all of the "client" MBA
> +  registers.
> +
> +  A "client" of an MBA instance has access to the "global" MBA registers (if
> +  they exist) and one or more sets of "client" MBA registers.
> +
> +  These bindings are focused on describing the MBA from the point of view of
> +  a single client.
> +
> +  As per above, a client may have access to several sets of MBA "client"
> +  registers. Each set of "client" registers represents a logical mailbox
> +  "channel". However, because each channel may have different configuration
> +  parameters and a mailbox "channel" in typical usage means one of a number
> +  of identical channels, each channel in a Google MailBox Array is typically
> +  referred to as a full "mailbox" and the whole collection of mailboxes as
> +  the "mailbox array".
> +
> +  Mailboxes in an MBA instance have these features:
> +  * 1 to 256 32-bit words of shared memory.
> +  * The ability for the client to ring the main doorbell of the host and be
> +    notified when the host Acks the doorbell.
> +  * The ability for the host to ring the main doorbell of the client and be
> +    notified when the client Acks the doorbell.
> +
> +  Some mailboxes may also have the ability to have counted doorbells. This
> +  means that the receiver of the doorbell can tell how many times it rung.
> +  This is intended for implementing "queued" mailboxes. See below.
> +
> +  The MBA hardware doesn't have any specific directionality. That is to
> +  say, both the host and the client have full read and write access to
> +  their shared memory. All mailbox instances have doorbells going both from
> +  the client to the host as well as the host to the client.
> +
> +  The mailboxes can only be used for communication if the host and client
> +  both agree on conventions. These conventions are described in the
> +  device tree as they describe how the remote firmware is expecting to
> +  communicate.
> +
> +  Current known in-use conventions:
> +  1. An RX mailbox with payloads that are of a well-defined size.
> +     On mailboxes of this type, the host is the only one to write shared
> +     memory. After placing a fixed-size message in shared memory, it rings
> +     the main doorbell of the client. The client reads the message and Acks
> +     the doorbell.
> +  2. A TX mailbox with payloads that could vary in size.
> +     On mailboxes of this type, the mailbox client is the only one to write
> +     shared memory. The client always writes a payload to the start of shared
> +     memory and rings the main host doorbell. The client then looks for the
> +     host to Ack the doorbell. The clients of the mailbox have ways to know
> +     the size of any given message.
> +  3. A half-duplex TX/RX mailbox. This is a mailbox that can switch between
> +     convention #1 and #2 above. Since both sides write data to the start of
> +     shared memory, the two sides must have some convention to know whose
> +     turn it is to send a message.
> +  4. A "queued" RX mailbox with a payload of a well-defined size.
> +     This type of mailbox is only possible if the MBA instance can count
> +     doorbells. On mailboxes of this type, the host is the only one to write
> +     shared memory. When the client doorbell rings, the client reads a
> +     fixed-size from the next "slot" in shared memory and then updates its
> +     internal state. The shared memory is treated as a circular queue.
> +  5. A "queued" TX mailbox with a payload of a well-defined size.
> +     This type of mailbox is only possible if the MBA instance can count
> +     doorbells. On mailboxes of this type, the mailbox client is the only one
> +     to write shared memory. The shared memory is treated as a circular queue.
> +     The client writes a fixed-sized payload to the next "slot" in the shared
> +     memory (where the slot size is determined by the client's first transfer),
> +     updates its internal state, and rings the host doorbell. The client can
> +     keep writing more messages as long as the circular queue isn't full. The
> +     client gets an interrupt when the host Acks a doorbell and can tell how
> +     many doorbells still haven't been Acked.
> +
> +  Conventions will be supported with a small number of properties specified
> +  for each mailbox.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - google,lga-mailbox-array
> +      - const: google,mailbox-array

Don't use generic fallback. Just the SoCs.


> +
> +  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?

> +
> +  ranges: true
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 1
> +
> +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.


> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +
> +      interrupts:
> +        maxItems: 1
> +
> +      "#mbox-cells":
> +        const: 0
> +
> +      google,rx-payload-words:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        maximum: 256
> +        default: 0
> +        description:
> +          The number of 32-bit words in each mailbox message from the remote
> +          processor. May be 0 for doorbell-only. If not specified this is
> +          assumed to be 0.
> +
> +      google,mba-queue-mode:
> +        type: boolean
> +        description:
> +          The remote processor is expecting the shared memory to be treated
> +          as a circular queue and that there may be several outstanding
> +          messages at once. Only usable on instances with counted doorbell
> +          interrupts.
> +
> +    required:
> +      - reg
> +      - interrupts
> +      - "#mbox-cells"
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - ranges
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    soc {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +
> +      cpm_ap_ns_mba: mailbox-array@5240000 {

Drop all unused labels.


Best regards,
Krzysztof

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

[Doug Anderson]

Hi,

On Tue, Jul 14, 2026 at 9:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 15/07/2026 00:21, Douglas Anderson wrote:
> > Introduce bindings for the MailBox Array IP block present in Laguna
> > SoCs (AKA "lga", AKA "Google Tensor G5").
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> >  .../bindings/mailbox/google,mba.yaml          | 216 ++++++++++++++++++
>
> Filename must match compatible.

Whoops! Will fix in v2.


> > +properties:
> > +  compatible:
> > +    items:
> > +      - enum:
> > +          - google,lga-mailbox-array
> > +      - const: google,mailbox-array
>
> Don't use generic fallback. Just the SoCs.

Sure, if you insist.

In general the "mba" hardware is designed with enough identification
registers that we should be able to autodetect which variant we're on.
Thus, my hope is to not ever need to reference the SoC-specific
variant in the driver itself. It's not the end of the world to use the
"google,lga-mailbox-array" as the generic, I guess...

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

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

Sure, it means that if someone unexpectedly makes a new Google
mailbox-array that's totally incompatible then the
"google,mailbox-array" sounds too generic, but that doesn't feel like
the end of the world. You could call the new mailbox array designed in
the year 2037 the "google,2037-mailbox-array" and things would overall
be less confusing than using the "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.

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?


> > +
> > +  ranges: true
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 1
> > +
> > +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.
* Has its own interrupt.
* Can have a different amount of memory for messages.
* Can have its own conventions for communication.

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
an array describing the communication conventions for each channel.
Can it be done? Yes. Is it ugly? Also, yes.

Further evidence that the hardware design intended "a bunch of
mailboxes" rather than "a mailbox with channels" is that the IP block
is called a "mailbox array". ;-)


> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    soc {
> > +      #address-cells = <2>;
> > +      #size-cells = <2>;
> > +
> > +      cpm_ap_ns_mba: mailbox-array@5240000 {
>
> Drop all unused labels.

Sounds good.

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

[Krzysztof Kozlowski]

On 15/07/2026 18:49, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 14, 2026 at 9:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 15/07/2026 00:21, Douglas Anderson wrote:
>>> Introduce bindings for the MailBox Array IP block present in Laguna
>>> SoCs (AKA "lga", AKA "Google Tensor G5").
>>>
>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>> ---
>>>
>>>  .../bindings/mailbox/google,mba.yaml          | 216 ++++++++++++++++++
>>
>> Filename must match compatible.
> 
> Whoops! Will fix in v2.
> 
> 
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - google,lga-mailbox-array
>>> +      - const: google,mailbox-array
>>
>> Don't use generic fallback. Just the SoCs.
> 
> Sure, if you insist.
> 
> In general the "mba" hardware is designed with enough identification
> registers that we should be able to autodetect which variant we're on.
> Thus, my hope is to not ever need to reference the SoC-specific
> variant in the driver itself. It's not the end of the world to use the
> "google,lga-mailbox-array" as the generic, I guess...
> 
> 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...

> 
> Sure, it means that if someone unexpectedly makes a new Google
> mailbox-array that's totally incompatible then the
> "google,mailbox-array" sounds too generic, but that doesn't feel like
> the end of the world. You could call the new mailbox array designed in

And there is simple solution, just use SoC compatibles. Everything is
elegant, simple and accurate.

> the year 2037 the "google,2037-mailbox-array" and things would overall
> be less confusing than using the "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/ ?

> 

> 
>>> +
>>> +  ranges: true
>>> +
>>> +  "#address-cells":
>>> +    const: 1
>>> +
>>> +  "#size-cells":
>>> +    const: 1
>>> +
>>> +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...

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

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.

> 
> 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.

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.

> an array describing the communication conventions for each channel.
> Can it be done? Yes. Is it ugly? Also, yes.
> 
> Further evidence that the hardware design intended "a bunch of
> mailboxes" rather than "a mailbox with channels" is that the IP block
> is called a "mailbox array". ;-)
Best regards,
Krzysztof

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

[Doug Anderson]

Hi,

On Wed, Jul 15, 2026 at 10:47 PM Krzysztof Kozlowski <krzk@kernel.org> 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

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

[Doug Anderson]

Hi,

On Thu, Jul 16, 2026 at 9:31 AM Doug Anderson <dianders@chromium.org> wrote:
>
> > >>> +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.

FWIW, I still feel fairly strongly about the fact that #mailbox-cells
shuld be 0 here and the fact that each mailbox in this mailbox array
is best expressed in the device-tree by its own node since. In other
words, the best model for this hardware is an array of single-channel
mailboxes, not one mailbox with many channels. The main reason is that
each mailbox in the array is _not_ homogeneous and is not intended to
be. The main argument is that every mailbox in the array can have a
different message-buffer size. While we can discover the
message-buffer size of each mailbox in the array at runtime by reading
hardware registers, the different message-buffer sizes suggest that
each mailbox in the array is designed to be able to use a different
message-passing convention. The message-passing convention is defined
by the firmware on the remote side and is _not_ discoverable, so we
need a place in the device tree to describe it for each mailbox. Each
mailbox in the array having its own node is the proper place to put
information about this convention.

Krzysztof: Do you still oppose this? I feel pretty strongly, so before
changing this to some awkward bindings, I'd love to get confirmation.
I'd also be interested if anyone else on the CC list has opinions on
the topic.

-Doug

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

[Jassi Brar]

Hi,
  Replying in one place to the two main points of contention ...

1) Compatible string :-
   I too think leaving it too generic is a bit bold ... we often think
it is final but more often it turns out to not be so. But I also don't
particularly like the idea of naming it after the SoC because
controller IPs are usually not tied to a SoC. So giving it a
controller version specific name should be good. I hope we treat all
controllers as potentially 3rd reusable blocks rather than a part of
SoC's identity.

2) Single-channel Controllers vs Multi-channel Controller :-
  Looking at the description of h/w, especially the separate and
optional register sets and the fact that we are not talking runtime
ad-hoc links between two endpoints, I lean towards single-channel
controllers implementation. That seems like tidier dts+code.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Wed, Jul 29, 2026 at 6:27 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> Hi,
>   Replying in one place to the two main points of contention ...
>
> 1) Compatible string :-
>    I too think leaving it too generic is a bit bold ... we often think
> it is final but more often it turns out to not be so. But I also don't
> particularly like the idea of naming it after the SoC because
> controller IPs are usually not tied to a SoC. So giving it a
> controller version specific name should be good. I hope we treat all
> controllers as potentially 3rd reusable blocks rather than a part of
> SoC's identity.

Jassi: How strongly do you feel about the above? It seems like
Krzysztof and Rob both feel strongly that any type of generic
compatible string (including a versioned generic name) is not OK. They
seem to strongly believe it should be named after the first SoC that
was upstreamed that contained the IP block.

Will you object if I send a v2 with "google,lga-mailbox-array" as the
compatible string and no generic?

Personally, I don't think this is worth fighting more about, but if
you feel strongly about it then I guess we need to resolve things
between you and the DT maintainers before I can send a v2?


> 2) Single-channel Controllers vs Multi-channel Controller :-
>   Looking at the description of h/w, especially the separate and
> optional register sets and the fact that we are not talking runtime
> ad-hoc links between two endpoints, I lean towards single-channel
> controllers implementation. That seems like tidier dts+code.

Thanks for your opinion. Unless I hear more thoughts on this, I'd be
inclined to send v2 while keeping one node for each single-channel
mailbox.

Jassi: do you want to review anything else in this series before I
send a v2? ...or I can just send a v2 and we can do further review
there. :-)


-Doug

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

[Rob Herring]

On Wed, Jul 15, 2026 at 09:49:14AM -0700, Doug Anderson wrote:
> Hi,
> 
> On Tue, Jul 14, 2026 at 9:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 15/07/2026 00:21, Douglas Anderson wrote:
> > > Introduce bindings for the MailBox Array IP block present in Laguna
> > > SoCs (AKA "lga", AKA "Google Tensor G5").
> > >
> > > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > > ---
> > >
> > >  .../bindings/mailbox/google,mba.yaml          | 216 ++++++++++++++++++
> >
> > Filename must match compatible.
> 
> Whoops! Will fix in v2.
> 
> 
> > > +properties:
> > > +  compatible:
> > > +    items:
> > > +      - enum:
> > > +          - google,lga-mailbox-array
> > > +      - const: google,mailbox-array
> >
> > Don't use generic fallback. Just the SoCs.
> 
> Sure, if you insist.
> 
> In general the "mba" hardware is designed with enough identification
> registers that we should be able to autodetect which variant we're on.
> Thus, my hope is to not ever need to reference the SoC-specific
> variant in the driver itself. It's not the end of the world to use the
> "google,lga-mailbox-array" as the generic, I guess...
> 
> 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
> 
> 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

I find 4 strings nicer than 5 strings.

> Sure, it means that if someone unexpectedly makes a new Google
> mailbox-array that's totally incompatible then the
> "google,mailbox-array" sounds too generic, but that doesn't feel like
> the end of the world. You could call the new mailbox array designed in
> the year 2037 the "google,2037-mailbox-array" and things would overall
> be less confusing than using the "google,lga-mailbox-array" as the
> generic.

What exactly do we need to do to stop having this conversation? We've 
done this scheme and version numbers and it never ends well.

The reality is the h/w folks can't help themselves from changing things, 
so nothing remains unchanged for very many generations.

Rob

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

[Doug Anderson]

Hi,

On Wed, Jul 22, 2026 at 7:10 AM Rob Herring <robh@kernel.org> wrote:
>
> > Sure, it means that if someone unexpectedly makes a new Google
> > mailbox-array that's totally incompatible then the
> > "google,mailbox-array" sounds too generic, but that doesn't feel like
> > the end of the world. You could call the new mailbox array designed in
> > the year 2037 the "google,2037-mailbox-array" and things would overall
> > be less confusing than using the "google,lga-mailbox-array" as the
> > generic.
>
> What exactly do we need to do to stop having this conversation? We've
> done this scheme and version numbers and it never ends well.
>
> The reality is the h/w folks can't help themselves from changing things,
> so nothing remains unchanged for very many generations.

Stop having the conversation with me, or with everyone?

With me, I'll stop pushing since I think I understand pretty well
where you and Krzysztof stand on the matter now. Even here, I wasn't
pushing hard on the matter but I was trying to understand exactly
where the boundary was. As I understood it, things with enough
"identification" to disambiguate themselves could use something more
generic, especially if no preexisting binding existed. I clearly
misunderstood. Mea culpa for the noise on this one.

I'm not sure how to avoid repeating the conversation with the rest of
the world. I think this will always be a confusing topic.

-Doug

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

[Douglas Anderson]

Add a driver for the MailBox Array IP block present in Laguna SoCs
(AKA "lga", AKA "Google Tensor G5"). The mailbox hardware and theory
of operation is described in detail in the devicetree bindings.

This driver requires two improvements to the mailbox core in order to
function properly. Notably:
* mailbox: Add support for mailbox controllers that can queue
* mailbox: Find a matching mailbox by fwnode rather than device

The goog-mba hardware and conventions used to communicate to remote
processors is described in detail in the bindings file. See the
bindings patch ("dt-bindings: mailbox: goog-mba: Add goog-mba mailbox
bindings").

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---
This driver is a rewrite from the downstream driver used in Pixel
phones and thus is only lightly tested. The downstream driver needed
to jump through some awkward hoops in order to work around the above
two patches not being present in the mailbox core.

 MAINTAINERS                              |   8 +
 drivers/mailbox/Kconfig                  |   8 +
 drivers/mailbox/Makefile                 |   2 +
 drivers/mailbox/goog-mba-priv.h          | 108 +++++
 drivers/mailbox/goog-mba-trace.h         | 183 ++++++++
 drivers/mailbox/goog-mba.c               | 567 +++++++++++++++++++++++
 include/linux/mailbox/goog-mba-message.h |  38 ++
 7 files changed, 914 insertions(+)
 create mode 100644 drivers/mailbox/goog-mba-priv.h
 create mode 100644 drivers/mailbox/goog-mba-trace.h
 create mode 100644 drivers/mailbox/goog-mba.c
 create mode 100644 include/linux/mailbox/goog-mba-message.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f37a81950e25..09da5b3a0e86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11070,6 +11070,14 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git
 F:	drivers/firmware/google/
 F:	include/linux/coreboot.h
 
+GOOGLE MAILBOX ARRAY
+M:	Douglas Anderson <dianders@chromium.org>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/mailbox/google,mba.yaml
+F:	drivers/mailbox/goog-mba*
+F:	include/linux/mailbox/goog-mba-message.h
+
 GOOGLE TENSOR SoC SUPPORT
 M:	Peter Griffin <peter.griffin@linaro.org>
 R:	André Draszik <andre.draszik@linaro.org>
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 3062ee352f78..ddd08e473b57 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -399,4 +399,12 @@ config RISCV_SBI_MPXY_MBOX
 	  or HS-mode hypervisor). Say Y here, unless you are sure you do not
 	  need this.
 
+config GOOG_MBA_MBOX
+	tristate "Google Tensor MBA Mailbox"
+	help
+	  An implementation of the Google MailBox Array (MBA) driver present in
+	  Google Tensor SoCs starting with Laguna (G5). It is used for
+	  inter-processor communication between the application processor and
+	  co-processors.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 944d8ea39f34..56b18641f50e 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -84,3 +84,5 @@ obj-$(CONFIG_CIX_MBOX)	+= cix-mailbox.o
 obj-$(CONFIG_BCM74110_MAILBOX)	+= bcm74110-mailbox.o
 
 obj-$(CONFIG_RISCV_SBI_MPXY_MBOX)	+= riscv-sbi-mpxy-mbox.o
+
+obj-$(CONFIG_GOOG_MBA_MBOX)	+= goog-mba.o
diff --git a/drivers/mailbox/goog-mba-priv.h b/drivers/mailbox/goog-mba-priv.h
new file mode 100644
index 000000000000..bfe03993a7d4
--- /dev/null
+++ b/drivers/mailbox/goog-mba-priv.h
@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Google LLC
+ */
+
+#ifndef _GOOG_MBA_PRIV_H_
+#define _GOOG_MBA_PRIV_H_
+
+#include <linux/mailbox_controller.h>
+
+struct goog_mbox_info {
+	/** @mbox: Mailbox controller structure. */
+	struct mbox_controller mbox;
+
+	/** @np: Device tree node */
+	struct device_node *np;
+
+	/** @chan: Mailbox channel structure; only 1 channel per mailbox. */
+	struct mbox_chan chan;
+
+	/** @mba: Pointer to the mailbox array containing this mailbox. */
+	struct goog_mba_info *mba;
+
+	/** @iomem: IO memory associated with this mailbox. */
+	void __iomem *iomem;
+
+	/** @irq: Linux IRQ associated with this mailbox. */
+	int irq;
+
+	/** @msg_buffer_words: Number of 32-bit words present in hardware. */
+	unsigned int msg_buffer_words;
+
+	/**
+	 * @tx_payload_words: Number of 32-bit words in a tx mailbox message.
+	 *
+	 * In queue mode this is detected on the first TX transfer and subsequent
+	 * transfers must match.
+	 */
+	unsigned int tx_payload_words;
+
+	/** @rx_payload_words: Number of 32-bit words in a rx mailbox message. */
+	unsigned int rx_payload_words;
+
+	/**
+	 * @rx_buffer: Memory storage for payload when receiving
+	 *
+	 * When we get a message from the other side we copy it here before
+	 * acknowledging the message and passing it to the client. Buffer
+	 * is `payload_words * 4` bytes big.
+	 */
+	u32 *rx_buffer;
+
+	/**
+	 * @queue_mode: If true, the other side uses the "queue mode" protocol.
+	 *
+	 * In the "queue mode" protocol, we can send more than one message at
+	 * once and we treat the message buffer like a circular queue, with
+	 * each entry being `payload_words` big.
+	 */
+	bool queue_mode;
+
+
+	/* QUEUE MODE ONLY BELOW */
+
+	/**
+	 * @tx_idx: For queue mode, index into msg buffer to write the next msg.
+	 *
+	 * Always between 0 and msg_buffer_words - 1. Increments by payload_words
+	 * after each transmission and wraps to 0 if it's == msg_buffer_words.
+	 */
+	unsigned int tx_idx;
+
+	/**
+	 * @rx_idx: For queue mode, index into msg buffer to read the next msg.
+	 *
+	 * Always between 0 and msg_buffer_words - 1. Increments by rx_payload_words
+	 * after each reception and wraps to 0 if it's == msg_buffer_words.
+	 */
+	unsigned int rx_idx;
+
+	/**
+	 * @lock: For queue mode, protects outstanding_msgs
+	 *
+	 * We update `outstanding_msgs` in the interrupt handler and when
+	 * queuing up a message. This protects those two accesses.
+	 */
+	spinlock_t lock;
+
+	/**
+	 * @outstanding_msgs: For queue mode, num msgs we've written but not acked
+	 *
+	 * After we start each transmission we grab the `lock` and increment
+	 * this by 1. In the interrupt handler when we see that some messages
+	 * were transferred we decrease this and send out the proper number
+	 * of acks.
+	 */
+	unsigned int outstanding_msgs;
+};
+
+struct goog_mba_info {
+	/** @dev: Pointer to the `struct device` */
+	struct device *dev;
+
+	/** @global_iomem: Pointer to global IO memory, or NULL */
+	void __iomem *global_iomem;
+};
+
+#endif /* _GOOG_MBA_PRIV_H_ */
diff --git a/drivers/mailbox/goog-mba-trace.h b/drivers/mailbox/goog-mba-trace.h
new file mode 100644
index 000000000000..02be79757fcb
--- /dev/null
+++ b/drivers/mailbox/goog-mba-trace.h
@@ -0,0 +1,183 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM goog_mba
+
+#if !defined(_GOOG_MBA_TRACE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _GOOG_MBA_TRACE_H
+
+#include <linux/tracepoint.h>
+
+#include "goog-mba-priv.h"
+
+TRACE_EVENT(
+	goog_mba_process_nq_txdone,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox),
+
+	TP_ARGS(goog_mbox),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+	),
+
+	TP_printk("%s %s", __get_str(dev_name), __get_str(name))
+);
+
+TRACE_EVENT(
+	goog_mba_process_q_txdone,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox, u32 reqs_completed, u32 outstanding_msgs),
+
+	TP_ARGS(goog_mbox, reqs_completed, outstanding_msgs),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+		__field(u32, outstanding_msgs)
+		__field(u32, reqs_completed)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+		__entry->outstanding_msgs = outstanding_msgs;
+		__entry->reqs_completed = reqs_completed;
+	),
+
+	TP_printk("%s %s: reqs_completed=%u outstanding_msgs=%u",
+		  __get_str(dev_name), __get_str(name), __entry->reqs_completed,
+		  __entry->outstanding_msgs)
+);
+
+TRACE_EVENT(
+	goog_mba_send_data_nq,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox, const u32 *payload,
+		 unsigned int payload_words),
+
+	TP_ARGS(goog_mbox, payload, payload_words),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+		__field(u32, payload_words)
+		__dynamic_array(u32, payload, payload_words)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+		__entry->payload_words = payload_words;
+		memcpy(__get_dynamic_array(payload), payload,
+		       payload_words * sizeof(u32));
+	),
+
+	TP_printk("%s %s: data=%s",
+		  __get_str(dev_name), __get_str(name),
+		  __print_array(__get_dynamic_array(payload),
+				__entry->payload_words, sizeof(u32)))
+);
+
+TRACE_EVENT(
+	goog_mba_send_data_q,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox, const u32 *payload,
+		 unsigned int payload_words),
+
+	TP_ARGS(goog_mbox, payload, payload_words),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+		__field(u32, tx_idx)
+		__field(u32, payload_words)
+		__dynamic_array(u32, payload, payload_words)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+		__entry->tx_idx = goog_mbox->tx_idx;
+		__entry->payload_words = payload_words;
+		memcpy(__get_dynamic_array(payload), payload,
+		       payload_words * sizeof(u32));
+	),
+
+	TP_printk("%s %s: tx_idx=%u data=%s",
+		  __get_str(dev_name), __get_str(name), __entry->tx_idx,
+		  __print_array(__get_dynamic_array(payload),
+				__entry->payload_words, sizeof(u32)))
+);
+
+TRACE_EVENT(
+	goog_mba_process_nq_rx,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox),
+
+	TP_ARGS(goog_mbox),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+		__field(u32, rx_payload_words)
+		__dynamic_array(u32, payload, goog_mbox->rx_payload_words)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+		__entry->rx_payload_words = goog_mbox->rx_payload_words;
+		memcpy(__get_dynamic_array(payload), goog_mbox->rx_buffer,
+		       goog_mbox->rx_payload_words * sizeof(u32));
+	),
+
+	TP_printk("%s %s: data=%s",
+		  __get_str(dev_name), __get_str(name),
+		  __print_array(__get_dynamic_array(payload),
+				__entry->rx_payload_words, sizeof(u32)))
+);
+
+TRACE_EVENT(
+	goog_mba_process_q_rx,
+
+	TP_PROTO(const struct goog_mbox_info *goog_mbox),
+
+	TP_ARGS(goog_mbox),
+
+	TP_STRUCT__entry(
+		__string(dev_name, dev_name(goog_mbox->mba->dev))
+		__string(name, goog_mbox->np->full_name)
+		__field(u32, rx_idx)
+		__field(u32, rx_payload_words)
+		__dynamic_array(u32, payload, goog_mbox->rx_payload_words)
+	),
+
+	TP_fast_assign(
+		__assign_str(dev_name);
+		__assign_str(name);
+		__entry->rx_idx = goog_mbox->rx_idx;
+		__entry->rx_payload_words = goog_mbox->rx_payload_words;
+		memcpy(__get_dynamic_array(payload), goog_mbox->rx_buffer,
+		       goog_mbox->rx_payload_words * sizeof(u32));
+	),
+
+	TP_printk("%s %s: rx_idx=%u data=%s",
+		  __get_str(dev_name), __get_str(name), __entry->rx_idx,
+		  __print_array(__get_dynamic_array(payload),
+				__entry->rx_payload_words, sizeof(u32)))
+);
+
+#endif /* _GOOG_MBA_TRACE_H */
+
+/* This part must be outside protection */
+#undef TRACE_INCLUDE_PATH
+#define TRACE_INCLUDE_PATH ../drivers/mailbox
+#undef TRACE_INCLUDE_FILE
+#define TRACE_INCLUDE_FILE goog-mba-trace
+#include <trace/define_trace.h>
diff --git a/drivers/mailbox/goog-mba.c b/drivers/mailbox/goog-mba.c
new file mode 100644
index 000000000000..54b6ed787b71
--- /dev/null
+++ b/drivers/mailbox/goog-mba.c
@@ -0,0 +1,567 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Google MailBox Array (MBA) Driver
+ *
+ * Copyright (c) 2025 Google LLC
+ */
+
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/goog-mba-message.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "goog-mba-priv.h"
+
+#define CREATE_TRACE_POINTS
+#include "goog-mba-trace.h"
+
+#define CLIENT_IRQ_TRIG_OFFSET		0x0
+#define SET_HOST_IRQ			0x1
+
+#define CLIENT_IRQ_CONFIG_OFFSET	0x4
+#define ENABLE_HOST_AUTO_ACK		BIT(8)
+#define CLIENT_IRQ_MASK_MSG_INT		BIT(16)
+#define CLIENT_IRQ_MASK_ACK_INT		BIT(24)
+
+#define CLIENT_IRQ_STATUS_OFFSET	0x8
+#define CLIENT_IRQ_STATUS_MSG_INT	0x1
+#define CLIENT_IRQ_STATUS_ACK_INT	0x100
+
+#define CLIENT_MBA_IP_VER		0x30
+#define CLIENT_NUM_MSG_REG		0x34
+#define CLIENT_OUTSTANDING_MSG		0x10
+
+#define GLOBAL_NUM_MSG_REG_OFFSET(x)	(0x10 + ((x) * 4))
+#define GLOBAL_MBA_IP_VER_OFFSET	4
+
+#define CLIENT_CLIENT_DOORBELL_TRIG	0x20
+#define CLIENT_DOORBELL_MASK_OFFSET	0x24
+#define CLIENT_DOORBELL_STATUS_OFFSET	0x28
+#define CLIENT_HOST_DOORBELL_OFFSET	0x38
+#define CLIENT_CLIENT_DOORBELL_OFFSET	0x3c
+
+#define MAX_MBA_CHANNELS		1
+#define NR_PHANDLE_ARG_COUNT		1
+
+#define MSG_OFFSET(i)			(0x100 + (i) * sizeof(u32))
+
+#define MBA_IP_MAJOR_VER_1		0x1
+#define MBA_IP_MAJOR_VER_3		0x3
+
+#define MBA_IP_MAJOR_VER_SHIFT		24
+#define MBA_IP_MAJOR_VER_MASK		0xff
+#define MBA_IP_MINOR_VER_SHIFT		16
+#define MBA_IP_MINOR_VER_MASK		0xff
+#define MBA_IP_INCREMENTAL_VER_SHIFT	0
+#define MBA_IP_INCREMENTAL_VER_MASK	0xffff
+
+/**
+ * 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);
+}
+
+/**
+ * goog_mba_handle_rx_interrupt() - Handle interrupt that remote send a msg.
+ * @goog_mbox: The mailbox info.
+ */
+static void goog_mba_handle_rx_interrupt(struct goog_mbox_info *goog_mbox)
+{
+	struct goog_mba_rx_msg msg = {
+		.payload = goog_mbox->rx_buffer,
+		.payload_words = goog_mbox->rx_payload_words,
+	};
+	int i;
+
+	for (i = 0; i < goog_mbox->rx_payload_words; i++)
+		goog_mbox->rx_buffer[i] = readl(goog_mbox->iomem +
+						MSG_OFFSET(goog_mbox->rx_idx + i));
+
+	if (goog_mbox->queue_mode) {
+		goog_mbox->rx_idx = (goog_mbox->rx_idx + goog_mbox->rx_payload_words) %
+				    goog_mbox->msg_buffer_words;
+		trace_goog_mba_process_q_rx(goog_mbox);
+	} else {
+		trace_goog_mba_process_nq_rx(goog_mbox);
+	}
+
+	/*
+	 * Ack the interrupt after we've read the message to local memory but
+	 * before passing it to the client. This frees up space for the remote
+	 * processor to send another message as the client is processing this one.
+	 */
+	writel(CLIENT_IRQ_STATUS_MSG_INT, goog_mbox->iomem + CLIENT_IRQ_STATUS_OFFSET);
+
+	mbox_chan_received_data(&goog_mbox->chan, &msg);
+}
+
+/**
+ * goog_mba_isr() - Main interrupt routine
+ * @irq: The IRQ number.
+ * @data: Data passed when registering (the goog_mbox_info for this IRQ).
+ *
+ * Return: IRQ_HANDLED if IRQ was handled; IRQ_NONE if no interrupt was found.
+ */
+static irqreturn_t goog_mba_isr(int irq, void *data)
+{
+	struct goog_mbox_info *goog_mbox = data;
+	u32 irq_status;
+
+	irq_status = readl(goog_mbox->iomem + CLIENT_IRQ_STATUS_OFFSET);
+	if (!irq_status)
+		return IRQ_NONE;
+
+	while (true) {
+		if (irq_status & CLIENT_IRQ_STATUS_ACK_INT)
+			goog_mba_handle_tx_interrupt(goog_mbox);
+
+		if (irq_status & CLIENT_IRQ_STATUS_MSG_INT) {
+			goog_mba_handle_rx_interrupt(goog_mbox);
+
+			/*
+			 * In queue mode the RX interrupt will re-assert itself
+			 * right after we clear it if there is more than one
+			 * message waiting. As an optimization to avoid returning
+			 * and immediately re-triggering our interrupt, re-check.
+			 *
+			 * NOTE: we don't need to loop for the TX (Ack) case
+			 * since TX interrupts aren't counted in the same way.
+			 */
+			if (goog_mbox->queue_mode)
+				irq_status = readl(goog_mbox->iomem + CLIENT_IRQ_STATUS_OFFSET);
+			else
+				break;
+		} else {
+			break;
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * goog_mba_send_data() - Mailbox op for send_data.
+ * @chan: The Linux mbox_chan structure associated with the mailbox.
+ * @msg: The mailbox message, expected to be of type `struct goog_mba_tx_msg`.
+ *	 If NULL, we'll assume a 0-byte doorbell-only message.
+ *
+ * Return: 0 if the data was sent; -EBUSY if the queue was full; other
+ *	   negative error values for other problems.
+ */
+static int goog_mba_send_data(struct mbox_chan *chan, void *msg)
+{
+	struct goog_mbox_info *goog_mbox = chan->con_priv;
+	struct device *dev = goog_mbox->mba->dev;
+	bool queue_mode = goog_mbox->queue_mode;
+	unsigned int payload_words = 0;
+	const u32 *payload = NULL;
+	bool is_init = false;
+	unsigned int tx_idx;
+	unsigned int i;
+	unsigned long flags;
+
+	if (msg) {
+		struct goog_mba_tx_msg *mba_msg = msg;
+
+		payload_words = mba_msg->payload_words;
+		payload = mba_msg->payload;
+		is_init = mba_msg->init;
+	}
+
+	if (payload_words > goog_mbox->msg_buffer_words) {
+		dev_err(dev, "%pOFP: Payload too big: %u > %u\n",
+			goog_mbox->np, payload_words, goog_mbox->msg_buffer_words);
+		return -EINVAL;
+	}
+
+	/*
+	 * Currently all queue-mode transfers need to be the same size and
+	 * need to evenly divide the message buffer. If this is the first
+	 * transfer, we need to validate/store the payload_words. For subsequent
+	 * transfers we just need to confirm it hasn't changed.
+	 */
+	if (queue_mode) {
+		if (!goog_mbox->tx_payload_words && payload_words) {
+			if (goog_mbox->msg_buffer_words % payload_words != 0) {
+				dev_err(dev, "%pOFP: Invalid initial TX queue payload words: %u\n",
+					goog_mbox->np, payload_words);
+				return -EINVAL;
+			}
+			goog_mbox->tx_payload_words = payload_words;
+		} else if (payload_words != goog_mbox->tx_payload_words) {
+			dev_err(dev, "%pOFP: Payload size mismatch: %u vs %u\n",
+				goog_mbox->np, payload_words, goog_mbox->tx_payload_words);
+			return -EINVAL;
+		}
+	}
+
+	/*
+	 * HW can handle full duplex but there is no current scheme for dividing
+	 * up the message buffer between TX and RX portions. Non-queue mode
+	 * could work half-duplex, but that doesn't make sense in queue mode.
+	 * If space is reserved for RX then disallow using it for TX.
+	 */
+	if (queue_mode && payload_words && goog_mbox->rx_payload_words) {
+		dev_err(dev, "%pOFP: Can't TX if buffer is used for RX\n", goog_mbox->np);
+		return -EINVAL;
+	}
+
+	if (queue_mode) {
+		bool out_of_space;
+
+		spin_lock_irqsave(&goog_mbox->lock, flags);
+		out_of_space = payload_words &&
+			       (goog_mbox->outstanding_msgs >=
+				goog_mbox->msg_buffer_words / payload_words);
+		spin_unlock_irqrestore(&goog_mbox->lock, flags);
+
+		/*
+		 * IMPORTANT: don't print an error for this. It's normal and
+		 * expected that the core will keep trying to queue messages
+		 * until the controller reports -EBUSY.
+		 */
+		if (out_of_space)
+			return -EBUSY;
+
+		tx_idx = goog_mbox->tx_idx;
+		trace_goog_mba_send_data_q(goog_mbox, payload, payload_words);
+	} else {
+		tx_idx = 0;
+		trace_goog_mba_send_data_nq(goog_mbox, payload, payload_words);
+	}
+
+	for (i = 0; i < payload_words; i++)
+		writel(payload[i], goog_mbox->iomem + MSG_OFFSET(tx_idx + i));
+
+	if (queue_mode) {
+		if (is_init)
+			goog_mbox->tx_idx = 0;
+		else
+			goog_mbox->tx_idx = (goog_mbox->tx_idx + payload_words) %
+					    goog_mbox->msg_buffer_words;
+	}
+
+	/*
+	 * We need to be careful on how we deal with `outstanding_msgs`.
+	 * The hardware will give us an interrupt every time it transfers
+	 * a message, but if it transfers two messages before our interrupt
+	 * handler fires then we'll still only get one interrupt. We have
+	 * to look at the difference between the hardware's idea of
+	 * `outstanding_msgs` and ours to figure out how many messages were
+	 * actually transferred.
+	 *
+	 * We need to start the transfer and increment our concept of
+	 * `outstanding_msgs` in lockstep so protect against the interrupt
+	 * handler also touching `outstanding_msgs`.
+	 */
+	if (queue_mode)
+		spin_lock_irqsave(&goog_mbox->lock, flags);
+
+	writel(SET_HOST_IRQ, goog_mbox->iomem + CLIENT_IRQ_TRIG_OFFSET);
+
+	if (queue_mode) {
+		goog_mbox->outstanding_msgs++;
+		spin_unlock_irqrestore(&goog_mbox->lock, flags);
+	}
+
+	return 0;
+}
+
+/**
+ * goog_mba_startup() - Mailbox op for startup.
+ * @chan: The Linux mbox_chan structure associated with the mailbox.
+ *
+ * Return: 0 for OK; negative error code if problems.
+ */
+static int goog_mba_startup(struct mbox_chan *chan)
+{
+	struct goog_mbox_info *goog_mbox = chan->con_priv;
+
+	writel(ENABLE_HOST_AUTO_ACK | CLIENT_IRQ_MASK_MSG_INT | CLIENT_IRQ_MASK_ACK_INT,
+	       goog_mbox->iomem + CLIENT_IRQ_CONFIG_OFFSET);
+	enable_irq(goog_mbox->irq);
+
+	return 0;
+}
+
+/**
+ * goog_mba_shutdown() - Mailbox op for shutdown.
+ * @chan: The Linux mbox_chan structure associated with the mailbox.
+ */
+static void goog_mba_shutdown(struct mbox_chan *chan)
+{
+	struct goog_mbox_info *goog_mbox = chan->con_priv;
+
+	disable_irq(goog_mbox->irq);
+	writel(0x0, goog_mbox->iomem + CLIENT_IRQ_CONFIG_OFFSET);
+}
+
+static const struct mbox_chan_ops goog_mba_chan_ops = {
+	.send_data = goog_mba_send_data,
+	.startup = goog_mba_startup,
+	.shutdown = goog_mba_shutdown,
+};
+
+/**
+ * goog_mba_get_msg_buf_words() - Return # msg buffer words in HW for this mailbox.
+ * @goog_mbox: The mailbox info.
+ * @np: The device tree node associated with this mailbox.
+ *
+ * Return: The number of 32-bit words in the hardware message buffer.
+ */
+static unsigned int goog_mba_get_msg_buf_words(struct goog_mbox_info *goog_mbox,
+					       struct device_node *np)
+{
+	struct goog_mba_info *mba = goog_mbox->mba;
+	struct resource res;
+	unsigned int index;
+
+	/*
+	 * On newer IP there's no global space and the register moved to the
+	 * client address space.
+	 */
+	if (!mba->global_iomem)
+		return readl(goog_mbox->iomem + CLIENT_NUM_MSG_REG);
+
+	/*
+	 * On older IP we need to find the index so we can look up the value
+	 * in the global memory. Our index is based on the physical address
+	 * of the mbox since on older hardware mailboxes are 4K apart.
+	 */
+	if (of_address_to_resource(np, 0, &res)) {
+		dev_err(mba->dev, "%pOFP: Failed to find physical address\n", np);
+		return 0;
+	}
+	index = (res.start & 0x1ffff) / 0x1000;
+
+	return readl(mba->global_iomem + GLOBAL_NUM_MSG_REG_OFFSET(index));
+}
+
+/**
+ * of_node_put_void() - of_node_put() for passing to devm_add_action_or_reset().
+ * @data: Our struct device_node pointer.
+ */
+static void of_node_put_void(void *data)
+{
+	of_node_put(data);
+}
+
+/**
+ * goog_mba_mbox_init() - Initialize one single mailbox.
+ * @goog_mbox: The mailbox info.
+ * @mba: The array containing this mailbox.
+ * @np: The device tree node associated with this mailbox.
+ *
+ * Return: 0 or a negative error code. If an error is returned, the error has
+ *	   already been logged.
+ */
+static int goog_mba_mbox_init(struct goog_mbox_info *goog_mbox,
+			      struct goog_mba_info *mba, struct device_node *np)
+{
+	struct mbox_controller *mbox = &goog_mbox->mbox;
+	struct mbox_chan *chan = &goog_mbox->chan;
+	struct device *dev = mba->dev;
+	unsigned int msg_buffer_words;
+	u32 rx_payload_words;
+	int ret;
+
+	goog_mbox->mba = mba;
+
+	of_node_get(np);
+	ret = devm_add_action_or_reset(dev, of_node_put_void, np);
+	if (ret)
+		return ret;
+	goog_mbox->np = np;
+
+	goog_mbox->iomem = devm_of_iomap(dev, np, 0, NULL);
+	if (IS_ERR(goog_mbox->iomem))
+		return dev_err_probe(dev, PTR_ERR(goog_mbox->iomem),
+				     "%pOFP: Failed to map memory\n", np);
+
+	/* Start with all interrupts disabled and cleared */
+	writel(0x0, goog_mbox->iomem + CLIENT_IRQ_CONFIG_OFFSET);
+	writel(CLIENT_IRQ_STATUS_MSG_INT | CLIENT_IRQ_STATUS_ACK_INT,
+	       goog_mbox->iomem + CLIENT_IRQ_STATUS_OFFSET);
+
+	goog_mbox->irq = of_irq_get(np, 0);
+	if (goog_mbox->irq <= 0) {
+		if (goog_mbox->irq == 0)
+			goog_mbox->irq = -EINVAL;
+		return dev_err_probe(dev, goog_mbox->irq, "%pOFP: Failed to get IRQ\n", np);
+	}
+	ret = devm_request_irq(dev, goog_mbox->irq, goog_mba_isr,
+			       IRQF_NO_SUSPEND | IRQF_NO_AUTOEN,
+			       NULL, goog_mbox);
+	if (ret)
+		return dev_err_probe(dev, ret, "%pOFP: Failed to request interrupt\n", np);
+
+	goog_mbox->queue_mode = of_property_read_bool(np, "google,mba-queue-mode");
+
+	rx_payload_words = 0;
+	ret = of_property_read_u32(np, "google,rx-payload-words", &rx_payload_words);
+	goog_mbox->rx_payload_words = rx_payload_words;
+
+	msg_buffer_words = goog_mba_get_msg_buf_words(goog_mbox, np);
+	goog_mbox->msg_buffer_words = msg_buffer_words;
+
+	if (goog_mbox->queue_mode && !msg_buffer_words)
+		return dev_err_probe(dev, -EINVAL,
+				     "%pOFP: Queue mode requires non-zero buffer\n", np);
+
+	if (msg_buffer_words < rx_payload_words)
+		return dev_err_probe(dev, -EINVAL,
+				     "%pOFP: Buffer (%u) < RX Payload (%u)\n",
+				     np, msg_buffer_words, rx_payload_words);
+	if (goog_mbox->queue_mode && rx_payload_words && msg_buffer_words % rx_payload_words != 0)
+		return dev_err_probe(dev, -EINVAL,
+				     "%pOFP: Queue buffer (%u) must be multiple of RX payload (%u)\n",
+				     np, msg_buffer_words, rx_payload_words);
+
+	if (rx_payload_words) {
+		goog_mbox->rx_buffer = devm_kcalloc(dev, rx_payload_words,
+						    sizeof(*goog_mbox->rx_buffer), GFP_KERNEL);
+		if (!goog_mbox->rx_buffer)
+			return -ENOMEM;
+	}
+
+	mbox->fwnode = of_fwnode_handle(np);
+	mbox->dev = dev;
+	mbox->ops = &goog_mba_chan_ops;
+	mbox->chans = chan;
+	mbox->num_chans = 1;
+	mbox->txdone_irq = true;
+	mbox->has_queue = goog_mbox->queue_mode;
+	spin_lock_init(&goog_mbox->lock);
+
+	chan->con_priv = goog_mbox;
+
+	ret = devm_mbox_controller_register(dev, mbox);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "%pOFP: Failed to register mailbox controller\n", np);
+
+	return 0;
+}
+
+/**
+ * goog_mba_probe() - MBA probe routine.
+ * @pdev: Our platform device.
+ *
+ * Return: 0 or a negative error code.
+ */
+static int goog_mba_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct goog_mbox_info *goog_mboxes;
+	struct goog_mba_info *mba;
+	unsigned int num_mboxes;
+	unsigned int i;
+	int ret;
+
+	mba = devm_kzalloc(dev, sizeof(*mba), GFP_KERNEL);
+	if (!mba)
+		return -ENOMEM;
+	mba->dev = dev;
+
+	num_mboxes = of_get_available_child_count(dev->of_node);
+	goog_mboxes = devm_kzalloc(dev, sizeof(*goog_mboxes) * num_mboxes, GFP_KERNEL);
+	if (!goog_mboxes)
+		return -ENOMEM;
+
+	/*
+	 * The first memory range is the memory range that the other side
+	 * of the mailbox uses. The second memory range is the shared/global
+	 * range. Note that newer versions of the IP don't have the global
+	 * range, so mba->global_iomem is left NULL on newer IP.
+	 */
+	if (platform_get_resource(pdev, IORESOURCE_MEM, 1)) {
+		mba->global_iomem = devm_platform_ioremap_resource(pdev, 1);
+		if (IS_ERR(mba->global_iomem))
+			return dev_err_probe(dev, PTR_ERR(mba->global_iomem),
+					     "Failed to iomap global region\n");
+	}
+
+	i = 0;
+	for_each_available_child_of_node_scoped(dev->of_node, child_np) {
+		ret = goog_mba_mbox_init(&goog_mboxes[i], mba, child_np);
+		if (ret)
+			return ret;
+		i++;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id goog_mba_match[] = {
+	{ .compatible = "google,mailbox-array" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, goog_mba_match);
+
+static struct platform_driver mba = {
+	.driver = {
+		.name = "goog-mba",
+		.of_match_table = goog_mba_match,
+	},
+	.probe = goog_mba_probe,
+};
+
+module_platform_driver(mba);
+
+MODULE_DESCRIPTION("Google MailBox Array (MBA) Driver");
+MODULE_AUTHOR("Douglas Anderson <dianders@chromium.org>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mailbox/goog-mba-message.h b/include/linux/mailbox/goog-mba-message.h
new file mode 100644
index 000000000000..5c3d47e3ded3
--- /dev/null
+++ b/include/linux/mailbox/goog-mba-message.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Google MailBox Array (MBA) Mailbox Message
+ *
+ * Copyright (c) 2025 Google LLC
+ */
+
+#ifndef _LINUX_MAILBOX_GOOG_MBA_MESSAGE_H_
+#define _LINUX_MAILBOX_GOOG_MBA_MESSAGE_H_
+
+#include <linux/types.h>
+
+struct goog_mba_tx_msg {
+	/** @payload: The contents of the message to send. */
+	const u32 *payload;
+
+	/** @payload_words: The number of 32-bit words in the payload. */
+	u8 payload_words;
+
+	/**
+	 * @init: Initialize queue settings after sending.
+	 *
+	 * Tell the mailbox driver that this is a special "initialize"
+	 * message for a queue-based mailbox. This allows the mailbox driver
+	 * to keep its state synced with the remote side of the mailbox.
+	 */
+	bool init;
+};
+
+struct goog_mba_rx_msg {
+	/** @payload: The contents of the message received. */
+	const u32 *payload;
+
+	/** @payload_words: The number of 32-bit words in the payload. */
+	u8 payload_words;
+};
+
+#endif /* _LINUX_MAILBOX_GOOG_MBA_MESSAGE_H_ */
-- 
2.55.0.141.g00534a21ce-goog

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

[Jassi Brar]

Hi Douglas,

On Tue, Jul 14, 2026 at 5:24 PM Douglas Anderson <dianders@chromium.org> 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?

Regards,
Jassi

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@gmail.com> wrote:
>
> Hi Douglas,
>
> On Tue, Jul 14, 2026 at 5:24 PM Douglas Anderson <dianders@chromium.org> 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

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

[Jassi Brar]

On Wed, Sep 9, 2026 at 10:53 AM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sat, Sep 5, 2026 at 6:20 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >
> > Hi Douglas,
> >
> > On Tue, Jul 14, 2026 at 5:24 PM Douglas Anderson <dianders@chromium.org> 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
>
I don't mean that ... though even that may be more acceptable to the
clients than you think. What class of clients are there?

I am suggesting to consider TX-Is-Done when send_data() returns, i.e
tx submitted is seen as transferred. You anyway  don't catch
transmission errors. That way you fill all slots without blocking on
the first message and achieve this same throughput.
Without knowing the clients I am not sure if that can't be done.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Wed, Sep 9, 2026 at 9:34 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Wed, Sep 9, 2026 at 10:53 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > Hi,
> >
> > On Sat, Sep 5, 2026 at 6:20 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> > >
> > > Hi Douglas,
> > >
> > > On Tue, Jul 14, 2026 at 5:24 PM Douglas Anderson <dianders@chromium.org> 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
> >
> I don't mean that ...

Ah, sorry for misunderstanding!

> though even that may be more acceptable to the
> clients than you think. What class of clients are there?

As far as I can tell, the mailbox controller is used for pretty much
everything in the system, so latency is pretty critical. Want a clock
turned on? You gotta send a mailbox message. Want a power domain
turned on? Mailbox message.

I don't think we can just drop the queuing support...


> I am suggesting to consider TX-Is-Done when send_data() returns, i.e
> tx submitted is seen as transferred. You anyway  don't catch
> transmission errors. That way you fill all slots without blocking on
> the first message and achieve this same throughput.
> Without knowing the clients I am not sure if that can't be done.

We need to know the txdone and thus we can't do what you're proposing.
In general, queuing mailboxes are used in cases where the remote side
enables a resource like a clock or power domain. Different tasks in
the system may independently turn on/off these resources, so allowing
them to work "in parallel" makes sense. ...but each task needs to know
for sure when its resource is finished turning on. If we just imagine
3 clocks we want to turn on.

clk_enable(clk_a)
clk_enable(clk_b)
clk_enable(clk_c)

Those 3 calls can be made in 3 different contexts from 3 different
drivers. We want all 3 enables happening in parallel with each other
(not one after another), but each call needs to know when its function
is done.

Does that make sense?

-Doug

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

[Jassi Brar]

On Wed, Sep 9, 2026 at 11:57 AM Doug Anderson <dianders@chromium.org> wrote:
>
> > I am suggesting to consider TX-Is-Done when send_data() returns, i.e
> > tx submitted is seen as transferred. You anyway  don't catch
> > transmission errors. That way you fill all slots without blocking on
> > the first message and achieve this same throughput.
> > Without knowing the clients I am not sure if that can't be done.
>
> We need to know the txdone and thus we can't do what you're proposing.
> In general, queuing mailboxes are used in cases where the remote side
> enables a resource like a clock or power domain. Different tasks in
> the system may independently turn on/off these resources, so allowing
> them to work "in parallel" makes sense
>
It will still work in parallel - you still keep writing to the h/w
fifo without waiting for previous ones to finish as long as there is
space. Just like you get here.

. ...but each task needs to know
> for sure when its resource is finished turning on. If we just imagine
> 3 clocks we want to turn on.
>
> clk_enable(clk_a)
> clk_enable(clk_b)
> clk_enable(clk_c)
>
> Those 3 calls can be made in 3 different contexts from 3 different
> drivers. We want all 3 enables happening in parallel with each other
> (not one after another), but each call needs to know when its function
> is done.
>
I don't see why not?  The three requests will be queued into h/w fifo
in the order they arrive without any block - just as you do now.

Btw, if you mean they need to be enabled in parallel literally, they
should not be called from three different drivers. Instead they should
be modelled as one "composite" clock.

> Does that make sense?
>
The usage makes sense but I still don't see why existing api can't work.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Sat, Sep 19, 2026 at 11:32 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Wed, Sep 9, 2026 at 11:57 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > > I am suggesting to consider TX-Is-Done when send_data() returns, i.e
> > > tx submitted is seen as transferred. You anyway  don't catch
> > > transmission errors. That way you fill all slots without blocking on
> > > the first message and achieve this same throughput.
> > > Without knowing the clients I am not sure if that can't be done.
> >
> > We need to know the txdone and thus we can't do what you're proposing.
> > In general, queuing mailboxes are used in cases where the remote side
> > enables a resource like a clock or power domain. Different tasks in
> > the system may independently turn on/off these resources, so allowing
> > them to work "in parallel" makes sense
> >
> It will still work in parallel - you still keep writing to the h/w
> fifo without waiting for previous ones to finish as long as there is
> space. Just like you get here.
>
> . ...but each task needs to know
> > for sure when its resource is finished turning on. If we just imagine
> > 3 clocks we want to turn on.
> >
> > clk_enable(clk_a)
> > clk_enable(clk_b)
> > clk_enable(clk_c)
> >
> > Those 3 calls can be made in 3 different contexts from 3 different
> > drivers. We want all 3 enables happening in parallel with each other
> > (not one after another), but each call needs to know when its function
> > is done.
> >
> I don't see why not?  The three requests will be queued into h/w fifo
> in the order they arrive without any block - just as you do now.
>
> Btw, if you mean they need to be enabled in parallel literally, they
> should not be called from three different drivers. Instead they should
> be modelled as one "composite" clock.
>
> > Does that make sense?
> >
> The usage makes sense but I still don't see why existing api can't work.

I must be missing something because I don't see how the existing API
can work. Let me explain in more detail and maybe you can tell me what
I've got wrong.

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.

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.
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.

Does the above example make sense? Can you explain how I could make
things work without changing the mailbox core?

-Doug

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

[Jassi Brar]

On Sat, Sep 19, 2026 at 3:40 PM Doug Anderson <dianders@chromium.org> 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.


> 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.


> 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. We can keep the core
unchanged and the driver much simpler if we simply use the buffering
in the core.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Sun, Sep 20, 2026 at 1:30 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> On Sat, Sep 19, 2026 at 3:40 PM Doug Anderson <dianders@chromium.org> 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.

> 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.

-Doug

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@chromium.org> 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

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

[Doug Anderson]

Hi,

On Sun, Sep 20, 2026 at 3:46 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> > 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.

Fair enough. I've jumped into a pre-existing design. Let me see if I
can find old information or gather evidence myself. Then with real
data we can figure out what makes sense. OK, gathered some data...

FWIW, to explain things clearly, I have been simplifying by saying
that just clock prepare/unprepare goes over this channel. In reality,
there is much more traffic. On Pixel 10, the mailbox using queue mode
like this connects to the "CPM" (central power manager). Looking at
the device tree, we see the following things using this mailbox:
* One of the main clock controllers in the system.
* Most of the power domains in the system
* Devfreq controllers
* Thermal controllers
* A GPIO controller
* A reset controller
* An interrupt controller
* A RTC
* A pile of other stuff

So basically a whole crap-ton of resources are managed over this
mailbox. The team designing this Phone apparently decided that the
mailbox is one of the primary communication pipelines in the system.
Yes, everything that communicates over the mailbox needs to be able to
sleep, but that doesn't mean we shouldn't keep it fast if possible.

On an off-the-shelf Pixel 10, one can spy on the CPM mailbox like this:

echo 1 > /sys/kernel/tracing/events/goog_mba_ctrl/enable
echo 1 > /sys/kernel/tracing/tracing_on
echo "" > /sys/kernel/tracing/trace
cat /sys/kernel/tracing/trace_pipe | grep 'process_q_t\|send_data_q'

When I do that, messages spew by pretty much constantly showing just
how busy this mailbox is. I can see instances where the queue is
actually utilized like this:

cat /sys/kernel/tracing/trace_pipe | \
  grep 'process_q_t\|send_data_q' | \
  grep -C10 'eqs_completed=[^1]\|anding_msg=[^0]'

If I do that, it's quite easy to see the queue being used. If I use
the device actively, I get several hits per second. A few examples
traces (using sed to shorten them slightly):

6946.884142: send_q: {0xa0016969,0xf,0x2,0x0} tx_q_wr_ptr=4
6946.884496: send_q: {0xa0026969,0x7,0x1,0x0} tx_q_wr_ptr=5
6946.884851: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=0
6946.884880: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=0

Here you can see that it took 709 us to get the response to the first
message. ...but, luckily we didn't have to wait for that 709 us before
sending the second message. That means we still got both responses at
once. Note that things aren't always so slow. The next messages
through this mailbox only took 42 us.

6946.885000: send_q: {0xa0036969,0x7,0x1,0x0} tx_q_wr_ptr=6
6946.885042: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0

Here's a pretty big usage of the queue. I assume the CPM was busy at the time:

7751.863328: send_q: {0xa0050708,0x1020001,0x0,0x0} tx_q_wr_ptr=7
7751.863619: send_q: {0xa0060708,0x1030001,0x0,0x0} tx_q_wr_ptr=0
7751.863850: send_q: {0xa0070708,0x1000001,0x0,0x0} tx_q_wr_ptr=1
7751.864123: send_q: {0xa0080708,0x1010001,0x0,0x0} tx_q_wr_ptr=2
7751.864329: q_txdone: tx_q_rd_ptr=7 reqs_completed=4 outstanding_msgs=0
7751.864338: q_txdone: tx_q_rd_ptr=0 reqs_completed=3 outstanding_msgs=0
7751.864347: q_txdone: tx_q_rd_ptr=1 reqs_completed=2 outstanding_msgs=0
7751.864378: q_txdone: tx_q_rd_ptr=2 reqs_completed=1 outstanding_msgs=0

A full millisecond before the first response, but luckily we got them
all at once.

...and this one is pretty interesting here:

7782.321344: send_q: {0xa0040708,0x30001,0x0,0x0} tx_q_wr_ptr=4
7782.321481: send_q: {0xa0150708,0x10001,0x0,0x0} tx_q_wr_ptr=5
7782.321769: send_q: {0xa0060708,0x1,0x0,0x0} tx_q_wr_ptr=6
7782.321799: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=1
7782.321813: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=1
7782.321842: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0

This one is interesting because we can see that the remote side first
ACKed two of the three outstanding messages. Then a short time later
it managed to ACK the last message.

...and just to show the queue being used quite often, here are two
instances right in a row (it's not hard to see this happening):

8521.693559: send_q: {0xa0010708,0x1030001,0x0,0x0} tx_q_wr_ptr=5
8521.693649: send_q: {0xa0120708,0x10001,0x0,0x0} tx_q_wr_ptr=6
8521.693690: q_txdone: tx_q_rd_ptr=5 reqs_completed=2 outstanding_msgs=0
8521.693701: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0
8521.693920: send_q: {0xa0030708,0x1,0x0,0x0} tx_q_wr_ptr=7
8521.693956: q_txdone: tx_q_rd_ptr=7 reqs_completed=1 outstanding_msgs=0
8521.694171: send_q: {0xa0040705,0x2,0x14,0x1} tx_q_wr_ptr=0
8521.694194: send_q: {0xa0150708,0x1000001,0x0,0x0} tx_q_wr_ptr=1
8521.694214: q_txdone: tx_q_rd_ptr=0 reqs_completed=2 outstanding_msgs=0
8521.694226: q_txdone: tx_q_rd_ptr=1 reqs_completed=1 outstanding_msgs=0

...and in case you want to see an even bigger use of the queue, here
are 6 queued up at once:

9223.248560: send_q: {0xa0060403,0x3050600,0x0,0x0} tx_q_wr_ptr=7
9223.248598: q_txdone: tx_q_rd_ptr=7 reqs_completed=1 outstanding_msgs=0
9223.248824: send_q: {0xa007000b,0x1,0x418,0x7} tx_q_wr_ptr=0
9223.248960: send_q: {0xa008000b,0x1,0x18,0x7} tx_q_wr_ptr=1
9223.249289: send_q: {0xa0090008,0xa1800,0x0,0x0} tx_q_wr_ptr=2
9223.249489: send_q: {0xa00a0008,0x1803,0x0,0x0} tx_q_wr_ptr=3
9223.249612: send_q: {0xa00b0008,0xa1900,0x0,0x0} tx_q_wr_ptr=4
9223.250046: send_q: {0xa01c0403,0x3050600,0xffffffff,0x0} tx_q_wr_ptr=5
9223.250232: q_txdone: tx_q_rd_ptr=0 reqs_completed=6 outstanding_msgs=0
9223.250236: q_txdone: tx_q_rd_ptr=1 reqs_completed=5 outstanding_msgs=0
9223.250237: q_txdone: tx_q_rd_ptr=2 reqs_completed=4 outstanding_msgs=0
9223.250239: q_txdone: tx_q_rd_ptr=3 reqs_completed=3 outstanding_msgs=0
9223.250240: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=0
9223.250241: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=0
9223.250406: send_q: {0xa00d0008,0x1903,0x0,0x0} tx_q_wr_ptr=6
9223.250437: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0


This looks pretty clearly worth it. Have I convinced you? Is there
other data you'd like to see?


> 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.

FWIW, the downstream code in Pixel does what I think you're
suggesting. I can confidently say that, while it doesn't require
changes to the mailbox core, it is much more convoluted and
complicated. It also bleeds into the device-tree representation, which
doesn't feel great.

Just to be concrete, I'll document how the downstream driver works. If
this isn't what you were thinking, please correct me.

Back to our simplified "clk_mailbox" driver. We'll say that our
"clk_mailbox" driver talks over a single mailbox to the remote
processor. Let's say each message is 4 words big. The message space is
32-words big. 32 / 4 = 8 which means this space is divided into 8
queue slots. Downstream represents each of these queue slots as a
generic channel. That means that, in the device tree, our
"clk_mailbox" driver looks looks like this:

mboxes = <&cpm_tx_mba 0>,
         <&cpm_tx_mba 1>,
         <&cpm_tx_mba 2>,
         <&cpm_tx_mba 3>,
         <&cpm_tx_mba 4>,
         <&cpm_tx_mba 5>,
         <&cpm_tx_mba 6>,
         <&cpm_tx_mba 7>;

Then the "clk_mailbox" driver is in charge of rotating through each of
the channels. First it writes to channel 0, then it writes to channel
1, etc. This works with no changes to the core, but...

1. IMO, it's ugly. Logically, this is one communication channel
between the processor running Linux and the remote processor. We
shouldn't represent it as 8 channels. It feels especially bad to leak
this into the device tree.

2. The "clk_mailbox" driver needs to re-implement queuing and can't
use the mailbox core's queue. This is because the remote side
absolutely requires strict adherance to the "rotation" protocol in
order for it to receive messages. It expects a message in slot 0, then
slot 1, then slot 2, etc. If we used the mailbox core's queue, this
would break the protocol.

3. It may involve code duplication in the future. Currently, the only
driver using "queue mode" is the "CPM", but other drivers could
conceivably use it since many of the "LGA MBA" blocks support queue
mode in hardware.


I'm also a little confused about the resistance. I don't feel like the
mailbox core change is that complicated. The diffstat shows 58
insertions and 17 deletions. 14 of those added lines are comments.
While we certainly don't want to add useless APIs, to me this truly
seems like the correct way to add the functionality. It also doesn't
seem absurd to me that some future mailbox controller out there will
also support queuing like this.

-Doug

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

[Jassi Brar]

On Mon, Sep 21, 2026 at 5:19 PM Doug Anderson <dianders@chromium.org> wrote:
>
> Hi,
>
> On Sun, Sep 20, 2026 at 3:46 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
> >
> > > 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.
>
> Fair enough. I've jumped into a pre-existing design. Let me see if I
> can find old information or gather evidence myself. Then with real
> data we can figure out what makes sense. OK, gathered some data...
>
> FWIW, to explain things clearly, I have been simplifying by saying
> that just clock prepare/unprepare goes over this channel. In reality,
> there is much more traffic. On Pixel 10, the mailbox using queue mode
> like this connects to the "CPM" (central power manager). Looking at
> the device tree, we see the following things using this mailbox:
> * One of the main clock controllers in the system.
> * Most of the power domains in the system
> * Devfreq controllers
> * Thermal controllers
> * A GPIO controller
> * A reset controller
> * An interrupt controller
> * A RTC
> * A pile of other stuff
>
> So basically a whole crap-ton of resources are managed over this
> mailbox. The team designing this Phone apparently decided that the
> mailbox is one of the primary communication pipelines in the system.
> Yes, everything that communicates over the mailbox needs to be able to
> sleep, but that doesn't mean we shouldn't keep it fast if possible.
>
> On an off-the-shelf Pixel 10, one can spy on the CPM mailbox like this:
>
> echo 1 > /sys/kernel/tracing/events/goog_mba_ctrl/enable
> echo 1 > /sys/kernel/tracing/tracing_on
> echo "" > /sys/kernel/tracing/trace
> cat /sys/kernel/tracing/trace_pipe | grep 'process_q_t\|send_data_q'
>
> When I do that, messages spew by pretty much constantly showing just
> how busy this mailbox is. I can see instances where the queue is
> actually utilized like this:
>
> cat /sys/kernel/tracing/trace_pipe | \
>   grep 'process_q_t\|send_data_q' | \
>   grep -C10 'eqs_completed=[^1]\|anding_msg=[^0]'
>
> If I do that, it's quite easy to see the queue being used. If I use
> the device actively, I get several hits per second. A few examples
> traces (using sed to shorten them slightly):
>
> 6946.884142: send_q: {0xa0016969,0xf,0x2,0x0} tx_q_wr_ptr=4
> 6946.884496: send_q: {0xa0026969,0x7,0x1,0x0} tx_q_wr_ptr=5
> 6946.884851: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=0
> 6946.884880: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=0
>
> Here you can see that it took 709 us to get the response to the first
> message. ...but, luckily we didn't have to wait for that 709 us before
> sending the second message. That means we still got both responses at
> once. Note that things aren't always so slow. The next messages
> through this mailbox only took 42 us.
>
> 6946.885000: send_q: {0xa0036969,0x7,0x1,0x0} tx_q_wr_ptr=6
> 6946.885042: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0
>
> Here's a pretty big usage of the queue. I assume the CPM was busy at the time:
>
> 7751.863328: send_q: {0xa0050708,0x1020001,0x0,0x0} tx_q_wr_ptr=7
> 7751.863619: send_q: {0xa0060708,0x1030001,0x0,0x0} tx_q_wr_ptr=0
> 7751.863850: send_q: {0xa0070708,0x1000001,0x0,0x0} tx_q_wr_ptr=1
> 7751.864123: send_q: {0xa0080708,0x1010001,0x0,0x0} tx_q_wr_ptr=2
> 7751.864329: q_txdone: tx_q_rd_ptr=7 reqs_completed=4 outstanding_msgs=0
> 7751.864338: q_txdone: tx_q_rd_ptr=0 reqs_completed=3 outstanding_msgs=0
> 7751.864347: q_txdone: tx_q_rd_ptr=1 reqs_completed=2 outstanding_msgs=0
> 7751.864378: q_txdone: tx_q_rd_ptr=2 reqs_completed=1 outstanding_msgs=0
>
> A full millisecond before the first response, but luckily we got them
> all at once.
>
> ...and this one is pretty interesting here:
>
> 7782.321344: send_q: {0xa0040708,0x30001,0x0,0x0} tx_q_wr_ptr=4
> 7782.321481: send_q: {0xa0150708,0x10001,0x0,0x0} tx_q_wr_ptr=5
> 7782.321769: send_q: {0xa0060708,0x1,0x0,0x0} tx_q_wr_ptr=6
> 7782.321799: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=1
> 7782.321813: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=1
> 7782.321842: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0
>
> This one is interesting because we can see that the remote side first
> ACKed two of the three outstanding messages. Then a short time later
> it managed to ACK the last message.
>
> ...and just to show the queue being used quite often, here are two
> instances right in a row (it's not hard to see this happening):
>
> 8521.693559: send_q: {0xa0010708,0x1030001,0x0,0x0} tx_q_wr_ptr=5
> 8521.693649: send_q: {0xa0120708,0x10001,0x0,0x0} tx_q_wr_ptr=6
> 8521.693690: q_txdone: tx_q_rd_ptr=5 reqs_completed=2 outstanding_msgs=0
> 8521.693701: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0
> 8521.693920: send_q: {0xa0030708,0x1,0x0,0x0} tx_q_wr_ptr=7
> 8521.693956: q_txdone: tx_q_rd_ptr=7 reqs_completed=1 outstanding_msgs=0
> 8521.694171: send_q: {0xa0040705,0x2,0x14,0x1} tx_q_wr_ptr=0
> 8521.694194: send_q: {0xa0150708,0x1000001,0x0,0x0} tx_q_wr_ptr=1
> 8521.694214: q_txdone: tx_q_rd_ptr=0 reqs_completed=2 outstanding_msgs=0
> 8521.694226: q_txdone: tx_q_rd_ptr=1 reqs_completed=1 outstanding_msgs=0
>
> ...and in case you want to see an even bigger use of the queue, here
> are 6 queued up at once:
>
> 9223.248560: send_q: {0xa0060403,0x3050600,0x0,0x0} tx_q_wr_ptr=7
> 9223.248598: q_txdone: tx_q_rd_ptr=7 reqs_completed=1 outstanding_msgs=0
> 9223.248824: send_q: {0xa007000b,0x1,0x418,0x7} tx_q_wr_ptr=0
> 9223.248960: send_q: {0xa008000b,0x1,0x18,0x7} tx_q_wr_ptr=1
> 9223.249289: send_q: {0xa0090008,0xa1800,0x0,0x0} tx_q_wr_ptr=2
> 9223.249489: send_q: {0xa00a0008,0x1803,0x0,0x0} tx_q_wr_ptr=3
> 9223.249612: send_q: {0xa00b0008,0xa1900,0x0,0x0} tx_q_wr_ptr=4
> 9223.250046: send_q: {0xa01c0403,0x3050600,0xffffffff,0x0} tx_q_wr_ptr=5
> 9223.250232: q_txdone: tx_q_rd_ptr=0 reqs_completed=6 outstanding_msgs=0
> 9223.250236: q_txdone: tx_q_rd_ptr=1 reqs_completed=5 outstanding_msgs=0
> 9223.250237: q_txdone: tx_q_rd_ptr=2 reqs_completed=4 outstanding_msgs=0
> 9223.250239: q_txdone: tx_q_rd_ptr=3 reqs_completed=3 outstanding_msgs=0
> 9223.250240: q_txdone: tx_q_rd_ptr=4 reqs_completed=2 outstanding_msgs=0
> 9223.250241: q_txdone: tx_q_rd_ptr=5 reqs_completed=1 outstanding_msgs=0
> 9223.250406: send_q: {0xa00d0008,0x1903,0x0,0x0} tx_q_wr_ptr=6
> 9223.250437: q_txdone: tx_q_rd_ptr=6 reqs_completed=1 outstanding_msgs=0
>
>
> This looks pretty clearly worth it. Have I convinced you? Is there
> other data you'd like to see?
>
OK, let's work with the assumption that it is really needed.  Though I
would still explore the option of lazy/debounced disabling of clocks
and other resources controlled by the remote host.

>
> > 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.
>
> FWIW, the downstream code in Pixel does what I think you're
> suggesting. I can confidently say that, while it doesn't require
> changes to the mailbox core, it is much more convoluted and
> complicated. It also bleeds into the device-tree representation, which
> doesn't feel great.
>
> Just to be concrete, I'll document how the downstream driver works. If
> this isn't what you were thinking, please correct me.
>
> Back to our simplified "clk_mailbox" driver. We'll say that our
> "clk_mailbox" driver talks over a single mailbox to the remote
> processor. Let's say each message is 4 words big. The message space is
> 32-words big. 32 / 4 = 8 which means this space is divided into 8
> queue slots. Downstream represents each of these queue slots as a
> generic channel. That means that, in the device tree, our
> "clk_mailbox" driver looks looks like this:
>
> mboxes = <&cpm_tx_mba 0>,
>          <&cpm_tx_mba 1>,
>          <&cpm_tx_mba 2>,
>          <&cpm_tx_mba 3>,
>          <&cpm_tx_mba 4>,
>          <&cpm_tx_mba 5>,
>          <&cpm_tx_mba 6>,
>          <&cpm_tx_mba 7>;
>
> Then the "clk_mailbox" driver is in charge of rotating through each of
> the channels. First it writes to channel 0, then it writes to channel
> 1, etc. This works with no changes to the core, but...
>
I was thinking something like   mboxes = <&cpm_tx_mba>
clk_mailbox simply asks for some channel, gets allocated the next free
slot. The mailbox controller driver keeps track of the channel-slot
map locally and is responsible for managing contiguous slots and
completing the tx upon receiving ack.

> I'm also a little confused about the resistance. I don't feel like the
> mailbox core change is that complicated. The diffstat shows 58
> insertions and 17 deletions. 14 of those added lines are comments.
> While we certainly don't want to add useless APIs, to me this truly
> seems like the correct way to add the functionality. It also doesn't
> seem absurd to me that some future mailbox controller out there will
> also support queuing like this.
>
It is not the diff stat but about inserting a flag in the api to
introduce special case behavior. It is like adding one person to the
party introduces N-1 handshakes - the has_queue flag doesn't play well
with other configurations and may allow future platforms to abuse
has_queue to implement hacks.

Regards
Jassi

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

[Doug Anderson]

Hi,

On Mon, Sep 21, 2026 at 5:11 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> > > 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.
> >
> > FWIW, the downstream code in Pixel does what I think you're
> > suggesting. I can confidently say that, while it doesn't require
> > changes to the mailbox core, it is much more convoluted and
> > complicated. It also bleeds into the device-tree representation, which
> > doesn't feel great.
> >
> > Just to be concrete, I'll document how the downstream driver works. If
> > this isn't what you were thinking, please correct me.
> >
> > Back to our simplified "clk_mailbox" driver. We'll say that our
> > "clk_mailbox" driver talks over a single mailbox to the remote
> > processor. Let's say each message is 4 words big. The message space is
> > 32-words big. 32 / 4 = 8 which means this space is divided into 8
> > queue slots. Downstream represents each of these queue slots as a
> > generic channel. That means that, in the device tree, our
> > "clk_mailbox" driver looks looks like this:
> >
> > mboxes = <&cpm_tx_mba 0>,
> >          <&cpm_tx_mba 1>,
> >          <&cpm_tx_mba 2>,
> >          <&cpm_tx_mba 3>,
> >          <&cpm_tx_mba 4>,
> >          <&cpm_tx_mba 5>,
> >          <&cpm_tx_mba 6>,
> >          <&cpm_tx_mba 7>;
> >
> > Then the "clk_mailbox" driver is in charge of rotating through each of
> > the channels. First it writes to channel 0, then it writes to channel
> > 1, etc. This works with no changes to the core, but...
> >
> I was thinking something like   mboxes = <&cpm_tx_mba>
> clk_mailbox simply asks for some channel, gets allocated the next free
> slot. The mailbox controller driver keeps track of the channel-slot
> map locally and is responsible for managing contiguous slots and
> completing the tx upon receiving ack.

Hmmm. OK, so I guess you're saying that every time "clk_mailbox" wants
to send a message, it calls mbox_request_channel(). It always requests
the same channel over and over again, but underneath the mailbox's
fw_xlate() function returns the next distinct channel? Then once
"clk_mailbox" sees the tx_done then it calls mbox_free_channel()? I
guess it would need to fork the mbox_free_channel() into a delayed
work function since mbox_free_channel() requires grabbing a mutex and
tx_done() is called from atomic context. It would be up to
"clk_mailbox" to ensure that it used the interface properly: always
send the message on the next allocated mailbox, always free the
channels in order, etc. Also, "clk_mailbox" would still need to
implement its own queue to handle the case where there were no more
free channels.

One thing that is important is that "clk_mailbox" needs to know when
"tx_done". If we didn't need that, everything would be vastly simpler:
just allocate the channel, send the message, and free it right away.

Did I understand your suggestion correctly this time, or am I still
not getting it?

Assuming I understood correctly, my thoughts would be:
1. At least this doesn't bleed into the device tree, which is great.
2. I'm a little worried about all the overhead involved in constantly
requesting / freeing channels. Maybe it's not as bad as I fear, but
those functions don't seem intended for constant calls. Even if the
overhead isn't that bad, the extra overhead needed to fork the free
call to delayed work doesn't seem great.
3. It feels like trying to manage this from "clk_mailbox" is going to
be a bunch of complicated code, including managing our own queue since
we still can't use the mailbox core's queue.

Overall, it feels like an bunch of awkward code. It feels like a hack
that a downstream kernel module would do simply because they didn't
want to improve the mailbox core...


> > I'm also a little confused about the resistance. I don't feel like the
> > mailbox core change is that complicated. The diffstat shows 58
> > insertions and 17 deletions. 14 of those added lines are comments.
> > While we certainly don't want to add useless APIs, to me this truly
> > seems like the correct way to add the functionality. It also doesn't
> > seem absurd to me that some future mailbox controller out there will
> > also support queuing like this.
> >
> It is not the diff stat but about inserting a flag in the api to
> introduce special case behavior. It is like adding one person to the
> party introduces N-1 handshakes -

Sure, it's new core code for a single client. ...but some client has
to be the first. The idea of a mailbox controller being able to queue
data doesn't feel like an absurd feature that nobody would ever need
again. Heck, once the core supports the feature it seems like someone
out there will figure out how to make their existing hardware work in
"queue-mode" and improve its performance.


> the has_queue flag doesn't play well
> with other configurations and may allow future platforms to abuse
> has_queue to implement hacks.

I don't really understand this part. Can you give any examples? How
does "has_queue" not play well with other configurations? You mean the
fact that my code right now only works with "MBOX_TXDONE_BY_IRQ"? I
don't think relaxing that would be very hard. ...and sure, people will
implement hacks no matter what API you give them (the "allocate a new
channel for each message" might qualify as one such hack?), but it
doesn't feel like "has_queue" is especially prone to abuse, is it?

-Doug

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

[Jassi Brar]

On Tue, Sep 22, 2026 at 11:01 AM Doug Anderson <dianders@chromium.org> wrote:
> >
> > > > 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.
> > >
> > > FWIW, the downstream code in Pixel does what I think you're
> > > suggesting. I can confidently say that, while it doesn't require
> > > changes to the mailbox core, it is much more convoluted and
> > > complicated. It also bleeds into the device-tree representation, which
> > > doesn't feel great.
> > >
> > > Just to be concrete, I'll document how the downstream driver works. If
> > > this isn't what you were thinking, please correct me.
> > >
> > > Back to our simplified "clk_mailbox" driver. We'll say that our
> > > "clk_mailbox" driver talks over a single mailbox to the remote
> > > processor. Let's say each message is 4 words big. The message space is
> > > 32-words big. 32 / 4 = 8 which means this space is divided into 8
> > > queue slots. Downstream represents each of these queue slots as a
> > > generic channel. That means that, in the device tree, our
> > > "clk_mailbox" driver looks looks like this:
> > >
> > > mboxes = <&cpm_tx_mba 0>,
> > >          <&cpm_tx_mba 1>,
> > >          <&cpm_tx_mba 2>,
> > >          <&cpm_tx_mba 3>,
> > >          <&cpm_tx_mba 4>,
> > >          <&cpm_tx_mba 5>,
> > >          <&cpm_tx_mba 6>,
> > >          <&cpm_tx_mba 7>;
> > >
> > > Then the "clk_mailbox" driver is in charge of rotating through each of
> > > the channels. First it writes to channel 0, then it writes to channel
> > > 1, etc. This works with no changes to the core, but...
> > >
> > I was thinking something like   mboxes = <&cpm_tx_mba>
> > clk_mailbox simply asks for some channel, gets allocated the next free
> > slot. The mailbox controller driver keeps track of the channel-slot
> > map locally and is responsible for managing contiguous slots and
> > completing the tx upon receiving ack.
>
> Hmmm. OK, so I guess you're saying that every time "clk_mailbox" wants
> to send a message, it calls mbox_request_channel(). It always requests
> the same channel over and over again, but underneath the mailbox's
> fw_xlate() function returns the next distinct channel? Then once
> "clk_mailbox" sees the tx_done then it calls mbox_free_channel()? I
> guess it would need to fork the mbox_free_channel() into a delayed
> work function since mbox_free_channel() requires grabbing a mutex and
> tx_done() is called from atomic context. It would be up to
> "clk_mailbox" to ensure that it used the interface properly: always
> send the message on the next allocated mailbox, always free the
> channels in order, etc. Also, "clk_mailbox" would still need to
> implement its own queue to handle the case where there were no more
> free channels.
>
> One thing that is important is that "clk_mailbox" needs to know when
> "tx_done". If we didn't need that, everything would be vastly simpler:
> just allocate the channel, send the message, and free it right away.
>
> Did I understand your suggestion correctly this time, or am I still
> not getting it?
>
> Assuming I understood correctly, my thoughts would be:
> 1. At least this doesn't bleed into the device tree, which is great.
> 2. I'm a little worried about all the overhead involved in constantly
> requesting / freeing channels. Maybe it's not as bad as I fear, but
> those functions don't seem intended for constant calls. Even if the
> overhead isn't that bad, the extra overhead needed to fork the free
> call to delayed work doesn't seem great.
> 3. It feels like trying to manage this from "clk_mailbox" is going to
> be a bunch of complicated code, including managing our own queue since
> we still can't use the mailbox core's queue.
>
It should be a lot simpler and neater than you anticipate.

I agree we should avoid channel request-release for every transfer.
Not due to overhead concerns but because that makes code cleaner.
(lets not forget the remote handles requests serially and it is just
the latency of incoming ack irq and programming the next xfer that we
can and want to avoid. So acquiring/releasing a channel should not be
the bottleneck).

So I think clk_mailbox can acquire all 8 channels during probe and
maintain a list of idle channels from which it picks one and uses it
for an incoming clk_prepare(). If all channels are busy, the
clk_prepare() will sleep/block on a wait-queue which is nudged by
tx_done of a transfer after the channel is added back to the idle
list.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Tue, Sep 22, 2026 at 5:23 PM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> So I think clk_mailbox can acquire all 8 channels during probe and
> maintain a list of idle channels from which it picks one and uses it
> for an incoming clk_prepare(). If all channels are busy, the
> clk_prepare() will sleep/block on a wait-queue which is nudged by
> tx_done of a transfer after the channel is added back to the idle
> list.

Sorry for the delay in responding. I wanted to prototype the change
and needed time to look at it.

So I think the crux of your current suggestion is that we have a
non-idempotent "fw_xlate" function, right? We call it multiple times
with the same arguments and it returns a different channel each time?
In my prototype, it looked like this:

```C
static struct mbox_chan *goog_mba_fw_xlate(struct mbox_controller *mbox,
                                      const struct fwnode_reference_args *sp)
{
  int i;

  if (sp->nargs)
    return ERR_PTR(-EINVAL);

  for (i = 0; i < mbox->num_chans; i++) {
    if (!mbox->chans[i].cl)
      return &mbox->chans[i];
  }

  return ERR_PTR(-EBUSY);
}
```

Then the client loops around and requests the same channel over and
over again until it gets -EBUSY? Like:

```C
for (i = 0; i < FIFO_MAX; i++) {
  client
  chan[i] = mbox_request_channel(client[i], TX_CHAN);
  if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
    break;
}
fifo_depth = i;
```

I _guess_ that works, but it still feels like a bit of a hack to me.
You said you were worried about people abusing the "has_queue" API.
The above feels like it's abusing the "fw_xlate" API, turning it from
something that is normally a "lookup" into an allocator function.

+Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
something that looks right to you?


I researched whether other upstream drivers use of_xlate() /
fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
which appears to be doing something similar. However, upon deeper
digging it seems like "exynos-mailbox" isn't using this dynamic
allocation for any compelling reason. It looks like, really,
"exynos-mailbox" should just be returning one channel. The client
(exynos-acpm) could just use the same channel for everything since:
* It actually gets the _real_ channel ID out of the data.
* It doesn't care about txdone.
* All it does is ring a doorbell and there's no queueing.


In any case, if using fw_xlate() as an allocator is truly the only way
to proceed, I'll finish my prototype and send a v2, but I'm still
skeptical that this is better than just adding queuing into the core.
Speaking of which, I actually want to go back to something you said
earlier. I asked a bit about this but I don't think I saw a response
(sorry if I missed it!):

> It is not the diff stat but about inserting a flag in the api to
> introduce special case behavior. It is like adding one person to the
> party introduces N-1 handshakes - the has_queue flag doesn't play well
> with other configurations and may allow future platforms to abuse
> has_queue to implement hacks.

Can you elaborate more on this?

1. How does "has_queue" not play well with other configurations?
Everything should behave the same if "has_queue" isn't set, right? Are
you saying that it will make the code too hard to understand, or
something?

2. How does this allow future programs to abuse "has_queue"? Won't
they need to submit mailbox controllers to the mailbox subsystem,
meaning they'll have to go through you? If someone is using
"has_queue" to do a hack, can't you just NAK them?


One other thing that my prototype turned up: If I do the "fw_xlate()
as an allocator" solution, I believe I need to change the DT bindings
by adding a "tx-payload-size" attribute, or I need to jump through a
pile of awkward hoops. The reason is that I suddenly need to know the
number of FIFO entries much earlier. The v1 of my patch simply figured
out the "tx-payload-size" based on the first message sent, but now we
need it earlier.

While that's maybe not the end of the world, it's always unfortunate
when we have to change the DT bindings to accommodate the software
design.

-Doug

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

[Krzysztof Kozlowski]

On 28/09/2026 23:34, Doug Anderson wrote:
> 
> ```C
> static struct mbox_chan *goog_mba_fw_xlate(struct mbox_controller *mbox,
>                                       const struct fwnode_reference_args *sp)
> {
>   int i;
> 
>   if (sp->nargs)
>     return ERR_PTR(-EINVAL);
> 
>   for (i = 0; i < mbox->num_chans; i++) {
>     if (!mbox->chans[i].cl)
>       return &mbox->chans[i];
>   }
> 
>   return ERR_PTR(-EBUSY);
> }
> ```
> 
> Then the client loops around and requests the same channel over and
> over again until it gets -EBUSY? Like:
> 
> ```C
> for (i = 0; i < FIFO_MAX; i++) {
>   client
>   chan[i] = mbox_request_channel(client[i], TX_CHAN);
>   if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
>     break;
> }
> fifo_depth = i;
> ```
> 
> I _guess_ that works, but it still feels like a bit of a hack to me.
> You said you were worried about people abusing the "has_queue" API.
> The above feels like it's abusing the "fw_xlate" API, turning it from
> something that is normally a "lookup" into an allocator function.
> 
> +Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
> something that looks right to you?

There is no allocation in your xlate code above, so this is not that
terrible as we talked on LPC.

> 
> 
> I researched whether other upstream drivers use of_xlate() /
> fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
> which appears to be doing something similar. However, upon deeper
> digging it seems like "exynos-mailbox" isn't using this dynamic
> allocation for any compelling reason. It looks like, really,
> "exynos-mailbox" should just be returning one channel. The client
> (exynos-acpm) could just use the same channel for everything since:
> * It actually gets the _real_ channel ID out of the data.
> * It doesn't care about txdone.
> * All it does is ring a doorbell and there's no queueing.
> 
> 
> In any case, if using fw_xlate() as an allocator is truly the only way
> to proceed, I'll finish my prototype and send a v2, but I'm still

Why fw_xlate() would be an allocator? Can you extend your code to show that?

I think doing any allocation in xlate() is calls is fundamentally wrong.
These should not modify the state of the device, so no allocations, no
device_link_add() etc.

Why? There is simply no corresponding xlate_destroy() call. It's also
confusing, because the meaning is to translate from one domain resource
to another, not perform actual resource allocation.

> skeptical that this is better than just adding queuing into the core.
> Speaking of which, I actually want to go back to something you said
> earlier. I asked a bit about this but I don't think I saw a response
> (sorry if I missed it!):

Best regards,
Krzysztof

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

[Doug Anderson]

Hi,

On Thu, Oct 8, 2026 at 11:36 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 28/09/2026 23:34, Doug Anderson wrote:
> >
> > ```C
> > static struct mbox_chan *goog_mba_fw_xlate(struct mbox_controller *mbox,
> >                                       const struct fwnode_reference_args *sp)
> > {
> >   int i;
> >
> >   if (sp->nargs)
> >     return ERR_PTR(-EINVAL);
> >
> >   for (i = 0; i < mbox->num_chans; i++) {
> >     if (!mbox->chans[i].cl)
> >       return &mbox->chans[i];
> >   }
> >
> >   return ERR_PTR(-EBUSY);
> > }
> > ```
> >
> > Then the client loops around and requests the same channel over and
> > over again until it gets -EBUSY? Like:
> >
> > ```C
> > for (i = 0; i < FIFO_MAX; i++) {
> >   client
> >   chan[i] = mbox_request_channel(client[i], TX_CHAN);
> >   if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
> >     break;
> > }
> > fifo_depth = i;
> > ```
> >
> > I _guess_ that works, but it still feels like a bit of a hack to me.
> > You said you were worried about people abusing the "has_queue" API.
> > The above feels like it's abusing the "fw_xlate" API, turning it from
> > something that is normally a "lookup" into an allocator function.
> >
> > +Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
> > something that looks right to you?
>
> There is no allocation in your xlate code above, so this is not that
> terrible as we talked on LPC.
>
> >
> >
> > I researched whether other upstream drivers use of_xlate() /
> > fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
> > which appears to be doing something similar. However, upon deeper
> > digging it seems like "exynos-mailbox" isn't using this dynamic
> > allocation for any compelling reason. It looks like, really,
> > "exynos-mailbox" should just be returning one channel. The client
> > (exynos-acpm) could just use the same channel for everything since:
> > * It actually gets the _real_ channel ID out of the data.
> > * It doesn't care about txdone.
> > * All it does is ring a doorbell and there's no queueing.
> >
> >
> > In any case, if using fw_xlate() as an allocator is truly the only way
> > to proceed, I'll finish my prototype and send a v2, but I'm still
>
> Why fw_xlate() would be an allocator? Can you extend your code to show that?
>
> I think doing any allocation in xlate() is calls is fundamentally wrong.
> These should not modify the state of the device, so no allocations, no
> device_link_add() etc.

It's not _calling_ an allocator, it _is_ an allocator. It is walking
through the array of channels and returning the first free one. Then
that free channel is "allocated" to the client.


> Why? There is simply no corresponding xlate_destroy() call. It's also
> confusing, because the meaning is to translate from one domain resource
> to another, not perform actual resource allocation.

In this case, there is no leak because it's relying on knowledge of
how the mailbox core will be using fw_xlate() to finish the allocation
and then relying on the knowledge of the mailbox core to free. It
works like this (simplfied, see mbox_request_channel() for full code):

```
scoped_guard(mutex, &con_mutex) {
  list_for_each_entry(mbox, &mbox_cons, node) {
    if (device_match_fwnode(mbox->dev, fwspec.fwnode)) {
      // The below "allocates" the first free channel associated w/ the node.
      chan = mbox->fw_xlate(mbox, &fwspec);
      if (!IS_ERR(chan))
        break;
    }
  }
  if (!IS_ERR(chan))
    // This finishes the allocation.
    chan->cl = client;
}
```

Said more simply, in the mailbox driver, there is an array of channels
associated with a given "fw_node". These are "allocated" like this:
1. mbox_request_channel() grabs its global lock.
2. The mailbox driver's fw_xlate() is called to find the first "free"
channel associated with the fw_node (a channel with no "client").
3. mbox_request_channel() sets the "client" field in the channel to
finish allocation.
4. mbox_request_channel() drops its global lock.

Does that make sense?

Is that a design that looks good to you? If DT folks have no
objections to that, I'll send a new patch that works like that.

-Doug

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

[Jassi Brar]

Hi Doug,

On Fri, Oct 9, 2026 at 3:42 AM Doug Anderson <dianders@chromium.org> wrote:
> > >
> > >   for (i = 0; i < mbox->num_chans; i++) {
> > >     if (!mbox->chans[i].cl)
> > >       return &mbox->chans[i];
> > >   }
> > >
> > >   return ERR_PTR(-EBUSY);
> > > }
> > > ```
> > >
> > > Then the client loops around and requests the same channel over and
> > > over again until it gets -EBUSY? Like:
> > >
> > > ```C
> > > for (i = 0; i < FIFO_MAX; i++) {
> > >   client
> > >   chan[i] = mbox_request_channel(client[i], TX_CHAN);
> > >   if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
> > >     break;
> > > }
> > > fifo_depth = i;
> > > ```
> > >
> > > I _guess_ that works, but it still feels like a bit of a hack to me.
> > > You said you were worried about people abusing the "has_queue" API.
> > > The above feels like it's abusing the "fw_xlate" API, turning it from
> > > something that is normally a "lookup" into an allocator function.
> > >
> > > +Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
> > > something that looks right to you?
> >
> > There is no allocation in your xlate code above, so this is not that
> > terrible as we talked on LPC.
> >
> > >
> > >
> > > I researched whether other upstream drivers use of_xlate() /
> > > fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
> > > which appears to be doing something similar. However, upon deeper
> > > digging it seems like "exynos-mailbox" isn't using this dynamic
> > > allocation for any compelling reason. It looks like, really,
> > > "exynos-mailbox" should just be returning one channel. The client
> > > (exynos-acpm) could just use the same channel for everything since:
> > > * It actually gets the _real_ channel ID out of the data.
> > > * It doesn't care about txdone.
> > > * All it does is ring a doorbell and there's no queueing.
> > >
> > >
> > > In any case, if using fw_xlate() as an allocator is truly the only way
> > > to proceed, I'll finish my prototype and send a v2, but I'm still
> >
> > Why fw_xlate() would be an allocator? Can you extend your code to show that?
> >
> > I think doing any allocation in xlate() is calls is fundamentally wrong.
> > These should not modify the state of the device, so no allocations, no
> > device_link_add() etc.
>
> It's not _calling_ an allocator, it _is_ an allocator. It is walking
> through the array of channels and returning the first free one. Then
> that free channel is "allocated" to the client.
>
You mean _assigned_.
Allocation means when there are some resources reserved that must be
released at some point later .... which is what the mailbox controller
driver does in probe() and remove().
In xlate() the platform just decides which channel to assign to the
incoming request -- some platforms take the hint from device-tree,
your platform has flexibility and can simply assign the first free
found.

I will try to explain in detail your questions in your last post, but
I am still not convinced you need to modify the api.

Regards,
Jassi

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

[Doug Anderson]

Hi,

On Fri, Oct 9, 2026 at 8:43 AM Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
> Hi Doug,
>
> On Fri, Oct 9, 2026 at 3:42 AM Doug Anderson <dianders@chromium.org> wrote:
> > > >
> > > >   for (i = 0; i < mbox->num_chans; i++) {
> > > >     if (!mbox->chans[i].cl)
> > > >       return &mbox->chans[i];
> > > >   }
> > > >
> > > >   return ERR_PTR(-EBUSY);
> > > > }
> > > > ```
> > > >
> > > > Then the client loops around and requests the same channel over and
> > > > over again until it gets -EBUSY? Like:
> > > >
> > > > ```C
> > > > for (i = 0; i < FIFO_MAX; i++) {
> > > >   client
> > > >   chan[i] = mbox_request_channel(client[i], TX_CHAN);
> > > >   if (IS_ERR(chan[i]) && PTR_ERR(chan[i]) == -EBUSY)
> > > >     break;
> > > > }
> > > > fifo_depth = i;
> > > > ```
> > > >
> > > > I _guess_ that works, but it still feels like a bit of a hack to me.
> > > > You said you were worried about people abusing the "has_queue" API.
> > > > The above feels like it's abusing the "fw_xlate" API, turning it from
> > > > something that is normally a "lookup" into an allocator function.
> > > >
> > > > +Rob, Saravana, and devicetree@vger.kernel.org. DT folks: is the above
> > > > something that looks right to you?
> > >
> > > There is no allocation in your xlate code above, so this is not that
> > > terrible as we talked on LPC.
> > >
> > > >
> > > >
> > > > I researched whether other upstream drivers use of_xlate() /
> > > > fw_xlate() as an "allocator" like this. I did find "exynos-mailbox,"
> > > > which appears to be doing something similar. However, upon deeper
> > > > digging it seems like "exynos-mailbox" isn't using this dynamic
> > > > allocation for any compelling reason. It looks like, really,
> > > > "exynos-mailbox" should just be returning one channel. The client
> > > > (exynos-acpm) could just use the same channel for everything since:
> > > > * It actually gets the _real_ channel ID out of the data.
> > > > * It doesn't care about txdone.
> > > > * All it does is ring a doorbell and there's no queueing.
> > > >
> > > >
> > > > In any case, if using fw_xlate() as an allocator is truly the only way
> > > > to proceed, I'll finish my prototype and send a v2, but I'm still
> > >
> > > Why fw_xlate() would be an allocator? Can you extend your code to show that?
> > >
> > > I think doing any allocation in xlate() is calls is fundamentally wrong.
> > > These should not modify the state of the device, so no allocations, no
> > > device_link_add() etc.
> >
> > It's not _calling_ an allocator, it _is_ an allocator. It is walking
> > through the array of channels and returning the first free one. Then
> > that free channel is "allocated" to the client.
> >
> You mean _assigned_.
> Allocation means when there are some resources reserved that must be
> released at some point later .... which is what the mailbox controller
> driver does in probe() and remove().
> In xlate() the platform just decides which channel to assign to the
> incoming request -- some platforms take the hint from device-tree,
> your platform has flexibility and can simply assign the first free
> found.

In this case, mbox_request_channel() plus the fw_xlate() seem to me to
act like an allocator. There is a pool of channels and each call
allocates one. mbox_release_channel() releases the channel. This is in
contrast to normal cases where fw_xlate() always returns the same
channel. Sure, I guess in that case it still "allocates" that one
slot, but that feels more like someone requesting/reserving a channel
rather than allocating the next free channel.

...but I guess the root of the problem is that it just seems
unexpected to me that fw_xlate() is taking the same input "fw_node"
and then is returning different mailboxes.

If this is what everyone wants then I agree that it can work. I just
want to confirm since it doesn't feel very clean to me.


> I will try to explain in detail your questions in your last post, but
> I am still not convinced you need to modify the api.

Sure, anything you can do to clarify is appreciated! I'd love to be
able to send a v2 and move forward, but I don't want to do so while
things are still unresolved.

-Doug

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

[Markus Elfring]

…
> +++ b/drivers/mailbox/goog-mba.c
> @@ -0,0 +1,567 @@
…
> +static int goog_mba_send_data(struct mbox_chan *chan, void *msg)
> +{
…
> +		spin_lock_irqsave(&goog_mbox->lock, flags);
> +		out_of_space = payload_words &&
> +			       (goog_mbox->outstanding_msgs >=
> +				goog_mbox->msg_buffer_words / payload_words);
> +		spin_unlock_irqrestore(&goog_mbox->lock, flags);
…

Under which circumstances would you become interested to apply a statement
like “guard(spinlock_irqsave)(&goog_mbox->lock);”?
https://elixir.bootlin.com/linux/v7.3-rc6/source/include/linux/spinlock.h#L642-L645

Regards,
Markus