| Message ID | 20210907145501.69161-3-sven@svenpeter.dev |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Apple Mailbox Controller support | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dt-meta-schema | success | |
| robh/dtbs-check | success |
> + - description: > + M3 mailboxes are an older variant with a slightly different MMIO > + interface still found on the M1. > + items: > + - const: apple,t8103-m3-mailbox Would be nice to document an example of where an M3 mailbox is found. > + interrupts: > + minItems: 4 > + items: > + - description: send fifo is empty interrupt > + - description: send fifo is not empty interrupt > + - description: receive fifo is empty interrupt > + - description: receive fifo is not empty interrupt > + > + interrupt-names: > + minItems: 4 > + items: > + - const: send-empty > + - const: send-not-empty > + - const: recv-empty > + - const: recv-not-empty If the names became not-constant the asprintf thing goes away, not sure that's better or worse. > + clocks: > + description: > + Reference to the clock gate phandle(s) if required for this mailbox. > + Optional since not all mailboxes are attached to a clock gate. Do we do anything with the clocks at this point?
Hi, On Tue, Sep 7, 2021, at 20:56, Alyssa Rosenzweig wrote: > > + - description: > > + M3 mailboxes are an older variant with a slightly different MMIO > > + interface still found on the M1. > > + items: > > + - const: apple,t8103-m3-mailbox > > Would be nice to document an example of where an M3 mailbox is found. Sure, I can add a comment that this is used for the coprocessor controlling Thunderbolt. > > > + interrupts: > > + minItems: 4 > > + items: > > + - description: send fifo is empty interrupt > > + - description: send fifo is not empty interrupt > > + - description: receive fifo is empty interrupt > > + - description: receive fifo is not empty interrupt > > + > > + interrupt-names: > > + minItems: 4 > > + items: > > + - const: send-empty > > + - const: send-not-empty > > + - const: recv-empty > > + - const: recv-not-empty > > If the names became not-constant the asprintf thing goes away, not sure > that's better or worse. I'm not sure I understand your comment here. This property just gives a name to the interrupts so that they can be referenced by that instead of a magic number between 0 and 4 in the driver. > > > + clocks: > > + description: > > + Reference to the clock gate phandle(s) if required for this mailbox. > > + Optional since not all mailboxes are attached to a clock gate. > > Do we do anything with the clocks at this point? > The device tree bindings describe the hardware (as best as we can without proper documentation) and some of these mailboxes have clock gates which need to be turned on before accessing their MMIO. This driver already tries to do that and works fine with the downstream clock driver(s) we have. Sven
> > > + - description: > > > + M3 mailboxes are an older variant with a slightly different MMIO > > > + interface still found on the M1. > > > + items: > > > + - const: apple,t8103-m3-mailbox > > > > Would be nice to document an example of where an M3 mailbox is found. > > Sure, I can add a comment that this is used for the coprocessor controlling Thunderbolt. That raises another issue ... how do we know the M3 code works at all without TB support yet? It "looks" correct but some of the IRQ handling stuff is nontrivial. > > > + interrupts: > > > + minItems: 4 > > > + items: > > > + - description: send fifo is empty interrupt > > > + - description: send fifo is not empty interrupt > > > + - description: receive fifo is empty interrupt > > > + - description: receive fifo is not empty interrupt > > > + > > > + interrupt-names: > > > + minItems: 4 > > > + items: > > > + - const: send-empty > > > + - const: send-not-empty > > > + - const: recv-empty > > > + - const: recv-not-empty > > > > If the names became not-constant the asprintf thing goes away, not sure > > that's better or worse. > > I'm not sure I understand your comment here. This property just gives a name > to the interrupts so that they can be referenced by that instead of a magic > number between 0 and 4 in the driver. D'oh, right, retracted. (Both this comment and the corresponding comment on the driver itself). Sorry about that. > > > + clocks: > > > + description: > > > + Reference to the clock gate phandle(s) if required for this mailbox. > > > + Optional since not all mailboxes are attached to a clock gate. > > > > Do we do anything with the clocks at this point? > > > > The device tree bindings describe the hardware (as best as we can without proper > documentation) and some of these mailboxes have clock gates which need to be turned > on before accessing their MMIO. This driver already tries to do that and works fine > with the downstream clock driver(s) we have. Good enough for me, thanks for clarifying 👍 Commit r-b, though Rob will surely point out problems and I'll need to rereview 😉
On Tue, Sep 7, 2021, at 22:48, Alyssa Rosenzweig wrote: > > > > + - description: > > > > + M3 mailboxes are an older variant with a slightly different MMIO > > > > + interface still found on the M1. > > > > + items: > > > > + - const: apple,t8103-m3-mailbox > > > > > > Would be nice to document an example of where an M3 mailbox is found. > > > > Sure, I can add a comment that this is used for the coprocessor controlling Thunderbolt. > > That raises another issue ... how do we know the M3 code works at all > without TB support yet? It "looks" correct but some of the IRQ handling > stuff is nontrivial. Enabling the mailbox interface just requires a few clocks to be ungated. Then I injected messages manually to verify that the code works. In addition I also brought up parts of the Thunderbolt controller which then allowed the co-processor on the other end of the mailbox to boot. After that I was also able to successfully talk to that processor using the same protocol used by most other processors. > > > > > + interrupts: > > > > + minItems: 4 > > > > + items: > > > > + - description: send fifo is empty interrupt > > > > + - description: send fifo is not empty interrupt > > > > + - description: receive fifo is empty interrupt > > > > + - description: receive fifo is not empty interrupt > > > > + > > > > + interrupt-names: > > > > + minItems: 4 > > > > + items: > > > > + - const: send-empty > > > > + - const: send-not-empty > > > > + - const: recv-empty > > > > + - const: recv-not-empty > > > > > > If the names became not-constant the asprintf thing goes away, not sure > > > that's better or worse. > > > > I'm not sure I understand your comment here. This property just gives a name > > to the interrupts so that they can be referenced by that instead of a magic > > number between 0 and 4 in the driver. > > D'oh, right, retracted. (Both this comment and the corresponding comment > on the driver itself). Sorry about that. > > > > > + clocks: > > > > + description: > > > > + Reference to the clock gate phandle(s) if required for this mailbox. > > > > + Optional since not all mailboxes are attached to a clock gate. > > > > > > Do we do anything with the clocks at this point? > > > > > > > The device tree bindings describe the hardware (as best as we can without proper > > documentation) and some of these mailboxes have clock gates which need to be turned > > on before accessing their MMIO. This driver already tries to do that and works fine > > with the downstream clock driver(s) we have. > > Good enough for me, thanks for clarifying 👍 > > Commit r-b, though Rob will surely point out problems and I'll need to > rereview 😉 > Thanks! Sven
> From: Sven Peter <sven@svenpeter.dev> > Date: Tue, 7 Sep 2021 16:55:00 +0200 > > Apple mailbox controller are found on the M1 and are used for > communication with various co-processors. > > Signed-off-by: Sven Peter <sven@svenpeter.dev> > --- > .../bindings/mailbox/apple,mailbox.yaml | 81 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 82 insertions(+) > create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
diff --git a/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml new file mode 100644 index 000000000000..360d8a162cc5 --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mailbox/apple,mailbox.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Apple Mailbox Controller + +maintainers: + - Hector Martin <marcan@marcan.st> + - Sven Peter <sven@svenpeter.dev> + +description: + The Apple mailbox consists of two FIFOs used to exchange 64+32 bit + messages between the main CPU and a co-processor. Multiple instances + of this mailbox can be found on Apple SoCs. + One of the two FIFOs is used to send data to a co-processor while the other + FIFO is used for the other direction. + Various clients implement different IPC protocols based on these simple + messages and shared memory buffers. + +properties: + compatible: + oneOf: + - description: + ASC mailboxes are the most common variant found on the M1. + items: + - const: apple,t8103-asc-mailbox + + - description: + M3 mailboxes are an older variant with a slightly different MMIO + interface still found on the M1. + items: + - const: apple,t8103-m3-mailbox + + reg: + maxItems: 1 + + interrupts: + minItems: 4 + items: + - description: send fifo is empty interrupt + - description: send fifo is not empty interrupt + - description: receive fifo is empty interrupt + - description: receive fifo is not empty interrupt + + interrupt-names: + minItems: 4 + items: + - const: send-empty + - const: send-not-empty + - const: recv-empty + - const: recv-not-empty + + clocks: + description: + Reference to the clock gate phandle(s) if required for this mailbox. + Optional since not all mailboxes are attached to a clock gate. + + "#mbox-cells": + const: 0 + +required: + - compatible + - reg + - interrupts + - interrupt-names + - "#mbox-cells" + +additionalProperties: false + +examples: + - | + mailbox@77408000 { + compatible = "apple,t8103-asc-mailbox"; + reg = <0x77408000 0x4000>; + interrupts = <1 583 4>, <1 584 4>, <1 585 4>, <1 586 4>; + interrupt-names = "send-empty", "send-not-empty", + "recv-empty", "recv-not-empty"; + #mbox-cells = <0>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index cad1289793db..47de27282c98 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1720,6 +1720,7 @@ C: irc://irc.oftc.net/asahi-dev T: git https://github.com/AsahiLinux/linux.git F: Documentation/devicetree/bindings/arm/apple.yaml F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml +F: Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml F: arch/arm64/boot/dts/apple/ F: drivers/irqchip/irq-apple-aic.c
Apple mailbox controller are found on the M1 and are used for communication with various co-processors. Signed-off-by: Sven Peter <sven@svenpeter.dev> --- .../bindings/mailbox/apple,mailbox.yaml | 81 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 82 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml