diff mbox

[v2,3/6] mailbox: Add support for ST's Mailbox IP

Message ID 1437990272-23111-4-git-send-email-lee.jones@linaro.org
State New
Headers show

Commit Message

Lee Jones July 27, 2015, 9:44 a.m. UTC
ST's platforms currently support a maximum of 5 Mailboxes, one for
each of the supported co-processors situated on the platform.  Each
Mailbox is divided up into 4 instances which consist of 32 channels.
Messages are passed between the application and co-processors using
shared memory areas.  It is the Client's responsibility to manage
these areas.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mailbox/Kconfig       |   7 +
 drivers/mailbox/Makefile      |   2 +
 drivers/mailbox/mailbox-sti.c | 527 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 536 insertions(+)
 create mode 100644 drivers/mailbox/mailbox-sti.c

Comments

Lee Jones July 30, 2015, 11:45 a.m. UTC | #1
On Wed, 29 Jul 2015, Paul Bolle wrote:

> On ma, 2015-07-27 at 10:44 +0100, Lee Jones wrote:
> > --- /dev/null
> > +++ b/drivers/mailbox/mailbox-sti.c
> 
> > +static int sti_mbox_probe(struct platform_device *pdev)
> > +{
> > +	[...]
> > +
> > +	match = of_match_device(sti_mailbox_match, &pdev->dev);
> > +	if (!match) {
> > +		dev_err(&pdev->dev, "No configuration found\n");
> > +		return -ENODEV;
> > +	}
> > +	pdev->dev.platform_data = (struct sti_mbox_pdata *) match->data;
> > +
> > +	[...]
> > +}
> 
> > +static struct platform_driver sti_mbox_driver = {
> > +	.probe = sti_mbox_probe,
> > +	.remove = sti_mbox_remove,
> > +	.driver = {
> > +		.name = "sti-mailbox",
> > +		.of_match_table = sti_mailbox_match,
> > +	},
> > +};
> > +module_platform_driver(sti_mbox_driver);
> 
> > +MODULE_ALIAS("platform:mailbox-sti");
> 
> It seems this alias is only useful if there's a corresponding struct
> platform_device with a "mailbox-sti" .name. I couldn't spot that
> platform_device.
> 
> Besides, if I read sti_max_probe() correctly, without OF support loading
> this module won't accomplish much. So what would another way of
> autoloading this module buy you?

I think this line can be safely removed.
Lee Jones July 30, 2015, 1:31 p.m. UTC | #2
On Thu, 30 Jul 2015, Paul Bolle wrote:

> On do, 2015-07-30 at 12:45 +0100, Lee Jones wrote:
> > On Wed, 29 Jul 2015, Paul Bolle wrote:
> > > Besides, if I read sti_max_probe() correctly, without OF support
> > > loading
> > > this module won't accomplish much. So what would another way of
> > > autoloading this module buy you?
> > 
> > I think this line can be safely removed.
> 
> Looking at this patch a little more I notice there's no line reading
>      MODULE_DEVICE_TABLE(of, sti_mailbox_match);
> 
> The three mailbox drivers that currently support OF do have such a line.
> So "another way of autoloading" was rather sloppy as now I don't see how
> this driver, if build as a module, will get auto-loaded. Maybe there's
> an auto-loading mechanism that I'm not aware of here. Or perhaps auto
> -loading is not needed.
> 
> Similar question for patch 5/6 and the lack of a line reading
>     MODULE_DEVICE_TABLE(of, mbox_test_match);

Yes, both of those lines should be present.

FYW, I don't test these drivers as modules and have no desire to.  I
always build everything into a single kernel binary, so any feedback
regarding the module enabling code is appreciated.
Lee Jones Aug. 14, 2015, 6:33 a.m. UTC | #3
On Thu, 13 Aug 2015, Jassi Brar wrote:
> On Mon, Jul 27, 2015 at 3:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> 
> > +
> > +static bool sti_mbox_tx_is_ready(struct mbox_chan *chan)
> > +{
> > +       struct sti_channel *chan_info = chan->con_priv;
> > +       struct sti_mbox_device *mdev = chan_info->mdev;
> > +       unsigned int instance = chan_info->instance;
> > +       unsigned int channel = chan_info->channel;
> > +       void __iomem *base = MBOX_BASE(mdev, instance);
> > +
> > +       if (!(chan_info->direction & MBOX_TX))
> > +               return false;
> >
> Here the 'direction' is gotten via DT node of the client i.e, you
> expect consumer drivers to tell the provider what its limitations are?
> 
> IMO if some physical channel can't do TX then that should be either
> hardcoded inside the controller driver or learnt via DT node of the
> _controller_.

That's a fair point.  I need to create a new property similar to the
already existing 'read-only'.  I guess 'tx-only' is equivalent.

> > +static struct mbox_chan *sti_mbox_xlate(struct mbox_controller *mbox,
> > +                                       const struct of_phandle_args *spec)
> > +{
> > +       struct sti_mbox_device *mdev = dev_get_drvdata(mbox->dev);
> > +       struct sti_mbox_pdata *pdata = dev_get_platdata(mdev->dev);
> > +       struct sti_channel *chan_info;
> > +       struct mbox_chan *chan = NULL;
> > +       unsigned int instance  = spec->args[0];
> > +       unsigned int channel   = spec->args[1];
> > +       unsigned int direction = spec->args[2];
> > +       int i;
> > +
> > +       /* Bounds checking */
> > +       if (instance >= pdata->num_inst || channel  >= pdata->num_chan) {
> > +               dev_err(mbox->dev,
> > +                       "Invalid channel requested instance: %d channel: %d\n",
> > +                       instance, channel);
> > +               return NULL;
> return ERR_PTR(-EINVAL)

I can handle all these, no problem.

> > +       }
> > +
> > +       for (i = 0; i < mbox->num_chans; i++) {
> > +               chan_info = mbox->chans[i].con_priv;
> > +
> > +               /* Is requested channel free? */
> > +               if (direction != MBOX_LOOPBACK &&
> >
> Consider this example when 2 clients ask for same physical channel but
> in different modes.
>            mboxes = <&mboxA 0 1 MBOX_TX>;
>            mboxes = <&mboxA 0 1 MBOX_LOOPBACK>;
> 
> You happily assign 2 virtual channels backed by one physical channel
> {mboxA, 0, 1}. The 2 clients think they can freely do startup(),
> shutdown() and send_data() on the channels. But obviously we are
> screwed with races like
>    client1.startup()
>     -> client2.startup()
>         -> client2.send_data()
>             -> client2.shutdown()
>                 -> client1.send_data()  XXXX

Good catch and a fair point.  As you say, it's unlikely to happen, but
I would like to prevent it in any case.

>  Now you can shove in some more checks to 'fix' the race OR you can
> simply expose only physical channels.

We can't expose all of the channels.  There are too many and would
take up too much *unused* memory.

I don't want to have an endless stream of checks either, but we should
try to cover the bases.  I think smarter (rather than more) checks is
the answer.  I'll have a think about it.

> Practically no client would ever
> ask it to do what it can't, and for the hypothetical possibility that
> some does, just return error.

Right.  Smarter error checking here and returning an error on a bad
config is what I plan to do.

[...]
Lee Jones Aug. 14, 2015, 10:41 a.m. UTC | #4
On Fri, 14 Aug 2015, Jassi Brar wrote:

> On Fri, Aug 14, 2015 at 12:03 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Thu, 13 Aug 2015, Jassi Brar wrote:
> >> On Mon, Jul 27, 2015 at 3:14 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >>
> >> > +
> >> > +static bool sti_mbox_tx_is_ready(struct mbox_chan *chan)
> >> > +{
> >> > +       struct sti_channel *chan_info = chan->con_priv;
> >> > +       struct sti_mbox_device *mdev = chan_info->mdev;
> >> > +       unsigned int instance = chan_info->instance;
> >> > +       unsigned int channel = chan_info->channel;
> >> > +       void __iomem *base = MBOX_BASE(mdev, instance);
> >> > +
> >> > +       if (!(chan_info->direction & MBOX_TX))
> >> > +               return false;
> >> >
> >> Here the 'direction' is gotten via DT node of the client i.e, you
> >> expect consumer drivers to tell the provider what its limitations are?
> >>
> >> IMO if some physical channel can't do TX then that should be either
> >> hardcoded inside the controller driver or learnt via DT node of the
> >> _controller_.
> >
> > That's a fair point.
> .
> 
> >  I need to create a new property similar to the
> > already existing 'read-only'.  I guess 'tx-only' is equivalent.
> >
> Just to be clear, if you must have such a property it should come from
> the _controller_ node.

Correct.  That's the plan.

> However at one point you said,  "Only the A9 (Mbox 0) can Rx."
>   Which sounds like the 'simplex' constraint is not coming from the
> mailbox controller but from the remote endpoints that don't RX+TX
> except for one of them. That makes more sense than a controller with
> differently capable physical channels. If that is indeed the
> situation, then the controller is actually 'duplex' and there should
> be no tx-only/rx-only property anywhere. Everything automatically
> falls into place because client drivers are written for specific
> targets and, unless you write some code, there can be no TX call to a
> remote that doesn't listen.

Unfortunately it's a restriction of the hardware (or the controller as
you call it, although it's not really a controller).  There is only
one IRQ for Rx'ing and that's wired up to the A9's mailbox (Mailbox
0).  If one of the remote processors attempted to send a message
through any of the other mailboxes (other than the Mailbox 0), then no
one would hear the doorbell ring and the message would go unserviced.

> >> > +
> >> > +       for (i = 0; i < mbox->num_chans; i++) {
> >> > +               chan_info = mbox->chans[i].con_priv;
> >> > +
> >> > +               /* Is requested channel free? */
> >> > +               if (direction != MBOX_LOOPBACK &&
> >> >
> >> Consider this example when 2 clients ask for same physical channel but
> >> in different modes.
> >>            mboxes = <&mboxA 0 1 MBOX_TX>;
> >>            mboxes = <&mboxA 0 1 MBOX_LOOPBACK>;
> >>
> >> You happily assign 2 virtual channels backed by one physical channel
> >> {mboxA, 0, 1}. The 2 clients think they can freely do startup(),
> >> shutdown() and send_data() on the channels. But obviously we are
> >> screwed with races like
> >>    client1.startup()
> >>     -> client2.startup()
> >>         -> client2.send_data()
> >>             -> client2.shutdown()
> >>                 -> client1.send_data()  XXXX
> >
> > Good catch and a fair point.  As you say, it's unlikely to happen, but
> > I would like to prevent it in any case.
> >
> No, such races are a practical problem. We must own them.
> 
> I was talking about problems that arise because someone wrote bad
> DT... those are not 'practical' problems because there are too many
> ways to screw up with bad DT properties that if we try to check for
> them we'll go insane.

The only thing I'm having trouble with protecting at the moment is
other clients _also_ requesting a LOOPBACK channel.  I would like to
check which clients have already requested one/them, however that
information is not available until _after_ xlate() has been called,
which is pretty frustrating.  Perhaps I'll put a comment in instead.

> >>  Now you can shove in some more checks to 'fix' the race OR you can
> >> simply expose only physical channels.
> >
> > We can't expose all of the channels.  There are too many and would
> > take up too much *unused* memory.
> >
> I am aware of that. I said expose _only_ physical channels, not _all_ :)

It's impossible to know which physical channels will be used by
clients and subsequently which physical channels to expose.
diff mbox

Patch

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index e269f08..2cc4c39 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -70,4 +70,11 @@  config BCM2835_MBOX
 	  the services of the Videocore. Say Y here if you want to use the
 	  BCM2835 Mailbox.
 
+config STI_MBOX
+	tristate "STI Mailbox framework support"
+	depends on ARCH_STI && OF
+	help
+	  Mailbox implementation for STMicroelectonics family chips with
+	  hardware for interprocessor communication.
+
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 8e6d822..7cb4766 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -13,3 +13,5 @@  obj-$(CONFIG_PCC)		+= pcc.o
 obj-$(CONFIG_ALTERA_MBOX)	+= mailbox-altera.o
 
 obj-$(CONFIG_BCM2835_MBOX)	+= bcm2835-mailbox.o
+
+obj-$(CONFIG_STI_MBOX)		+= mailbox-sti.o
diff --git a/drivers/mailbox/mailbox-sti.c b/drivers/mailbox/mailbox-sti.c
new file mode 100644
index 0000000..54679d8
--- /dev/null
+++ b/drivers/mailbox/mailbox-sti.c
@@ -0,0 +1,527 @@ 
+/*
+ * STi Mailbox
+ *
+ * Copyright (C) 2015 ST Microelectronics
+ *
+ * Author: Lee Jones <lee.jones@linaro.org> for ST Microelectronics
+ *
+ * Based on the original driver written by;
+ *   Alexandre Torgue, Olivier Lebreton and Loic Pallardy
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#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_device.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <dt-bindings/mailbox/mailbox.h>
+
+#include "mailbox.h"
+
+#define STI_MBOX_INST_MAX	4      /* RAM saving: Max supported instances */
+#define STI_MBOX_CHAN_MAX	20     /* RAM saving: Max supported channels  */
+
+#define STI_IRQ_VAL_OFFSET	0x04   /* Read interrupt status	              */
+#define STI_IRQ_SET_OFFSET	0x24   /* Generate a Tx channel interrupt     */
+#define STI_IRQ_CLR_OFFSET	0x44   /* Clear pending Rx interrupts	      */
+#define STI_ENA_VAL_OFFSET	0x64   /* Read enable status		      */
+#define STI_ENA_SET_OFFSET	0x84   /* Enable a channel		      */
+#define STI_ENA_CLR_OFFSET	0xa4   /* Disable a channel		      */
+
+#define MBOX_BASE(mdev, inst)   ((mdev)->base + (inst * 4))
+
+/**
+ * STi Mailbox device data
+ *
+ * An IP Mailbox is currently composed of 4 instances
+ * Each instance is currently composed of 32 channels
+ * This means that we have 128 channels per Mailbox
+ * A channel an be used for TX or RX
+ *
+ * @dev:	Device to which it is attached
+ * @mbox:	Representation of a communication channel controller
+ * @base:	Base address of the register mapping region
+ * @name:	Name of the mailbox
+ * @enabled:	Local copy of enabled channels
+ * @lock:	Mutex protecting enabled status
+ */
+struct sti_mbox_device {
+	struct device		*dev;
+	struct mbox_controller	*mbox;
+	void __iomem		*base;
+	const char		*name;
+	u32			enabled[STI_MBOX_INST_MAX];
+	spinlock_t		lock;
+};
+
+/**
+ * STi Mailbox platform specfic configuration
+ *
+ * @num_inst:	Maximum number of instances in one HW Mailbox
+ * @num_chan:	Maximum number of channel per instance
+ */
+struct sti_mbox_pdata {
+	unsigned int		num_inst;
+	unsigned int		num_chan;
+};
+
+/**
+ * STi Mailbox allocated channel information
+ *
+ * @mdev:	Pointer to parent Mailbox device
+ * @instance:	Instance number channel resides in
+ * @channel:	Channel number pertaining to this container
+ * @direction:	Direction which data will travel in through the channel (Tx/Rx)
+ */
+struct sti_channel {
+	struct sti_mbox_device	*mdev;
+	unsigned int		instance;
+	unsigned int		channel;
+	unsigned int		direction;
+};
+
+static inline bool sti_mbox_channel_is_enabled(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	unsigned long flags;
+
+	spin_lock_irqsave(&mdev->lock, flags);
+	return mdev->enabled[instance] & BIT(channel);
+	spin_unlock_irqrestore(&mdev->lock, flags);
+}
+
+static inline
+struct mbox_chan *sti_mbox_to_channel(struct mbox_controller *mbox,
+				      unsigned int instance,
+				      unsigned int channel)
+{
+	struct sti_channel *chan_info;
+	int i;
+
+	for (i = 0; i < mbox->num_chans; i++) {
+		chan_info = mbox->chans[i].con_priv;
+		if (chan_info &&
+		    chan_info->instance == instance &&
+		    chan_info->channel == channel)
+			return &mbox->chans[i];
+	}
+
+	dev_err(mbox->dev,
+		"Channel not registered: instance: %d channel: %d\n",
+		instance, channel);
+
+	return NULL;
+}
+
+static void sti_mbox_enable_channel(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	unsigned long flags;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	spin_lock_irqsave(&mdev->lock, flags);
+	mdev->enabled[instance] |= BIT(channel);
+	writel_relaxed(BIT(channel), base + STI_ENA_SET_OFFSET);
+	spin_unlock_irqrestore(&mdev->lock, flags);
+}
+
+static void sti_mbox_disable_channel(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	unsigned long flags;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	spin_lock_irqsave(&mdev->lock, flags);
+	mdev->enabled[instance] &= ~BIT(channel);
+	writel_relaxed(BIT(channel), base + STI_ENA_CLR_OFFSET);
+	spin_unlock_irqrestore(&mdev->lock, flags);
+}
+
+static void sti_mbox_clear_irq(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	writel_relaxed(BIT(channel), base + STI_IRQ_CLR_OFFSET);
+}
+
+static struct mbox_chan *sti_mbox_irq_to_channel(struct sti_mbox_device *mdev,
+						 unsigned int instance)
+{
+	struct mbox_controller *mbox = mdev->mbox;
+	struct mbox_chan *chan = NULL;
+	unsigned int channel;
+	unsigned long bits;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	bits = readl_relaxed(base + STI_IRQ_VAL_OFFSET);
+	if (!bits)
+		/* No IRQs fired in specified instance */
+		return NULL;
+
+	/* An IRQ has fired, find the associated channel */
+	for (channel = 0; bits; channel++) {
+		if (!test_and_clear_bit(channel, &bits))
+			continue;
+
+		chan = sti_mbox_to_channel(mbox, instance, channel);
+		if (chan) {
+			dev_dbg(mbox->dev,
+				"IRQ fired on instance: %d channel: %d\n",
+				instance, channel);
+			break;
+		}
+	}
+
+	return chan;
+}
+
+static irqreturn_t sti_mbox_thread_handler(int irq, void *data)
+{
+	struct sti_mbox_device *mdev = data;
+	struct sti_mbox_pdata *pdata = dev_get_platdata(mdev->dev);
+	struct mbox_chan *chan;
+	unsigned int instance;
+
+	for (instance = 0; instance < pdata->num_inst; instance++) {
+keep_looking:
+		chan = sti_mbox_irq_to_channel(mdev, instance);
+		if (!chan)
+			continue;
+
+		mbox_chan_received_data(chan, NULL);
+		sti_mbox_clear_irq(chan);
+		sti_mbox_enable_channel(chan);
+		goto keep_looking;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t sti_mbox_irq_handler(int irq, void *data)
+{
+	struct sti_mbox_device *mdev = data;
+	struct sti_mbox_pdata *pdata = dev_get_platdata(mdev->dev);
+	struct sti_channel *chan_info;
+	struct mbox_chan *chan;
+	unsigned int instance;
+	int ret = IRQ_NONE;
+
+	for (instance = 0; instance < pdata->num_inst; instance++) {
+		chan = sti_mbox_irq_to_channel(mdev, instance);
+		if (!chan)
+			continue;
+		chan_info = chan->con_priv;
+
+		if (!sti_mbox_channel_is_enabled(chan)) {
+			dev_warn(mdev->dev,
+				 "Unexpected IRQ: %s\n"
+				 "  instance: %d: channel: %d [enabled: %x]\n",
+				 mdev->name, chan_info->instance,
+				 chan_info->channel, mdev->enabled[instance]);
+
+			/* Only handle IRQ if no other valid IRQs were found */
+			if (ret == IRQ_NONE)
+				ret = IRQ_HANDLED;
+			continue;
+		}
+
+		sti_mbox_disable_channel(chan);
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	if (ret == IRQ_NONE)
+		dev_err(mdev->dev, "Spurious IRQ - was a channel requested?\n");
+
+	return ret;
+}
+
+static bool sti_mbox_tx_is_ready(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	if (!(chan_info->direction & MBOX_TX))
+		return false;
+
+	if (!(readl_relaxed(base + STI_ENA_VAL_OFFSET) & BIT(channel))) {
+		dev_dbg(mdev->dev, "Mbox: %s: inst: %d, chan: %d disabled\n",
+			mdev->name, instance, channel);
+		return false;
+	}
+
+	if (readl_relaxed(base + STI_IRQ_VAL_OFFSET) & BIT(channel)) {
+		dev_dbg(mdev->dev, "Mbox: %s: inst: %d, chan: %d not ready\n",
+			mdev->name, instance, channel);
+		return false;
+	}
+
+	return true;
+}
+
+static int sti_mbox_send_data(struct mbox_chan *chan, void *data)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct sti_mbox_device *mdev = chan_info->mdev;
+	unsigned int instance = chan_info->instance;
+	unsigned int channel = chan_info->channel;
+	void __iomem *base = MBOX_BASE(mdev, instance);
+
+	/* Send event to co-processor */
+	writel_relaxed(BIT(channel), base + STI_IRQ_SET_OFFSET);
+
+	dev_dbg(mdev->dev,
+		"Sent via Mailbox %s: instance: %d channel: %d\n",
+		mdev->name, instance, channel);
+
+	return 0;
+}
+
+static int sti_mbox_startup_chan(struct mbox_chan *chan)
+{
+	sti_mbox_clear_irq(chan);
+	sti_mbox_enable_channel(chan);
+
+	return 0;
+}
+
+static void sti_mbox_shutdown_chan(struct mbox_chan *chan)
+{
+	struct sti_channel *chan_info = chan->con_priv;
+	struct mbox_controller *mbox = chan_info->mdev->mbox;
+	int i;
+
+	for (i = 0; i < mbox->num_chans; i++)
+		if (chan == &mbox->chans[i])
+			break;
+
+	if (mbox->num_chans == i) {
+		dev_warn(mbox->dev, "Request to free non-existent channel\n");
+		return;
+	}
+
+	/* Reset channel */
+	sti_mbox_disable_channel(chan);
+	sti_mbox_clear_irq(chan);
+	chan->con_priv = NULL;
+}
+
+static struct mbox_chan *sti_mbox_xlate(struct mbox_controller *mbox,
+					const struct of_phandle_args *spec)
+{
+	struct sti_mbox_device *mdev = dev_get_drvdata(mbox->dev);
+	struct sti_mbox_pdata *pdata = dev_get_platdata(mdev->dev);
+	struct sti_channel *chan_info;
+	struct mbox_chan *chan = NULL;
+	unsigned int instance  = spec->args[0];
+	unsigned int channel   = spec->args[1];
+	unsigned int direction = spec->args[2];
+	int i;
+
+	/* Bounds checking */
+	if (instance >= pdata->num_inst || channel  >= pdata->num_chan) {
+		dev_err(mbox->dev,
+			"Invalid channel requested instance: %d channel: %d\n",
+			instance, channel);
+		return NULL;
+	}
+
+	for (i = 0; i < mbox->num_chans; i++) {
+		chan_info = mbox->chans[i].con_priv;
+
+		/* Is requested channel free? */
+		if (direction != MBOX_LOOPBACK &&
+		    chan_info &&
+		    mbox->dev == chan_info->mdev->dev &&
+		    instance == chan_info->instance &&
+		    channel == chan_info->channel) {
+			dev_err(mbox->dev, "Channel in use\n");
+			return NULL;
+		}
+
+		/*
+		 * Find the first free slot, then continue checking
+		 * to see if requested channel is in use
+		 */
+		if (!chan && !chan_info)
+			chan = &mbox->chans[i];
+	}
+
+	if (!chan) {
+		dev_err(mbox->dev, "No free channels left\n");
+		return NULL;
+	}
+
+	chan_info = devm_kzalloc(mbox->dev, sizeof(*chan_info), GFP_KERNEL);
+	if (!chan_info)
+		return NULL;
+
+	chan_info->mdev		= mdev;
+	chan_info->instance	= instance;
+	chan_info->channel	= channel;
+	chan_info->direction	= direction;
+
+	chan->con_priv = chan_info;
+
+	dev_info(mbox->dev,
+		 "Mbox: %s: Created channel:\n"
+		 "  instance: %d channel: %d direction: %s\n",
+		 mdev->name, instance, channel,
+		direction == MBOX_LOOPBACK ? "Loopback" :
+		direction == MBOX_TX ? "Tx" : "Rx");
+
+	return chan;
+}
+
+static struct mbox_chan_ops sti_mbox_ops = {
+	.startup	= sti_mbox_startup_chan,
+	.shutdown	= sti_mbox_shutdown_chan,
+	.send_data	= sti_mbox_send_data,
+	.last_tx_done	= sti_mbox_tx_is_ready,
+};
+
+static const struct sti_mbox_pdata mbox_stih407_pdata = {
+	.num_inst	= 4,
+	.num_chan	= 32,
+};
+
+static const struct of_device_id sti_mailbox_match[] = {
+	{
+		.compatible = "st,stih407-mailbox",
+		.data = (void *)&mbox_stih407_pdata
+	},
+	{ }
+};
+
+static int sti_mbox_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+	struct mbox_controller *mbox;
+	struct sti_mbox_device *mdev;
+	struct device_node *np = pdev->dev.of_node;
+	struct mbox_chan *chans;
+	struct resource *res;
+	int irq;
+	int ret;
+
+	match = of_match_device(sti_mailbox_match, &pdev->dev);
+	if (!match) {
+		dev_err(&pdev->dev, "No configuration found\n");
+		return -ENODEV;
+	}
+	pdev->dev.platform_data = (struct sti_mbox_pdata *) match->data;
+
+	mdev = devm_kzalloc(&pdev->dev, sizeof(*mdev), GFP_KERNEL);
+	if (!mdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, mdev);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mdev->base = devm_ioremap_resource(&pdev->dev, res);
+	if (!mdev->base)
+		return -ENOMEM;
+
+	ret = of_property_read_string(np, "mbox-name", &mdev->name);
+	if (ret)
+		mdev->name = np->full_name;
+
+	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	chans = devm_kzalloc(&pdev->dev,
+			     sizeof(*chans) * STI_MBOX_CHAN_MAX, GFP_KERNEL);
+	if (!chans)
+		return -ENOMEM;
+
+	mdev->dev		= &pdev->dev;
+	mdev->mbox		= mbox;
+
+	spin_lock_init(&mdev->lock);
+
+	/* STi Mailbox does not have a Tx-Done or Tx-Ready IRQ */
+	mbox->txdone_irq	= false;
+	mbox->txdone_poll	= true;
+	mbox->txpoll_period	= 100;
+	mbox->ops		= &sti_mbox_ops;
+	mbox->dev		= mdev->dev;
+	mbox->of_xlate		= sti_mbox_xlate;
+	mbox->chans		= chans;
+	mbox->num_chans		= STI_MBOX_CHAN_MAX;
+
+	ret = mbox_controller_register(mbox);
+	if (ret)
+		return ret;
+
+	/* It's okay for Tx Mailboxes to not supply IRQs */
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_info(&pdev->dev,
+			 "%s: Registered Tx only Mailbox\n", mdev->name);
+		return 0;
+	}
+
+	ret = devm_request_threaded_irq(&pdev->dev, irq,
+					sti_mbox_irq_handler,
+					sti_mbox_thread_handler,
+					IRQF_ONESHOT, mdev->name, mdev);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't claim IRQ %d\n", irq);
+		mbox_controller_unregister(mbox);
+		return -EINVAL;
+	}
+
+	dev_info(&pdev->dev, "%s: Registered Tx/Rx Mailbox\n", mdev->name);
+
+	return 0;
+}
+
+static int sti_mbox_remove(struct platform_device *pdev)
+{
+	struct sti_mbox_device *mdev = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(mdev->mbox);
+
+	return 0;
+}
+
+static struct platform_driver sti_mbox_driver = {
+	.probe = sti_mbox_probe,
+	.remove = sti_mbox_remove,
+	.driver = {
+		.name = "sti-mailbox",
+		.of_match_table = sti_mailbox_match,
+	},
+};
+module_platform_driver(sti_mbox_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("STMicroelectronics Mailbox Controller");
+MODULE_AUTHOR("Lee Jones <lee.jones@linaro.org");
+MODULE_ALIAS("platform:mailbox-sti");