All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Apple Mailbox Controller support
@ 2021-09-16 15:49 ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel

Hi,

This is the second version of my series which adds support for the mailbox
controllers found on the Apple M1.

v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/

Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
your comments with the following changes:

 - switched to txdone_irq instead of introducing a new mode
 - switched to a threaded interrupt handler for receiving messages
 - added co-processor examples to the device tree binding 
 - reformatted the register defines and clarified multiple comments

Best,

Sven

Sven Peter (2):
  dt-bindings: mailbox: Add Apple mailbox bindings
  mailbox: apple: Add driver for Apple mailboxes

 .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
 MAINTAINERS                                   |   3 +
 drivers/mailbox/Kconfig                       |  12 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
 include/linux/apple-mailbox.h                 |  18 +
 6 files changed, 550 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
 create mode 100644 drivers/mailbox/apple-mailbox.c
 create mode 100644 include/linux/apple-mailbox.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/2] Apple Mailbox Controller support
@ 2021-09-16 15:49 ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel

Hi,

This is the second version of my series which adds support for the mailbox
controllers found on the Apple M1.

v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/

Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
your comments with the following changes:

 - switched to txdone_irq instead of introducing a new mode
 - switched to a threaded interrupt handler for receiving messages
 - added co-processor examples to the device tree binding 
 - reformatted the register defines and clarified multiple comments

Best,

Sven

Sven Peter (2):
  dt-bindings: mailbox: Add Apple mailbox bindings
  mailbox: apple: Add driver for Apple mailboxes

 .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
 MAINTAINERS                                   |   3 +
 drivers/mailbox/Kconfig                       |  12 +
 drivers/mailbox/Makefile                      |   2 +
 drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
 include/linux/apple-mailbox.h                 |  18 +
 6 files changed, 550 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
 create mode 100644 drivers/mailbox/apple-mailbox.c
 create mode 100644 include/linux/apple-mailbox.h

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] dt-bindings: mailbox: Add Apple mailbox bindings
  2021-09-16 15:49 ` Sven Peter
@ 2021-09-16 15:49   ` Sven Peter
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel, Mark Kettenis

Apple mailbox controller are found on the M1 and are used for
communication with various co-processors.

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
 .../bindings/mailbox/apple,mailbox.yaml       | 84 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
new file mode 100644
index 000000000000..79b0cef08232
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
@@ -0,0 +1,84 @@
+# 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 used
+          for example for the display controller, the system management
+          controller and the NVMe coprocessor.
+        items:
+          - const: apple,t8103-asc-mailbox
+
+      - description:
+          M3 mailboxes are an older variant with a slightly different MMIO
+          interface still found on the M1. It is used for the Thunderbolt
+          co-processors.
+        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
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/2] dt-bindings: mailbox: Add Apple mailbox bindings
@ 2021-09-16 15:49   ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel, Mark Kettenis

Apple mailbox controller are found on the M1 and are used for
communication with various co-processors.

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
 .../bindings/mailbox/apple,mailbox.yaml       | 84 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml

diff --git a/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
new file mode 100644
index 000000000000..79b0cef08232
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
@@ -0,0 +1,84 @@
+# 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 used
+          for example for the display controller, the system management
+          controller and the NVMe coprocessor.
+        items:
+          - const: apple,t8103-asc-mailbox
+
+      - description:
+          M3 mailboxes are an older variant with a slightly different MMIO
+          interface still found on the M1. It is used for the Thunderbolt
+          co-processors.
+        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
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-09-16 15:49 ` Sven Peter
@ 2021-09-16 15:49   ` Sven Peter
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel

Apple SoCs such as the M1 come with various co-processors. Mailboxes
are used to communicate with those. This driver adds support for
two variants of those mailboxes.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
 MAINTAINERS                     |   2 +
 drivers/mailbox/Kconfig         |  12 +
 drivers/mailbox/Makefile        |   2 +
 drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
 include/linux/apple-mailbox.h   |  18 ++
 5 files changed, 465 insertions(+)
 create mode 100644 drivers/mailbox/apple-mailbox.c
 create mode 100644 include/linux/apple-mailbox.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 47de27282c98..cf0500bbea5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1724,8 +1724,10 @@ 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
+F:	drivers/mailbox/apple-mailbox.c
 F:	include/dt-bindings/interrupt-controller/apple-aic.h
 F:	include/dt-bindings/pinctrl/apple.h
+F:	include/linux/apple-mailbox.h
 
 ARM/ARTPEC MACHINE SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c9fc06c7e685..d9cd3606040e 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -8,6 +8,18 @@ menuconfig MAILBOX
 
 if MAILBOX
 
+config APPLE_MAILBOX
+	tristate "Apple Mailbox driver"
+	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
+	default ARCH_APPLE
+	help
+	  Apple SoCs have various co-processors required for certain
+	  peripherals to work (NVMe, display controller, etc.). This
+	  driver adds support for the mailbox controller used to
+	  communicate with those.
+
+	  Say Y here if you have a Apple SoC.
+
 config ARM_MHU
 	tristate "ARM MHU Mailbox"
 	depends on ARM_AMBA
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c2089f04887e..338cc05e5431 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
 obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
 
 obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
+
+obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
new file mode 100644
index 000000000000..2622a6369075
--- /dev/null
+++ b/drivers/mailbox/apple-mailbox.c
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple mailbox driver
+ *
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ *
+ * This driver adds support for two mailbox variants (called ASC and M3 by
+ * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
+ * exchange 64+32 bit messages between the main CPU and a co-processor.
+ * Various coprocessors implement different IPC protocols based on these simple
+ * messages and shared memory buffers.
+ *
+ * Both the main CPU and the co-processor see the same set of registers but
+ * the first FIFO (A2I) is always used to transfer messages from the application
+ * processor (us) to the I/O processor and the second one (I2A) for the
+ * other direction.
+ */
+
+#include <linux/apple-mailbox.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/gfp.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
+#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
+
+#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
+#define APPLE_ASC_MBOX_A2I_SEND0   0x800
+#define APPLE_ASC_MBOX_A2I_SEND1   0x808
+#define APPLE_ASC_MBOX_A2I_RECV0   0x810
+#define APPLE_ASC_MBOX_A2I_RECV1   0x818
+
+#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
+#define APPLE_ASC_MBOX_I2A_SEND0   0x820
+#define APPLE_ASC_MBOX_I2A_SEND1   0x828
+#define APPLE_ASC_MBOX_I2A_RECV0   0x830
+#define APPLE_ASC_MBOX_I2A_RECV1   0x838
+
+#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
+#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
+
+#define APPLE_M3_MBOX_A2I_CONTROL 0x50
+#define APPLE_M3_MBOX_A2I_SEND0	  0x60
+#define APPLE_M3_MBOX_A2I_SEND1	  0x68
+#define APPLE_M3_MBOX_A2I_RECV0	  0x70
+#define APPLE_M3_MBOX_A2I_RECV1	  0x78
+
+#define APPLE_M3_MBOX_I2A_CONTROL 0x80
+#define APPLE_M3_MBOX_I2A_SEND0	  0x90
+#define APPLE_M3_MBOX_I2A_SEND1	  0x98
+#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
+#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
+
+#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
+#define APPLE_M3_MBOX_IRQ_ACK		0x4c
+#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
+#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
+#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
+#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
+
+#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
+#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
+#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
+#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
+#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
+
+struct apple_mbox_hw {
+	unsigned int control_full;
+	unsigned int control_empty;
+
+	unsigned int a2i_control;
+	unsigned int a2i_send0;
+	unsigned int a2i_send1;
+
+	unsigned int i2a_control;
+	unsigned int i2a_recv0;
+	unsigned int i2a_recv1;
+
+	bool has_irq_controls;
+	unsigned int irq_enable;
+	unsigned int irq_ack;
+	unsigned int irq_bit_recv_not_empty;
+	unsigned int irq_bit_send_empty;
+};
+
+static const struct apple_mbox_hw apple_mbox_asc_hw = {
+	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
+	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
+
+	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
+	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
+	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
+
+	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
+	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
+	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
+
+	.has_irq_controls = false,
+};
+
+static const struct apple_mbox_hw apple_mbox_m3_hw = {
+	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
+	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
+
+	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
+	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
+	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
+
+	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
+	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
+	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
+
+	.has_irq_controls = true,
+	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
+	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
+	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
+	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
+};
+
+static const struct of_device_id apple_mbox_of_match[] = {
+	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
+	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
+	{}
+};
+MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
+
+struct apple_mbox {
+	void __iomem *regs;
+	const struct apple_mbox_hw *hw;
+
+	int irq_recv_not_empty;
+	int irq_send_empty;
+
+	struct clk_bulk_data *clks;
+	int num_clks;
+
+	struct mbox_chan chan;
+
+	struct device *dev;
+	struct mbox_controller controller;
+};
+
+static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
+{
+	u32 mbox_ctrl =
+		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
+
+	return !(mbox_ctrl & apple_mbox->hw->control_full);
+}
+
+static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
+			      struct apple_mbox_msg *msg)
+{
+	if (!apple_mbox_hw_can_send(apple_mbox))
+		return -EBUSY;
+
+	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
+
+	/*
+	 * This message may be related to a shared memory buffer and we must
+	 * ensure all previous writes to normal memory are visible before
+	 * submitting it.
+	 */
+	dma_wmb();
+
+	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
+	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
+		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
+
+	return 0;
+}
+
+static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
+{
+	u32 mbox_ctrl =
+		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
+
+	return !(mbox_ctrl & apple_mbox->hw->control_empty);
+}
+
+static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
+			      struct apple_mbox_msg *msg)
+{
+	if (!apple_mbox_hw_can_recv(apple_mbox))
+		return -ENOMSG;
+
+	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
+	msg->msg1 = FIELD_GET(
+		APPLE_MBOX_MSG1_MSG,
+		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
+
+	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
+
+	/*
+	 * This message may be related to a shared memory buffer and we must
+	 * ensure any following reads from normal memory only happen after
+	 * having read this message.
+	 */
+	dma_rmb();
+
+	return 0;
+}
+
+static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+	struct apple_mbox_msg *msg = data;
+	int ret;
+
+	ret = apple_mbox_hw_send(apple_mbox, msg);
+	if (ret)
+		return ret;
+
+	/*
+	 * The interrupt is level triggered and will keep firing as long as the
+	 * FIFO is empty. It will also keep firing if the FIFO was empty
+	 * at any point in the past until it has been acknowledged at the
+	 * mailbox level. By acknowledging it here we can ensure that we will
+	 * only get the interrupt once the FIFO has been cleared again.
+	 * If the FIFO is already empty before the ack it will fire again
+	 * immediately after the ack.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_ack);
+	}
+	enable_irq(apple_mbox->irq_send_empty);
+
+	return 0;
+}
+
+static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
+{
+	struct apple_mbox *apple_mbox = data;
+
+	/*
+	 * We don't need to acknowledge the interrupt at the mailbox level
+	 * here even if supported by the hardware. It will keep firing but that
+	 * doesn't matter since it's disabled at the main interrupt controller.
+	 * apple_mbox_chan_send_data will acknowledge it before enabling
+	 * it at the main controller again.
+	 */
+	disable_irq_nosync(apple_mbox->irq_send_empty);
+	mbox_chan_txdone(&apple_mbox->chan, 0);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
+{
+	struct apple_mbox *apple_mbox = data;
+	struct apple_mbox_msg msg;
+
+	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
+		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
+
+	/*
+	 * The interrupt will keep firing even if there are no more messages
+	 * unless we also acknowledge it at the mailbox level here.
+	 * There's no race if a message comes in between the check in the while
+	 * loop above and the ack below: If a new messages arrives inbetween
+	 * those two the interrupt will just fire again immediately after the
+	 * ack since it's level triggered.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_ack);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
+					     const struct of_phandle_args *spec)
+{
+	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
+
+	if (spec->args_count != 0)
+		return ERR_PTR(-EINVAL);
+	if (apple_mbox->chan.con_priv)
+		return ERR_PTR(-EBUSY);
+
+	apple_mbox->chan.con_priv = apple_mbox;
+	return &apple_mbox->chan;
+}
+
+static int apple_mbox_chan_startup(struct mbox_chan *chan)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+
+	/*
+	 * Only some variants of this mailbox HW provide interrupt control
+	 * at the mailbox level. We therefore need to handle enabling/disabling
+	 * interrupts at the main interrupt controller anyway for hardware that
+	 * doesn't. Just always keep the interrupts we care about enabled at
+	 * the mailbox level so that both hardware revisions behave almost
+	 * the same.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
+				       apple_mbox->hw->irq_bit_send_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_enable);
+	}
+
+	enable_irq(apple_mbox->irq_recv_not_empty);
+	return 0;
+}
+
+static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+
+	disable_irq(apple_mbox->irq_recv_not_empty);
+}
+
+static const struct mbox_chan_ops apple_mbox_ops = {
+	.send_data = apple_mbox_chan_send_data,
+	.startup = apple_mbox_chan_startup,
+	.shutdown = apple_mbox_chan_shutdown,
+};
+
+static int apple_mbox_probe(struct platform_device *pdev)
+{
+	int ret;
+	const struct of_device_id *match;
+	char *irqname;
+	struct apple_mbox *mbox;
+	struct device *dev = &pdev->dev;
+
+	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
+	if (!match)
+		return -EINVAL;
+	if (!match->data)
+		return -EINVAL;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, mbox);
+
+	mbox->dev = dev;
+	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mbox->regs))
+		return PTR_ERR(mbox->regs);
+
+	mbox->hw = match->data;
+	mbox->irq_recv_not_empty =
+		platform_get_irq_byname(pdev, "recv-not-empty");
+	if (mbox->irq_recv_not_empty < 0)
+		return -ENODEV;
+
+	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
+	if (mbox->irq_send_empty < 0)
+		return -ENODEV;
+
+	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
+	if (ret < 0)
+		return ret;
+	mbox->num_clks = ret;
+
+	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
+	if (ret)
+		return ret;
+
+	mbox->controller.dev = mbox->dev;
+	mbox->controller.num_chans = 1;
+	mbox->controller.chans = &mbox->chan;
+	mbox->controller.ops = &apple_mbox_ops;
+	mbox->controller.of_xlate = &apple_mbox_of_xlate;
+	mbox->controller.txdone_irq = true;
+
+	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-recv", dev_name(dev));
+	if (!irqname) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+	ret = devm_request_threaded_irq(dev, mbox->irq_recv_not_empty, NULL,
+					apple_mbox_recv_irq,
+					IRQF_NO_AUTOEN | IRQF_ONESHOT, irqname,
+					mbox);
+	if (ret)
+		goto err_clk_disable;
+
+	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-send", dev_name(dev));
+	if (!irqname) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+	ret = devm_request_irq(dev, mbox->irq_send_empty,
+			       apple_mbox_send_empty_irq, IRQF_NO_AUTOEN,
+			       irqname, mbox);
+	if (ret)
+		goto err_clk_disable;
+
+	ret = devm_mbox_controller_register(dev, &mbox->controller);
+	if (ret)
+		goto err_clk_disable;
+	return ret;
+
+err_clk_disable:
+	clk_bulk_disable_unprepare(mbox->num_clks, mbox->clks);
+	return ret;
+}
+
+static int apple_mbox_remove(struct platform_device *pdev)
+{
+	struct apple_mbox *apple_mbox = dev_get_drvdata(&pdev->dev);
+
+	clk_bulk_disable_unprepare(apple_mbox->num_clks, apple_mbox->clks);
+	return 0;
+}
+
+static struct platform_driver apple_mbox_driver = {
+	.driver = {
+		.name = "apple-mailbox",
+		.of_match_table = apple_mbox_of_match,
+	},
+	.probe = apple_mbox_probe,
+	.remove = apple_mbox_remove,
+};
+module_platform_driver(apple_mbox_driver);
+
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_AUTHOR("Sven Peter <sven@svenpeter.dev>");
+MODULE_DESCRIPTION("Apple Mailbox driver");
diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
new file mode 100644
index 000000000000..c455e0f9c73b
--- /dev/null
+++ b/include/linux/apple-mailbox.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Apple mailbox message format
+ *
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ */
+
+#ifndef _LINUX_APPLE_MAILBOX_H_
+#define _LINUX_APPLE_MAILBOX_H_
+
+#include <linux/types.h>
+
+struct apple_mbox_msg {
+	u64 msg0;
+	u32 msg1;
+};
+
+#endif
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-09-16 15:49   ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-09-16 15:49 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Alyssa Rosenzweig, Mohamed Mediouni, Stan Skowronek, devicetree,
	linux-arm-kernel, linux-kernel

Apple SoCs such as the M1 come with various co-processors. Mailboxes
are used to communicate with those. This driver adds support for
two variants of those mailboxes.

Signed-off-by: Sven Peter <sven@svenpeter.dev>
---
 MAINTAINERS                     |   2 +
 drivers/mailbox/Kconfig         |  12 +
 drivers/mailbox/Makefile        |   2 +
 drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
 include/linux/apple-mailbox.h   |  18 ++
 5 files changed, 465 insertions(+)
 create mode 100644 drivers/mailbox/apple-mailbox.c
 create mode 100644 include/linux/apple-mailbox.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 47de27282c98..cf0500bbea5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1724,8 +1724,10 @@ 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
+F:	drivers/mailbox/apple-mailbox.c
 F:	include/dt-bindings/interrupt-controller/apple-aic.h
 F:	include/dt-bindings/pinctrl/apple.h
+F:	include/linux/apple-mailbox.h
 
 ARM/ARTPEC MACHINE SUPPORT
 M:	Jesper Nilsson <jesper.nilsson@axis.com>
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c9fc06c7e685..d9cd3606040e 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -8,6 +8,18 @@ menuconfig MAILBOX
 
 if MAILBOX
 
+config APPLE_MAILBOX
+	tristate "Apple Mailbox driver"
+	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
+	default ARCH_APPLE
+	help
+	  Apple SoCs have various co-processors required for certain
+	  peripherals to work (NVMe, display controller, etc.). This
+	  driver adds support for the mailbox controller used to
+	  communicate with those.
+
+	  Say Y here if you have a Apple SoC.
+
 config ARM_MHU
 	tristate "ARM MHU Mailbox"
 	depends on ARM_AMBA
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c2089f04887e..338cc05e5431 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
 obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
 
 obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
+
+obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
new file mode 100644
index 000000000000..2622a6369075
--- /dev/null
+++ b/drivers/mailbox/apple-mailbox.c
@@ -0,0 +1,431 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+/*
+ * Apple mailbox driver
+ *
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ *
+ * This driver adds support for two mailbox variants (called ASC and M3 by
+ * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
+ * exchange 64+32 bit messages between the main CPU and a co-processor.
+ * Various coprocessors implement different IPC protocols based on these simple
+ * messages and shared memory buffers.
+ *
+ * Both the main CPU and the co-processor see the same set of registers but
+ * the first FIFO (A2I) is always used to transfer messages from the application
+ * processor (us) to the I/O processor and the second one (I2A) for the
+ * other direction.
+ */
+
+#include <linux/apple-mailbox.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/gfp.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
+#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
+
+#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
+#define APPLE_ASC_MBOX_A2I_SEND0   0x800
+#define APPLE_ASC_MBOX_A2I_SEND1   0x808
+#define APPLE_ASC_MBOX_A2I_RECV0   0x810
+#define APPLE_ASC_MBOX_A2I_RECV1   0x818
+
+#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
+#define APPLE_ASC_MBOX_I2A_SEND0   0x820
+#define APPLE_ASC_MBOX_I2A_SEND1   0x828
+#define APPLE_ASC_MBOX_I2A_RECV0   0x830
+#define APPLE_ASC_MBOX_I2A_RECV1   0x838
+
+#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
+#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
+
+#define APPLE_M3_MBOX_A2I_CONTROL 0x50
+#define APPLE_M3_MBOX_A2I_SEND0	  0x60
+#define APPLE_M3_MBOX_A2I_SEND1	  0x68
+#define APPLE_M3_MBOX_A2I_RECV0	  0x70
+#define APPLE_M3_MBOX_A2I_RECV1	  0x78
+
+#define APPLE_M3_MBOX_I2A_CONTROL 0x80
+#define APPLE_M3_MBOX_I2A_SEND0	  0x90
+#define APPLE_M3_MBOX_I2A_SEND1	  0x98
+#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
+#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
+
+#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
+#define APPLE_M3_MBOX_IRQ_ACK		0x4c
+#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
+#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
+#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
+#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
+
+#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
+#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
+#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
+#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
+#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
+
+struct apple_mbox_hw {
+	unsigned int control_full;
+	unsigned int control_empty;
+
+	unsigned int a2i_control;
+	unsigned int a2i_send0;
+	unsigned int a2i_send1;
+
+	unsigned int i2a_control;
+	unsigned int i2a_recv0;
+	unsigned int i2a_recv1;
+
+	bool has_irq_controls;
+	unsigned int irq_enable;
+	unsigned int irq_ack;
+	unsigned int irq_bit_recv_not_empty;
+	unsigned int irq_bit_send_empty;
+};
+
+static const struct apple_mbox_hw apple_mbox_asc_hw = {
+	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
+	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
+
+	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
+	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
+	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
+
+	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
+	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
+	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
+
+	.has_irq_controls = false,
+};
+
+static const struct apple_mbox_hw apple_mbox_m3_hw = {
+	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
+	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
+
+	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
+	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
+	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
+
+	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
+	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
+	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
+
+	.has_irq_controls = true,
+	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
+	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
+	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
+	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
+};
+
+static const struct of_device_id apple_mbox_of_match[] = {
+	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
+	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
+	{}
+};
+MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
+
+struct apple_mbox {
+	void __iomem *regs;
+	const struct apple_mbox_hw *hw;
+
+	int irq_recv_not_empty;
+	int irq_send_empty;
+
+	struct clk_bulk_data *clks;
+	int num_clks;
+
+	struct mbox_chan chan;
+
+	struct device *dev;
+	struct mbox_controller controller;
+};
+
+static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
+{
+	u32 mbox_ctrl =
+		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
+
+	return !(mbox_ctrl & apple_mbox->hw->control_full);
+}
+
+static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
+			      struct apple_mbox_msg *msg)
+{
+	if (!apple_mbox_hw_can_send(apple_mbox))
+		return -EBUSY;
+
+	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
+
+	/*
+	 * This message may be related to a shared memory buffer and we must
+	 * ensure all previous writes to normal memory are visible before
+	 * submitting it.
+	 */
+	dma_wmb();
+
+	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
+	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
+		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
+
+	return 0;
+}
+
+static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
+{
+	u32 mbox_ctrl =
+		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
+
+	return !(mbox_ctrl & apple_mbox->hw->control_empty);
+}
+
+static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
+			      struct apple_mbox_msg *msg)
+{
+	if (!apple_mbox_hw_can_recv(apple_mbox))
+		return -ENOMSG;
+
+	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
+	msg->msg1 = FIELD_GET(
+		APPLE_MBOX_MSG1_MSG,
+		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
+
+	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
+
+	/*
+	 * This message may be related to a shared memory buffer and we must
+	 * ensure any following reads from normal memory only happen after
+	 * having read this message.
+	 */
+	dma_rmb();
+
+	return 0;
+}
+
+static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+	struct apple_mbox_msg *msg = data;
+	int ret;
+
+	ret = apple_mbox_hw_send(apple_mbox, msg);
+	if (ret)
+		return ret;
+
+	/*
+	 * The interrupt is level triggered and will keep firing as long as the
+	 * FIFO is empty. It will also keep firing if the FIFO was empty
+	 * at any point in the past until it has been acknowledged at the
+	 * mailbox level. By acknowledging it here we can ensure that we will
+	 * only get the interrupt once the FIFO has been cleared again.
+	 * If the FIFO is already empty before the ack it will fire again
+	 * immediately after the ack.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_ack);
+	}
+	enable_irq(apple_mbox->irq_send_empty);
+
+	return 0;
+}
+
+static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
+{
+	struct apple_mbox *apple_mbox = data;
+
+	/*
+	 * We don't need to acknowledge the interrupt at the mailbox level
+	 * here even if supported by the hardware. It will keep firing but that
+	 * doesn't matter since it's disabled at the main interrupt controller.
+	 * apple_mbox_chan_send_data will acknowledge it before enabling
+	 * it at the main controller again.
+	 */
+	disable_irq_nosync(apple_mbox->irq_send_empty);
+	mbox_chan_txdone(&apple_mbox->chan, 0);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
+{
+	struct apple_mbox *apple_mbox = data;
+	struct apple_mbox_msg msg;
+
+	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
+		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
+
+	/*
+	 * The interrupt will keep firing even if there are no more messages
+	 * unless we also acknowledge it at the mailbox level here.
+	 * There's no race if a message comes in between the check in the while
+	 * loop above and the ack below: If a new messages arrives inbetween
+	 * those two the interrupt will just fire again immediately after the
+	 * ack since it's level triggered.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_ack);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
+					     const struct of_phandle_args *spec)
+{
+	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
+
+	if (spec->args_count != 0)
+		return ERR_PTR(-EINVAL);
+	if (apple_mbox->chan.con_priv)
+		return ERR_PTR(-EBUSY);
+
+	apple_mbox->chan.con_priv = apple_mbox;
+	return &apple_mbox->chan;
+}
+
+static int apple_mbox_chan_startup(struct mbox_chan *chan)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+
+	/*
+	 * Only some variants of this mailbox HW provide interrupt control
+	 * at the mailbox level. We therefore need to handle enabling/disabling
+	 * interrupts at the main interrupt controller anyway for hardware that
+	 * doesn't. Just always keep the interrupts we care about enabled at
+	 * the mailbox level so that both hardware revisions behave almost
+	 * the same.
+	 */
+	if (apple_mbox->hw->has_irq_controls) {
+		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
+				       apple_mbox->hw->irq_bit_send_empty,
+			       apple_mbox->regs + apple_mbox->hw->irq_enable);
+	}
+
+	enable_irq(apple_mbox->irq_recv_not_empty);
+	return 0;
+}
+
+static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
+{
+	struct apple_mbox *apple_mbox = chan->con_priv;
+
+	disable_irq(apple_mbox->irq_recv_not_empty);
+}
+
+static const struct mbox_chan_ops apple_mbox_ops = {
+	.send_data = apple_mbox_chan_send_data,
+	.startup = apple_mbox_chan_startup,
+	.shutdown = apple_mbox_chan_shutdown,
+};
+
+static int apple_mbox_probe(struct platform_device *pdev)
+{
+	int ret;
+	const struct of_device_id *match;
+	char *irqname;
+	struct apple_mbox *mbox;
+	struct device *dev = &pdev->dev;
+
+	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
+	if (!match)
+		return -EINVAL;
+	if (!match->data)
+		return -EINVAL;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, mbox);
+
+	mbox->dev = dev;
+	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(mbox->regs))
+		return PTR_ERR(mbox->regs);
+
+	mbox->hw = match->data;
+	mbox->irq_recv_not_empty =
+		platform_get_irq_byname(pdev, "recv-not-empty");
+	if (mbox->irq_recv_not_empty < 0)
+		return -ENODEV;
+
+	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
+	if (mbox->irq_send_empty < 0)
+		return -ENODEV;
+
+	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
+	if (ret < 0)
+		return ret;
+	mbox->num_clks = ret;
+
+	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
+	if (ret)
+		return ret;
+
+	mbox->controller.dev = mbox->dev;
+	mbox->controller.num_chans = 1;
+	mbox->controller.chans = &mbox->chan;
+	mbox->controller.ops = &apple_mbox_ops;
+	mbox->controller.of_xlate = &apple_mbox_of_xlate;
+	mbox->controller.txdone_irq = true;
+
+	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-recv", dev_name(dev));
+	if (!irqname) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+	ret = devm_request_threaded_irq(dev, mbox->irq_recv_not_empty, NULL,
+					apple_mbox_recv_irq,
+					IRQF_NO_AUTOEN | IRQF_ONESHOT, irqname,
+					mbox);
+	if (ret)
+		goto err_clk_disable;
+
+	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-send", dev_name(dev));
+	if (!irqname) {
+		ret = -ENOMEM;
+		goto err_clk_disable;
+	}
+	ret = devm_request_irq(dev, mbox->irq_send_empty,
+			       apple_mbox_send_empty_irq, IRQF_NO_AUTOEN,
+			       irqname, mbox);
+	if (ret)
+		goto err_clk_disable;
+
+	ret = devm_mbox_controller_register(dev, &mbox->controller);
+	if (ret)
+		goto err_clk_disable;
+	return ret;
+
+err_clk_disable:
+	clk_bulk_disable_unprepare(mbox->num_clks, mbox->clks);
+	return ret;
+}
+
+static int apple_mbox_remove(struct platform_device *pdev)
+{
+	struct apple_mbox *apple_mbox = dev_get_drvdata(&pdev->dev);
+
+	clk_bulk_disable_unprepare(apple_mbox->num_clks, apple_mbox->clks);
+	return 0;
+}
+
+static struct platform_driver apple_mbox_driver = {
+	.driver = {
+		.name = "apple-mailbox",
+		.of_match_table = apple_mbox_of_match,
+	},
+	.probe = apple_mbox_probe,
+	.remove = apple_mbox_remove,
+};
+module_platform_driver(apple_mbox_driver);
+
+MODULE_LICENSE("Dual MIT/GPL");
+MODULE_AUTHOR("Sven Peter <sven@svenpeter.dev>");
+MODULE_DESCRIPTION("Apple Mailbox driver");
diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
new file mode 100644
index 000000000000..c455e0f9c73b
--- /dev/null
+++ b/include/linux/apple-mailbox.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/*
+ * Apple mailbox message format
+ *
+ * Copyright (C) 2021 The Asahi Linux Contributors
+ */
+
+#ifndef _LINUX_APPLE_MAILBOX_H_
+#define _LINUX_APPLE_MAILBOX_H_
+
+#include <linux/types.h>
+
+struct apple_mbox_msg {
+	u64 msg0;
+	u32 msg1;
+};
+
+#endif
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Apple Mailbox Controller support
  2021-09-16 15:49 ` Sven Peter
@ 2021-09-19 11:47   ` Alyssa Rosenzweig
  -1 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-09-19 11:47 UTC (permalink / raw)
  To: Sven Peter
  Cc: Jassi Brar, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

I've dropped v1 from my tree and cherry-picked "mailbox: apple: Add
driver for Apple mailboxes". NVMe and DCP both still work, so this is

	Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

Thanks!

On Thu , Sep 16, 2021 at 05:49:09PM +0200, Sven Peter wrote:
> Hi,
> 
> This is the second version of my series which adds support for the mailbox
> controllers found on the Apple M1.
> 
> v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/
> 
> Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
> your comments with the following changes:
> 
>  - switched to txdone_irq instead of introducing a new mode
>  - switched to a threaded interrupt handler for receiving messages
>  - added co-processor examples to the device tree binding 
>  - reformatted the register defines and clarified multiple comments
> 
> Best,
> 
> Sven
> 
> Sven Peter (2):
>   dt-bindings: mailbox: Add Apple mailbox bindings
>   mailbox: apple: Add driver for Apple mailboxes
> 
>  .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
>  MAINTAINERS                                   |   3 +
>  drivers/mailbox/Kconfig                       |  12 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
>  include/linux/apple-mailbox.h                 |  18 +
>  6 files changed, 550 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
> 
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Apple Mailbox Controller support
@ 2021-09-19 11:47   ` Alyssa Rosenzweig
  0 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-09-19 11:47 UTC (permalink / raw)
  To: Sven Peter
  Cc: Jassi Brar, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

I've dropped v1 from my tree and cherry-picked "mailbox: apple: Add
driver for Apple mailboxes". NVMe and DCP both still work, so this is

	Tested-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

Thanks!

On Thu , Sep 16, 2021 at 05:49:09PM +0200, Sven Peter wrote:
> Hi,
> 
> This is the second version of my series which adds support for the mailbox
> controllers found on the Apple M1.
> 
> v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/
> 
> Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
> your comments with the following changes:
> 
>  - switched to txdone_irq instead of introducing a new mode
>  - switched to a threaded interrupt handler for receiving messages
>  - added co-processor examples to the device tree binding 
>  - reformatted the register defines and clarified multiple comments
> 
> Best,
> 
> Sven
> 
> Sven Peter (2):
>   dt-bindings: mailbox: Add Apple mailbox bindings
>   mailbox: apple: Add driver for Apple mailboxes
> 
>  .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
>  MAINTAINERS                                   |   3 +
>  drivers/mailbox/Kconfig                       |  12 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
>  include/linux/apple-mailbox.h                 |  18 +
>  6 files changed, 550 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
> 
> -- 
> 2.25.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-09-16 15:49   ` Sven Peter
@ 2021-09-19 11:51     ` Alyssa Rosenzweig
  -1 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-09-19 11:51 UTC (permalink / raw)
  To: Sven Peter
  Cc: Jassi Brar, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
...
> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)

It might make sense to combine these to just

> +#define APPLE_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_MBOX_CONTROL_EMPTY BIT(17)

..unless we have a reason to think the status bits otherwise differ.

Regardless of that minor nit, this is

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-09-19 11:51     ` Alyssa Rosenzweig
  0 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-09-19 11:51 UTC (permalink / raw)
  To: Sven Peter
  Cc: Jassi Brar, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
...
> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)

It might make sense to combine these to just

> +#define APPLE_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_MBOX_CONTROL_EMPTY BIT(17)

..unless we have a reason to think the status bits otherwise differ.

Regardless of that minor nit, this is

Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: mailbox: Add Apple mailbox bindings
  2021-09-16 15:49   ` Sven Peter
@ 2021-09-21 22:08     ` Rob Herring
  -1 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-09-21 22:08 UTC (permalink / raw)
  To: Sven Peter
  Cc: Mark Kettenis, Mohamed Mediouni, Stan Skowronek, Mark Kettenis,
	Rob Herring, linux-arm-kernel, Alyssa Rosenzweig, Jassi Brar,
	linux-kernel, devicetree, Hector Martin

On Thu, 16 Sep 2021 17:49:10 +0200, Sven Peter wrote:
> Apple mailbox controller are found on the M1 and are used for
> communication with various co-processors.
> 
> Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>  .../bindings/mailbox/apple,mailbox.yaml       | 84 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 1/2] dt-bindings: mailbox: Add Apple mailbox bindings
@ 2021-09-21 22:08     ` Rob Herring
  0 siblings, 0 replies; 24+ messages in thread
From: Rob Herring @ 2021-09-21 22:08 UTC (permalink / raw)
  To: Sven Peter
  Cc: Mark Kettenis, Mohamed Mediouni, Stan Skowronek, Mark Kettenis,
	Rob Herring, linux-arm-kernel, Alyssa Rosenzweig, Jassi Brar,
	linux-kernel, devicetree, Hector Martin

On Thu, 16 Sep 2021 17:49:10 +0200, Sven Peter wrote:
> Apple mailbox controller are found on the M1 and are used for
> communication with various co-processors.
> 
> Reviewed-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> Reviewed-by: Mark Kettenis <kettenis@openbsd.org>
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>  .../bindings/mailbox/apple,mailbox.yaml       | 84 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Apple Mailbox Controller support
  2021-09-16 15:49 ` Sven Peter
@ 2021-10-16  9:21   ` Sven Peter
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-16  9:21 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

On Thu, Sep 16, 2021, at 17:49, Sven Peter wrote:
> Hi,
>
> This is the second version of my series which adds support for the mailbox
> controllers found on the Apple M1.
>
> v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/
>
> Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
> your comments with the following changes:
>
>  - switched to txdone_irq instead of introducing a new mode
>  - switched to a threaded interrupt handler for receiving messages
>  - added co-processor examples to the device tree binding 
>  - reformatted the register defines and clarified multiple comments
>
> Best,
>
> Sven
>
> Sven Peter (2):
>   dt-bindings: mailbox: Add Apple mailbox bindings
>   mailbox: apple: Add driver for Apple mailboxes
>
>  .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
>  MAINTAINERS                                   |   3 +
>  drivers/mailbox/Kconfig                       |  12 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
>  include/linux/apple-mailbox.h                 |  18 +
>  6 files changed, 550 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
>
> -- 
> 2.25.1

Hi Jassi,

Have you had a chance to look at this series yet?


Thanks,


Sven

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 0/2] Apple Mailbox Controller support
@ 2021-10-16  9:21   ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-16  9:21 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel

On Thu, Sep 16, 2021, at 17:49, Sven Peter wrote:
> Hi,
>
> This is the second version of my series which adds support for the mailbox
> controllers found on the Apple M1.
>
> v1: https://lore.kernel.org/lkml/20210907145501.69161-1-sven@svenpeter.dev/
>
> Thanks to Jassi, Mark and Alyssa for the initial review. I've addressed
> your comments with the following changes:
>
>  - switched to txdone_irq instead of introducing a new mode
>  - switched to a threaded interrupt handler for receiving messages
>  - added co-processor examples to the device tree binding 
>  - reformatted the register defines and clarified multiple comments
>
> Best,
>
> Sven
>
> Sven Peter (2):
>   dt-bindings: mailbox: Add Apple mailbox bindings
>   mailbox: apple: Add driver for Apple mailboxes
>
>  .../bindings/mailbox/apple,mailbox.yaml       |  84 ++++
>  MAINTAINERS                                   |   3 +
>  drivers/mailbox/Kconfig                       |  12 +
>  drivers/mailbox/Makefile                      |   2 +
>  drivers/mailbox/apple-mailbox.c               | 431 ++++++++++++++++++
>  include/linux/apple-mailbox.h                 |  18 +
>  6 files changed, 550 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
>
> -- 
> 2.25.1

Hi Jassi,

Have you had a chance to look at this series yet?


Thanks,


Sven

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-09-16 15:49   ` Sven Peter
@ 2021-10-16 19:04     ` Jassi Brar
  -1 siblings, 0 replies; 24+ messages in thread
From: Jassi Brar @ 2021-10-16 19:04 UTC (permalink / raw)
  To: Sven Peter
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

On Thu, Sep 16, 2021 at 10:49 AM Sven Peter <sven@svenpeter.dev> wrote:

....
> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
> +       .control_full = APPLE_ASC_MBOX_CONTROL_FULL,
> +       .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
> +
> +       .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
> +       .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
> +       .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
> +
> +       .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
> +       .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
> +       .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
> +
> +       .has_irq_controls = false,
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
> +       .control_full = APPLE_M3_MBOX_CONTROL_FULL,
> +       .control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
> +
> +       .a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
> +       .a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
> +       .a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
> +
> +       .i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
> +       .i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
> +       .i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
> +
> +       .has_irq_controls = true,
> +       .irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
> +       .irq_ack = APPLE_M3_MBOX_IRQ_ACK,
> +       .irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
> +       .irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
> +};
> +
> +static const struct of_device_id apple_mbox_of_match[] = {
> +       { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
> +       { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
> +
Traditionally, these go at the end of file.

....
> +
> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
> +                             struct apple_mbox_msg *msg)
> +{
> +       if (!apple_mbox_hw_can_send(apple_mbox))
> +               return -EBUSY;
> +
> +       dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +       /*
> +        * This message may be related to a shared memory buffer and we must
> +        * ensure all previous writes to normal memory are visible before
> +        * submitting it.
> +        */
> +       dma_wmb();
> +
This may cause unnecessary overhead.
mbox_client.tx_prepare() is where the SHMEM data is copied, which
should also call dma_wmb() just before return.
......

> +
> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
> +                             struct apple_mbox_msg *msg)
> +{
> +       if (!apple_mbox_hw_can_recv(apple_mbox))
> +               return -ENOMSG;
> +
> +       msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
> +       msg->msg1 = FIELD_GET(
> +               APPLE_MBOX_MSG1_MSG,
> +               readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
> +
> +       dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +       /*
> +        * This message may be related to a shared memory buffer and we must
> +        * ensure any following reads from normal memory only happen after
> +        * having read this message.
> +        */
> +       dma_rmb();
> +
.... similarly should be done by the client as the first thing in recv callback.


> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
> +                                            const struct of_phandle_args *spec)
> +{
> +       struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
> +
> +       if (spec->args_count != 0)
> +               return ERR_PTR(-EINVAL);
> +       if (apple_mbox->chan.con_priv)
> +               return ERR_PTR(-EBUSY);
> +
> +       apple_mbox->chan.con_priv = apple_mbox;
> +       return &apple_mbox->chan;
> +}
> +
we could do without of_xlate callback, by assigning  chan.con_priv
already in the .probe()


> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
> +
> +struct apple_mbox_msg {
> +       u64 msg0;
> +       u32 msg1;
> +};
> +
Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
them out as such. But of course, I don't know the h/w details so I may
be wrong.

Cheers!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-10-16 19:04     ` Jassi Brar
  0 siblings, 0 replies; 24+ messages in thread
From: Jassi Brar @ 2021-10-16 19:04 UTC (permalink / raw)
  To: Sven Peter
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

On Thu, Sep 16, 2021 at 10:49 AM Sven Peter <sven@svenpeter.dev> wrote:

....
> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
> +       .control_full = APPLE_ASC_MBOX_CONTROL_FULL,
> +       .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
> +
> +       .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
> +       .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
> +       .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
> +
> +       .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
> +       .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
> +       .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
> +
> +       .has_irq_controls = false,
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
> +       .control_full = APPLE_M3_MBOX_CONTROL_FULL,
> +       .control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
> +
> +       .a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
> +       .a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
> +       .a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
> +
> +       .i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
> +       .i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
> +       .i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
> +
> +       .has_irq_controls = true,
> +       .irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
> +       .irq_ack = APPLE_M3_MBOX_IRQ_ACK,
> +       .irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
> +       .irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
> +};
> +
> +static const struct of_device_id apple_mbox_of_match[] = {
> +       { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
> +       { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
> +       {}
> +};
> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
> +
Traditionally, these go at the end of file.

....
> +
> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
> +                             struct apple_mbox_msg *msg)
> +{
> +       if (!apple_mbox_hw_can_send(apple_mbox))
> +               return -EBUSY;
> +
> +       dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +       /*
> +        * This message may be related to a shared memory buffer and we must
> +        * ensure all previous writes to normal memory are visible before
> +        * submitting it.
> +        */
> +       dma_wmb();
> +
This may cause unnecessary overhead.
mbox_client.tx_prepare() is where the SHMEM data is copied, which
should also call dma_wmb() just before return.
......

> +
> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
> +                             struct apple_mbox_msg *msg)
> +{
> +       if (!apple_mbox_hw_can_recv(apple_mbox))
> +               return -ENOMSG;
> +
> +       msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
> +       msg->msg1 = FIELD_GET(
> +               APPLE_MBOX_MSG1_MSG,
> +               readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
> +
> +       dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +       /*
> +        * This message may be related to a shared memory buffer and we must
> +        * ensure any following reads from normal memory only happen after
> +        * having read this message.
> +        */
> +       dma_rmb();
> +
.... similarly should be done by the client as the first thing in recv callback.


> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
> +                                            const struct of_phandle_args *spec)
> +{
> +       struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
> +
> +       if (spec->args_count != 0)
> +               return ERR_PTR(-EINVAL);
> +       if (apple_mbox->chan.con_priv)
> +               return ERR_PTR(-EBUSY);
> +
> +       apple_mbox->chan.con_priv = apple_mbox;
> +       return &apple_mbox->chan;
> +}
> +
we could do without of_xlate callback, by assigning  chan.con_priv
already in the .probe()


> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
> +
> +struct apple_mbox_msg {
> +       u64 msg0;
> +       u32 msg1;
> +};
> +
Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
them out as such. But of course, I don't know the h/w details so I may
be wrong.

Cheers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-10-16 19:04     ` Jassi Brar
@ 2021-10-16 19:16       ` Alyssa Rosenzweig
  -1 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-10-16 19:16 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

> > +struct apple_mbox_msg {
> > +       u64 msg0;
> > +       u32 msg1;
> > +};
> > +
> Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
> them out as such. But of course, I don't know the h/w details so I may
> be wrong.

AIUI -- It's a 96-bit message (msg0 is general purpose, msg1 is used for
dispatch). This goes for both RX and TX.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-10-16 19:16       ` Alyssa Rosenzweig
  0 siblings, 0 replies; 24+ messages in thread
From: Alyssa Rosenzweig @ 2021-10-16 19:16 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Sven Peter, Rob Herring, Mark Kettenis, Hector Martin,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

> > +struct apple_mbox_msg {
> > +       u64 msg0;
> > +       u32 msg1;
> > +};
> > +
> Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
> them out as such. But of course, I don't know the h/w details so I may
> be wrong.

AIUI -- It's a 96-bit message (msg0 is general purpose, msg1 is used for
dispatch). This goes for both RX and TX.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-10-16 19:04     ` Jassi Brar
@ 2021-10-16 19:17       ` Sven Peter
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-16 19:17 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

Hi Jassi,

Thanks a lot for the review!

On Sat, Oct 16, 2021, at 21:04, Jassi Brar wrote:
> On Thu, Sep 16, 2021 at 10:49 AM Sven Peter <sven@svenpeter.dev> wrote:
>
> ....
>> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
>> +       .control_full = APPLE_ASC_MBOX_CONTROL_FULL,
>> +       .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
>> +
>> +       .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
>> +       .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
>> +       .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
>> +
>> +       .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
>> +       .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
>> +       .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
>> +
>> +       .has_irq_controls = false,
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
>> +       .control_full = APPLE_M3_MBOX_CONTROL_FULL,
>> +       .control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
>> +
>> +       .a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
>> +       .a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
>> +       .a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
>> +
>> +       .i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
>> +       .i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
>> +       .i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
>> +
>> +       .has_irq_controls = true,
>> +       .irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
>> +       .irq_ack = APPLE_M3_MBOX_IRQ_ACK,
>> +       .irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
>> +       .irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
>> +};
>> +
>> +static const struct of_device_id apple_mbox_of_match[] = {
>> +       { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
>> +       { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
>> +       {}
>> +};
>> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
>> +
> Traditionally, these go at the end of file.

Ack.

>
> ....
>> +
>> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
>> +                             struct apple_mbox_msg *msg)
>> +{
>> +       if (!apple_mbox_hw_can_send(apple_mbox))
>> +               return -EBUSY;
>> +
>> +       dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +       /*
>> +        * This message may be related to a shared memory buffer and we must
>> +        * ensure all previous writes to normal memory are visible before
>> +        * submitting it.
>> +        */
>> +       dma_wmb();
>> +
> This may cause unnecessary overhead.
> mbox_client.tx_prepare() is where the SHMEM data is copied, which
> should also call dma_wmb() just before return.
> ......
>

Ok, I'll just drop it here then.

>> +
>> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
>> +                             struct apple_mbox_msg *msg)
>> +{
>> +       if (!apple_mbox_hw_can_recv(apple_mbox))
>> +               return -ENOMSG;
>> +
>> +       msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
>> +       msg->msg1 = FIELD_GET(
>> +               APPLE_MBOX_MSG1_MSG,
>> +               readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
>> +
>> +       dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +       /*
>> +        * This message may be related to a shared memory buffer and we must
>> +        * ensure any following reads from normal memory only happen after
>> +        * having read this message.
>> +        */
>> +       dma_rmb();
>> +
> .... similarly should be done by the client as the first thing in recv callback.

Ok.

>
>
>> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
>> +                                            const struct of_phandle_args *spec)
>> +{
>> +       struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
>> +
>> +       if (spec->args_count != 0)
>> +               return ERR_PTR(-EINVAL);
>> +       if (apple_mbox->chan.con_priv)
>> +               return ERR_PTR(-EBUSY);
>> +
>> +       apple_mbox->chan.con_priv = apple_mbox;
>> +       return &apple_mbox->chan;
>> +}
>> +
> we could do without of_xlate callback, by assigning  chan.con_priv
> already in the .probe()

Sure, will do that.

>
>
>> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
>> +
>> +struct apple_mbox_msg {
>> +       u64 msg0;
>> +       u32 msg1;
>> +};
>> +
> Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
> them out as such. But of course, I don't know the h/w details so I may
> be wrong.

This hardware essentially only has a single channel.
Depending on the firmware running on the other side sometimes msg1 is used
as an endpoint index and sometimes 8bits of msg0 are. This is similar to the
discussion about the raspberry pi mailbox hardware [1].



Thanks,


Sven


[1] https://lore.kernel.org/all/550C7534.8070100@wwwdotorg.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-10-16 19:17       ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-16 19:17 UTC (permalink / raw)
  To: Jassi Brar
  Cc: Rob Herring, Mark Kettenis, Hector Martin, Alyssa Rosenzweig,
	Mohamed Mediouni, Stan Skowronek, Devicetree List,
	linux-arm-kernel, Linux Kernel Mailing List

Hi Jassi,

Thanks a lot for the review!

On Sat, Oct 16, 2021, at 21:04, Jassi Brar wrote:
> On Thu, Sep 16, 2021 at 10:49 AM Sven Peter <sven@svenpeter.dev> wrote:
>
> ....
>> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
>> +       .control_full = APPLE_ASC_MBOX_CONTROL_FULL,
>> +       .control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
>> +
>> +       .a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
>> +       .a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
>> +       .a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
>> +
>> +       .i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
>> +       .i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
>> +       .i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
>> +
>> +       .has_irq_controls = false,
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
>> +       .control_full = APPLE_M3_MBOX_CONTROL_FULL,
>> +       .control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
>> +
>> +       .a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
>> +       .a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
>> +       .a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
>> +
>> +       .i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
>> +       .i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
>> +       .i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
>> +
>> +       .has_irq_controls = true,
>> +       .irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
>> +       .irq_ack = APPLE_M3_MBOX_IRQ_ACK,
>> +       .irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
>> +       .irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
>> +};
>> +
>> +static const struct of_device_id apple_mbox_of_match[] = {
>> +       { .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
>> +       { .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
>> +       {}
>> +};
>> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
>> +
> Traditionally, these go at the end of file.

Ack.

>
> ....
>> +
>> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
>> +                             struct apple_mbox_msg *msg)
>> +{
>> +       if (!apple_mbox_hw_can_send(apple_mbox))
>> +               return -EBUSY;
>> +
>> +       dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +       /*
>> +        * This message may be related to a shared memory buffer and we must
>> +        * ensure all previous writes to normal memory are visible before
>> +        * submitting it.
>> +        */
>> +       dma_wmb();
>> +
> This may cause unnecessary overhead.
> mbox_client.tx_prepare() is where the SHMEM data is copied, which
> should also call dma_wmb() just before return.
> ......
>

Ok, I'll just drop it here then.

>> +
>> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
>> +                             struct apple_mbox_msg *msg)
>> +{
>> +       if (!apple_mbox_hw_can_recv(apple_mbox))
>> +               return -ENOMSG;
>> +
>> +       msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
>> +       msg->msg1 = FIELD_GET(
>> +               APPLE_MBOX_MSG1_MSG,
>> +               readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
>> +
>> +       dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +       /*
>> +        * This message may be related to a shared memory buffer and we must
>> +        * ensure any following reads from normal memory only happen after
>> +        * having read this message.
>> +        */
>> +       dma_rmb();
>> +
> .... similarly should be done by the client as the first thing in recv callback.

Ok.

>
>
>> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
>> +                                            const struct of_phandle_args *spec)
>> +{
>> +       struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
>> +
>> +       if (spec->args_count != 0)
>> +               return ERR_PTR(-EINVAL);
>> +       if (apple_mbox->chan.con_priv)
>> +               return ERR_PTR(-EBUSY);
>> +
>> +       apple_mbox->chan.con_priv = apple_mbox;
>> +       return &apple_mbox->chan;
>> +}
>> +
> we could do without of_xlate callback, by assigning  chan.con_priv
> already in the .probe()

Sure, will do that.

>
>
>> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
>> +
>> +struct apple_mbox_msg {
>> +       u64 msg0;
>> +       u32 msg1;
>> +};
>> +
> Aren't msg0/1 the Tx and Rx channels? If so you may want to separate
> them out as such. But of course, I don't know the h/w details so I may
> be wrong.

This hardware essentially only has a single channel.
Depending on the firmware running on the other side sometimes msg1 is used
as an endpoint index and sometimes 8bits of msg0 are. This is similar to the
discussion about the raspberry pi mailbox hardware [1].



Thanks,


Sven


[1] https://lore.kernel.org/all/550C7534.8070100@wwwdotorg.org/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-09-16 15:49   ` Sven Peter
@ 2021-10-17 11:25     ` Mark Kettenis
  -1 siblings, 0 replies; 24+ messages in thread
From: Mark Kettenis @ 2021-10-17 11:25 UTC (permalink / raw)
  To: Sven Peter
  Cc: jassisinghbrar, sven, robh+dt, marcan, alyssa, mohamed.mediouni,
	stan, devicetree, linux-arm-kernel, linux-kernel

> From: Sven Peter <sven@svenpeter.dev>
> Date: Thu, 16 Sep 2021 17:49:11 +0200

Hi Sven,

> Apple SoCs such as the M1 come with various co-processors. Mailboxes
> are used to communicate with those. This driver adds support for
> two variants of those mailboxes.
> 
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>  MAINTAINERS                     |   2 +
>  drivers/mailbox/Kconfig         |  12 +
>  drivers/mailbox/Makefile        |   2 +
>  drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
>  include/linux/apple-mailbox.h   |  18 ++
>  5 files changed, 465 insertions(+)
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 47de27282c98..cf0500bbea5b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1724,8 +1724,10 @@ 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
> +F:	drivers/mailbox/apple-mailbox.c
>  F:	include/dt-bindings/interrupt-controller/apple-aic.h
>  F:	include/dt-bindings/pinctrl/apple.h
> +F:	include/linux/apple-mailbox.h
>  
>  ARM/ARTPEC MACHINE SUPPORT
>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c9fc06c7e685..d9cd3606040e 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -8,6 +8,18 @@ menuconfig MAILBOX
>  
>  if MAILBOX
>  
> +config APPLE_MAILBOX
> +	tristate "Apple Mailbox driver"
> +	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
> +	default ARCH_APPLE
> +	help
> +	  Apple SoCs have various co-processors required for certain
> +	  peripherals to work (NVMe, display controller, etc.). This
> +	  driver adds support for the mailbox controller used to
> +	  communicate with those.
> +
> +	  Say Y here if you have a Apple SoC.
> +
>  config ARM_MHU
>  	tristate "ARM MHU Mailbox"
>  	depends on ARM_AMBA
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index c2089f04887e..338cc05e5431 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
>  obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
>  
>  obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
> +
> +obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
> diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
> new file mode 100644
> index 000000000000..2622a6369075
> --- /dev/null
> +++ b/drivers/mailbox/apple-mailbox.c
> @@ -0,0 +1,431 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple mailbox driver
> + *
> + * Copyright (C) 2021 The Asahi Linux Contributors
> + *
> + * This driver adds support for two mailbox variants (called ASC and M3 by
> + * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
> + * exchange 64+32 bit messages between the main CPU and a co-processor.
> + * Various coprocessors implement different IPC protocols based on these simple
> + * messages and shared memory buffers.
> + *
> + * Both the main CPU and the co-processor see the same set of registers but
> + * the first FIFO (A2I) is always used to transfer messages from the application
> + * processor (us) to the I/O processor and the second one (I2A) for the
> + * other direction.
> + */
> +
> +#include <linux/apple-mailbox.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/gfp.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
> +
> +#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
> +#define APPLE_ASC_MBOX_A2I_SEND0   0x800
> +#define APPLE_ASC_MBOX_A2I_SEND1   0x808
> +#define APPLE_ASC_MBOX_A2I_RECV0   0x810
> +#define APPLE_ASC_MBOX_A2I_RECV1   0x818
> +
> +#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
> +#define APPLE_ASC_MBOX_I2A_SEND0   0x820
> +#define APPLE_ASC_MBOX_I2A_SEND1   0x828
> +#define APPLE_ASC_MBOX_I2A_RECV0   0x830
> +#define APPLE_ASC_MBOX_I2A_RECV1   0x838
> +
> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
> +
> +#define APPLE_M3_MBOX_A2I_CONTROL 0x50
> +#define APPLE_M3_MBOX_A2I_SEND0	  0x60
> +#define APPLE_M3_MBOX_A2I_SEND1	  0x68
> +#define APPLE_M3_MBOX_A2I_RECV0	  0x70
> +#define APPLE_M3_MBOX_A2I_RECV1	  0x78
> +
> +#define APPLE_M3_MBOX_I2A_CONTROL 0x80
> +#define APPLE_M3_MBOX_I2A_SEND0	  0x90
> +#define APPLE_M3_MBOX_I2A_SEND1	  0x98
> +#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
> +#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
> +
> +#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
> +#define APPLE_M3_MBOX_IRQ_ACK		0x4c
> +#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
> +#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
> +#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
> +#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
> +
> +#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
> +#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
> +#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
> +#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
> +#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
> +
> +struct apple_mbox_hw {
> +	unsigned int control_full;
> +	unsigned int control_empty;
> +
> +	unsigned int a2i_control;
> +	unsigned int a2i_send0;
> +	unsigned int a2i_send1;
> +
> +	unsigned int i2a_control;
> +	unsigned int i2a_recv0;
> +	unsigned int i2a_recv1;
> +
> +	bool has_irq_controls;
> +	unsigned int irq_enable;
> +	unsigned int irq_ack;
> +	unsigned int irq_bit_recv_not_empty;
> +	unsigned int irq_bit_send_empty;
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
> +	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
> +	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
> +
> +	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
> +	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
> +	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
> +
> +	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
> +	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
> +	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
> +
> +	.has_irq_controls = false,
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
> +	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
> +	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
> +
> +	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
> +	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
> +	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
> +
> +	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
> +	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
> +	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
> +
> +	.has_irq_controls = true,
> +	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
> +	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
> +	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
> +	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
> +};
> +
> +static const struct of_device_id apple_mbox_of_match[] = {
> +	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
> +	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
> +
> +struct apple_mbox {
> +	void __iomem *regs;
> +	const struct apple_mbox_hw *hw;
> +
> +	int irq_recv_not_empty;
> +	int irq_send_empty;
> +
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +
> +	struct mbox_chan chan;
> +
> +	struct device *dev;
> +	struct mbox_controller controller;
> +};
> +
> +static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
> +{
> +	u32 mbox_ctrl =
> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
> +
> +	return !(mbox_ctrl & apple_mbox->hw->control_full);
> +}
> +
> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
> +			      struct apple_mbox_msg *msg)
> +{
> +	if (!apple_mbox_hw_can_send(apple_mbox))
> +		return -EBUSY;
> +
> +	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +	/*
> +	 * This message may be related to a shared memory buffer and we must
> +	 * ensure all previous writes to normal memory are visible before
> +	 * submitting it.
> +	 */
> +	dma_wmb();
> +
> +	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
> +	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
> +		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
> +
> +	return 0;
> +}
> +
> +static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
> +{
> +	u32 mbox_ctrl =
> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
> +
> +	return !(mbox_ctrl & apple_mbox->hw->control_empty);
> +}
> +
> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
> +			      struct apple_mbox_msg *msg)
> +{
> +	if (!apple_mbox_hw_can_recv(apple_mbox))
> +		return -ENOMSG;
> +
> +	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
> +	msg->msg1 = FIELD_GET(
> +		APPLE_MBOX_MSG1_MSG,
> +		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
> +
> +	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +	/*
> +	 * This message may be related to a shared memory buffer and we must
> +	 * ensure any following reads from normal memory only happen after
> +	 * having read this message.
> +	 */
> +	dma_rmb();
> +
> +	return 0;
> +}
> +
> +static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +	struct apple_mbox_msg *msg = data;
> +	int ret;
> +
> +	ret = apple_mbox_hw_send(apple_mbox, msg);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The interrupt is level triggered and will keep firing as long as the
> +	 * FIFO is empty. It will also keep firing if the FIFO was empty
> +	 * at any point in the past until it has been acknowledged at the
> +	 * mailbox level. By acknowledging it here we can ensure that we will
> +	 * only get the interrupt once the FIFO has been cleared again.
> +	 * If the FIFO is already empty before the ack it will fire again
> +	 * immediately after the ack.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
> +	}
> +	enable_irq(apple_mbox->irq_send_empty);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
> +{
> +	struct apple_mbox *apple_mbox = data;
> +
> +	/*
> +	 * We don't need to acknowledge the interrupt at the mailbox level
> +	 * here even if supported by the hardware. It will keep firing but that
> +	 * doesn't matter since it's disabled at the main interrupt controller.
> +	 * apple_mbox_chan_send_data will acknowledge it before enabling
> +	 * it at the main controller again.
> +	 */
> +	disable_irq_nosync(apple_mbox->irq_send_empty);
> +	mbox_chan_txdone(&apple_mbox->chan, 0);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
> +{
> +	struct apple_mbox *apple_mbox = data;
> +	struct apple_mbox_msg msg;
> +
> +	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
> +		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
> +
> +	/*
> +	 * The interrupt will keep firing even if there are no more messages
> +	 * unless we also acknowledge it at the mailbox level here.
> +	 * There's no race if a message comes in between the check in the while
> +	 * loop above and the ack below: If a new messages arrives inbetween
> +	 * those two the interrupt will just fire again immediately after the
> +	 * ack since it's level triggered.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
> +					     const struct of_phandle_args *spec)
> +{
> +	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
> +
> +	if (spec->args_count != 0)
> +		return ERR_PTR(-EINVAL);
> +	if (apple_mbox->chan.con_priv)
> +		return ERR_PTR(-EBUSY);
> +
> +	apple_mbox->chan.con_priv = apple_mbox;
> +	return &apple_mbox->chan;
> +}
> +
> +static int apple_mbox_chan_startup(struct mbox_chan *chan)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +
> +	/*
> +	 * Only some variants of this mailbox HW provide interrupt control
> +	 * at the mailbox level. We therefore need to handle enabling/disabling
> +	 * interrupts at the main interrupt controller anyway for hardware that
> +	 * doesn't. Just always keep the interrupts we care about enabled at
> +	 * the mailbox level so that both hardware revisions behave almost
> +	 * the same.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
> +				       apple_mbox->hw->irq_bit_send_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_enable);
> +	}
> +
> +	enable_irq(apple_mbox->irq_recv_not_empty);
> +	return 0;
> +}
> +
> +static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +
> +	disable_irq(apple_mbox->irq_recv_not_empty);
> +}
> +
> +static const struct mbox_chan_ops apple_mbox_ops = {
> +	.send_data = apple_mbox_chan_send_data,
> +	.startup = apple_mbox_chan_startup,
> +	.shutdown = apple_mbox_chan_shutdown,
> +};
> +
> +static int apple_mbox_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	const struct of_device_id *match;
> +	char *irqname;
> +	struct apple_mbox *mbox;
> +	struct device *dev = &pdev->dev;
> +
> +	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
> +	if (!match)
> +		return -EINVAL;
> +	if (!match->data)
> +		return -EINVAL;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, mbox);
> +
> +	mbox->dev = dev;
> +	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(mbox->regs))
> +		return PTR_ERR(mbox->regs);
> +
> +	mbox->hw = match->data;
> +	mbox->irq_recv_not_empty =
> +		platform_get_irq_byname(pdev, "recv-not-empty");
> +	if (mbox->irq_recv_not_empty < 0)
> +		return -ENODEV;
> +
> +	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
> +	if (mbox->irq_send_empty < 0)
> +		return -ENODEV;
> +
> +	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
> +	if (ret < 0)
> +		return ret;
> +	mbox->num_clks = ret;
> +
> +	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
> +	if (ret)
> +		return ret;

Given that we now realized that the clocks are really power domains,
the clock code should probably be dropped.  Makes the driver smaller
since power domains are handle automatically by the core device code.

> +
> +	mbox->controller.dev = mbox->dev;
> +	mbox->controller.num_chans = 1;
> +	mbox->controller.chans = &mbox->chan;
> +	mbox->controller.ops = &apple_mbox_ops;
> +	mbox->controller.of_xlate = &apple_mbox_of_xlate;
> +	mbox->controller.txdone_irq = true;
> +
> +	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-recv", dev_name(dev));
> +	if (!irqname) {
> +		ret = -ENOMEM;
> +		goto err_clk_disable;
> +	}
> +	ret = devm_request_threaded_irq(dev, mbox->irq_recv_not_empty, NULL,
> +					apple_mbox_recv_irq,
> +					IRQF_NO_AUTOEN | IRQF_ONESHOT, irqname,
> +					mbox);
> +	if (ret)
> +		goto err_clk_disable;
> +
> +	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-send", dev_name(dev));
> +	if (!irqname) {
> +		ret = -ENOMEM;
> +		goto err_clk_disable;
> +	}
> +	ret = devm_request_irq(dev, mbox->irq_send_empty,
> +			       apple_mbox_send_empty_irq, IRQF_NO_AUTOEN,
> +			       irqname, mbox);
> +	if (ret)
> +		goto err_clk_disable;
> +
> +	ret = devm_mbox_controller_register(dev, &mbox->controller);
> +	if (ret)
> +		goto err_clk_disable;
> +	return ret;
> +
> +err_clk_disable:
> +	clk_bulk_disable_unprepare(mbox->num_clks, mbox->clks);
> +	return ret;
> +}
> +
> +static int apple_mbox_remove(struct platform_device *pdev)
> +{
> +	struct apple_mbox *apple_mbox = dev_get_drvdata(&pdev->dev);
> +
> +	clk_bulk_disable_unprepare(apple_mbox->num_clks, apple_mbox->clks);
> +	return 0;
> +}
> +
> +static struct platform_driver apple_mbox_driver = {
> +	.driver = {
> +		.name = "apple-mailbox",
> +		.of_match_table = apple_mbox_of_match,
> +	},
> +	.probe = apple_mbox_probe,
> +	.remove = apple_mbox_remove,
> +};
> +module_platform_driver(apple_mbox_driver);
> +
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_AUTHOR("Sven Peter <sven@svenpeter.dev>");
> +MODULE_DESCRIPTION("Apple Mailbox driver");
> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
> new file mode 100644
> index 000000000000..c455e0f9c73b
> --- /dev/null
> +++ b/include/linux/apple-mailbox.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> +/*
> + * Apple mailbox message format
> + *
> + * Copyright (C) 2021 The Asahi Linux Contributors
> + */
> +
> +#ifndef _LINUX_APPLE_MAILBOX_H_
> +#define _LINUX_APPLE_MAILBOX_H_
> +
> +#include <linux/types.h>
> +
> +struct apple_mbox_msg {
> +	u64 msg0;
> +	u32 msg1;
> +};
> +
> +#endif
> -- 
> 2.25.1
> 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-10-17 11:25     ` Mark Kettenis
  0 siblings, 0 replies; 24+ messages in thread
From: Mark Kettenis @ 2021-10-17 11:25 UTC (permalink / raw)
  To: Sven Peter
  Cc: jassisinghbrar, sven, robh+dt, marcan, alyssa, mohamed.mediouni,
	stan, devicetree, linux-arm-kernel, linux-kernel

> From: Sven Peter <sven@svenpeter.dev>
> Date: Thu, 16 Sep 2021 17:49:11 +0200

Hi Sven,

> Apple SoCs such as the M1 come with various co-processors. Mailboxes
> are used to communicate with those. This driver adds support for
> two variants of those mailboxes.
> 
> Signed-off-by: Sven Peter <sven@svenpeter.dev>
> ---
>  MAINTAINERS                     |   2 +
>  drivers/mailbox/Kconfig         |  12 +
>  drivers/mailbox/Makefile        |   2 +
>  drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
>  include/linux/apple-mailbox.h   |  18 ++
>  5 files changed, 465 insertions(+)
>  create mode 100644 drivers/mailbox/apple-mailbox.c
>  create mode 100644 include/linux/apple-mailbox.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 47de27282c98..cf0500bbea5b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1724,8 +1724,10 @@ 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
> +F:	drivers/mailbox/apple-mailbox.c
>  F:	include/dt-bindings/interrupt-controller/apple-aic.h
>  F:	include/dt-bindings/pinctrl/apple.h
> +F:	include/linux/apple-mailbox.h
>  
>  ARM/ARTPEC MACHINE SUPPORT
>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
> index c9fc06c7e685..d9cd3606040e 100644
> --- a/drivers/mailbox/Kconfig
> +++ b/drivers/mailbox/Kconfig
> @@ -8,6 +8,18 @@ menuconfig MAILBOX
>  
>  if MAILBOX
>  
> +config APPLE_MAILBOX
> +	tristate "Apple Mailbox driver"
> +	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
> +	default ARCH_APPLE
> +	help
> +	  Apple SoCs have various co-processors required for certain
> +	  peripherals to work (NVMe, display controller, etc.). This
> +	  driver adds support for the mailbox controller used to
> +	  communicate with those.
> +
> +	  Say Y here if you have a Apple SoC.
> +
>  config ARM_MHU
>  	tristate "ARM MHU Mailbox"
>  	depends on ARM_AMBA
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index c2089f04887e..338cc05e5431 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
>  obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
>  
>  obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
> +
> +obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
> diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
> new file mode 100644
> index 000000000000..2622a6369075
> --- /dev/null
> +++ b/drivers/mailbox/apple-mailbox.c
> @@ -0,0 +1,431 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +/*
> + * Apple mailbox driver
> + *
> + * Copyright (C) 2021 The Asahi Linux Contributors
> + *
> + * This driver adds support for two mailbox variants (called ASC and M3 by
> + * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
> + * exchange 64+32 bit messages between the main CPU and a co-processor.
> + * Various coprocessors implement different IPC protocols based on these simple
> + * messages and shared memory buffers.
> + *
> + * Both the main CPU and the co-processor see the same set of registers but
> + * the first FIFO (A2I) is always used to transfer messages from the application
> + * processor (us) to the I/O processor and the second one (I2A) for the
> + * other direction.
> + */
> +
> +#include <linux/apple-mailbox.h>
> +#include <linux/clk.h>
> +#include <linux/device.h>
> +#include <linux/gfp.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/mailbox_controller.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/types.h>
> +
> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
> +
> +#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
> +#define APPLE_ASC_MBOX_A2I_SEND0   0x800
> +#define APPLE_ASC_MBOX_A2I_SEND1   0x808
> +#define APPLE_ASC_MBOX_A2I_RECV0   0x810
> +#define APPLE_ASC_MBOX_A2I_RECV1   0x818
> +
> +#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
> +#define APPLE_ASC_MBOX_I2A_SEND0   0x820
> +#define APPLE_ASC_MBOX_I2A_SEND1   0x828
> +#define APPLE_ASC_MBOX_I2A_RECV0   0x830
> +#define APPLE_ASC_MBOX_I2A_RECV1   0x838
> +
> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
> +
> +#define APPLE_M3_MBOX_A2I_CONTROL 0x50
> +#define APPLE_M3_MBOX_A2I_SEND0	  0x60
> +#define APPLE_M3_MBOX_A2I_SEND1	  0x68
> +#define APPLE_M3_MBOX_A2I_RECV0	  0x70
> +#define APPLE_M3_MBOX_A2I_RECV1	  0x78
> +
> +#define APPLE_M3_MBOX_I2A_CONTROL 0x80
> +#define APPLE_M3_MBOX_I2A_SEND0	  0x90
> +#define APPLE_M3_MBOX_I2A_SEND1	  0x98
> +#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
> +#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
> +
> +#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
> +#define APPLE_M3_MBOX_IRQ_ACK		0x4c
> +#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
> +#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
> +#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
> +#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
> +
> +#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
> +#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
> +#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
> +#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
> +#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
> +
> +struct apple_mbox_hw {
> +	unsigned int control_full;
> +	unsigned int control_empty;
> +
> +	unsigned int a2i_control;
> +	unsigned int a2i_send0;
> +	unsigned int a2i_send1;
> +
> +	unsigned int i2a_control;
> +	unsigned int i2a_recv0;
> +	unsigned int i2a_recv1;
> +
> +	bool has_irq_controls;
> +	unsigned int irq_enable;
> +	unsigned int irq_ack;
> +	unsigned int irq_bit_recv_not_empty;
> +	unsigned int irq_bit_send_empty;
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
> +	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
> +	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
> +
> +	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
> +	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
> +	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
> +
> +	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
> +	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
> +	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
> +
> +	.has_irq_controls = false,
> +};
> +
> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
> +	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
> +	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
> +
> +	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
> +	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
> +	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
> +
> +	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
> +	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
> +	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
> +
> +	.has_irq_controls = true,
> +	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
> +	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
> +	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
> +	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
> +};
> +
> +static const struct of_device_id apple_mbox_of_match[] = {
> +	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
> +	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
> +
> +struct apple_mbox {
> +	void __iomem *regs;
> +	const struct apple_mbox_hw *hw;
> +
> +	int irq_recv_not_empty;
> +	int irq_send_empty;
> +
> +	struct clk_bulk_data *clks;
> +	int num_clks;
> +
> +	struct mbox_chan chan;
> +
> +	struct device *dev;
> +	struct mbox_controller controller;
> +};
> +
> +static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
> +{
> +	u32 mbox_ctrl =
> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
> +
> +	return !(mbox_ctrl & apple_mbox->hw->control_full);
> +}
> +
> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
> +			      struct apple_mbox_msg *msg)
> +{
> +	if (!apple_mbox_hw_can_send(apple_mbox))
> +		return -EBUSY;
> +
> +	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +	/*
> +	 * This message may be related to a shared memory buffer and we must
> +	 * ensure all previous writes to normal memory are visible before
> +	 * submitting it.
> +	 */
> +	dma_wmb();
> +
> +	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
> +	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
> +		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
> +
> +	return 0;
> +}
> +
> +static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
> +{
> +	u32 mbox_ctrl =
> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
> +
> +	return !(mbox_ctrl & apple_mbox->hw->control_empty);
> +}
> +
> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
> +			      struct apple_mbox_msg *msg)
> +{
> +	if (!apple_mbox_hw_can_recv(apple_mbox))
> +		return -ENOMSG;
> +
> +	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
> +	msg->msg1 = FIELD_GET(
> +		APPLE_MBOX_MSG1_MSG,
> +		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
> +
> +	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
> +
> +	/*
> +	 * This message may be related to a shared memory buffer and we must
> +	 * ensure any following reads from normal memory only happen after
> +	 * having read this message.
> +	 */
> +	dma_rmb();
> +
> +	return 0;
> +}
> +
> +static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +	struct apple_mbox_msg *msg = data;
> +	int ret;
> +
> +	ret = apple_mbox_hw_send(apple_mbox, msg);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The interrupt is level triggered and will keep firing as long as the
> +	 * FIFO is empty. It will also keep firing if the FIFO was empty
> +	 * at any point in the past until it has been acknowledged at the
> +	 * mailbox level. By acknowledging it here we can ensure that we will
> +	 * only get the interrupt once the FIFO has been cleared again.
> +	 * If the FIFO is already empty before the ack it will fire again
> +	 * immediately after the ack.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
> +	}
> +	enable_irq(apple_mbox->irq_send_empty);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
> +{
> +	struct apple_mbox *apple_mbox = data;
> +
> +	/*
> +	 * We don't need to acknowledge the interrupt at the mailbox level
> +	 * here even if supported by the hardware. It will keep firing but that
> +	 * doesn't matter since it's disabled at the main interrupt controller.
> +	 * apple_mbox_chan_send_data will acknowledge it before enabling
> +	 * it at the main controller again.
> +	 */
> +	disable_irq_nosync(apple_mbox->irq_send_empty);
> +	mbox_chan_txdone(&apple_mbox->chan, 0);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
> +{
> +	struct apple_mbox *apple_mbox = data;
> +	struct apple_mbox_msg msg;
> +
> +	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
> +		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
> +
> +	/*
> +	 * The interrupt will keep firing even if there are no more messages
> +	 * unless we also acknowledge it at the mailbox level here.
> +	 * There's no race if a message comes in between the check in the while
> +	 * loop above and the ack below: If a new messages arrives inbetween
> +	 * those two the interrupt will just fire again immediately after the
> +	 * ack since it's level triggered.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
> +					     const struct of_phandle_args *spec)
> +{
> +	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
> +
> +	if (spec->args_count != 0)
> +		return ERR_PTR(-EINVAL);
> +	if (apple_mbox->chan.con_priv)
> +		return ERR_PTR(-EBUSY);
> +
> +	apple_mbox->chan.con_priv = apple_mbox;
> +	return &apple_mbox->chan;
> +}
> +
> +static int apple_mbox_chan_startup(struct mbox_chan *chan)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +
> +	/*
> +	 * Only some variants of this mailbox HW provide interrupt control
> +	 * at the mailbox level. We therefore need to handle enabling/disabling
> +	 * interrupts at the main interrupt controller anyway for hardware that
> +	 * doesn't. Just always keep the interrupts we care about enabled at
> +	 * the mailbox level so that both hardware revisions behave almost
> +	 * the same.
> +	 */
> +	if (apple_mbox->hw->has_irq_controls) {
> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
> +				       apple_mbox->hw->irq_bit_send_empty,
> +			       apple_mbox->regs + apple_mbox->hw->irq_enable);
> +	}
> +
> +	enable_irq(apple_mbox->irq_recv_not_empty);
> +	return 0;
> +}
> +
> +static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
> +{
> +	struct apple_mbox *apple_mbox = chan->con_priv;
> +
> +	disable_irq(apple_mbox->irq_recv_not_empty);
> +}
> +
> +static const struct mbox_chan_ops apple_mbox_ops = {
> +	.send_data = apple_mbox_chan_send_data,
> +	.startup = apple_mbox_chan_startup,
> +	.shutdown = apple_mbox_chan_shutdown,
> +};
> +
> +static int apple_mbox_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	const struct of_device_id *match;
> +	char *irqname;
> +	struct apple_mbox *mbox;
> +	struct device *dev = &pdev->dev;
> +
> +	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
> +	if (!match)
> +		return -EINVAL;
> +	if (!match->data)
> +		return -EINVAL;
> +
> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
> +	if (!mbox)
> +		return -ENOMEM;
> +	platform_set_drvdata(pdev, mbox);
> +
> +	mbox->dev = dev;
> +	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(mbox->regs))
> +		return PTR_ERR(mbox->regs);
> +
> +	mbox->hw = match->data;
> +	mbox->irq_recv_not_empty =
> +		platform_get_irq_byname(pdev, "recv-not-empty");
> +	if (mbox->irq_recv_not_empty < 0)
> +		return -ENODEV;
> +
> +	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
> +	if (mbox->irq_send_empty < 0)
> +		return -ENODEV;
> +
> +	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
> +	if (ret < 0)
> +		return ret;
> +	mbox->num_clks = ret;
> +
> +	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
> +	if (ret)
> +		return ret;

Given that we now realized that the clocks are really power domains,
the clock code should probably be dropped.  Makes the driver smaller
since power domains are handle automatically by the core device code.

> +
> +	mbox->controller.dev = mbox->dev;
> +	mbox->controller.num_chans = 1;
> +	mbox->controller.chans = &mbox->chan;
> +	mbox->controller.ops = &apple_mbox_ops;
> +	mbox->controller.of_xlate = &apple_mbox_of_xlate;
> +	mbox->controller.txdone_irq = true;
> +
> +	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-recv", dev_name(dev));
> +	if (!irqname) {
> +		ret = -ENOMEM;
> +		goto err_clk_disable;
> +	}
> +	ret = devm_request_threaded_irq(dev, mbox->irq_recv_not_empty, NULL,
> +					apple_mbox_recv_irq,
> +					IRQF_NO_AUTOEN | IRQF_ONESHOT, irqname,
> +					mbox);
> +	if (ret)
> +		goto err_clk_disable;
> +
> +	irqname = devm_kasprintf(dev, GFP_KERNEL, "%s-send", dev_name(dev));
> +	if (!irqname) {
> +		ret = -ENOMEM;
> +		goto err_clk_disable;
> +	}
> +	ret = devm_request_irq(dev, mbox->irq_send_empty,
> +			       apple_mbox_send_empty_irq, IRQF_NO_AUTOEN,
> +			       irqname, mbox);
> +	if (ret)
> +		goto err_clk_disable;
> +
> +	ret = devm_mbox_controller_register(dev, &mbox->controller);
> +	if (ret)
> +		goto err_clk_disable;
> +	return ret;
> +
> +err_clk_disable:
> +	clk_bulk_disable_unprepare(mbox->num_clks, mbox->clks);
> +	return ret;
> +}
> +
> +static int apple_mbox_remove(struct platform_device *pdev)
> +{
> +	struct apple_mbox *apple_mbox = dev_get_drvdata(&pdev->dev);
> +
> +	clk_bulk_disable_unprepare(apple_mbox->num_clks, apple_mbox->clks);
> +	return 0;
> +}
> +
> +static struct platform_driver apple_mbox_driver = {
> +	.driver = {
> +		.name = "apple-mailbox",
> +		.of_match_table = apple_mbox_of_match,
> +	},
> +	.probe = apple_mbox_probe,
> +	.remove = apple_mbox_remove,
> +};
> +module_platform_driver(apple_mbox_driver);
> +
> +MODULE_LICENSE("Dual MIT/GPL");
> +MODULE_AUTHOR("Sven Peter <sven@svenpeter.dev>");
> +MODULE_DESCRIPTION("Apple Mailbox driver");
> diff --git a/include/linux/apple-mailbox.h b/include/linux/apple-mailbox.h
> new file mode 100644
> index 000000000000..c455e0f9c73b
> --- /dev/null
> +++ b/include/linux/apple-mailbox.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> +/*
> + * Apple mailbox message format
> + *
> + * Copyright (C) 2021 The Asahi Linux Contributors
> + */
> +
> +#ifndef _LINUX_APPLE_MAILBOX_H_
> +#define _LINUX_APPLE_MAILBOX_H_
> +
> +#include <linux/types.h>
> +
> +struct apple_mbox_msg {
> +	u64 msg0;
> +	u32 msg1;
> +};
> +
> +#endif
> -- 
> 2.25.1
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
  2021-10-17 11:25     ` Mark Kettenis
@ 2021-10-17 11:27       ` Sven Peter
  -1 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-17 11:27 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Jassi Brar, Rob Herring, Hector Martin, Alyssa Rosenzweig,
	mohamed.mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel


Hi Mark,

On Sun, Oct 17, 2021, at 13:25, Mark Kettenis wrote:
>> From: Sven Peter <sven@svenpeter.dev>
>> Date: Thu, 16 Sep 2021 17:49:11 +0200
>
> Hi Sven,
>
>> Apple SoCs such as the M1 come with various co-processors. Mailboxes
>> are used to communicate with those. This driver adds support for
>> two variants of those mailboxes.
>> 
>> Signed-off-by: Sven Peter <sven@svenpeter.dev>
>> ---
>>  MAINTAINERS                     |   2 +
>>  drivers/mailbox/Kconfig         |  12 +
>>  drivers/mailbox/Makefile        |   2 +
>>  drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
>>  include/linux/apple-mailbox.h   |  18 ++
>>  5 files changed, 465 insertions(+)
>>  create mode 100644 drivers/mailbox/apple-mailbox.c
>>  create mode 100644 include/linux/apple-mailbox.h
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 47de27282c98..cf0500bbea5b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1724,8 +1724,10 @@ 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
>> +F:	drivers/mailbox/apple-mailbox.c
>>  F:	include/dt-bindings/interrupt-controller/apple-aic.h
>>  F:	include/dt-bindings/pinctrl/apple.h
>> +F:	include/linux/apple-mailbox.h
>>  
>>  ARM/ARTPEC MACHINE SUPPORT
>>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index c9fc06c7e685..d9cd3606040e 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -8,6 +8,18 @@ menuconfig MAILBOX
>>  
>>  if MAILBOX
>>  
>> +config APPLE_MAILBOX
>> +	tristate "Apple Mailbox driver"
>> +	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
>> +	default ARCH_APPLE
>> +	help
>> +	  Apple SoCs have various co-processors required for certain
>> +	  peripherals to work (NVMe, display controller, etc.). This
>> +	  driver adds support for the mailbox controller used to
>> +	  communicate with those.
>> +
>> +	  Say Y here if you have a Apple SoC.
>> +
>>  config ARM_MHU
>>  	tristate "ARM MHU Mailbox"
>>  	depends on ARM_AMBA
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index c2089f04887e..338cc05e5431 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
>>  obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
>>  
>>  obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
>> +
>> +obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
>> diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
>> new file mode 100644
>> index 000000000000..2622a6369075
>> --- /dev/null
>> +++ b/drivers/mailbox/apple-mailbox.c
>> @@ -0,0 +1,431 @@
>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>> +/*
>> + * Apple mailbox driver
>> + *
>> + * Copyright (C) 2021 The Asahi Linux Contributors
>> + *
>> + * This driver adds support for two mailbox variants (called ASC and M3 by
>> + * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
>> + * exchange 64+32 bit messages between the main CPU and a co-processor.
>> + * Various coprocessors implement different IPC protocols based on these simple
>> + * messages and shared memory buffers.
>> + *
>> + * Both the main CPU and the co-processor see the same set of registers but
>> + * the first FIFO (A2I) is always used to transfer messages from the application
>> + * processor (us) to the I/O processor and the second one (I2A) for the
>> + * other direction.
>> + */
>> +
>> +#include <linux/apple-mailbox.h>
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/gfp.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/mailbox_controller.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/types.h>
>> +
>> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
>> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
>> +
>> +#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
>> +#define APPLE_ASC_MBOX_A2I_SEND0   0x800
>> +#define APPLE_ASC_MBOX_A2I_SEND1   0x808
>> +#define APPLE_ASC_MBOX_A2I_RECV0   0x810
>> +#define APPLE_ASC_MBOX_A2I_RECV1   0x818
>> +
>> +#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
>> +#define APPLE_ASC_MBOX_I2A_SEND0   0x820
>> +#define APPLE_ASC_MBOX_I2A_SEND1   0x828
>> +#define APPLE_ASC_MBOX_I2A_RECV0   0x830
>> +#define APPLE_ASC_MBOX_I2A_RECV1   0x838
>> +
>> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
>> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
>> +
>> +#define APPLE_M3_MBOX_A2I_CONTROL 0x50
>> +#define APPLE_M3_MBOX_A2I_SEND0	  0x60
>> +#define APPLE_M3_MBOX_A2I_SEND1	  0x68
>> +#define APPLE_M3_MBOX_A2I_RECV0	  0x70
>> +#define APPLE_M3_MBOX_A2I_RECV1	  0x78
>> +
>> +#define APPLE_M3_MBOX_I2A_CONTROL 0x80
>> +#define APPLE_M3_MBOX_I2A_SEND0	  0x90
>> +#define APPLE_M3_MBOX_I2A_SEND1	  0x98
>> +#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
>> +#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
>> +
>> +#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
>> +#define APPLE_M3_MBOX_IRQ_ACK		0x4c
>> +#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
>> +#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
>> +#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
>> +#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
>> +
>> +#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
>> +#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
>> +#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
>> +#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
>> +#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
>> +
>> +struct apple_mbox_hw {
>> +	unsigned int control_full;
>> +	unsigned int control_empty;
>> +
>> +	unsigned int a2i_control;
>> +	unsigned int a2i_send0;
>> +	unsigned int a2i_send1;
>> +
>> +	unsigned int i2a_control;
>> +	unsigned int i2a_recv0;
>> +	unsigned int i2a_recv1;
>> +
>> +	bool has_irq_controls;
>> +	unsigned int irq_enable;
>> +	unsigned int irq_ack;
>> +	unsigned int irq_bit_recv_not_empty;
>> +	unsigned int irq_bit_send_empty;
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
>> +	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
>> +	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
>> +
>> +	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
>> +	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
>> +	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
>> +
>> +	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
>> +	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
>> +	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
>> +
>> +	.has_irq_controls = false,
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
>> +	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
>> +	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
>> +
>> +	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
>> +	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
>> +	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
>> +
>> +	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
>> +	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
>> +	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
>> +
>> +	.has_irq_controls = true,
>> +	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
>> +	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
>> +	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
>> +	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
>> +};
>> +
>> +static const struct of_device_id apple_mbox_of_match[] = {
>> +	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
>> +	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
>> +
>> +struct apple_mbox {
>> +	void __iomem *regs;
>> +	const struct apple_mbox_hw *hw;
>> +
>> +	int irq_recv_not_empty;
>> +	int irq_send_empty;
>> +
>> +	struct clk_bulk_data *clks;
>> +	int num_clks;
>> +
>> +	struct mbox_chan chan;
>> +
>> +	struct device *dev;
>> +	struct mbox_controller controller;
>> +};
>> +
>> +static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
>> +{
>> +	u32 mbox_ctrl =
>> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
>> +
>> +	return !(mbox_ctrl & apple_mbox->hw->control_full);
>> +}
>> +
>> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
>> +			      struct apple_mbox_msg *msg)
>> +{
>> +	if (!apple_mbox_hw_can_send(apple_mbox))
>> +		return -EBUSY;
>> +
>> +	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +	/*
>> +	 * This message may be related to a shared memory buffer and we must
>> +	 * ensure all previous writes to normal memory are visible before
>> +	 * submitting it.
>> +	 */
>> +	dma_wmb();
>> +
>> +	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
>> +	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
>> +		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
>> +
>> +	return 0;
>> +}
>> +
>> +static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
>> +{
>> +	u32 mbox_ctrl =
>> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
>> +
>> +	return !(mbox_ctrl & apple_mbox->hw->control_empty);
>> +}
>> +
>> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
>> +			      struct apple_mbox_msg *msg)
>> +{
>> +	if (!apple_mbox_hw_can_recv(apple_mbox))
>> +		return -ENOMSG;
>> +
>> +	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
>> +	msg->msg1 = FIELD_GET(
>> +		APPLE_MBOX_MSG1_MSG,
>> +		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
>> +
>> +	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +	/*
>> +	 * This message may be related to a shared memory buffer and we must
>> +	 * ensure any following reads from normal memory only happen after
>> +	 * having read this message.
>> +	 */
>> +	dma_rmb();
>> +
>> +	return 0;
>> +}
>> +
>> +static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +	struct apple_mbox_msg *msg = data;
>> +	int ret;
>> +
>> +	ret = apple_mbox_hw_send(apple_mbox, msg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * The interrupt is level triggered and will keep firing as long as the
>> +	 * FIFO is empty. It will also keep firing if the FIFO was empty
>> +	 * at any point in the past until it has been acknowledged at the
>> +	 * mailbox level. By acknowledging it here we can ensure that we will
>> +	 * only get the interrupt once the FIFO has been cleared again.
>> +	 * If the FIFO is already empty before the ack it will fire again
>> +	 * immediately after the ack.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
>> +	}
>> +	enable_irq(apple_mbox->irq_send_empty);
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = data;
>> +
>> +	/*
>> +	 * We don't need to acknowledge the interrupt at the mailbox level
>> +	 * here even if supported by the hardware. It will keep firing but that
>> +	 * doesn't matter since it's disabled at the main interrupt controller.
>> +	 * apple_mbox_chan_send_data will acknowledge it before enabling
>> +	 * it at the main controller again.
>> +	 */
>> +	disable_irq_nosync(apple_mbox->irq_send_empty);
>> +	mbox_chan_txdone(&apple_mbox->chan, 0);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = data;
>> +	struct apple_mbox_msg msg;
>> +
>> +	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
>> +		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
>> +
>> +	/*
>> +	 * The interrupt will keep firing even if there are no more messages
>> +	 * unless we also acknowledge it at the mailbox level here.
>> +	 * There's no race if a message comes in between the check in the while
>> +	 * loop above and the ack below: If a new messages arrives inbetween
>> +	 * those two the interrupt will just fire again immediately after the
>> +	 * ack since it's level triggered.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
>> +					     const struct of_phandle_args *spec)
>> +{
>> +	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
>> +
>> +	if (spec->args_count != 0)
>> +		return ERR_PTR(-EINVAL);
>> +	if (apple_mbox->chan.con_priv)
>> +		return ERR_PTR(-EBUSY);
>> +
>> +	apple_mbox->chan.con_priv = apple_mbox;
>> +	return &apple_mbox->chan;
>> +}
>> +
>> +static int apple_mbox_chan_startup(struct mbox_chan *chan)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +
>> +	/*
>> +	 * Only some variants of this mailbox HW provide interrupt control
>> +	 * at the mailbox level. We therefore need to handle enabling/disabling
>> +	 * interrupts at the main interrupt controller anyway for hardware that
>> +	 * doesn't. Just always keep the interrupts we care about enabled at
>> +	 * the mailbox level so that both hardware revisions behave almost
>> +	 * the same.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
>> +				       apple_mbox->hw->irq_bit_send_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_enable);
>> +	}
>> +
>> +	enable_irq(apple_mbox->irq_recv_not_empty);
>> +	return 0;
>> +}
>> +
>> +static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +
>> +	disable_irq(apple_mbox->irq_recv_not_empty);
>> +}
>> +
>> +static const struct mbox_chan_ops apple_mbox_ops = {
>> +	.send_data = apple_mbox_chan_send_data,
>> +	.startup = apple_mbox_chan_startup,
>> +	.shutdown = apple_mbox_chan_shutdown,
>> +};
>> +
>> +static int apple_mbox_probe(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	const struct of_device_id *match;
>> +	char *irqname;
>> +	struct apple_mbox *mbox;
>> +	struct device *dev = &pdev->dev;
>> +
>> +	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
>> +	if (!match)
>> +		return -EINVAL;
>> +	if (!match->data)
>> +		return -EINVAL;
>> +
>> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
>> +	if (!mbox)
>> +		return -ENOMEM;
>> +	platform_set_drvdata(pdev, mbox);
>> +
>> +	mbox->dev = dev;
>> +	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(mbox->regs))
>> +		return PTR_ERR(mbox->regs);
>> +
>> +	mbox->hw = match->data;
>> +	mbox->irq_recv_not_empty =
>> +		platform_get_irq_byname(pdev, "recv-not-empty");
>> +	if (mbox->irq_recv_not_empty < 0)
>> +		return -ENODEV;
>> +
>> +	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
>> +	if (mbox->irq_send_empty < 0)
>> +		return -ENODEV;
>> +
>> +	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
>> +	if (ret < 0)
>> +		return ret;
>> +	mbox->num_clks = ret;
>> +
>> +	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
>> +	if (ret)
>> +		return ret;
>
> Given that we now realized that the clocks are really power domains,
> the clock code should probably be dropped.  Makes the driver smaller
> since power domains are handle automatically by the core device code.
>

Yup, thanks for pointing that out! I'll drop it from the code
and the binding for v3.



Sven

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes
@ 2021-10-17 11:27       ` Sven Peter
  0 siblings, 0 replies; 24+ messages in thread
From: Sven Peter @ 2021-10-17 11:27 UTC (permalink / raw)
  To: Mark Kettenis
  Cc: Jassi Brar, Rob Herring, Hector Martin, Alyssa Rosenzweig,
	mohamed.mediouni, Stan Skowronek, devicetree, linux-arm-kernel,
	linux-kernel


Hi Mark,

On Sun, Oct 17, 2021, at 13:25, Mark Kettenis wrote:
>> From: Sven Peter <sven@svenpeter.dev>
>> Date: Thu, 16 Sep 2021 17:49:11 +0200
>
> Hi Sven,
>
>> Apple SoCs such as the M1 come with various co-processors. Mailboxes
>> are used to communicate with those. This driver adds support for
>> two variants of those mailboxes.
>> 
>> Signed-off-by: Sven Peter <sven@svenpeter.dev>
>> ---
>>  MAINTAINERS                     |   2 +
>>  drivers/mailbox/Kconfig         |  12 +
>>  drivers/mailbox/Makefile        |   2 +
>>  drivers/mailbox/apple-mailbox.c | 431 ++++++++++++++++++++++++++++++++
>>  include/linux/apple-mailbox.h   |  18 ++
>>  5 files changed, 465 insertions(+)
>>  create mode 100644 drivers/mailbox/apple-mailbox.c
>>  create mode 100644 include/linux/apple-mailbox.h
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 47de27282c98..cf0500bbea5b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1724,8 +1724,10 @@ 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
>> +F:	drivers/mailbox/apple-mailbox.c
>>  F:	include/dt-bindings/interrupt-controller/apple-aic.h
>>  F:	include/dt-bindings/pinctrl/apple.h
>> +F:	include/linux/apple-mailbox.h
>>  
>>  ARM/ARTPEC MACHINE SUPPORT
>>  M:	Jesper Nilsson <jesper.nilsson@axis.com>
>> diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
>> index c9fc06c7e685..d9cd3606040e 100644
>> --- a/drivers/mailbox/Kconfig
>> +++ b/drivers/mailbox/Kconfig
>> @@ -8,6 +8,18 @@ menuconfig MAILBOX
>>  
>>  if MAILBOX
>>  
>> +config APPLE_MAILBOX
>> +	tristate "Apple Mailbox driver"
>> +	depends on ARCH_APPLE || (ARM64 && COMPILE_TEST)
>> +	default ARCH_APPLE
>> +	help
>> +	  Apple SoCs have various co-processors required for certain
>> +	  peripherals to work (NVMe, display controller, etc.). This
>> +	  driver adds support for the mailbox controller used to
>> +	  communicate with those.
>> +
>> +	  Say Y here if you have a Apple SoC.
>> +
>>  config ARM_MHU
>>  	tristate "ARM MHU Mailbox"
>>  	depends on ARM_AMBA
>> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
>> index c2089f04887e..338cc05e5431 100644
>> --- a/drivers/mailbox/Makefile
>> +++ b/drivers/mailbox/Makefile
>> @@ -58,3 +58,5 @@ obj-$(CONFIG_SUN6I_MSGBOX)	+= sun6i-msgbox.o
>>  obj-$(CONFIG_SPRD_MBOX)		+= sprd-mailbox.o
>>  
>>  obj-$(CONFIG_QCOM_IPCC)		+= qcom-ipcc.o
>> +
>> +obj-$(CONFIG_APPLE_MAILBOX)	+= apple-mailbox.o
>> diff --git a/drivers/mailbox/apple-mailbox.c b/drivers/mailbox/apple-mailbox.c
>> new file mode 100644
>> index 000000000000..2622a6369075
>> --- /dev/null
>> +++ b/drivers/mailbox/apple-mailbox.c
>> @@ -0,0 +1,431 @@
>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>> +/*
>> + * Apple mailbox driver
>> + *
>> + * Copyright (C) 2021 The Asahi Linux Contributors
>> + *
>> + * This driver adds support for two mailbox variants (called ASC and M3 by
>> + * Apple) found in Apple SoCs such as the M1. It consists of two FIFOs used to
>> + * exchange 64+32 bit messages between the main CPU and a co-processor.
>> + * Various coprocessors implement different IPC protocols based on these simple
>> + * messages and shared memory buffers.
>> + *
>> + * Both the main CPU and the co-processor see the same set of registers but
>> + * the first FIFO (A2I) is always used to transfer messages from the application
>> + * processor (us) to the I/O processor and the second one (I2A) for the
>> + * other direction.
>> + */
>> +
>> +#include <linux/apple-mailbox.h>
>> +#include <linux/clk.h>
>> +#include <linux/device.h>
>> +#include <linux/gfp.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/mailbox_controller.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/types.h>
>> +
>> +#define APPLE_ASC_MBOX_CONTROL_FULL  BIT(16)
>> +#define APPLE_ASC_MBOX_CONTROL_EMPTY BIT(17)
>> +
>> +#define APPLE_ASC_MBOX_A2I_CONTROL 0x110
>> +#define APPLE_ASC_MBOX_A2I_SEND0   0x800
>> +#define APPLE_ASC_MBOX_A2I_SEND1   0x808
>> +#define APPLE_ASC_MBOX_A2I_RECV0   0x810
>> +#define APPLE_ASC_MBOX_A2I_RECV1   0x818
>> +
>> +#define APPLE_ASC_MBOX_I2A_CONTROL 0x114
>> +#define APPLE_ASC_MBOX_I2A_SEND0   0x820
>> +#define APPLE_ASC_MBOX_I2A_SEND1   0x828
>> +#define APPLE_ASC_MBOX_I2A_RECV0   0x830
>> +#define APPLE_ASC_MBOX_I2A_RECV1   0x838
>> +
>> +#define APPLE_M3_MBOX_CONTROL_FULL  BIT(16)
>> +#define APPLE_M3_MBOX_CONTROL_EMPTY BIT(17)
>> +
>> +#define APPLE_M3_MBOX_A2I_CONTROL 0x50
>> +#define APPLE_M3_MBOX_A2I_SEND0	  0x60
>> +#define APPLE_M3_MBOX_A2I_SEND1	  0x68
>> +#define APPLE_M3_MBOX_A2I_RECV0	  0x70
>> +#define APPLE_M3_MBOX_A2I_RECV1	  0x78
>> +
>> +#define APPLE_M3_MBOX_I2A_CONTROL 0x80
>> +#define APPLE_M3_MBOX_I2A_SEND0	  0x90
>> +#define APPLE_M3_MBOX_I2A_SEND1	  0x98
>> +#define APPLE_M3_MBOX_I2A_RECV0	  0xa0
>> +#define APPLE_M3_MBOX_I2A_RECV1	  0xa8
>> +
>> +#define APPLE_M3_MBOX_IRQ_ENABLE	0x48
>> +#define APPLE_M3_MBOX_IRQ_ACK		0x4c
>> +#define APPLE_M3_MBOX_IRQ_A2I_EMPTY	BIT(0)
>> +#define APPLE_M3_MBOX_IRQ_A2I_NOT_EMPTY BIT(1)
>> +#define APPLE_M3_MBOX_IRQ_I2A_EMPTY	BIT(2)
>> +#define APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY BIT(3)
>> +
>> +#define APPLE_MBOX_MSG1_OUTCNT GENMASK(56, 52)
>> +#define APPLE_MBOX_MSG1_INCNT  GENMASK(51, 48)
>> +#define APPLE_MBOX_MSG1_OUTPTR GENMASK(47, 44)
>> +#define APPLE_MBOX_MSG1_INPTR  GENMASK(43, 40)
>> +#define APPLE_MBOX_MSG1_MSG    GENMASK(31, 0)
>> +
>> +struct apple_mbox_hw {
>> +	unsigned int control_full;
>> +	unsigned int control_empty;
>> +
>> +	unsigned int a2i_control;
>> +	unsigned int a2i_send0;
>> +	unsigned int a2i_send1;
>> +
>> +	unsigned int i2a_control;
>> +	unsigned int i2a_recv0;
>> +	unsigned int i2a_recv1;
>> +
>> +	bool has_irq_controls;
>> +	unsigned int irq_enable;
>> +	unsigned int irq_ack;
>> +	unsigned int irq_bit_recv_not_empty;
>> +	unsigned int irq_bit_send_empty;
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_asc_hw = {
>> +	.control_full = APPLE_ASC_MBOX_CONTROL_FULL,
>> +	.control_empty = APPLE_ASC_MBOX_CONTROL_EMPTY,
>> +
>> +	.a2i_control = APPLE_ASC_MBOX_A2I_CONTROL,
>> +	.a2i_send0 = APPLE_ASC_MBOX_A2I_SEND0,
>> +	.a2i_send1 = APPLE_ASC_MBOX_A2I_SEND1,
>> +
>> +	.i2a_control = APPLE_ASC_MBOX_I2A_CONTROL,
>> +	.i2a_recv0 = APPLE_ASC_MBOX_I2A_RECV0,
>> +	.i2a_recv1 = APPLE_ASC_MBOX_I2A_RECV1,
>> +
>> +	.has_irq_controls = false,
>> +};
>> +
>> +static const struct apple_mbox_hw apple_mbox_m3_hw = {
>> +	.control_full = APPLE_M3_MBOX_CONTROL_FULL,
>> +	.control_empty = APPLE_M3_MBOX_CONTROL_EMPTY,
>> +
>> +	.a2i_control = APPLE_M3_MBOX_A2I_CONTROL,
>> +	.a2i_send0 = APPLE_M3_MBOX_A2I_SEND0,
>> +	.a2i_send1 = APPLE_M3_MBOX_A2I_SEND1,
>> +
>> +	.i2a_control = APPLE_M3_MBOX_I2A_CONTROL,
>> +	.i2a_recv0 = APPLE_M3_MBOX_I2A_RECV0,
>> +	.i2a_recv1 = APPLE_M3_MBOX_I2A_RECV1,
>> +
>> +	.has_irq_controls = true,
>> +	.irq_enable = APPLE_M3_MBOX_IRQ_ENABLE,
>> +	.irq_ack = APPLE_M3_MBOX_IRQ_ACK,
>> +	.irq_bit_recv_not_empty = APPLE_M3_MBOX_IRQ_I2A_NOT_EMPTY,
>> +	.irq_bit_send_empty = APPLE_M3_MBOX_IRQ_A2I_EMPTY,
>> +};
>> +
>> +static const struct of_device_id apple_mbox_of_match[] = {
>> +	{ .compatible = "apple,t8103-asc-mailbox", .data = &apple_mbox_asc_hw },
>> +	{ .compatible = "apple,t8103-m3-mailbox", .data = &apple_mbox_m3_hw },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, apple_mbox_of_match);
>> +
>> +struct apple_mbox {
>> +	void __iomem *regs;
>> +	const struct apple_mbox_hw *hw;
>> +
>> +	int irq_recv_not_empty;
>> +	int irq_send_empty;
>> +
>> +	struct clk_bulk_data *clks;
>> +	int num_clks;
>> +
>> +	struct mbox_chan chan;
>> +
>> +	struct device *dev;
>> +	struct mbox_controller controller;
>> +};
>> +
>> +static bool apple_mbox_hw_can_send(struct apple_mbox *apple_mbox)
>> +{
>> +	u32 mbox_ctrl =
>> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->a2i_control);
>> +
>> +	return !(mbox_ctrl & apple_mbox->hw->control_full);
>> +}
>> +
>> +static int apple_mbox_hw_send(struct apple_mbox *apple_mbox,
>> +			      struct apple_mbox_msg *msg)
>> +{
>> +	if (!apple_mbox_hw_can_send(apple_mbox))
>> +		return -EBUSY;
>> +
>> +	dev_dbg(apple_mbox->dev, "> TX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +	/*
>> +	 * This message may be related to a shared memory buffer and we must
>> +	 * ensure all previous writes to normal memory are visible before
>> +	 * submitting it.
>> +	 */
>> +	dma_wmb();
>> +
>> +	writeq_relaxed(msg->msg0, apple_mbox->regs + apple_mbox->hw->a2i_send0);
>> +	writeq_relaxed(FIELD_PREP(APPLE_MBOX_MSG1_MSG, msg->msg1),
>> +		       apple_mbox->regs + apple_mbox->hw->a2i_send1);
>> +
>> +	return 0;
>> +}
>> +
>> +static bool apple_mbox_hw_can_recv(struct apple_mbox *apple_mbox)
>> +{
>> +	u32 mbox_ctrl =
>> +		readl_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_control);
>> +
>> +	return !(mbox_ctrl & apple_mbox->hw->control_empty);
>> +}
>> +
>> +static int apple_mbox_hw_recv(struct apple_mbox *apple_mbox,
>> +			      struct apple_mbox_msg *msg)
>> +{
>> +	if (!apple_mbox_hw_can_recv(apple_mbox))
>> +		return -ENOMSG;
>> +
>> +	msg->msg0 = readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv0);
>> +	msg->msg1 = FIELD_GET(
>> +		APPLE_MBOX_MSG1_MSG,
>> +		readq_relaxed(apple_mbox->regs + apple_mbox->hw->i2a_recv1));
>> +
>> +	dev_dbg(apple_mbox->dev, "< RX %016llx %08x\n", msg->msg0, msg->msg1);
>> +
>> +	/*
>> +	 * This message may be related to a shared memory buffer and we must
>> +	 * ensure any following reads from normal memory only happen after
>> +	 * having read this message.
>> +	 */
>> +	dma_rmb();
>> +
>> +	return 0;
>> +}
>> +
>> +static int apple_mbox_chan_send_data(struct mbox_chan *chan, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +	struct apple_mbox_msg *msg = data;
>> +	int ret;
>> +
>> +	ret = apple_mbox_hw_send(apple_mbox, msg);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/*
>> +	 * The interrupt is level triggered and will keep firing as long as the
>> +	 * FIFO is empty. It will also keep firing if the FIFO was empty
>> +	 * at any point in the past until it has been acknowledged at the
>> +	 * mailbox level. By acknowledging it here we can ensure that we will
>> +	 * only get the interrupt once the FIFO has been cleared again.
>> +	 * If the FIFO is already empty before the ack it will fire again
>> +	 * immediately after the ack.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_send_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
>> +	}
>> +	enable_irq(apple_mbox->irq_send_empty);
>> +
>> +	return 0;
>> +}
>> +
>> +static irqreturn_t apple_mbox_send_empty_irq(int irq, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = data;
>> +
>> +	/*
>> +	 * We don't need to acknowledge the interrupt at the mailbox level
>> +	 * here even if supported by the hardware. It will keep firing but that
>> +	 * doesn't matter since it's disabled at the main interrupt controller.
>> +	 * apple_mbox_chan_send_data will acknowledge it before enabling
>> +	 * it at the main controller again.
>> +	 */
>> +	disable_irq_nosync(apple_mbox->irq_send_empty);
>> +	mbox_chan_txdone(&apple_mbox->chan, 0);
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t apple_mbox_recv_irq(int irq, void *data)
>> +{
>> +	struct apple_mbox *apple_mbox = data;
>> +	struct apple_mbox_msg msg;
>> +
>> +	while (apple_mbox_hw_recv(apple_mbox, &msg) == 0)
>> +		mbox_chan_received_data(&apple_mbox->chan, (void *)&msg);
>> +
>> +	/*
>> +	 * The interrupt will keep firing even if there are no more messages
>> +	 * unless we also acknowledge it at the mailbox level here.
>> +	 * There's no race if a message comes in between the check in the while
>> +	 * loop above and the ack below: If a new messages arrives inbetween
>> +	 * those two the interrupt will just fire again immediately after the
>> +	 * ack since it's level triggered.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_ack);
>> +	}
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static struct mbox_chan *apple_mbox_of_xlate(struct mbox_controller *mbox,
>> +					     const struct of_phandle_args *spec)
>> +{
>> +	struct apple_mbox *apple_mbox = dev_get_drvdata(mbox->dev);
>> +
>> +	if (spec->args_count != 0)
>> +		return ERR_PTR(-EINVAL);
>> +	if (apple_mbox->chan.con_priv)
>> +		return ERR_PTR(-EBUSY);
>> +
>> +	apple_mbox->chan.con_priv = apple_mbox;
>> +	return &apple_mbox->chan;
>> +}
>> +
>> +static int apple_mbox_chan_startup(struct mbox_chan *chan)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +
>> +	/*
>> +	 * Only some variants of this mailbox HW provide interrupt control
>> +	 * at the mailbox level. We therefore need to handle enabling/disabling
>> +	 * interrupts at the main interrupt controller anyway for hardware that
>> +	 * doesn't. Just always keep the interrupts we care about enabled at
>> +	 * the mailbox level so that both hardware revisions behave almost
>> +	 * the same.
>> +	 */
>> +	if (apple_mbox->hw->has_irq_controls) {
>> +		writel_relaxed(apple_mbox->hw->irq_bit_recv_not_empty |
>> +				       apple_mbox->hw->irq_bit_send_empty,
>> +			       apple_mbox->regs + apple_mbox->hw->irq_enable);
>> +	}
>> +
>> +	enable_irq(apple_mbox->irq_recv_not_empty);
>> +	return 0;
>> +}
>> +
>> +static void apple_mbox_chan_shutdown(struct mbox_chan *chan)
>> +{
>> +	struct apple_mbox *apple_mbox = chan->con_priv;
>> +
>> +	disable_irq(apple_mbox->irq_recv_not_empty);
>> +}
>> +
>> +static const struct mbox_chan_ops apple_mbox_ops = {
>> +	.send_data = apple_mbox_chan_send_data,
>> +	.startup = apple_mbox_chan_startup,
>> +	.shutdown = apple_mbox_chan_shutdown,
>> +};
>> +
>> +static int apple_mbox_probe(struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	const struct of_device_id *match;
>> +	char *irqname;
>> +	struct apple_mbox *mbox;
>> +	struct device *dev = &pdev->dev;
>> +
>> +	match = of_match_node(apple_mbox_of_match, pdev->dev.of_node);
>> +	if (!match)
>> +		return -EINVAL;
>> +	if (!match->data)
>> +		return -EINVAL;
>> +
>> +	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
>> +	if (!mbox)
>> +		return -ENOMEM;
>> +	platform_set_drvdata(pdev, mbox);
>> +
>> +	mbox->dev = dev;
>> +	mbox->regs = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(mbox->regs))
>> +		return PTR_ERR(mbox->regs);
>> +
>> +	mbox->hw = match->data;
>> +	mbox->irq_recv_not_empty =
>> +		platform_get_irq_byname(pdev, "recv-not-empty");
>> +	if (mbox->irq_recv_not_empty < 0)
>> +		return -ENODEV;
>> +
>> +	mbox->irq_send_empty = platform_get_irq_byname(pdev, "send-empty");
>> +	if (mbox->irq_send_empty < 0)
>> +		return -ENODEV;
>> +
>> +	ret = devm_clk_bulk_get_all(dev, &mbox->clks);
>> +	if (ret < 0)
>> +		return ret;
>> +	mbox->num_clks = ret;
>> +
>> +	ret = clk_bulk_prepare_enable(mbox->num_clks, mbox->clks);
>> +	if (ret)
>> +		return ret;
>
> Given that we now realized that the clocks are really power domains,
> the clock code should probably be dropped.  Makes the driver smaller
> since power domains are handle automatically by the core device code.
>

Yup, thanks for pointing that out! I'll drop it from the code
and the binding for v3.



Sven

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-10-17 11:29 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-16 15:49 [PATCH v2 0/2] Apple Mailbox Controller support Sven Peter
2021-09-16 15:49 ` Sven Peter
2021-09-16 15:49 ` [PATCH v2 1/2] dt-bindings: mailbox: Add Apple mailbox bindings Sven Peter
2021-09-16 15:49   ` Sven Peter
2021-09-21 22:08   ` Rob Herring
2021-09-21 22:08     ` Rob Herring
2021-09-16 15:49 ` [PATCH v2 2/2] mailbox: apple: Add driver for Apple mailboxes Sven Peter
2021-09-16 15:49   ` Sven Peter
2021-09-19 11:51   ` Alyssa Rosenzweig
2021-09-19 11:51     ` Alyssa Rosenzweig
2021-10-16 19:04   ` Jassi Brar
2021-10-16 19:04     ` Jassi Brar
2021-10-16 19:16     ` Alyssa Rosenzweig
2021-10-16 19:16       ` Alyssa Rosenzweig
2021-10-16 19:17     ` Sven Peter
2021-10-16 19:17       ` Sven Peter
2021-10-17 11:25   ` Mark Kettenis
2021-10-17 11:25     ` Mark Kettenis
2021-10-17 11:27     ` Sven Peter
2021-10-17 11:27       ` Sven Peter
2021-09-19 11:47 ` [PATCH v2 0/2] Apple Mailbox Controller support Alyssa Rosenzweig
2021-09-19 11:47   ` Alyssa Rosenzweig
2021-10-16  9:21 ` Sven Peter
2021-10-16  9:21   ` Sven Peter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.