diff mbox

[3/3] mailbox: sunxi-msgbox: Add a new mailbox driver

Message ID 20180228022714.30068-4-samuel@sholland.org (mailing list archive)
State New, archived
Headers show

Commit Message

Samuel Holland Feb. 28, 2018, 2:27 a.m. UTC
Recent Allwinner sunxi SoCs contain a "hardware message box" used for
communication between the ARM CPUs and the ARISC management processor
(the platform's equivalent of the ARM SCP). Add a driver for it, so it
can be used for SCPI or other communication protocols.

The hardware contains 8 unidirectional 4-message-deep FIFOs. To fit the
mailbox framework API, this driver combines them into 4 bidirectional
pairs of channels, and only allows one message in each channel to be
queued at a time.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 drivers/mailbox/Kconfig        |   7 +
 drivers/mailbox/Makefile       |   2 +
 drivers/mailbox/sunxi-msgbox.c | 323 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 332 insertions(+)
 create mode 100644 drivers/mailbox/sunxi-msgbox.c

Comments

Maxime Ripard Feb. 28, 2018, 8:32 a.m. UTC | #1
On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
> +	/*
> +	 * The failure path should not disable the clock or assert the reset,
> +	 * because the PSCI implementation in firmware relies on this device
> +	 * being functional. Claiming the clock in this driver is required to
> +	 * prevent Linux from turning it off.
> +	 */
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
> +		return ret;
> +	}

You don't need it to be always on though. You only need it to be
enabled when you access the registers (on both sides I guess?). So you
could very well enable the clock in your registers accessors in Linux,
and do the same in the ARISC firmware. That should work.

Maxime
Jassi Brar Feb. 28, 2018, 9:16 a.m. UTC | #2
On Wed, Feb 28, 2018 at 7:57 AM, Samuel Holland <samuel@sholland.org> wrote:
....

> +/*
> + * The message box hardware provides 8 unidirectional channels. As the mailbox
> + * framework expects them to be bidirectional
>
That is incorrect. Mailbox framework does not require a channel to be
TX and RX capable.

You should expose each channel as per its physical capability, let the
client configure it for direction (if its bidirectional) and acquire
separate channels for RX and TX if it needs to.

Cheers!
Samuel Holland Feb. 28, 2018, 5:19 p.m. UTC | #3
Hi,

On 02/28/18 02:32, Maxime Ripard wrote:
> On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
>> +	/*
>> +	 * The failure path should not disable the clock or assert the reset,
>> +	 * because the PSCI implementation in firmware relies on this device
>> +	 * being functional. Claiming the clock in this driver is required to
>> +	 * prevent Linux from turning it off.
>> +	 */
>> +	ret = clk_prepare_enable(clk);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
>> +		return ret;
>> +	}
> 
> You don't need it to be always on though. You only need it to be
> enabled when you access the registers (on both sides I guess?). So you
> could very well enable the clock in your registers accessors in Linux,
> and do the same in the ARISC firmware. That should work.

It does need to always be on because the *PSCI* implementation (ATF) also uses
SCPI concurrently with Linux (on a separate channel). Turning off the clock
anywhere in Linux could turn it off in the middle of a PSCI SCPI call on a
different CPU, causing ATF to hang in EL3 (which would be very bad).

> Maxime

Regards,
Samuel
Samuel Holland Feb. 28, 2018, 5:51 p.m. UTC | #4
Hi,

On 02/28/18 03:16, Jassi Brar wrote:
> On Wed, Feb 28, 2018 at 7:57 AM, Samuel Holland <samuel@sholland.org> wrote:
> ....
> 
>> +/*
>> + * The message box hardware provides 8 unidirectional channels. As the mailbox
>> + * framework expects them to be bidirectional
>>
> That is incorrect. Mailbox framework does not require a channel to be
> TX and RX capable.

Sorry, it would be more accurate to say that the intended mailbox _client_
expects the channels to be bidirectional.

> You should expose each channel as per its physical capability, let the
> client configure it for direction (if its bidirectional) and acquire
> separate channels for RX and TX if it needs to.

Is there any way for the mailbox framework to inform the client that a channel
is uni/bidirectional? Or if the channel supports RX/TX specifically? I couldn't
find any.

It looks like all of the clients assume one case or the other, based on the
hardware they were initially designed to support. For example arm_scpi expects
one channel per client, while ti_sci and st_remoteproc expect two separate RX
and TX channels per client.

Since there's no API for the client to know the hardware properties, modifying
arm_scpi to support unidirectional mailbox channels would break support for all
existing bidirectional mailboxes.

To expose the physical channels individually, there would need to be an API to:
a) Determine if a mailbox channel supports unidirectional or bidirectional
   operation.
b) If a channel is unidirectional, determine which direction(s) it supports.
c) If a unidirectional channel supports both directions, configure which
   direction it should run in.

Since that API doesn't currently exist, I wrote the driver to match what the
intended client expected (as all other mailbox controller drivers currently do).

> Cheers!

Regards,
Samuel
Jassi Brar Feb. 28, 2018, 6:14 p.m. UTC | #5
On Wed, Feb 28, 2018 at 11:21 PM, Samuel Holland <samuel@sholland.org> wrote:
> Hi,
>
> On 02/28/18 03:16, Jassi Brar wrote:
>> On Wed, Feb 28, 2018 at 7:57 AM, Samuel Holland <samuel@sholland.org> wrote:
>> ....
>>
>>> +/*
>>> + * The message box hardware provides 8 unidirectional channels. As the mailbox
>>> + * framework expects them to be bidirectional
>>>
>> That is incorrect. Mailbox framework does not require a channel to be
>> TX and RX capable.
>
> Sorry, it would be more accurate to say that the intended mailbox _client_
> expects the channels to be bidirectional.
>
Mailbox clients are very mailbox provider specific. Your client driver
is unlikely to be reuseable over another controller+remote combo.
Your client has to know already what a physical channel can do (RX, TX
or RXTX). There is no api to provide that info.

thanks
Maxime Ripard March 1, 2018, 10:32 a.m. UTC | #6
On Wed, Feb 28, 2018 at 11:19:11AM -0600, Samuel Holland wrote:
> Hi,
> 
> On 02/28/18 02:32, Maxime Ripard wrote:
> > On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
> >> +	/*
> >> +	 * The failure path should not disable the clock or assert the reset,
> >> +	 * because the PSCI implementation in firmware relies on this device
> >> +	 * being functional. Claiming the clock in this driver is required to
> >> +	 * prevent Linux from turning it off.
> >> +	 */
> >> +	ret = clk_prepare_enable(clk);
> >> +	if (ret) {
> >> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
> >> +		return ret;
> >> +	}
> > 
> > You don't need it to be always on though. You only need it to be
> > enabled when you access the registers (on both sides I guess?). So you
> > could very well enable the clock in your registers accessors in Linux,
> > and do the same in the ARISC firmware. That should work.
> 
> It does need to always be on because the *PSCI* implementation (ATF) also uses
> SCPI concurrently with Linux (on a separate channel). Turning off the clock
> anywhere in Linux could turn it off in the middle of a PSCI SCPI call on a
> different CPU, causing ATF to hang in EL3 (which would be very bad).

Then the above code doesn't fix anything. You should mark the clock
critical, otherwise that clock will be turned off if the driver is
compiled as a module and not loaded (or loaded later), or if the
driver is not even compiled in.

Maxime
Andre Przywara March 1, 2018, 11:32 a.m. UTC | #7
Hi,

On 01/03/18 10:32, Maxime Ripard wrote:
> On Wed, Feb 28, 2018 at 11:19:11AM -0600, Samuel Holland wrote:
>> Hi,
>>
>> On 02/28/18 02:32, Maxime Ripard wrote:
>>> On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
>>>> +	/*
>>>> +	 * The failure path should not disable the clock or assert the reset,
>>>> +	 * because the PSCI implementation in firmware relies on this device
>>>> +	 * being functional. Claiming the clock in this driver is required to
>>>> +	 * prevent Linux from turning it off.
>>>> +	 */
>>>> +	ret = clk_prepare_enable(clk);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
>>>> +		return ret;
>>>> +	}
>>>
>>> You don't need it to be always on though. You only need it to be
>>> enabled when you access the registers (on both sides I guess?). So you
>>> could very well enable the clock in your registers accessors in Linux,
>>> and do the same in the ARISC firmware. That should work.
>>
>> It does need to always be on because the *PSCI* implementation (ATF) also uses
>> SCPI concurrently with Linux (on a separate channel). Turning off the clock
>> anywhere in Linux could turn it off in the middle of a PSCI SCPI call on a
>> different CPU, causing ATF to hang in EL3 (which would be very bad).
> 
> Then the above code doesn't fix anything. You should mark the clock
> critical, otherwise that clock will be turned off if the driver is
> compiled as a module and not loaded (or loaded later), or if the
> driver is not even compiled in.

... or if the module gets unloaded for some reason. So yes, I agree.
Actually I was hitting this problem before on several occasions:
- If firmware (either ATF or on the ARISC) wants to check temperatures,
it needs to rely on Linux not turning off the THS clock - which it does
at the moment when there is no Linux driver.
- If an EFI framebuffer needs to keep running in Linux, we should not
turn off the HDMI and DE clocks. simplefb solves this, but efifb has no
simple way of copying this approach.
- If a type 1 hypervisor like Xen uses the serial console (and exports a
virtual console to the guest), Linux turns off the now apparently unused
UART0 gate clock, killing Xen's console. So booting Xen on Allwinner
requires clk_ignore_unused at the moment.

There are ways to mitigate or workaround each one of these, but I was
wondering if we should look at a more general approach.

The most flexible seems to have some "clock victim" DT node, somewhat
mimicking the simplefb solution: One DT node which just references
clocks that are used by other (firmware) components in the system and
which can't be protected otherwise.
Normally this node would be empty, but firmware can add clock references
the moment it needs one clock. So ATF could add the THS clock, and Xen
could add the UART0 gate clock. This could be done at runtime by the
firmware or hypervisor. Xen for instance already amends the DT before
passing it on to Dom0, so copying all the clock references from the UART
to this node would be easy to implement.

Does that sound worthwhile to pursue? I could sketch up something if
that makes sense.

Cheers,
Andre.
Maxime Ripard March 1, 2018, 11:51 a.m. UTC | #8
On Thu, Mar 01, 2018 at 11:32:32AM +0000, Andre Przywara wrote:
> Hi,
> 
> On 01/03/18 10:32, Maxime Ripard wrote:
> > On Wed, Feb 28, 2018 at 11:19:11AM -0600, Samuel Holland wrote:
> >> Hi,
> >>
> >> On 02/28/18 02:32, Maxime Ripard wrote:
> >>> On Tue, Feb 27, 2018 at 08:27:14PM -0600, Samuel Holland wrote:
> >>>> +	/*
> >>>> +	 * The failure path should not disable the clock or assert the reset,
> >>>> +	 * because the PSCI implementation in firmware relies on this device
> >>>> +	 * being functional. Claiming the clock in this driver is required to
> >>>> +	 * prevent Linux from turning it off.
> >>>> +	 */
> >>>> +	ret = clk_prepare_enable(clk);
> >>>> +	if (ret) {
> >>>> +		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
> >>>> +		return ret;
> >>>> +	}
> >>>
> >>> You don't need it to be always on though. You only need it to be
> >>> enabled when you access the registers (on both sides I guess?). So you
> >>> could very well enable the clock in your registers accessors in Linux,
> >>> and do the same in the ARISC firmware. That should work.
> >>
> >> It does need to always be on because the *PSCI* implementation (ATF) also uses
> >> SCPI concurrently with Linux (on a separate channel). Turning off the clock
> >> anywhere in Linux could turn it off in the middle of a PSCI SCPI call on a
> >> different CPU, causing ATF to hang in EL3 (which would be very bad).
> > 
> > Then the above code doesn't fix anything. You should mark the clock
> > critical, otherwise that clock will be turned off if the driver is
> > compiled as a module and not loaded (or loaded later), or if the
> > driver is not even compiled in.
> 
> ... or if the module gets unloaded for some reason. So yes, I agree.
> Actually I was hitting this problem before on several occasions:
> - If firmware (either ATF or on the ARISC) wants to check temperatures,
> it needs to rely on Linux not turning off the THS clock - which it does
> at the moment when there is no Linux driver.
> - If an EFI framebuffer needs to keep running in Linux, we should not
> turn off the HDMI and DE clocks. simplefb solves this, but efifb has no
> simple way of copying this approach.
> - If a type 1 hypervisor like Xen uses the serial console (and exports a
> virtual console to the guest), Linux turns off the now apparently unused
> UART0 gate clock, killing Xen's console. So booting Xen on Allwinner
> requires clk_ignore_unused at the moment.
> 
> There are ways to mitigate or workaround each one of these, but I was
> wondering if we should look at a more general approach.
> 
> The most flexible seems to have some "clock victim" DT node, somewhat
> mimicking the simplefb solution: One DT node which just references
> clocks that are used by other (firmware) components in the system and
> which can't be protected otherwise.
> Normally this node would be empty, but firmware can add clock references
> the moment it needs one clock. So ATF could add the THS clock, and Xen
> could add the UART0 gate clock. This could be done at runtime by the
> firmware or hypervisor. Xen for instance already amends the DT before
> passing it on to Dom0, so copying all the clock references from the UART
> to this node would be easy to implement.
> 
> Does that sound worthwhile to pursue? I could sketch up something if
> that makes sense.

This makes sense, but I remember how heated the simplefb discussion
has been to introduce the clocks and regulator properties, so I'm not
sure this would be nice to encourage you to do that :)

Maxime
diff mbox

Patch

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ba2f1525f4ee..d6951b52498f 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -171,4 +171,11 @@  config BCM_FLEXRM_MBOX
 	  Mailbox implementation of the Broadcom FlexRM ring manager,
 	  which provides access to various offload engines on Broadcom
 	  SoCs. Say Y here if you want to use the Broadcom FlexRM.
+
+config SUNXI_MSGBOX
+	bool "Allwinner sunxi Message Box"
+	depends on ARCH_SUNXI
+	help
+	  Mailbox driver for the hardware message box used on Allwinner sunxi
+	  SoCs to communicate with the SCP.
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 4896f8dcae95..37a3e8f6a0eb 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -36,3 +36,5 @@  obj-$(CONFIG_BCM_FLEXRM_MBOX)	+= bcm-flexrm-mailbox.o
 obj-$(CONFIG_QCOM_APCS_IPC)	+= qcom-apcs-ipc-mailbox.o
 
 obj-$(CONFIG_TEGRA_HSP_MBOX)	+= tegra-hsp.o
+
+obj-$(CONFIG_SUNXI_MSGBOX)	+= sunxi-msgbox.o
diff --git a/drivers/mailbox/sunxi-msgbox.c b/drivers/mailbox/sunxi-msgbox.c
new file mode 100644
index 000000000000..16ff7aacfc55
--- /dev/null
+++ b/drivers/mailbox/sunxi-msgbox.c
@@ -0,0 +1,323 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2017 Samuel Holland <samuel@sholland.org>
+ *
+ * This driver was based on drivers/mailbox/bcm2835-mailbox.c and
+ * drivers/mailbox/rockchip-mailbox.c.
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/spinlock.h>
+
+/*
+ * The message box hardware provides 8 unidirectional channels. As the mailbox
+ * framework expects them to be bidirectional, create virtual channels out of
+ * pairs of opposite-direction hardware channels. The first channel in each pair
+ * is set up for AP->SCP communication, and the second channel is set up for
+ * SCP->AP transmission.
+ */
+#define NUM_CHANS		4
+
+/* These macros take a virtual channel number. */
+#define CTRL_REG(n)		(0x0000 + 0x4 * ((n) / 2))
+#define CTRL_MASK(n)		(0x1111 << 16 * ((n) % 2))
+#define CTRL_SET(n)		(0x0110 << 16 * ((n) % 2))
+
+#define IRQ_EN_REG		0x0060
+#define IRQ_STATUS_REG		0x0070
+#define RX_IRQ(n)		BIT(2 + 4 * (n))
+#define TX_IRQ(n)		BIT(1 + 4 * (n))
+
+#define REMOTE_IRQ_EN_REG	0x0040
+#define REMOTE_IRQ_STATUS_REG	0x0050
+#define REMOTE_RX_IRQ(n)	BIT(0 + 4 * (n))
+#define REMOTE_TX_IRQ(n)	BIT(3 + 4 * (n))
+
+#define RX_FIFO_STATUS_REG(n)	(0x0104 + 0x8 * (n))
+#define TX_FIFO_STATUS_REG(n)	(0x0100 + 0x8 * (n))
+#define FIFO_STATUS_MASK	BIT(0)
+
+#define RX_MSG_STATUS_REG(n)	(0x0144 + 0x8 * (n))
+#define TX_MSG_STATUS_REG(n)	(0x0140 + 0x8 * (n))
+#define MSG_STATUS_MASK		GENMASK(2, 0)
+
+#define RX_MSG_DATA_REG(n)	(0x0184 + 0x8 * (n))
+#define TX_MSG_DATA_REG(n)	(0x0180 + 0x8 * (n))
+
+struct sunxi_msgbox {
+	struct mbox_controller controller;
+	spinlock_t lock;
+	void __iomem *regs;
+};
+
+static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan);
+static bool sunxi_msgbox_peek_data(struct mbox_chan *chan);
+
+static inline int channel_number(struct mbox_chan *chan)
+{
+	return chan - chan->mbox->chans;
+}
+
+static inline struct sunxi_msgbox *channel_to_msgbox(struct mbox_chan *chan)
+{
+	return container_of(chan->mbox, struct sunxi_msgbox, controller);
+}
+
+static irqreturn_t sunxi_msgbox_irq(int irq, void *dev_id)
+{
+	struct mbox_chan *chan;
+	struct sunxi_msgbox *mbox = dev_id;
+	int n;
+	uint32_t msg, reg;
+
+	reg = readl(mbox->regs + IRQ_STATUS_REG);
+	for (n = 0; n < NUM_CHANS; ++n) {
+		if (!(reg & RX_IRQ(n)))
+			continue;
+		chan = &mbox->controller.chans[n];
+		while (sunxi_msgbox_peek_data(chan)) {
+			msg = readl(mbox->regs + RX_MSG_DATA_REG(n));
+			dev_dbg(mbox->controller.dev,
+				"Received 0x%08x on channel %d\n", msg, n);
+			mbox_chan_received_data(chan, &msg);
+		}
+		/* Clear the pending interrupt once the FIFO is empty. */
+		writel(RX_IRQ(n), mbox->regs + IRQ_STATUS_REG);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int sunxi_msgbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+	uint32_t msg = *(uint32_t *)data;
+
+	if (!sunxi_msgbox_last_tx_done(chan)) {
+		dev_dbg(mbox->controller.dev,
+			"Busy sending 0x%08x on channel %d\n", msg, n);
+		return -EBUSY;
+	}
+	writel(msg, mbox->regs + TX_MSG_DATA_REG(n));
+	dev_dbg(mbox->controller.dev,
+		"Sent 0x%08x on channel %d\n", msg, n);
+
+	return 0;
+}
+
+static int sunxi_msgbox_startup(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+	uint32_t reg;
+
+	/* Ensure FIFO directions are set properly. */
+	spin_lock(&mbox->lock);
+	reg = readl(mbox->regs + CTRL_REG(n));
+	writel((reg & ~CTRL_MASK(n)) | CTRL_SET(n), mbox->regs + CTRL_REG(n));
+	spin_unlock(&mbox->lock);
+
+	/* Clear existing messages in the receive FIFO. */
+	while (sunxi_msgbox_peek_data(chan))
+		readl(mbox->regs + RX_MSG_DATA_REG(n));
+
+	/* Clear and enable the receive interrupt. */
+	spin_lock(&mbox->lock);
+	reg = readl(mbox->regs + IRQ_STATUS_REG);
+	writel(reg | RX_IRQ(n), mbox->regs + IRQ_STATUS_REG);
+	reg = readl(mbox->regs + IRQ_EN_REG);
+	writel(reg | RX_IRQ(n), mbox->regs + IRQ_EN_REG);
+	spin_unlock(&mbox->lock);
+
+	dev_dbg(mbox->controller.dev, "Startup channel %d\n", n);
+
+	return 0;
+}
+
+static void sunxi_msgbox_shutdown(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+	uint32_t reg;
+
+	/* Disable the receive interrupt. */
+	spin_lock(&mbox->lock);
+	reg = readl(mbox->regs + IRQ_EN_REG);
+	writel(reg & ~RX_IRQ(n), mbox->regs + IRQ_EN_REG);
+	spin_unlock(&mbox->lock);
+
+	dev_dbg(mbox->controller.dev, "Shutdown channel %d\n", n);
+}
+
+static bool sunxi_msgbox_last_tx_done(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	/*
+	 * The message box hardware allows us to snoop on the other user's IRQ
+	 * statuses. Consider a message to be acknowledged when the reception
+	 * IRQ for that channel is cleared. As the hardware only allows clearing
+	 * the IRQ for a channel when the FIFO is empty, this still ensures that
+	 * the message has actually been read. Compared to checking the number
+	 * of messages in the FIFO, it also gives the receiver an opportunity to
+	 * perform minimal message handling (such as recording extra information
+	 * from a shared memory buffer) before acknowledging a message.
+	 */
+	return !(readl(mbox->regs + REMOTE_IRQ_STATUS_REG) & REMOTE_RX_IRQ(n));
+}
+
+static bool sunxi_msgbox_peek_data(struct mbox_chan *chan)
+{
+	struct sunxi_msgbox *mbox = channel_to_msgbox(chan);
+	int n = channel_number(chan);
+
+	return (readl(mbox->regs + RX_MSG_STATUS_REG(n)) & MSG_STATUS_MASK) > 0;
+}
+
+static const struct mbox_chan_ops sunxi_msgbox_chan_ops = {
+	.send_data	= sunxi_msgbox_send_data,
+	.startup	= sunxi_msgbox_startup,
+	.shutdown	= sunxi_msgbox_shutdown,
+	.last_tx_done	= sunxi_msgbox_last_tx_done,
+	.peek_data	= sunxi_msgbox_peek_data,
+};
+
+static struct mbox_chan *sunxi_msgbox_index_xlate(struct mbox_controller *mbox,
+	const struct of_phandle_args *sp)
+{
+	if (sp->args_count != 2)
+		return NULL;
+	/* Enforce this driver's assumed physical-to-virtual channel mapping. */
+	if ((sp->args[0] % 2) || (sp->args[1] != sp->args[0] + 1) ||
+	    (sp->args[0] > (NUM_CHANS - 1) * 2))
+		return NULL;
+
+	return &mbox->chans[sp->args[0] / 2];
+}
+
+static int sunxi_msgbox_probe(struct platform_device *pdev)
+{
+	struct clk *clk;
+	struct device *dev = &pdev->dev;
+	struct mbox_chan *chans;
+	struct reset_control *rst;
+	struct resource *res;
+	struct sunxi_msgbox *mbox;
+	int ret;
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	chans = devm_kcalloc(dev, NUM_CHANS, sizeof(*chans), GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	mbox->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(mbox->regs))
+		return PTR_ERR(mbox->regs);
+
+	clk = devm_clk_get(dev, "bus");
+	if (IS_ERR(clk)) {
+		dev_err(dev, "Failed to get bus clock\n");
+		return PTR_ERR(clk);
+	}
+
+	rst = devm_reset_control_get(dev, "bus");
+	if (IS_ERR(rst)) {
+		dev_err(dev, "Failed to get bus reset\n");
+		return PTR_ERR(rst);
+	}
+
+	/*
+	 * The failure path should not disable the clock or assert the reset,
+	 * because the PSCI implementation in firmware relies on this device
+	 * being functional. Claiming the clock in this driver is required to
+	 * prevent Linux from turning it off.
+	 */
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		dev_err(dev, "Failed to enable bus clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = reset_control_deassert(rst);
+	if (ret) {
+		dev_err(dev, "Failed to deassert bus reset: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable all interrupts. */
+	writel(0, mbox->regs + IRQ_EN_REG);
+
+	ret = devm_request_irq(dev, irq_of_parse_and_map(dev->of_node, 0),
+			       sunxi_msgbox_irq, 0, dev_name(dev), mbox);
+	if (ret) {
+		dev_err(dev, "Failed to register IRQ handler: %d\n", ret);
+		return ret;
+	}
+
+	mbox->controller.dev = dev;
+	mbox->controller.ops = &sunxi_msgbox_chan_ops;
+	mbox->controller.chans = chans;
+	mbox->controller.num_chans = NUM_CHANS;
+	mbox->controller.txdone_irq = false;
+	mbox->controller.txdone_poll = true;
+	mbox->controller.txpoll_period = 5;
+	mbox->controller.of_xlate = sunxi_msgbox_index_xlate;
+
+	spin_lock_init(&mbox->lock);
+	platform_set_drvdata(pdev, mbox);
+
+	ret = mbox_controller_register(&mbox->controller);
+	if (ret)
+		dev_err(dev, "Failed to register mailbox: %d\n", ret);
+
+	return ret;
+}
+
+static int sunxi_msgbox_remove(struct platform_device *pdev)
+{
+	struct sunxi_msgbox *mbox = platform_get_drvdata(pdev);
+	mbox_controller_unregister(&mbox->controller);
+
+	return 0;
+}
+
+static const struct of_device_id sunxi_msgbox_of_match[] = {
+	{ .compatible = "allwinner,sunxi-msgbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sunxi_msgbox_of_match);
+
+static struct platform_driver sunxi_msgbox_driver = {
+	.driver = {
+		.name = "sunxi-msgbox",
+		.of_match_table = sunxi_msgbox_of_match,
+	},
+	.probe		= sunxi_msgbox_probe,
+	.remove		= sunxi_msgbox_remove,
+};
+module_platform_driver(sunxi_msgbox_driver);
+
+MODULE_AUTHOR("Samuel Holland <samuel@sholland.org>");
+MODULE_DESCRIPTION("Allwinner sunxi Message Box");
+MODULE_LICENSE("GPL v2");