* [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 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 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