[RFC 0/6] mailbox: add common framework and port drivers

[RFC 0/6] mailbox: add common framework and port drivers

[Courtney Cavin]

There is currently no common framework for mailbox drivers, so this is my
attempt to come up with something suitable.  There seems to be a need for
making this generic, so I have attempted to do just that.  Most of this is
modeled pretty strongly after the pwm core, with some influences from the clock
core.

Looking at the existing use-cases, and some new ones, it would appear that the
requirements here are rather simple. We need essentially two things for
consumers:
	- put_message
	- callback for receiving messages

The code currently uses atomic notifiers for callbacks.  The common omap core
deals with fifos and work-queues in order to escape atomic contexts, but from
what I can see, this is unneeded.  I am also of the opinion that the contexts
can be much better managed in the drivers which are working with these
contexts, rather than generically.

Hopefully this will be suitable for the plethora of other drivers around the
kernel which implement mailboxes, as well.  In any case, I'm rather interested
to see what the rest of the world thinks.

Keep in mind that while the pl320 & omap code should compile, I don't currently
have a platform on which I can perform proper testing.  I also removed the
context save/restore code from omap2 mailbox support, because I think it should
be able to be done via driver suspend/resume, but haven't done a full
investigation just yet.

I'm also aware that breaking omap, just to fix it again probably isn't the best
course of action, and I'm open to suggestions.

-Courtney

Courtney Cavin (6):
  mailbox: add core framework
  mailbox: document bindings
  mailbox: pl320: migrate to mbox framework
  mailbox: omap: remove omap-specific framework
  mailbox: omap1: move to common mbox framework
  mailbox: omap2+: move to common mbox framework

 .../devicetree/bindings/mailbox/mailbox.txt        |  44 ++
 drivers/mailbox/Kconfig                            |  17 -
 drivers/mailbox/Makefile                           |   2 +-
 drivers/mailbox/core.c                             | 573 +++++++++++++++++++++
 drivers/mailbox/mailbox-omap1.c                    | 153 +++---
 drivers/mailbox/mailbox-omap2.c                    | 315 +++++------
 drivers/mailbox/omap-mailbox.c                     | 469 -----------------
 drivers/mailbox/omap-mbox.h                        |  67 ---
 drivers/mailbox/pl320-ipc.c                        | 258 +++++++---
 include/linux/mailbox.h                            |  29 +-
 include/linux/mbox.h                               | 175 +++++++
 include/linux/omap-mailbox.h                       |  45 +-
 12 files changed, 1261 insertions(+), 886 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mailbox.txt
 create mode 100644 drivers/mailbox/core.c
 delete mode 100644 drivers/mailbox/omap-mailbox.c
 delete mode 100644 drivers/mailbox/omap-mbox.h
 create mode 100644 include/linux/mbox.h

-- 
1.8.1.5

[RFC 1/6] mailbox: add core framework

[Courtney Cavin]

The mailbox drivers are fragmented, and some implement their own core.
Unify the drivers and implement common functionality in a framework.

Signed-off-by: Courtney Cavin <courtney.cavin-/MT0OVThwyLZJqsBc5GL+g@public.gmane.org>
---
 drivers/mailbox/Makefile |   1 +
 drivers/mailbox/core.c   | 573 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mbox.h     | 175 +++++++++++++++
 3 files changed, 749 insertions(+)
 create mode 100644 drivers/mailbox/core.c
 create mode 100644 include/linux/mbox.h

diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index e0facb3..53712ed 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,4 @@
+obj-$(CONFIG_MAILBOX)		+= core.o
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
 
 obj-$(CONFIG_OMAP_MBOX)		+= omap-mailbox.o
diff --git a/drivers/mailbox/core.c b/drivers/mailbox/core.c
new file mode 100644
index 0000000..0dc865e
--- /dev/null
+++ b/drivers/mailbox/core.c
@@ -0,0 +1,573 @@
+/*
+ * Generic mailbox implementation
+ *
+ * Copyright (C) 2014 Sony Mobile Communications, AB.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/mbox.h>
+
+static DEFINE_MUTEX(mbox_lock);
+static LIST_HEAD(mbox_adapters);
+
+static DEFINE_MUTEX(mbox_lookup_lock);
+static LIST_HEAD(mbox_lookup_list);
+
+static int __mbox_adapter_request(struct mbox_adapter *adap,
+		struct mbox_channel *chan)
+{
+	int rc;
+
+	if (chan->users > 0) {
+		chan->users++;
+		return 0;
+	}
+
+	if (!try_module_get(adap->ops->owner))
+		return -ENODEV;
+
+	if (adap->ops->request) {
+		rc = adap->ops->request(adap, chan);
+		if (rc) {
+			module_put(adap->ops->owner);
+			return rc;
+		}
+	}
+
+	chan->users++;
+
+	return 0;
+}
+
+static void __mbox_adapter_release(struct mbox_adapter *adap,
+		struct mbox_channel *chan)
+{
+	if (!adap || !chan)
+		return;
+
+	if (chan->users == 0) {
+		dev_err(adap->dev, "device already released\n");
+		return;
+	}
+
+	chan->users--;
+	if (chan->users > 0)
+		return;
+
+	if (adap->ops->release)
+		adap->ops->release(adap, chan);
+	module_put(adap->ops->owner);
+}
+
+static struct mbox_channel *
+mbox_adapter_request_channel(struct mbox_adapter *adap, unsigned int index)
+{
+	struct mbox_channel *chan;
+	int rc;
+
+	if (!adap || index >= adap->nchannels)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&adap->lock);
+	chan = &adap->channels[index];
+
+	rc = __mbox_adapter_request(adap, chan);
+	if (rc)
+		chan = ERR_PTR(rc);
+	mutex_unlock(&adap->lock);
+
+	return chan;
+}
+
+static void mbox_adapter_release_channel(struct mbox_adapter *adap,
+		struct mbox_channel *chan)
+{
+	if (!adap || !chan)
+		return;
+
+	mutex_lock(&adap->lock);
+	__mbox_adapter_release(adap, chan);
+	mutex_unlock(&adap->lock);
+}
+
+static int of_mbox_simple_xlate(struct mbox_adapter *adap,
+		const struct of_phandle_args *args)
+{
+	if (adap->of_n_cells < 1)
+		return -EINVAL;
+	if (args->args[0] >= adap->nchannels)
+		return -EINVAL;
+
+	return args->args[0];
+}
+
+static struct mbox_adapter *of_node_to_mbox_adapter(struct device_node *np)
+{
+	struct mbox_adapter *adap;
+
+	mutex_lock(&mbox_lock);
+	list_for_each_entry(adap, &mbox_adapters, list) {
+		if (adap->dev && adap->dev->of_node == np) {
+			mutex_unlock(&mbox_lock);
+			return adap;
+		}
+	}
+	mutex_unlock(&mbox_lock);
+
+	return ERR_PTR(-EPROBE_DEFER);
+}
+
+static void of_mbox_adapter_add(struct mbox_adapter *adap)
+{
+	if (!adap->dev)
+		return;
+
+	if (!adap->of_xlate) {
+		adap->of_xlate = of_mbox_simple_xlate;
+		adap->of_n_cells = 1;
+	}
+
+	of_node_get(adap->dev->of_node);
+}
+
+static void of_mbox_adapter_remove(struct mbox_adapter *adap)
+{
+	if (!adap->dev)
+		return;
+	of_node_put(adap->dev->of_node);
+}
+
+static struct mbox_channel *
+of_mbox_adapter_request_channel(struct device_node *np, const char *con_id)
+{
+	struct of_phandle_args args;
+	struct mbox_adapter *adap;
+	struct mbox_channel *chan;
+	int index = 0;
+	int rc;
+
+	if (con_id) {
+		index = of_property_match_string(np, "mbox-names", con_id);
+		if (index < 0)
+			return ERR_PTR(index);
+	}
+
+	rc = of_parse_phandle_with_args(np, "mbox", "#mbox-cells",
+			index, &args);
+	if (rc)
+		return ERR_PTR(rc);
+
+	adap = of_node_to_mbox_adapter(args.np);
+	if (IS_ERR(adap)) {
+		chan = ERR_CAST(adap);
+		goto out;
+	}
+
+	if (args.args_count != adap->of_n_cells) {
+		chan = ERR_PTR(-EINVAL);
+		goto out;
+	}
+
+	index = adap->of_xlate(adap, &args);
+	if (index < 0) {
+		chan = ERR_PTR(index);
+		goto out;
+	}
+
+	chan = mbox_adapter_request_channel(adap, index);
+
+out:
+	of_node_put(args.np);
+	return chan;
+}
+
+/**
+ * mbox_adapter_add() - register a new MBOX adapter
+ * @adap: the adapter to add
+ */
+int mbox_adapter_add(struct mbox_adapter *adap)
+{
+	struct mbox_channel *chan;
+	unsigned int i;
+
+	if (!adap || !adap->dev || !adap->ops || !adap->ops->put_message)
+		return -EINVAL;
+	if (adap->nchannels == 0)
+		return -EINVAL;
+
+	adap->channels = kzalloc(adap->nchannels * sizeof(*chan), GFP_KERNEL);
+	if (!adap->channels)
+		return -ENOMEM;
+
+	for (i = 0; i < adap->nchannels; ++i) {
+		chan = &adap->channels[i];
+		ATOMIC_INIT_NOTIFIER_HEAD(&chan->notifier);
+		chan->adapter = adap;
+		chan->id = i;
+	}
+	mutex_init(&adap->lock);
+
+	mutex_lock(&mbox_lock);
+	list_add(&adap->list, &mbox_adapters);
+
+	of_mbox_adapter_add(adap);
+	mutex_unlock(&mbox_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(mbox_adapter_add);
+
+/**
+ * mbox_adapter_remove() - unregisters a MBOX adapter
+ * @adap: the adapter to remove
+ *
+ * This function may return -EBUSY if the adapter provides a channel which
+ * is still requested.
+ */
+int mbox_adapter_remove(struct mbox_adapter *adap)
+{
+	unsigned int i;
+
+	mutex_lock(&mbox_lock);
+
+	for (i = 0; i < adap->nchannels; ++i) {
+		struct mbox_channel *chan = &adap->channels[i];
+		if (chan->users) {
+			mutex_unlock(&mbox_lock);
+			return -EBUSY;
+		}
+	}
+	list_del_init(&adap->list);
+
+	of_mbox_adapter_remove(adap);
+
+	mutex_unlock(&mbox_lock);
+
+	kfree(adap->channels);
+
+	return 0;
+}
+EXPORT_SYMBOL(mbox_adapter_remove);
+
+static int mbox_channel_put_message(struct mbox_channel *chan,
+		const void *data, unsigned int len)
+{
+	int rc;
+
+	mutex_lock(&chan->adapter->lock);
+	rc = chan->adapter->ops->put_message(chan->adapter, chan, data, len);
+	mutex_unlock(&chan->adapter->lock);
+
+	return rc;
+}
+
+/**
+ * mbox_channel_notify() - notify the core that a channel has a message
+ * @chan: the channel which has data
+ * @data: the location of said data
+ * @len: the length of specified data
+ *
+ * This function may be called from interrupt/no-sleep context.
+ */
+int mbox_channel_notify(struct mbox_channel *chan,
+		const void *data, unsigned int len)
+{
+	return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
+}
+EXPORT_SYMBOL(mbox_channel_notify);
+
+/**
+ * mbox_add_table() - add a lookup table for adapter consumers
+ * @table: array of consumers to register
+ * @num: number of consumers in array
+ */
+void __init mbox_add_table(struct mbox_lookup *table, unsigned int num)
+{
+	mutex_lock(&mbox_lookup_lock);
+	while (num--) {
+		if (table->provider && (table->dev_id || table->con_id))
+			list_add_tail(&table->list, &mbox_lookup_list);
+		table++;
+	}
+	mutex_unlock(&mbox_lookup_lock);
+}
+EXPORT_SYMBOL(mbox_add_table);
+
+static struct mbox_adapter *mbox_adapter_lookup(const char *name)
+{
+	struct mbox_adapter *adap;
+
+	if (!name)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&mbox_lock);
+	list_for_each_entry(adap, &mbox_adapters, list) {
+		const char *aname = dev_name(adap->dev);
+		if (aname && !strcmp(aname, name)) {
+			mutex_unlock(&mbox_lock);
+			return adap;
+		}
+	}
+	mutex_unlock(&mbox_lock);
+
+	return ERR_PTR(-ENODEV);
+}
+
+static struct mbox_adapter *
+mbox_channel_lookup(struct device *dev, const char *con_id, int *index)
+{
+	const char *dev_id = dev ? dev_name(dev) : NULL;
+	struct mbox_adapter *adap = ERR_PTR(-ENODEV);
+	struct mbox_lookup *lp;
+	int match, best;
+
+	best = 0;
+	mutex_lock(&mbox_lookup_lock);
+	list_for_each_entry(lp, &mbox_lookup_list, list) {
+		match = 0;
+		if (lp->dev_id) {
+			if (!dev_id || strcmp(lp->dev_id, dev_id))
+				continue;
+			match += 2;
+		}
+		if (lp->con_id) {
+			if (!con_id || strcmp(lp->con_id, con_id))
+				continue;
+			match += 1;
+		}
+		if (match <= best)
+			continue;
+		adap = mbox_adapter_lookup(lp->provider);
+		if (IS_ERR(adap))
+			continue;
+		if (index)
+			*index = lp->index;
+		if (match == 3)
+			break;
+		best = match;
+	}
+	mutex_unlock(&mbox_lookup_lock);
+
+	return adap;
+}
+
+/**
+ * struct mbox - MBOX channel consumer
+ * @chan: internal MBOX channel
+ * @nb: notifier to call on message available
+ */
+struct mbox {
+	struct mbox_channel *chan;
+	struct notifier_block *nb;
+};
+
+static struct mbox *
+__mbox_alloc(struct mbox_channel *chan, struct notifier_block *nb)
+{
+	struct mbox *mbox;
+
+	mbox = kzalloc(sizeof(*mbox), GFP_KERNEL);
+	if (mbox == NULL)
+		return ERR_PTR(-ENOMEM);
+	mbox->chan = chan;
+	mbox->nb = nb;
+
+	if (mbox->nb)
+		atomic_notifier_chain_register(&chan->notifier, mbox->nb);
+
+	return mbox;
+}
+
+/**
+ * mbox_request() - lookup and request a MBOX channel
+ * @dev: device for channel consumer
+ * @con_id: consumer name
+ * @nb: notifier block used for receiving messages
+ *
+ * The notifier is called as atomic on new messages, so you may not sleep
+ * in the notifier callback function.
+ */
+struct mbox *mbox_request(struct device *dev, const char *con_id,
+		struct notifier_block *nb)
+{
+	struct mbox_adapter *adap;
+	struct mbox_channel *chan;
+	struct mbox *mbox;
+	int index = 0;
+
+	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
+		return of_mbox_request(dev->of_node, con_id, nb);
+
+	adap = mbox_channel_lookup(dev, con_id, &index);
+	if (IS_ERR(adap))
+		return ERR_CAST(adap);
+
+	chan = mbox_adapter_request_channel(adap, index);
+	if (IS_ERR(chan))
+		return ERR_CAST(chan);
+
+	mbox = __mbox_alloc(chan, nb);
+	if (IS_ERR(mbox))
+		mbox_adapter_release_channel(adap, chan);
+	return mbox;
+}
+EXPORT_SYMBOL(mbox_request);
+
+/**
+ * mbox_release() - release a MBOX channel
+ * @mbox: the channel to release
+ *
+ * This releases a channel previously acquired by mbox_request(). Do not call
+ * this function on devm-allocated MBOX channels.
+ */
+void mbox_release(struct mbox *mbox)
+{
+	struct mbox_channel *chan = mbox->chan;
+	if (mbox->nb)
+		atomic_notifier_chain_unregister(&chan->notifier, mbox->nb);
+	mbox_adapter_release_channel(chan->adapter, chan);
+	kfree(mbox);
+}
+EXPORT_SYMBOL(mbox_release);
+
+/**
+ * mbox_put_message() - post a message to the MBOX channel
+ * @mbox: the channel to post to
+ * @data: location of the message
+ * @len: length of the message
+ *
+ * This function might sleep, and may not be called from interrupt context.
+ */
+int mbox_put_message(struct mbox *mbox, const void *data, unsigned int len)
+{
+	might_sleep();
+	return mbox_channel_put_message(mbox->chan, data, len);
+}
+EXPORT_SYMBOL(mbox_put_message);
+
+/**
+ * of_mbox_request() - lookup in DT and request a MBOX channel
+ * @np: device node for lookup
+ * @con_id: consumer id
+ * @nb: notifier block used for receiving messages
+ *
+ * The notifier is called as atomic on new messages, so you may not sleep
+ * in the notifier callback function.
+ */
+struct mbox *of_mbox_request(struct device_node *np, const char *con_id,
+		struct notifier_block *nb)
+{
+	struct mbox_channel *chan;
+	struct mbox *mbox;
+
+	chan = of_mbox_adapter_request_channel(np, con_id);
+	if (IS_ERR(chan))
+		return ERR_CAST(chan);
+
+	mbox = __mbox_alloc(chan, nb);
+	if (IS_ERR(mbox))
+		mbox_adapter_release_channel(chan->adapter, chan);
+	return mbox;
+}
+EXPORT_SYMBOL(of_mbox_request);
+
+static int devm_mbox_match(struct device *dev, void *res, void *data)
+{
+	struct mbox **p = res;
+
+	if (WARN_ON(!p || !*p))
+		return 0;
+
+	return *p == data;
+}
+
+static void __devm_mbox_release(struct device *dev, void *res)
+{
+	mbox_release(*(struct mbox **)res);
+}
+
+/**
+ * devm_mbox_release() - resource managed mbox_release()
+ * @dev: device for channel consumer
+ * @mbox: MBOX channel
+ *
+ * Release a MBOX channel previously allocated using devm_mbox_request() or
+ * devm_of_mbox_request().  Calling this function is usually not necessary,
+ * as devm-allocated resources are automatically released on driver detach.
+ */
+void devm_mbox_release(struct device *dev, struct mbox *mbox)
+{
+	WARN_ON(devres_release(dev, __devm_mbox_release,
+				devm_mbox_match, mbox));
+}
+EXPORT_SYMBOL(devm_mbox_release);
+
+/**
+ * devm_mbox_request() - resource managed mbox_request()
+ * @dev: device for channel consumer
+ * @con_id: consumer id
+ * @nb: notifier block used for receiving messages
+ *
+ * This function behaves like mbox_request() but the acquired channel
+ * will be automatically released on driver detach.
+ */
+struct mbox *devm_mbox_request(struct device *dev, const char *con_id,
+		struct notifier_block *nb)
+{
+	struct mbox **ptr;
+	struct mbox *mbox;
+
+	ptr = devres_alloc(__devm_mbox_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+	mbox = mbox_request(dev, con_id, nb);
+	if (!IS_ERR(mbox)) {
+		*ptr = mbox;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+	return mbox;
+}
+EXPORT_SYMBOL(devm_mbox_request);
+
+/**
+ * devm_mbox_request() - resource managed of_mbox_request()
+ * @dev: device for channel consumer
+ * @np: device node for lookup
+ * @con_id: consumer id
+ * @nb: notifier block used for receiving messages
+ *
+ * This function behaves like of_mbox_request() but the acquired channel
+ * will be automatically released on driver detach.
+ */
+struct mbox *devm_of_mbox_request(struct device *dev, struct device_node *np,
+		const char *con_id, struct notifier_block *nb)
+{
+	struct mbox **ptr;
+	struct mbox *mbox;
+
+	ptr = devres_alloc(__devm_mbox_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+	mbox = of_mbox_request(np, con_id, nb);
+	if (!IS_ERR(mbox)) {
+		*ptr = mbox;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+	return mbox;
+}
+EXPORT_SYMBOL(devm_of_mbox_request);
diff --git a/include/linux/mbox.h b/include/linux/mbox.h
new file mode 100644
index 0000000..d577b24
--- /dev/null
+++ b/include/linux/mbox.h
@@ -0,0 +1,175 @@
+#ifndef __LINUX_MBOX_H
+#define __LINUX_MBOX_H
+
+#include <linux/notifier.h>
+#include <linux/err.h>
+#include <linux/of.h>
+
+struct mbox_adapter;
+struct mbox_channel;
+
+/**
+ * struct mbox_adapter_ops - MBOX adapter operations
+ * @put_message: hook for putting messages in the channels MBOX
+ * @request: optional hook for requesting an MBOX channel
+ * @release: optional hook for releasing an MBOX channel
+ * @owner: helps prevent removal of modules exporting active MBOX channels
+ */
+struct mbox_adapter_ops {
+	int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
+				const void *, unsigned int);
+	int (*request)(struct mbox_adapter *, struct mbox_channel *);
+	int (*release)(struct mbox_adapter *, struct mbox_channel *);
+	struct module *owner;
+};
+
+struct mbox_channel {
+	unsigned int id;
+	unsigned int users;
+	struct mbox_adapter *adapter;
+	struct atomic_notifier_head notifier;
+};
+
+/**
+ * struct mbox_adapter - MBOX adapter abstraction
+ * @dev: device providing MBOX channels
+ * @ops: callback hooks for this adapter
+ * @nchannels: number of MBOX channels controlled by this adapter
+ * @channels: array of MBOX channels managed internally
+ * @of_xlate: OF translation hook for DT lookups
+ * @of_n_cells: number of cells for DT lookups
+ * @list: list node for internal use
+ * @lock: mutex for internal use
+ */
+struct mbox_adapter {
+	struct device *dev;
+	const struct mbox_adapter_ops *ops;
+	unsigned int nchannels;
+	struct mbox_channel *channels;
+	int (*of_xlate)(struct mbox_adapter *,
+			const struct of_phandle_args *args);
+	unsigned int of_n_cells;
+	struct list_head list;
+	struct mutex lock;
+};
+
+#if IS_ENABLED(CONFIG_MAILBOX)
+
+int mbox_adapter_add(struct mbox_adapter *adap);
+int mbox_adapter_remove(struct mbox_adapter *adap);
+
+int mbox_channel_notify(struct mbox_channel *chan,
+		const void *data, unsigned int len);
+
+#else
+
+static inline int mbox_adapter_add(struct mbox_adapter *adap)
+{
+	return -EINVAL;
+}
+static inline int mbox_adapter_remove(struct mbox_adapter *adap)
+{
+	return -EINVAL;
+}
+
+static inline int mbox_channel_notify(struct mbox_channel *chan,
+		const void *data, unsigned int len)
+{
+	return -EINVAL;
+}
+
+#endif
+
+struct mbox;
+
+#if IS_ENABLED(CONFIG_MAILBOX)
+
+struct mbox *mbox_request(struct device *dev, const char *con_id,
+		struct notifier_block *nb);
+void mbox_release(struct mbox *mbox);
+
+int mbox_put_message(struct mbox *mbox, const void *data, unsigned int len);
+
+struct mbox *of_mbox_request(struct device_node *np, const char *con_id,
+		struct notifier_block *nb);
+
+struct mbox *devm_mbox_request(struct device *dev, const char *con_id,
+		struct notifier_block *nb);
+
+struct mbox *devm_of_mbox_request(struct device *dev, struct device_node *np,
+		const char *con_id, struct notifier_block *nb);
+
+void devm_mbox_release(struct device *dev, struct mbox *mbox);
+
+#else
+
+static inline struct mbox *mbox_request(struct device *dev, const char *con_id,
+		struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+void mbox_release(struct mbox *mbox)
+{
+}
+
+static inline int mbox_put_message(struct mbox *mbox,
+		const void *data, unsigned int len)
+{
+	return -EINVAL;
+}
+
+static inline struct mbox *of_mbox_request(struct device_node *np,
+		const char *con_id, struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline struct mbox *devm_mbox_request(struct device *dev,
+		const char *con_id, struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline struct mbox *devm_of_mbox_request(struct device *dev,
+		struct device_node *np, const char *con_id,
+		struct notifier_block *nb)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+static inline void devm_mbox_release(struct device *dev, struct mbox *mbox)
+{
+}
+
+#endif
+
+struct mbox_lookup {
+	struct list_head list;
+	const char *provider;
+	unsigned int index;
+	const char *dev_id;
+	const char *con_id;
+};
+
+#define MBOX_LOOKUP(_provider, _index, _dev_id, _con_id)	\
+	{							\
+		.provider = _provider,				\
+		.index = _index,				\
+		.dev_id = _dev_id,				\
+		.con_id = _con_id,				\
+	}
+
+#if IS_ENABLED(CONFIG_MAILBOX)
+
+void mbox_add_table(struct mbox_lookup *table, unsigned int num);
+
+#else
+
+static inline void mbox_add_table(struct mbox_lookup *table, unsigned int num)
+{
+}
+
+#endif
+
+#endif /* __LINUX_MBOX_H */
-- 
1.8.1.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/6] mailbox: add core framework

[Arnd Bergmann]

On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> The mailbox drivers are fragmented, and some implement their own core.
> Unify the drivers and implement common functionality in a framework.
> 
> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>

This seems pretty cool overall, great to see someone getting at it@

> +static void of_mbox_adapter_add(struct mbox_adapter *adap)
> +{
> +	if (!adap->dev)
> +		return;
> +
> +	if (!adap->of_xlate) {
> +		adap->of_xlate = of_mbox_simple_xlate;
> +		adap->of_n_cells = 1;
> +	}
> +
> +	of_node_get(adap->dev->of_node);
> +}

You should probably check if of_n_cells matches the device node
#mbox-cells value, otherwise the xlate function will get confused.

> +
> +	mutex_lock(&mbox_lock);
> +	list_add(&adap->list, &mbox_adapters);
> +
> +	of_mbox_adapter_add(adap);
> +	mutex_unlock(&mbox_lock);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mbox_adapter_add);

Please use EXPORT_SYMBOL_GPL here and elsewhere.

> +/**
> + * mbox_channel_notify() - notify the core that a channel has a message
> + * @chan: the channel which has data
> + * @data: the location of said data
> + * @len: the length of specified data
> + *
> + * This function may be called from interrupt/no-sleep context.
> + */
> +int mbox_channel_notify(struct mbox_channel *chan,
> +		const void *data, unsigned int len)
> +{
> +	return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> +}
> +EXPORT_SYMBOL(mbox_channel_notify);

What is the reason to use a notifier chain here? Isn't a simple
callback function pointer enough? I would expect that each mailbox
can have exactly one consumer, not multiple ones.

> +/**
> + * mbox_add_table() - add a lookup table for adapter consumers
> + * @table: array of consumers to register
> + * @num: number of consumers in array
> + */
> +void __init mbox_add_table(struct mbox_lookup *table, unsigned int num)
> +{
> +	mutex_lock(&mbox_lookup_lock);
> +	while (num--) {
> +		if (table->provider && (table->dev_id || table->con_id))
> +			list_add_tail(&table->list, &mbox_lookup_list);
> +		table++;
> +	}
> +	mutex_unlock(&mbox_lookup_lock);
> +}
> +EXPORT_SYMBOL(mbox_add_table);

I don't understand this part of the API. Why do you need a separate
lookup table here? Isn't that what the DT lookup does already?

> +/**
> + * mbox_request() - lookup and request a MBOX channel
> + * @dev: device for channel consumer
> + * @con_id: consumer name
> + * @nb: notifier block used for receiving messages
> + *
> + * The notifier is called as atomic on new messages, so you may not sleep
> + * in the notifier callback function.
> + */
> +struct mbox *mbox_request(struct device *dev, const char *con_id,
> +		struct notifier_block *nb)
> +{
> +	struct mbox_adapter *adap;
> +	struct mbox_channel *chan;
> +	struct mbox *mbox;
> +	int index = 0;
> +
> +	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> +		return of_mbox_request(dev->of_node, con_id, nb);

What use case do you have in mind for !CONFIG_OF?

> +/**
> + * struct mbox_adapter_ops - MBOX adapter operations
> + * @put_message: hook for putting messages in the channels MBOX
> + * @request: optional hook for requesting an MBOX channel
> + * @release: optional hook for releasing an MBOX channel
> + * @owner: helps prevent removal of modules exporting active MBOX channels
> + */
> +struct mbox_adapter_ops {
> +	int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
> +				const void *, unsigned int);
> +	int (*request)(struct mbox_adapter *, struct mbox_channel *);
> +	int (*release)(struct mbox_adapter *, struct mbox_channel *);
> +	struct module *owner;
> +};

I think we will need a peek_message() callback for the upcoming
QMTM driver, to allow client drivers to get a message out before
the mailbox driver gets an IRQ. This will be used for IRQ mitigation
in the network driver.

	Arnd

Re: [RFC 1/6] mailbox: add core framework

[Courtney Cavin]

On Mon, Feb 10, 2014 at 03:11:00PM +0100, Arnd Bergmann wrote:
> On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> > The mailbox drivers are fragmented, and some implement their own core.
> > Unify the drivers and implement common functionality in a framework.
> > 
> > Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> 
> This seems pretty cool overall, great to see someone getting at it@

I'm glad to hear that there's some interest!

> > +static void of_mbox_adapter_add(struct mbox_adapter *adap)
> > +{
> > +	if (!adap->dev)
> > +		return;
> > +
> > +	if (!adap->of_xlate) {
> > +		adap->of_xlate = of_mbox_simple_xlate;
> > +		adap->of_n_cells = 1;
> > +	}
> > +
> > +	of_node_get(adap->dev->of_node);
> > +}
> 
> You should probably check if of_n_cells matches the device node
> #mbox-cells value, otherwise the xlate function will get confused.

Ok.  I was under the impression that the adapter implementations would
add something like that, but I see no reason why putting it here would
hurt.

> > +
> > +	mutex_lock(&mbox_lock);
> > +	list_add(&adap->list, &mbox_adapters);
> > +
> > +	of_mbox_adapter_add(adap);
> > +	mutex_unlock(&mbox_lock);
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(mbox_adapter_add);
> 
> Please use EXPORT_SYMBOL_GPL here and elsewhere.

Ok.

> > +/**
> > + * mbox_channel_notify() - notify the core that a channel has a message
> > + * @chan: the channel which has data
> > + * @data: the location of said data
> > + * @len: the length of specified data
> > + *
> > + * This function may be called from interrupt/no-sleep context.
> > + */
> > +int mbox_channel_notify(struct mbox_channel *chan,
> > +		const void *data, unsigned int len)
> > +{
> > +	return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> > +}
> > +EXPORT_SYMBOL(mbox_channel_notify);
> 
> What is the reason to use a notifier chain here? Isn't a simple
> callback function pointer enough? I would expect that each mailbox
> can have exactly one consumer, not multiple ones.

Mostly because I didn't see a reason not to.  While a callback function
(and private data) would probably be sufficient, I don't see a specific
reason why a mailbox cannot have multiple consumers, and the API
currently is designed around that concept.

> > +/**
> > + * mbox_add_table() - add a lookup table for adapter consumers
> > + * @table: array of consumers to register
> > + * @num: number of consumers in array
> > + */
> > +void __init mbox_add_table(struct mbox_lookup *table, unsigned int num)
> > +{
> > +	mutex_lock(&mbox_lookup_lock);
> > +	while (num--) {
> > +		if (table->provider && (table->dev_id || table->con_id))
> > +			list_add_tail(&table->list, &mbox_lookup_list);
> > +		table++;
> > +	}
> > +	mutex_unlock(&mbox_lookup_lock);
> > +}
> > +EXPORT_SYMBOL(mbox_add_table);
> 
> I don't understand this part of the API. Why do you need a separate
> lookup table here? Isn't that what the DT lookup does already?

It is.  The lookup/table stuff here is specifically for non-DT-based
mailboxes.

> > +/**
> > + * mbox_request() - lookup and request a MBOX channel
> > + * @dev: device for channel consumer
> > + * @con_id: consumer name
> > + * @nb: notifier block used for receiving messages
> > + *
> > + * The notifier is called as atomic on new messages, so you may not sleep
> > + * in the notifier callback function.
> > + */
> > +struct mbox *mbox_request(struct device *dev, const char *con_id,
> > +		struct notifier_block *nb)
> > +{
> > +	struct mbox_adapter *adap;
> > +	struct mbox_channel *chan;
> > +	struct mbox *mbox;
> > +	int index = 0;
> > +
> > +	if (IS_ENABLED(CONFIG_OF) && dev && dev->of_node)
> > +		return of_mbox_request(dev->of_node, con_id, nb);
> 
> What use case do you have in mind for !CONFIG_OF?

None particularly, except for the existing implementations in
drivers/mailbox.  I simply presumed it wouldn't hurt to implement lookup
tables similar to those the pwm core.

> > +/**
> > + * struct mbox_adapter_ops - MBOX adapter operations
> > + * @put_message: hook for putting messages in the channels MBOX
> > + * @request: optional hook for requesting an MBOX channel
> > + * @release: optional hook for releasing an MBOX channel
> > + * @owner: helps prevent removal of modules exporting active MBOX channels
> > + */
> > +struct mbox_adapter_ops {
> > +	int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
> > +				const void *, unsigned int);
> > +	int (*request)(struct mbox_adapter *, struct mbox_channel *);
> > +	int (*release)(struct mbox_adapter *, struct mbox_channel *);
> > +	struct module *owner;
> > +};
> 
> I think we will need a peek_message() callback for the upcoming
> QMTM driver, to allow client drivers to get a message out before
> the mailbox driver gets an IRQ. This will be used for IRQ mitigation
> in the network driver.

Eeek!  I'm not very fond of 'peek' functions, but I guess I can see a
reason for IRQ mitigation here.  I still cannot help but to try to think
my way out of implementing peek.

What would be the callback flow here?  There's no guarantee that a
mailbox implementation isn't implemented over a sleepy bus, which would
render peek somewhat useless.  Additionally, we have the adapter
protection mutex which can sleep anyway.  This means that a consumer can
not call peek from anywhere atomic, including a notifier, which I think
is your use-case.

Perhaps a FEED_ME return from a notifier, requesting more 'mail' if
available?

> 
> 	Arnd

Thanks for looking!  I appreciate the feedback.

-Courtney

Re: [RFC 1/6] mailbox: add core framework

[Rob Herring]

On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
>> The mailbox drivers are fragmented, and some implement their own core.
>> Unify the drivers and implement common functionality in a framework.
>>
>> Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>

[snip]

>> +int mbox_channel_notify(struct mbox_channel *chan,
>> +             const void *data, unsigned int len)
>> +{
>> +     return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
>> +}
>> +EXPORT_SYMBOL(mbox_channel_notify);
>
> What is the reason to use a notifier chain here? Isn't a simple
> callback function pointer enough? I would expect that each mailbox
> can have exactly one consumer, not multiple ones.

It probably can be a callback, but there can be multiple consumers. It
was only a notifier on the pl320 as there was no framework at the time
and to avoid creating custom interfaces between drivers. On highbank
for example, we can asynchronously receive the events for temperature
change, power off, and reset. So either there needs to be an event
demux somewhere or callbacks have to return whether they handled an
event or not.

Rob

Re: [RFC 1/6] mailbox: add core framework

[Josh Cartwright]

On Mon, Feb 10, 2014 at 11:52:05AM -0600, Rob Herring wrote:
> On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
[..]
> >> +int mbox_channel_notify(struct mbox_channel *chan,
> >> +             const void *data, unsigned int len)
> >> +{
> >> +     return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> >> +}
> >> +EXPORT_SYMBOL(mbox_channel_notify);
> >
> > What is the reason to use a notifier chain here? Isn't a simple
> > callback function pointer enough? I would expect that each mailbox
> > can have exactly one consumer, not multiple ones.
> 
> It probably can be a callback, but there can be multiple consumers. It
> was only a notifier on the pl320 as there was no framework at the time
> and to avoid creating custom interfaces between drivers. On highbank
> for example, we can asynchronously receive the events for temperature
> change, power off, and reset. So either there needs to be an event
> demux somewhere or callbacks have to return whether they handled an
> event or not.

I'm not familiar with highbank IPC, but with these requirements should
the mailbox core even bother with asynchronous notifier chain?  It
sounds like a better fit might be for the mailbox core to implement a
proper adapter-specific irqdomain and used a chained irq handler to
demux (or have consumers request with IRQF_SHARED in the shared case).

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [RFC 1/6] mailbox: add core framework

[Courtney Cavin]

On Mon, Feb 10, 2014 at 08:09:34PM +0100, Josh Cartwright wrote:
> On Mon, Feb 10, 2014 at 11:52:05AM -0600, Rob Herring wrote:
> > On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> [..]
> > >> +int mbox_channel_notify(struct mbox_channel *chan,
> > >> +             const void *data, unsigned int len)
> > >> +{
> > >> +     return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> > >> +}
> > >> +EXPORT_SYMBOL(mbox_channel_notify);
> > >
> > > What is the reason to use a notifier chain here? Isn't a simple
> > > callback function pointer enough? I would expect that each mailbox
> > > can have exactly one consumer, not multiple ones.
> > 
> > It probably can be a callback, but there can be multiple consumers. It
> > was only a notifier on the pl320 as there was no framework at the time
> > and to avoid creating custom interfaces between drivers. On highbank
> > for example, we can asynchronously receive the events for temperature
> > change, power off, and reset. So either there needs to be an event
> > demux somewhere or callbacks have to return whether they handled an
> > event or not.
> 
> I'm not familiar with highbank IPC, but with these requirements should
> the mailbox core even bother with asynchronous notifier chain?  It
> sounds like a better fit might be for the mailbox core to implement a
> proper adapter-specific irqdomain and used a chained irq handler to
> demux (or have consumers request with IRQF_SHARED in the shared case).

Although modeling this using irqdomains makes sense for the notification
bit, and would probably suit most adapters, there's the issue of data
being passed around which doesn't quite fit.  "Ok, I have mail... where
is it?"  Did you have something in mind for that?

Frankly, I don't see the notifier chain as being extraneous or
over-complicated here core-wise or implementation-wise, and unless I
understand Rob incorrectly, should suit the existing use-cases.  Am I
missing something?

-Courtney

Re: [RFC 1/6] mailbox: add core framework

[Rob Herring]

On Mon, Feb 10, 2014 at 1:59 PM, Courtney Cavin
<courtney.cavin@sonymobile.com> wrote:
> On Mon, Feb 10, 2014 at 08:09:34PM +0100, Josh Cartwright wrote:
>> On Mon, Feb 10, 2014 at 11:52:05AM -0600, Rob Herring wrote:
>> > On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > > On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
>> [..]
>> > >> +int mbox_channel_notify(struct mbox_channel *chan,
>> > >> +             const void *data, unsigned int len)
>> > >> +{
>> > >> +     return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
>> > >> +}
>> > >> +EXPORT_SYMBOL(mbox_channel_notify);
>> > >
>> > > What is the reason to use a notifier chain here? Isn't a simple
>> > > callback function pointer enough? I would expect that each mailbox
>> > > can have exactly one consumer, not multiple ones.
>> >
>> > It probably can be a callback, but there can be multiple consumers. It
>> > was only a notifier on the pl320 as there was no framework at the time
>> > and to avoid creating custom interfaces between drivers. On highbank
>> > for example, we can asynchronously receive the events for temperature
>> > change, power off, and reset. So either there needs to be an event
>> > demux somewhere or callbacks have to return whether they handled an
>> > event or not.
>>
>> I'm not familiar with highbank IPC, but with these requirements should
>> the mailbox core even bother with asynchronous notifier chain?  It
>> sounds like a better fit might be for the mailbox core to implement a
>> proper adapter-specific irqdomain and used a chained irq handler to
>> demux (or have consumers request with IRQF_SHARED in the shared case).
>
> Although modeling this using irqdomains makes sense for the notification
> bit, and would probably suit most adapters, there's the issue of data
> being passed around which doesn't quite fit.  "Ok, I have mail... where
> is it?"  Did you have something in mind for that?
>
> Frankly, I don't see the notifier chain as being extraneous or
> over-complicated here core-wise or implementation-wise, and unless I
> understand Rob incorrectly, should suit the existing use-cases.  Am I
> missing something?

Well, I think notifiers are not liked very much. I don't know that irq
handlers would be the right answer either as these are not h/w events
really and we may not want handlers to run in irq context. I would say
a callback similar to how the dma engine framework works is the right
answer. On the send side, you may want to have completion callbacks as
well.

Rob

Re: [RFC 1/6] mailbox: add core framework

[Courtney Cavin]

On Mon, Feb 10, 2014 at 09:45:07PM +0100, Rob Herring wrote:
> On Mon, Feb 10, 2014 at 1:59 PM, Courtney Cavin
> <courtney.cavin@sonymobile.com> wrote:
> > On Mon, Feb 10, 2014 at 08:09:34PM +0100, Josh Cartwright wrote:
> >> On Mon, Feb 10, 2014 at 11:52:05AM -0600, Rob Herring wrote:
> >> > On Mon, Feb 10, 2014 at 8:11 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > > On Friday 07 February 2014 16:50:14 Courtney Cavin wrote:
> >> [..]
> >> > >> +int mbox_channel_notify(struct mbox_channel *chan,
> >> > >> +             const void *data, unsigned int len)
> >> > >> +{
> >> > >> +     return atomic_notifier_call_chain(&chan->notifier, len, (void *)data);
> >> > >> +}
> >> > >> +EXPORT_SYMBOL(mbox_channel_notify);
> >> > >
> >> > > What is the reason to use a notifier chain here? Isn't a simple
> >> > > callback function pointer enough? I would expect that each mailbox
> >> > > can have exactly one consumer, not multiple ones.
> >> >
> >> > It probably can be a callback, but there can be multiple consumers. It
> >> > was only a notifier on the pl320 as there was no framework at the time
> >> > and to avoid creating custom interfaces between drivers. On highbank
> >> > for example, we can asynchronously receive the events for temperature
> >> > change, power off, and reset. So either there needs to be an event
> >> > demux somewhere or callbacks have to return whether they handled an
> >> > event or not.
> >>
> >> I'm not familiar with highbank IPC, but with these requirements should
> >> the mailbox core even bother with asynchronous notifier chain?  It
> >> sounds like a better fit might be for the mailbox core to implement a
> >> proper adapter-specific irqdomain and used a chained irq handler to
> >> demux (or have consumers request with IRQF_SHARED in the shared case).
> >
> > Although modeling this using irqdomains makes sense for the notification
> > bit, and would probably suit most adapters, there's the issue of data
> > being passed around which doesn't quite fit.  "Ok, I have mail... where
> > is it?"  Did you have something in mind for that?
> >
> > Frankly, I don't see the notifier chain as being extraneous or
> > over-complicated here core-wise or implementation-wise, and unless I
> > understand Rob incorrectly, should suit the existing use-cases.  Am I
> > missing something?
> 
> Well, I think notifiers are not liked very much. I don't know that irq
> handlers would be the right answer either as these are not h/w events
> really and we may not want handlers to run in irq context. I would say
> a callback similar to how the dma engine framework works is the right
> answer. On the send side, you may want to have completion callbacks as
> well.

While I'm not sure the dislike of notifiers entirely justifies not using
them here, where they seem to make sense, I can understand that they
might not fully implement what we need to expose.

Regarding handlers running in IRQ context: currently the API is designed
to do just that.  From the use-cases I've found, most message handlers
simply queue something to happen at a later point.  This is logical, as
the callback will be async, so you'll need to swap contexts or add locks
in your consumer anyway.

The dma engine is large and confused, so I'm not sure entirely which
part you are refering to.  I've looked at having async completion going
both ways, but what I see every time is code complication in both the
adapter and in the consumers in the simple use-case.  It doesn't really
make sense to make an API which makes things so generic that it becomes
hard to use.  What I tried to follow here when designing the API was
what I saw in the actual implementations, not what was future-proof:
	- Message receive callbacks may be called from IRQ context
	- Message send implementations may sleep

I think that these allowances enable the simple use-case to be very easy
to write, and the more complex use-cases still possible--albiet
sometimes at a higher level.

What I can do is try to alleviate implementation efforts of future
requirements--as honestly, we can't really say exactly what they are--by
turning the messages into structs themselves, so at a later point flags,
ack callbacks, and rainbows can be added.  I can then stop using
notifiers, and re-invent that functionality with a mbox_ prefix.

Comments?

-Courtney

Re: [RFC 1/6] mailbox: add core framework

[Arnd Bergmann]

On Monday 10 February 2014 16:23:48 Courtney Cavin wrote:

> While I'm not sure the dislike of notifiers entirely justifies not using
> them here, where they seem to make sense, I can understand that they
> might not fully implement what we need to expose.

I think we need to look at a few more examples of things that people
want to with the framework to come up with a good interface. It's
possible that we end up with multiple alternative notification
methods, but it would be good to come up with one that works well
for everyone.

> Regarding handlers running in IRQ context: currently the API is designed
> to do just that.  From the use-cases I've found, most message handlers
> simply queue something to happen at a later point.  This is logical, as
> the callback will be async, so you'll need to swap contexts or add locks
> in your consumer anyway.

Right. You may also have some handlers that need to run with extreme
latency constraints, so we need at least the option of getting the
callback from hardirq context, or possibly from softirq/tasklet
as in the dmaengine case.

> The dma engine is large and confused, so I'm not sure entirely which
> part you are refering to.  I've looked at having async completion going
> both ways, but what I see every time is code complication in both the
> adapter and in the consumers in the simple use-case.  It doesn't really
> make sense to make an API which makes things so generic that it becomes
> hard to use.  What I tried to follow here when designing the API was
> what I saw in the actual implementations, not what was future-proof:
> 	- Message receive callbacks may be called from IRQ context
> 	- Message send implementations may sleep

I can imagine cases where you want to send messages from softirq
context, or from the same context in which you received the incoming
mail, so it would be good to have the API flexible enough to deal
with that. As a first step, always allowing send to sleep seems
fine. Alternatively, we could have a 'sync' flag in the send
API, to choose between "arrange this message to be sent out as
soon as possible, but don't sleep" and "send this message and
make sure it has arrived at the other end" as you do now.

> What I can do is try to alleviate implementation efforts of future
> requirements--as honestly, we can't really say exactly what they are--by
> turning the messages into structs themselves, so at a later point flags,
> ack callbacks, and rainbows can be added.  I can then stop using
> notifiers, and re-invent that functionality with a mbox_ prefix.

I don't think there is a point in reimplementing notifiers under a
different name. The question is rather how we want to deal with
the 'multiple listener' case. If this case is the exception rather
than the rule, I'd prefer making the callback API handle only
single listeners and adding higher-level abstractions for the
cases where we do need multiple listeners, but it really depends
on what people need.

	Arnd

Re: [RFC 1/6] mailbox: add core framework

[Courtney Cavin]

On Tue, Feb 11, 2014 at 09:35:01AM +0100, Arnd Bergmann wrote:
> On Monday 10 February 2014 16:23:48 Courtney Cavin wrote:
> 
> > While I'm not sure the dislike of notifiers entirely justifies not using
> > them here, where they seem to make sense, I can understand that they
> > might not fully implement what we need to expose.
> 
> I think we need to look at a few more examples of things that people
> want to with the framework to come up with a good interface. It's
> possible that we end up with multiple alternative notification
> methods, but it would be good to come up with one that works well
> for everyone.
> 
> > Regarding handlers running in IRQ context: currently the API is designed
> > to do just that.  From the use-cases I've found, most message handlers
> > simply queue something to happen at a later point.  This is logical, as
> > the callback will be async, so you'll need to swap contexts or add locks
> > in your consumer anyway.
> 
> Right. You may also have some handlers that need to run with extreme
> latency constraints, so we need at least the option of getting the
> callback from hardirq context, or possibly from softirq/tasklet
> as in the dmaengine case.
> 
> > The dma engine is large and confused, so I'm not sure entirely which
> > part you are refering to.  I've looked at having async completion going
> > both ways, but what I see every time is code complication in both the
> > adapter and in the consumers in the simple use-case.  It doesn't really
> > make sense to make an API which makes things so generic that it becomes
> > hard to use.  What I tried to follow here when designing the API was
> > what I saw in the actual implementations, not what was future-proof:
> > 	- Message receive callbacks may be called from IRQ context
> > 	- Message send implementations may sleep
> 
> I can imagine cases where you want to send messages from softirq
> context, or from the same context in which you received the incoming
> mail, so it would be good to have the API flexible enough to deal
> with that. As a first step, always allowing send to sleep seems
> fine. Alternatively, we could have a 'sync' flag in the send
> API, to choose between "arrange this message to be sent out as
> soon as possible, but don't sleep" and "send this message and
> make sure it has arrived at the other end" as you do now.

Although I'm not sure there's currently a requirement for this, I can see that
this could be needed in the future.

> > What I can do is try to alleviate implementation efforts of future
> > requirements--as honestly, we can't really say exactly what they are--by
> > turning the messages into structs themselves, so at a later point flags,
> > ack callbacks, and rainbows can be added.  I can then stop using
> > notifiers, and re-invent that functionality with a mbox_ prefix.
> 
> I don't think there is a point in reimplementing notifiers under a
> different name. The question is rather how we want to deal with
> the 'multiple listener' case. If this case is the exception rather
> than the rule, I'd prefer making the callback API handle only
> single listeners and adding higher-level abstractions for the
> cases where we do need multiple listeners, but it really depends
> on what people need.

I wasn't actually planning on directly ripping-off notifiers under a new name.
Rather, as switching to message structs is probably a good idea anyway, I don't
think the notifier API properly represents that,... the void pointer is a bit
vague.  It could be that it would turn out as a wrapper around notifiers.  As
you said though, I do think this is the exception, so I'm fine with the single
callback idea, as long as Rob and Suman agree that this will be suitable for
their use-cases.

Then again, I think that the context management stuff is the exception as well,
and I think that can/should also be handled in a higher level.  Regardless, I
went ahead and drafted the async flags idea out anyway, so here's some
pseudo-code.  I also tried to shoe-horn in 'peek', and you can see how that
turns out.  Let me know if this is something like what you had in mind.

enum {
	MBOX_ASYNC	= BIT(0),
};

struct mbox_adapter_ops {
	...
	int (*put_message)(struct mbox_adapter *, struct mbox_channel *,
			const struct mbox_message *msg)
	int (*peek_message)(struct mbox_adapter *, struct mbox_channel *,
			struct mbox_message *msg)
};

struct mbox;

#define MBOX_FIFO_SIZE PAGE_SZ /* or not? */

struct mbox_channel {
	...
	unsigned long flags; /* MBOX_ASYNC, for now */
	struct mbox *consumer;
	struct kfifo_rec_ptr_1 rx_fifo;
	/**
	 * probably should be allocated only if user needs to call
	 * mbox_put_message with MBOX_ASYNC, instead of statically.
	 */
	STRUCT_KFIFO_REC_1(MBOX_FIFO_SIZE) tx_fifo;
	struct work_struct rx_work;
	struct work_struct tx_work;
}

struct mbox {
	struct mbox_channel *chan;
	struct mbox_adapter *adapter;
	void (*cb)(void *, struct mbox *, const struct mbox_message *);
	void *priv;
};

struct mbox_message {
	void *data;
	unsigned int len;
};

static void rx_work(struct work_struct *work)
{
	struct mbox_channel *chan;
	struct mbox_message msg;

	chan = container_of(work, struct mbox_channel, rx_work);
	msg.len = kfifo_out(&chan->rx_fifo, msg.data);
	chan->consumer->cb(chan->consumer, &msg);
}

/* called from adapter, typically in a IRQ handler */
int mbox_channel_notify(struct mbox_channel *chan,
		const struct mbox_message *msg)
{
	if (chan->flags & MBOX_ASYNC) {
		kfifo_in(&chan->rx_fifo, msg->data, msg->len);
		schedule_work(&chan->rx_work);
		return 0;
	}

	/*
	 * consumer may not sleep here, as they did not specify this
	 * requirement in channel flags when requesting
	 */
	return chan->consumer->cb(chan->consumer->priv, chan->consumer, msg);
}

static void tx_work(struct work_struct *work)
{
	struct mbox_channel *chan;
	struct mbox_message msg;
	char buf[PAGE_SZ]; /* should max size be specified by the adapter? */

	chan = container_of(work, struct mbox_channel, tx_work);
	msg.len = kfifo_out(&chan->tx_fifo, buf, sizeof(buf));
	msg.data = buf;

	mutex_lock(&chan->adapter->lock);
	chan->adapter->ops->put_message(chan->adapter, chan, &msg);
	mutex_unlock(&chan->adapter->lock);
}

/* called from consumer */
int mbox_put_message(struct mbox *mbox, const struct mbox_message *msg,
		unsigned long flags)
{
	int rc;

	if (flags & MBOX_ASYNC) {
		kfifo_in(&chan->tx_fifo, msg->data, msg->len);
		schedule_work(&mbox->chan->tx_work);
		return 0;
	}

	/**
	 * obviously, if not because of the lock, then because the adapter
	 * should wait for an ACK from it's controller if appropriate.
	 */
	might_sleep();

	mutex_lock(&mbox->adapter->lock);
	rc = mbox->adapter->ops->put_message(mbox->adapter, mbox->chan, msg);
	mutex_unlock(&mbox->adapter->lock);

	return rc;
}

/* called from consumer; illustrates the problem with peek */
int mbox_peek_message(struct mbox *mbox, struct mbox_message *msg)
{

	int rc;
	if (chan->flags & MBOX_ASYNC) {
		msg->len = kfifo_out_peek(&mbox->chan->rx_fifo,
				msg->data, msg->len);
		return 0;
	}

	/**
	 * It is unlikely that most adapters are able to properly implement
	 * this, so we have to allow for that.
	 */
	if (!mbox->adapter->ops->peek_message)
		return -EOPNOTSUPP;

	/**
	 * so this is where this lock makes things difficult, as this function
	 * might_sleep(), but only really because of the lock.  Either we can
	 * remove the lock and force the adapter to do its own locking
	 * spinlock-style, or we can accept the sleep here, which seems a bit
	 * stupid in a peek function.  Neither option is good.  Additionally,
	 * there's no guarantee that the adapter doesn't operate over a bus
	 * which itself might_sleep(), exacerbating the problem.
	 */
	mutex_lock(&mbox->adapter->lock);
	rc = mbox->adapter->ops->peek_message(mbox->adapter, mbox->chan, msg);
	mutex_lock(&mbox->adapter->lock);

	return rc;
}

struct mbox *mbox_request(struct device *dev, const char *con_id,
		void (*receive)(void *, struct mbox *, const struct mbox_message *),
		void *priv, unsigned long flags)
{
	struct mbox_channel *chan;
	struct mbox *mbox;
	int rc;
	...
	chan->flags = flags;
	if (flags & MBOX_ASYNC) {
		rc = kfifo_alloc(&chan->rx_fifo, MBOX_FIFO_SIZE, GFP_KERNEL);
		if (rc)
			return rc;
		INIT_WORK(&chan->rx_work, rx_work);
	}
	if (1 /* consumer plans on calling put_message with MBOX_ASYNC */) {
		INIT_KFIFO(chan->tx_fifo);
		INIT_WORK(&chan->tx_work, tx_work);
	}
	chan->consumer = mbox;
	mbox->cb = receive;
	mbox->priv = priv;
	...
	return mbox;
}

Re: [RFC 1/6] mailbox: add core framework

[Arnd Bergmann]

On Wednesday 12 February 2014, Courtney Cavin wrote:
> On Tue, Feb 11, 2014 at 09:35:01AM +0100, Arnd Bergmann wrote:
> > On Monday 10 February 2014 16:23:48 Courtney Cavin wrote:

> Then again, I think that the context management stuff is the exception as well,
> and I think that can/should also be handled in a higher level.  Regardless, I
> went ahead and drafted the async flags idea out anyway, so here's some
> pseudo-code.  I also tried to shoe-horn in 'peek', and you can see how that
> turns out.  Let me know if this is something like what you had in mind.

The async implementation looks good to me, assuming we actually need both
sync and async operations, which I can't tell for sure.

For the peek operation, it wouldn't work for the ethernet case, which
has to call it from atomic context in net_rx_action.

> 	/**
> 	 * so this is where this lock makes things difficult, as this function
> 	 * might_sleep(), but only really because of the lock.  Either we can
> 	 * remove the lock and force the adapter to do its own locking
> 	 * spinlock-style, or we can accept the sleep here, which seems a bit
> 	 * stupid in a peek function.  Neither option is good.  Additionally,
> 	 * there's no guarantee that the adapter doesn't operate over a bus
> 	 * which itself might_sleep(), exacerbating the problem.
> 	 */
> 	mutex_lock(&mbox->adapter->lock);
> 	rc = mbox->adapter->ops->peek_message(mbox->adapter, mbox->chan, msg);
> 	mutex_lock(&mbox->adapter->lock);

If we decide that peek() must not sleep, any driver that operates on a
slow bus could just always report "no data" here.
 
Moving the locking into the mbox driver here sounds appropriate.

	Arnd

Re: [RFC 1/6] mailbox: add core framework

[Courtney Cavin]

On Fri, Feb 14, 2014 at 08:48:25PM +0100, Arnd Bergmann wrote:
> On Wednesday 12 February 2014, Courtney Cavin wrote:
> > On Tue, Feb 11, 2014 at 09:35:01AM +0100, Arnd Bergmann wrote:
> > > On Monday 10 February 2014 16:23:48 Courtney Cavin wrote:
> 
> > Then again, I think that the context management stuff is the exception as well,
> > and I think that can/should also be handled in a higher level.  Regardless, I
> > went ahead and drafted the async flags idea out anyway, so here's some
> > pseudo-code.  I also tried to shoe-horn in 'peek', and you can see how that
> > turns out.  Let me know if this is something like what you had in mind.
> 
> The async implementation looks good to me, assuming we actually need both
> sync and async operations, which I can't tell for sure.

Yea, I would like some further input on that specifically.  I have added
Linus Walleij and Jassi Brar, who have had good input on mailboxes in
the past, and somehow I missed in this series.

> For the peek operation, it wouldn't work for the ethernet case, which
> has to call it from atomic context in net_rx_action.

It wouldn't work if the mbox is not requested with MBOX_ASYNC, but
otherwise that should be fine, as it would just peek into the kfifo.
That doesn't seem like a desirable method for ethernet use-case though,
as it ends up being two extra copies.

> > 	/**
> > 	 * so this is where this lock makes things difficult, as this function
> > 	 * might_sleep(), but only really because of the lock.  Either we can
> > 	 * remove the lock and force the adapter to do its own locking
> > 	 * spinlock-style, or we can accept the sleep here, which seems a bit
> > 	 * stupid in a peek function.  Neither option is good.  Additionally,
> > 	 * there's no guarantee that the adapter doesn't operate over a bus
> > 	 * which itself might_sleep(), exacerbating the problem.
> > 	 */
> > 	mutex_lock(&mbox->adapter->lock);
> > 	rc = mbox->adapter->ops->peek_message(mbox->adapter, mbox->chan, msg);
> > 	mutex_lock(&mbox->adapter->lock);
> 
> If we decide that peek() must not sleep, any driver that operates on a
> slow bus could just always report "no data" here.

Yes indeed, or it could just not implement peek, which seems reasonable.

> Moving the locking into the mbox driver here sounds appropriate.

I don't really like doing that for the entirety of the mbox core, as it
makes the simple adapters harder to write properly.  Since peek is not
a typical use-case, perhaps we could remove the locking for just peek,
and have a Big Fat Warning in the description of how to properly
implement it?

> 	Arnd

Thanks for the input!

-Courtney

[RFC 2/6] mailbox: document bindings

[Courtney Cavin]

Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
---
 .../devicetree/bindings/mailbox/mailbox.txt        | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/mailbox.txt

diff --git a/Documentation/devicetree/bindings/mailbox/mailbox.txt b/Documentation/devicetree/bindings/mailbox/mailbox.txt
new file mode 100644
index 0000000..846eb49
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/mailbox.txt
@@ -0,0 +1,44 @@
+Binding documentation for mailbox providers and consumers
+--
+
+Mailbox providers may be represented by any node in a device tree.  These
+nodes are designated as mailbox providers.  Consumers can then use a phandle
+to a mailbox provider, along with channel specifier information in order to
+get a mailbox.
+
+MAILBOX PROVIDERS
+
+#mbox-cells:
+	Usage: required
+	Type: u32
+	Desc: Number of cells in a mailbox specifier; Typically 1 for nodes
+	      which only need a channel index.
+
+
+Example:
+	mailbox: mailbox {
+		#mbox-cells = <1>;
+		...
+	};
+
+
+MAILBOX CONSUMERS
+
+mbox:
+	Usage: required
+	Type: < [phandle] [mailbox-specifier] >
+	Desc: List of phandle and mailbox specifier pairs, matching provider's
+	      #mbox-cells property
+
+mbox-names:
+	Usage: optional
+	Type: string array
+	Desc: List of mailbox input name strings sorted in the same order as
+	      the mbox property.  Consumer drivers should use mbox-names
+	      to match mailbox input names with mailbox specifiers.
+
+Example:
+	consumer {
+		mbox-names = "comms";
+		mbox = <&mailbox 0>;
+	};
-- 
1.8.1.5

[RFC 3/6] mailbox: pl320: migrate to mbox framework

[Courtney Cavin]

We don't remove the legacy methods here, but we mark them as deprecated
in the hopes that people with the ability to properly test modifications
can adapt its users.

Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
---
 drivers/mailbox/pl320-ipc.c | 258 ++++++++++++++++++++++++++++++++++----------
 include/linux/mailbox.h     |  29 ++++-
 2 files changed, 225 insertions(+), 62 deletions(-)

diff --git a/drivers/mailbox/pl320-ipc.c b/drivers/mailbox/pl320-ipc.c
index d873cba..b8da247 100644
--- a/drivers/mailbox/pl320-ipc.c
+++ b/drivers/mailbox/pl320-ipc.c
@@ -15,7 +15,6 @@
  */
 #include <linux/types.h>
 #include <linux/err.h>
-#include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
@@ -27,6 +26,7 @@
 #include <linux/amba/bus.h>
 
 #include <linux/mailbox.h>
+#include <linux/mbox.h>
 
 #define IPCMxSOURCE(m)		((m) * 0x40)
 #define IPCMxDSET(m)		(((m) * 0x40) + 0x004)
@@ -50,131 +50,162 @@
 #define A9_SOURCE		1
 #define M3_SOURCE		0
 
-static void __iomem *ipc_base;
-static int ipc_irq;
-static DEFINE_MUTEX(ipc_m1_lock);
-static DECLARE_COMPLETION(ipc_completion);
-static ATOMIC_NOTIFIER_HEAD(ipc_notifier);
+struct pl320 {
+	struct mbox_adapter adapter;
+	void __iomem *base;
+	int irq;
+	struct completion completion;
+};
 
-static inline void set_destination(int source, int mbox)
+static inline void set_destination(struct pl320 *pl, int source, int mbox)
 {
-	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxDSET(mbox));
-	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxMSET(mbox));
+	__raw_writel(CHAN_MASK(source), pl->base + IPCMxDSET(mbox));
+	__raw_writel(CHAN_MASK(source), pl->base + IPCMxMSET(mbox));
 }
 
-static inline void clear_destination(int source, int mbox)
+static inline void clear_destination(struct pl320 *pl, int source, int mbox)
 {
-	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxDCLEAR(mbox));
-	__raw_writel(CHAN_MASK(source), ipc_base + IPCMxMCLEAR(mbox));
+	__raw_writel(CHAN_MASK(source), pl->base + IPCMxDCLEAR(mbox));
+	__raw_writel(CHAN_MASK(source), pl->base + IPCMxMCLEAR(mbox));
 }
 
-static void __ipc_send(int mbox, u32 *data)
+static void __ipc_send(struct pl320 *pl, int mbox, const u32 *data)
 {
 	int i;
 	for (i = 0; i < 7; i++)
-		__raw_writel(data[i], ipc_base + IPCMxDR(mbox, i));
-	__raw_writel(0x1, ipc_base + IPCMxSEND(mbox));
+		__raw_writel(data[i], pl->base + IPCMxDR(mbox, i));
+	__raw_writel(0x1, pl->base + IPCMxSEND(mbox));
 }
 
-static u32 __ipc_rcv(int mbox, u32 *data)
+static u32 __ipc_rcv(struct pl320 *pl, int mbox, u32 *data)
 {
 	int i;
 	for (i = 0; i < 7; i++)
-		data[i] = __raw_readl(ipc_base + IPCMxDR(mbox, i));
+		data[i] = __raw_readl(pl->base + IPCMxDR(mbox, i));
 	return data[1];
 }
 
 /* blocking implmentation from the A9 side, not usuable in interrupts! */
-int pl320_ipc_transmit(u32 *data)
+static int pl320_ipc_put_message(struct mbox_adapter *adap,
+		struct mbox_channel *chan, const void *data, unsigned int len)
 {
+	struct pl320 *pl;
+	u32 repl[7];
 	int ret;
 
-	mutex_lock(&ipc_m1_lock);
+	if (len != 28 || chan->id != 0)
+		return -EINVAL;
 
-	init_completion(&ipc_completion);
-	__ipc_send(IPC_TX_MBOX, data);
-	ret = wait_for_completion_timeout(&ipc_completion,
+	pl = container_of(adap, struct pl320, adapter);
+	reinit_completion(&pl->completion);
+	__ipc_send(pl, IPC_TX_MBOX, data);
+	ret = wait_for_completion_timeout(&pl->completion,
 					  msecs_to_jiffies(1000));
-	if (ret == 0) {
-		ret = -ETIMEDOUT;
-		goto out;
-	}
+	if (ret == 0)
+		return -ETIMEDOUT;
+
+	ret = __ipc_rcv(pl, IPC_TX_MBOX, repl);
 
-	ret = __ipc_rcv(IPC_TX_MBOX, data);
-out:
-	mutex_unlock(&ipc_m1_lock);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(pl320_ipc_transmit);
 
 static irqreturn_t ipc_handler(int irq, void *dev)
 {
+	struct pl320 *pl = dev;
 	u32 irq_stat;
 	u32 data[7];
 
-	irq_stat = __raw_readl(ipc_base + IPCMMIS(1));
+	irq_stat = __raw_readl(pl->base + IPCMMIS(1));
 	if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
-		__raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
-		complete(&ipc_completion);
+		__raw_writel(0, pl->base + IPCMxSEND(IPC_TX_MBOX));
+		complete(&pl->completion);
 	}
 	if (irq_stat & MBOX_MASK(IPC_RX_MBOX)) {
-		__ipc_rcv(IPC_RX_MBOX, data);
-		atomic_notifier_call_chain(&ipc_notifier, data[0], data + 1);
-		__raw_writel(2, ipc_base + IPCMxSEND(IPC_RX_MBOX));
+		__ipc_rcv(pl, IPC_RX_MBOX, data);
+		mbox_channel_notify(&pl->adapter.channels[0], data, 28);
+		__raw_writel(2, pl->base + IPCMxSEND(IPC_RX_MBOX));
 	}
 
 	return IRQ_HANDLED;
 }
 
-int pl320_ipc_register_notifier(struct notifier_block *nb)
-{
-	return atomic_notifier_chain_register(&ipc_notifier, nb);
-}
-EXPORT_SYMBOL_GPL(pl320_ipc_register_notifier);
-
-int pl320_ipc_unregister_notifier(struct notifier_block *nb)
-{
-	return atomic_notifier_chain_unregister(&ipc_notifier, nb);
-}
-EXPORT_SYMBOL_GPL(pl320_ipc_unregister_notifier);
+static const struct mbox_adapter_ops pl320_mbox_ops = {
+	.owner = THIS_MODULE,
+	.put_message = pl320_ipc_put_message,
+};
 
 static int pl320_probe(struct amba_device *adev, const struct amba_id *id)
 {
+	struct pl320 *pl;
 	int ret;
 
-	ipc_base = ioremap(adev->res.start, resource_size(&adev->res));
-	if (ipc_base == NULL)
+	pl = devm_kzalloc(&adev->dev, sizeof(*pl), GFP_KERNEL);
+	if (pl == NULL)
+		return -ENOMEM;
+	pl->base = ioremap(adev->res.start, resource_size(&adev->res));
+	if (pl->base == NULL)
 		return -ENOMEM;
 
-	__raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
+	init_completion(&pl->completion);
 
-	ipc_irq = adev->irq[0];
-	ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
+	pl->adapter.dev = &adev->dev;
+	pl->adapter.ops = &pl320_mbox_ops;
+	pl->adapter.nchannels = 1;
+
+	ret = mbox_adapter_add(&pl->adapter);
+	if (ret)
+		goto err;
+
+	__raw_writel(0, pl->base + IPCMxSEND(IPC_TX_MBOX));
+
+	pl->irq = adev->irq[0];
+	ret = request_irq(pl->irq, ipc_handler, 0, dev_name(&adev->dev), pl);
 	if (ret < 0)
 		goto err;
 
 	/* Init slow mailbox */
 	__raw_writel(CHAN_MASK(A9_SOURCE),
-			ipc_base + IPCMxSOURCE(IPC_TX_MBOX));
+			pl->base + IPCMxSOURCE(IPC_TX_MBOX));
 	__raw_writel(CHAN_MASK(M3_SOURCE),
-			ipc_base + IPCMxDSET(IPC_TX_MBOX));
+			pl->base + IPCMxDSET(IPC_TX_MBOX));
 	__raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
-		     ipc_base + IPCMxMSET(IPC_TX_MBOX));
+		     pl->base + IPCMxMSET(IPC_TX_MBOX));
 
 	/* Init receive mailbox */
 	__raw_writel(CHAN_MASK(M3_SOURCE),
-			ipc_base + IPCMxSOURCE(IPC_RX_MBOX));
+			pl->base + IPCMxSOURCE(IPC_RX_MBOX));
 	__raw_writel(CHAN_MASK(A9_SOURCE),
-			ipc_base + IPCMxDSET(IPC_RX_MBOX));
+			pl->base + IPCMxDSET(IPC_RX_MBOX));
 	__raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
-		     ipc_base + IPCMxMSET(IPC_RX_MBOX));
+		     pl->base + IPCMxMSET(IPC_RX_MBOX));
 
+	amba_set_drvdata(adev, pl);
 	return 0;
 err:
-	iounmap(ipc_base);
+	iounmap(pl->base);
 	return ret;
 }
 
+static int pl320_remove(struct amba_device *adev)
+{
+	struct pl320 *pl;
+	int ret;
+
+	pl = amba_get_drvdata(adev);
+
+	disable_irq(pl->irq);
+
+	ret = mbox_adapter_remove(&pl->adapter);
+	if (ret) {
+		enable_irq(pl->irq);
+		return ret;
+	}
+
+	free_irq(pl->irq, pl);
+	iounmap(pl->base);
+	return 0;
+}
+
 static struct amba_id pl320_ids[] = {
 	{
 		.id	= 0x00041320,
@@ -189,6 +220,7 @@ static struct amba_driver pl320_driver = {
 	},
 	.id_table	= pl320_ids,
 	.probe		= pl320_probe,
+	.remove		= pl320_remove,
 };
 
 static int __init ipc_init(void)
@@ -196,3 +228,111 @@ static int __init ipc_init(void)
 	return amba_driver_register(&pl320_driver);
 }
 module_init(ipc_init);
+
+/* Legacy API */
+static struct mbox *pl320_mbox;
+static struct notifier_block *pl320_notifier;
+static DEFINE_SPINLOCK(pl320_legacy_lock);
+static DEFINE_MUTEX(pl320_mutex);
+
+static int __pl320_notify(struct notifier_block *nb,
+		unsigned long len, void *data)
+{
+	unsigned long flags;
+	u32 *mdata = data;
+	int rc;
+
+	spin_lock_irqsave(&pl320_legacy_lock, flags);
+	if (!pl320_notifier) {
+		spin_unlock_irqrestore(&pl320_legacy_lock, flags);
+		return NOTIFY_DONE;
+	}
+
+	rc = pl320_notifier->notifier_call(pl320_notifier,
+			mdata[0], mdata + 1);
+	spin_unlock_irqrestore(&pl320_legacy_lock, flags);
+	return rc;
+}
+
+static void __pl320_set_notifier(struct notifier_block *nb)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&pl320_legacy_lock, flags);
+	pl320_notifier = nb;
+	spin_unlock_irqrestore(&pl320_legacy_lock, flags);
+}
+
+static struct notifier_block pl320_nb = {
+	.notifier_call = __pl320_notify,
+};
+
+static int __pl320_legacy_setup(struct notifier_block *nb, bool exist_ok)
+{
+	int rc = 0;
+
+	if (WARN_ON(!exist_ok && pl320_mbox))
+		return -EBUSY;
+
+	if (pl320_mbox)
+		return 0;
+
+	__pl320_set_notifier(nb);
+
+	pl320_mbox = mbox_request(NULL, "pl320", &pl320_nb);
+	if (IS_ERR(pl320_mbox)) {
+		rc = PTR_ERR(pl320_mbox);
+		pl320_mbox = NULL;
+		__pl320_set_notifier(NULL);
+	}
+
+	return rc;
+}
+
+int __pl320_legacy_ipc_transmit(u32 *data)
+{
+	int rc;
+
+	mutex_lock(&pl320_mutex);
+	rc = __pl320_legacy_setup(NULL, true);
+	if (rc)
+		goto out;
+
+	rc = mbox_put_message(pl320_mbox, data, 7 * sizeof(*data));
+out:
+	mutex_unlock(&pl320_mutex);
+
+	return rc;
+}
+EXPORT_SYMBOL(__pl320_legacy_ipc_transmit);
+
+int __pl320_legacy_ipc_register_notifier(struct notifier_block *nb)
+{
+	int rc;
+
+	mutex_lock(&pl320_mutex);
+	rc = __pl320_legacy_setup(nb, false);
+	mutex_unlock(&pl320_mutex);
+
+	return rc;
+}
+EXPORT_SYMBOL(__pl320_legacy_ipc_register_notifier);
+
+int __pl320_legacy_ipc_unregister_notifier(struct notifier_block *nb)
+{
+	mutex_lock(&pl320_mutex);
+
+	if (WARN_ON(!pl320_mbox)) {
+		mutex_unlock(&pl320_mutex);
+		return -EINVAL;
+	}
+
+	mbox_release(pl320_mbox);
+	__pl320_set_notifier(NULL);
+	pl320_mbox = NULL;
+
+	mutex_unlock(&pl320_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL(__pl320_legacy_ipc_unregister_notifier);
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
index 5161f63..2330954 100644
--- a/include/linux/mailbox.h
+++ b/include/linux/mailbox.h
@@ -12,6 +12,29 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-int pl320_ipc_transmit(u32 *data);
-int pl320_ipc_register_notifier(struct notifier_block *nb);
-int pl320_ipc_unregister_notifier(struct notifier_block *nb);
+#ifndef PL320_MAILBOX_H
+#define PL320_MAILBOX_H
+
+#include <linux/compiler.h>
+#include <linux/mbox.h>
+
+int __pl320_legacy_ipc_transmit(u32 *data);
+int __pl320_legacy_ipc_register_notifier(struct notifier_block *nb);
+int __pl320_legacy_ipc_unregister_notifier(struct notifier_block *nb);
+
+static inline int __deprecated pl320_ipc_transmit(u32 *data)
+{
+	return __pl320_legacy_ipc_transmit(data);
+}
+static inline int __deprecated
+pl320_ipc_register_notifier(struct notifier_block *nb)
+{
+	return __pl320_legacy_ipc_register_notifier(nb);
+}
+static inline int __deprecated
+pl320_ipc_unregister_notifier(struct notifier_block *nb)
+{
+	return __pl320_legacy_ipc_unregister_notifier(nb);
+}
+
+#endif
-- 
1.8.1.5

Re: [RFC 3/6] mailbox: pl320: migrate to mbox framework

[Rob Herring]

On Fri, Feb 7, 2014 at 6:50 PM, Courtney Cavin
<courtney.cavin@sonymobile.com> wrote:
> We don't remove the legacy methods here, but we mark them as deprecated
> in the hopes that people with the ability to properly test modifications
> can adapt its users.

The DT for highbank is pretty much fixed at this point. So adopting
this will need a way to register without DT. Unfortunately, I don't
have access to h/w either ATM.

I should note that this driver is very much highbank specific and not
really a generic pl320 driver. The pl320 has up to 8 mailboxes and 8
interrupts. How it is used from there is a software decision. I've
never seen any other user, but it could be done quite differently from
how it is used in highbank. In the case of highbank, we assigned a tx
and rx mailbox. While both the management core and linux side have all
8 interrupts wired up, we have assigned an interrupt to each side. I
suppose you could have the interrupt tied to each mailbox, but really
they are unrelated in the pl320 as each mailbox message could have
multiple targets (interrupts). Probably splitting this between a pl320
lib and platform specific drivers would be the right split if there
are ever other users.


> -       ipc_irq = adev->irq[0];
> -       ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
> +       pl->adapter.dev = &adev->dev;
> +       pl->adapter.ops = &pl320_mbox_ops;
> +       pl->adapter.nchannels = 1;

Shouldn't this be 2? The 2 channels here are not a single
bi-directional channel in any way. They are completely independent and
have unrelated events. For example we originally defined having 3
mailboxes where we had 2 tx mailboxes for fast and slow messages, but
we ultimately decided everything could be a single tx mailbox. Event
completion is handled synchronously via the pl320's handshake
mechanism. I'd imagine you could have a protocol where you have async
completions via an rx mailbox instead.

Rob

Re: [RFC 3/6] mailbox: pl320: migrate to mbox framework

[Courtney Cavin]

On Mon, Feb 10, 2014 at 07:28:54PM +0100, Rob Herring wrote:
> On Fri, Feb 7, 2014 at 6:50 PM, Courtney Cavin
> <courtney.cavin@sonymobile.com> wrote:
> > We don't remove the legacy methods here, but we mark them as deprecated
> > in the hopes that people with the ability to properly test modifications
> > can adapt its users.
> 
> The DT for highbank is pretty much fixed at this point. So adopting
> this will need a way to register without DT. Unfortunately, I don't
> have access to h/w either ATM.

That's fine. The lookup table stuff (see mbox_channel_lookup() and
mbox_add_table()) should solve this.

Hopefully we'll find someone with a test-setup for this.

> I should note that this driver is very much highbank specific and not
> really a generic pl320 driver. The pl320 has up to 8 mailboxes and 8
> interrupts. How it is used from there is a software decision. I've
> never seen any other user, but it could be done quite differently from
> how it is used in highbank. In the case of highbank, we assigned a tx
> and rx mailbox. While both the management core and linux side have all
> 8 interrupts wired up, we have assigned an interrupt to each side. I
> suppose you could have the interrupt tied to each mailbox, but really
> they are unrelated in the pl320 as each mailbox message could have
> multiple targets (interrupts). Probably splitting this between a pl320
> lib and platform specific drivers would be the right split if there
> are ever other users.

I'm not exactly sure I follow, and I probably should lookup the pl320
spec, but from the way you describe it and the simplicity of the
existing implementation, it seems to me that one could easily implement
this using DT to decribe how the hardware is hooked up.

> > -       ipc_irq = adev->irq[0];
> > -       ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
> > +       pl->adapter.dev = &adev->dev;
> > +       pl->adapter.ops = &pl320_mbox_ops;
> > +       pl->adapter.nchannels = 1;
> 
> Shouldn't this be 2? The 2 channels here are not a single
> bi-directional channel in any way. They are completely independent and
> have unrelated events. For example we originally defined having 3
> mailboxes where we had 2 tx mailboxes for fast and slow messages, but
> we ultimately decided everything could be a single tx mailbox. Event
> completion is handled synchronously via the pl320's handshake
> mechanism. I'd imagine you could have a protocol where you have async
> completions via an rx mailbox instead.

I generally see a mailbox as having both rx & tx, and I thought that
this was what this driver was attempting to model, so I implemented it
that way.  If the channels are completely independent, there's no reason
why we can't model this as one rx, and one tx.  The proposed API for
this should be suitable.

If I understand what you mean, "event completion" in this case is an ACK
for each event.  Async event ACKing sounds pretty dirty, and I don't
really want to touch on that in the core implementation, but there
should be nothing stopping you from exposing a mailbox which uses
.put_message() to ACK an RX event.

> Rob

I'm glad you found this, as apparently I got your email address all
wrong.

Thanks for the comments! 

-Courtney

[RFC 4/6] mailbox: omap: remove omap-specific framework

[Courtney Cavin]

This framework is no longer needed, and the users should move over to
the new common framework.  Mark the existing implementations as broken,
and deprecate the api, as a stop-gap.

Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
---
 drivers/mailbox/Kconfig        |  19 +-
 drivers/mailbox/Makefile       |   1 -
 drivers/mailbox/omap-mailbox.c | 469 -----------------------------------------
 drivers/mailbox/omap-mbox.h    |  67 ------
 include/linux/omap-mailbox.h   |  45 +++-
 5 files changed, 40 insertions(+), 561 deletions(-)
 delete mode 100644 drivers/mailbox/omap-mailbox.c
 delete mode 100644 drivers/mailbox/omap-mbox.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index c8b5c13..6befc6e 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -16,17 +16,10 @@ config PL320_MBOX
 	  Management Engine, primarily for cpufreq. Say Y here if you want
 	  to use the PL320 IPCM support.
 
-config OMAP_MBOX
-	tristate
-	help
-	  This option is selected by any OMAP architecture specific mailbox
-	  driver such as CONFIG_OMAP1_MBOX or CONFIG_OMAP2PLUS_MBOX. This
-	  enables the common OMAP mailbox framework code.
-
 config OMAP1_MBOX
 	tristate "OMAP1 Mailbox framework support"
 	depends on ARCH_OMAP1
-	select OMAP_MBOX
+	depends on BROKEN
 	help
 	  Mailbox implementation for OMAP chips with hardware for
 	  interprocessor communication involving DSP in OMAP1. Say Y here
@@ -35,19 +28,11 @@ config OMAP1_MBOX
 config OMAP2PLUS_MBOX
 	tristate "OMAP2+ Mailbox framework support"
 	depends on ARCH_OMAP2PLUS
-	select OMAP_MBOX
+	depends on BROKEN
 	help
 	  Mailbox implementation for OMAP family chips with hardware for
 	  interprocessor communication involving DSP, IVA1.0 and IVA2 in
 	  OMAP2/3; or IPU, IVA HD and DSP in OMAP4/5. Say Y here if you
 	  want to use OMAP2+ Mailbox framework support.
 
-config OMAP_MBOX_KFIFO_SIZE
-	int "Mailbox kfifo default buffer size (bytes)"
-	depends on OMAP2PLUS_MBOX || OMAP1_MBOX
-	default 256
-	help
-	  Specify the default size of mailbox's kfifo buffers (bytes).
-	  This can also be changed at runtime (via the mbox_kfifo_size
-	  module parameter).
 endif
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 53712ed..c8e51a0 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,7 +1,6 @@
 obj-$(CONFIG_MAILBOX)		+= core.o
 obj-$(CONFIG_PL320_MBOX)	+= pl320-ipc.o
 
-obj-$(CONFIG_OMAP_MBOX)		+= omap-mailbox.o
 obj-$(CONFIG_OMAP1_MBOX)	+= mailbox_omap1.o
 mailbox_omap1-objs		:= mailbox-omap1.o
 obj-$(CONFIG_OMAP2PLUS_MBOX)	+= mailbox_omap2.o
diff --git a/drivers/mailbox/omap-mailbox.c b/drivers/mailbox/omap-mailbox.c
deleted file mode 100644
index d79a646..0000000
--- a/drivers/mailbox/omap-mailbox.c
+++ /dev/null
@@ -1,469 +0,0 @@
-/*
- * OMAP mailbox driver
- *
- * Copyright (C) 2006-2009 Nokia Corporation. All rights reserved.
- *
- * Contact: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License
- * version 2 as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
- * 02110-1301 USA
- *
- */
-
-#include <linux/interrupt.h>
-#include <linux/spinlock.h>
-#include <linux/mutex.h>
-#include <linux/delay.h>
-#include <linux/slab.h>
-#include <linux/kfifo.h>
-#include <linux/err.h>
-#include <linux/notifier.h>
-#include <linux/module.h>
-
-#include "omap-mbox.h"
-
-static struct omap_mbox **mboxes;
-
-static int mbox_configured;
-static DEFINE_MUTEX(mbox_configured_lock);
-
-static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
-module_param(mbox_kfifo_size, uint, S_IRUGO);
-MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo (bytes)");
-
-/* Mailbox FIFO handle functions */
-static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
-{
-	return mbox->ops->fifo_read(mbox);
-}
-static inline void mbox_fifo_write(struct omap_mbox *mbox, mbox_msg_t msg)
-{
-	mbox->ops->fifo_write(mbox, msg);
-}
-static inline int mbox_fifo_empty(struct omap_mbox *mbox)
-{
-	return mbox->ops->fifo_empty(mbox);
-}
-static inline int mbox_fifo_full(struct omap_mbox *mbox)
-{
-	return mbox->ops->fifo_full(mbox);
-}
-
-/* Mailbox IRQ handle functions */
-static inline void ack_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	if (mbox->ops->ack_irq)
-		mbox->ops->ack_irq(mbox, irq);
-}
-static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	return mbox->ops->is_irq(mbox, irq);
-}
-
-/*
- * message sender
- */
-static int __mbox_poll_for_space(struct omap_mbox *mbox)
-{
-	int ret = 0, i = 1000;
-
-	while (mbox_fifo_full(mbox)) {
-		if (mbox->ops->type == OMAP_MBOX_TYPE2)
-			return -1;
-		if (--i == 0)
-			return -1;
-		udelay(1);
-	}
-	return ret;
-}
-
-int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
-{
-	struct omap_mbox_queue *mq = mbox->txq;
-	int ret = 0, len;
-
-	spin_lock_bh(&mq->lock);
-
-	if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	if (kfifo_is_empty(&mq->fifo) && !__mbox_poll_for_space(mbox)) {
-		mbox_fifo_write(mbox, msg);
-		goto out;
-	}
-
-	len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
-	WARN_ON(len != sizeof(msg));
-
-	tasklet_schedule(&mbox->txq->tasklet);
-
-out:
-	spin_unlock_bh(&mq->lock);
-	return ret;
-}
-EXPORT_SYMBOL(omap_mbox_msg_send);
-
-void omap_mbox_save_ctx(struct omap_mbox *mbox)
-{
-	if (!mbox->ops->save_ctx) {
-		dev_err(mbox->dev, "%s:\tno save\n", __func__);
-		return;
-	}
-
-	mbox->ops->save_ctx(mbox);
-}
-EXPORT_SYMBOL(omap_mbox_save_ctx);
-
-void omap_mbox_restore_ctx(struct omap_mbox *mbox)
-{
-	if (!mbox->ops->restore_ctx) {
-		dev_err(mbox->dev, "%s:\tno restore\n", __func__);
-		return;
-	}
-
-	mbox->ops->restore_ctx(mbox);
-}
-EXPORT_SYMBOL(omap_mbox_restore_ctx);
-
-void omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	mbox->ops->enable_irq(mbox, irq);
-}
-EXPORT_SYMBOL(omap_mbox_enable_irq);
-
-void omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	mbox->ops->disable_irq(mbox, irq);
-}
-EXPORT_SYMBOL(omap_mbox_disable_irq);
-
-static void mbox_tx_tasklet(unsigned long tx_data)
-{
-	struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
-	struct omap_mbox_queue *mq = mbox->txq;
-	mbox_msg_t msg;
-	int ret;
-
-	while (kfifo_len(&mq->fifo)) {
-		if (__mbox_poll_for_space(mbox)) {
-			omap_mbox_enable_irq(mbox, IRQ_TX);
-			break;
-		}
-
-		ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
-								sizeof(msg));
-		WARN_ON(ret != sizeof(msg));
-
-		mbox_fifo_write(mbox, msg);
-	}
-}
-
-/*
- * Message receiver(workqueue)
- */
-static void mbox_rx_work(struct work_struct *work)
-{
-	struct omap_mbox_queue *mq =
-			container_of(work, struct omap_mbox_queue, work);
-	mbox_msg_t msg;
-	int len;
-
-	while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
-		len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
-		WARN_ON(len != sizeof(msg));
-
-		blocking_notifier_call_chain(&mq->mbox->notifier, len,
-								(void *)msg);
-		spin_lock_irq(&mq->lock);
-		if (mq->full) {
-			mq->full = false;
-			omap_mbox_enable_irq(mq->mbox, IRQ_RX);
-		}
-		spin_unlock_irq(&mq->lock);
-	}
-}
-
-/*
- * Mailbox interrupt handler
- */
-static void __mbox_tx_interrupt(struct omap_mbox *mbox)
-{
-	omap_mbox_disable_irq(mbox, IRQ_TX);
-	ack_mbox_irq(mbox, IRQ_TX);
-	tasklet_schedule(&mbox->txq->tasklet);
-}
-
-static void __mbox_rx_interrupt(struct omap_mbox *mbox)
-{
-	struct omap_mbox_queue *mq = mbox->rxq;
-	mbox_msg_t msg;
-	int len;
-
-	while (!mbox_fifo_empty(mbox)) {
-		if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg))) {
-			omap_mbox_disable_irq(mbox, IRQ_RX);
-			mq->full = true;
-			goto nomem;
-		}
-
-		msg = mbox_fifo_read(mbox);
-
-		len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
-		WARN_ON(len != sizeof(msg));
-
-		if (mbox->ops->type == OMAP_MBOX_TYPE1)
-			break;
-	}
-
-	/* no more messages in the fifo. clear IRQ source. */
-	ack_mbox_irq(mbox, IRQ_RX);
-nomem:
-	schedule_work(&mbox->rxq->work);
-}
-
-static irqreturn_t mbox_interrupt(int irq, void *p)
-{
-	struct omap_mbox *mbox = p;
-
-	if (is_mbox_irq(mbox, IRQ_TX))
-		__mbox_tx_interrupt(mbox);
-
-	if (is_mbox_irq(mbox, IRQ_RX))
-		__mbox_rx_interrupt(mbox);
-
-	return IRQ_HANDLED;
-}
-
-static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
-					void (*work) (struct work_struct *),
-					void (*tasklet)(unsigned long))
-{
-	struct omap_mbox_queue *mq;
-
-	mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
-	if (!mq)
-		return NULL;
-
-	spin_lock_init(&mq->lock);
-
-	if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
-		goto error;
-
-	if (work)
-		INIT_WORK(&mq->work, work);
-
-	if (tasklet)
-		tasklet_init(&mq->tasklet, tasklet, (unsigned long)mbox);
-	return mq;
-error:
-	kfree(mq);
-	return NULL;
-}
-
-static void mbox_queue_free(struct omap_mbox_queue *q)
-{
-	kfifo_free(&q->fifo);
-	kfree(q);
-}
-
-static int omap_mbox_startup(struct omap_mbox *mbox)
-{
-	int ret = 0;
-	struct omap_mbox_queue *mq;
-
-	mutex_lock(&mbox_configured_lock);
-	if (!mbox_configured++) {
-		if (likely(mbox->ops->startup)) {
-			ret = mbox->ops->startup(mbox);
-			if (unlikely(ret))
-				goto fail_startup;
-		} else
-			goto fail_startup;
-	}
-
-	if (!mbox->use_count++) {
-		mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
-		if (!mq) {
-			ret = -ENOMEM;
-			goto fail_alloc_txq;
-		}
-		mbox->txq = mq;
-
-		mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
-		if (!mq) {
-			ret = -ENOMEM;
-			goto fail_alloc_rxq;
-		}
-		mbox->rxq = mq;
-		mq->mbox = mbox;
-		ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
-							mbox->name, mbox);
-		if (unlikely(ret)) {
-			pr_err("failed to register mailbox interrupt:%d\n",
-									ret);
-			goto fail_request_irq;
-		}
-
-		omap_mbox_enable_irq(mbox, IRQ_RX);
-	}
-	mutex_unlock(&mbox_configured_lock);
-	return 0;
-
-fail_request_irq:
-	mbox_queue_free(mbox->rxq);
-fail_alloc_rxq:
-	mbox_queue_free(mbox->txq);
-fail_alloc_txq:
-	if (mbox->ops->shutdown)
-		mbox->ops->shutdown(mbox);
-	mbox->use_count--;
-fail_startup:
-	mbox_configured--;
-	mutex_unlock(&mbox_configured_lock);
-	return ret;
-}
-
-static void omap_mbox_fini(struct omap_mbox *mbox)
-{
-	mutex_lock(&mbox_configured_lock);
-
-	if (!--mbox->use_count) {
-		omap_mbox_disable_irq(mbox, IRQ_RX);
-		free_irq(mbox->irq, mbox);
-		tasklet_kill(&mbox->txq->tasklet);
-		flush_work(&mbox->rxq->work);
-		mbox_queue_free(mbox->txq);
-		mbox_queue_free(mbox->rxq);
-	}
-
-	if (likely(mbox->ops->shutdown)) {
-		if (!--mbox_configured)
-			mbox->ops->shutdown(mbox);
-	}
-
-	mutex_unlock(&mbox_configured_lock);
-}
-
-struct omap_mbox *omap_mbox_get(const char *name, struct notifier_block *nb)
-{
-	struct omap_mbox *_mbox, *mbox = NULL;
-	int i, ret;
-
-	if (!mboxes)
-		return ERR_PTR(-EINVAL);
-
-	for (i = 0; (_mbox = mboxes[i]); i++) {
-		if (!strcmp(_mbox->name, name)) {
-			mbox = _mbox;
-			break;
-		}
-	}
-
-	if (!mbox)
-		return ERR_PTR(-ENOENT);
-
-	if (nb)
-		blocking_notifier_chain_register(&mbox->notifier, nb);
-
-	ret = omap_mbox_startup(mbox);
-	if (ret) {
-		blocking_notifier_chain_unregister(&mbox->notifier, nb);
-		return ERR_PTR(-ENODEV);
-	}
-
-	return mbox;
-}
-EXPORT_SYMBOL(omap_mbox_get);
-
-void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb)
-{
-	blocking_notifier_chain_unregister(&mbox->notifier, nb);
-	omap_mbox_fini(mbox);
-}
-EXPORT_SYMBOL(omap_mbox_put);
-
-static struct class omap_mbox_class = { .name = "mbox", };
-
-int omap_mbox_register(struct device *parent, struct omap_mbox **list)
-{
-	int ret;
-	int i;
-
-	mboxes = list;
-	if (!mboxes)
-		return -EINVAL;
-
-	for (i = 0; mboxes[i]; i++) {
-		struct omap_mbox *mbox = mboxes[i];
-		mbox->dev = device_create(&omap_mbox_class,
-				parent, 0, mbox, "%s", mbox->name);
-		if (IS_ERR(mbox->dev)) {
-			ret = PTR_ERR(mbox->dev);
-			goto err_out;
-		}
-
-		BLOCKING_INIT_NOTIFIER_HEAD(&mbox->notifier);
-	}
-	return 0;
-
-err_out:
-	while (i--)
-		device_unregister(mboxes[i]->dev);
-	return ret;
-}
-EXPORT_SYMBOL(omap_mbox_register);
-
-int omap_mbox_unregister(void)
-{
-	int i;
-
-	if (!mboxes)
-		return -EINVAL;
-
-	for (i = 0; mboxes[i]; i++)
-		device_unregister(mboxes[i]->dev);
-	mboxes = NULL;
-	return 0;
-}
-EXPORT_SYMBOL(omap_mbox_unregister);
-
-static int __init omap_mbox_init(void)
-{
-	int err;
-
-	err = class_register(&omap_mbox_class);
-	if (err)
-		return err;
-
-	/* kfifo size sanity check: alignment and minimal size */
-	mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
-	mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size,
-							sizeof(mbox_msg_t));
-
-	return 0;
-}
-subsys_initcall(omap_mbox_init);
-
-static void __exit omap_mbox_exit(void)
-{
-	class_unregister(&omap_mbox_class);
-}
-module_exit(omap_mbox_exit);
-
-MODULE_LICENSE("GPL v2");
-MODULE_DESCRIPTION("omap mailbox: interrupt driven messaging");
-MODULE_AUTHOR("Toshihiro Kobayashi");
-MODULE_AUTHOR("Hiroshi DOYU");
diff --git a/drivers/mailbox/omap-mbox.h b/drivers/mailbox/omap-mbox.h
deleted file mode 100644
index 6cd38fc..0000000
--- a/drivers/mailbox/omap-mbox.h
+++ /dev/null
@@ -1,67 +0,0 @@
-/*
- * omap-mbox.h: OMAP mailbox internal definitions
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#ifndef OMAP_MBOX_H
-#define OMAP_MBOX_H
-
-#include <linux/device.h>
-#include <linux/interrupt.h>
-#include <linux/kfifo.h>
-#include <linux/spinlock.h>
-#include <linux/workqueue.h>
-#include <linux/omap-mailbox.h>
-
-typedef int __bitwise omap_mbox_type_t;
-#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
-#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
-
-struct omap_mbox_ops {
-	omap_mbox_type_t	type;
-	int		(*startup)(struct omap_mbox *mbox);
-	void		(*shutdown)(struct omap_mbox *mbox);
-	/* fifo */
-	mbox_msg_t	(*fifo_read)(struct omap_mbox *mbox);
-	void		(*fifo_write)(struct omap_mbox *mbox, mbox_msg_t msg);
-	int		(*fifo_empty)(struct omap_mbox *mbox);
-	int		(*fifo_full)(struct omap_mbox *mbox);
-	/* irq */
-	void		(*enable_irq)(struct omap_mbox *mbox,
-						omap_mbox_irq_t irq);
-	void		(*disable_irq)(struct omap_mbox *mbox,
-						omap_mbox_irq_t irq);
-	void		(*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
-	int		(*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
-	/* ctx */
-	void		(*save_ctx)(struct omap_mbox *mbox);
-	void		(*restore_ctx)(struct omap_mbox *mbox);
-};
-
-struct omap_mbox_queue {
-	spinlock_t		lock;
-	struct kfifo		fifo;
-	struct work_struct	work;
-	struct tasklet_struct	tasklet;
-	struct omap_mbox	*mbox;
-	bool full;
-};
-
-struct omap_mbox {
-	const char		*name;
-	unsigned int		irq;
-	struct omap_mbox_queue	*txq, *rxq;
-	struct omap_mbox_ops	*ops;
-	struct device		*dev;
-	void			*priv;
-	int			use_count;
-	struct blocking_notifier_head	notifier;
-};
-
-int omap_mbox_register(struct device *parent, struct omap_mbox **);
-int omap_mbox_unregister(void);
-
-#endif /* OMAP_MBOX_H */
diff --git a/include/linux/omap-mailbox.h b/include/linux/omap-mailbox.h
index f8322d9..89fb606 100644
--- a/include/linux/omap-mailbox.h
+++ b/include/linux/omap-mailbox.h
@@ -9,6 +9,9 @@
 #ifndef OMAP_MAILBOX_H
 #define OMAP_MAILBOX_H
 
+#include <linux/compiler.h>
+#include <linux/mbox.h>
+
 typedef u32 mbox_msg_t;
 struct omap_mbox;
 
@@ -16,14 +19,42 @@ typedef int __bitwise omap_mbox_irq_t;
 #define IRQ_TX ((__force omap_mbox_irq_t) 1)
 #define IRQ_RX ((__force omap_mbox_irq_t) 2)
 
-int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
+static inline int __deprecated omap_mbox_msg_send(struct omap_mbox *mbox,
+		mbox_msg_t msg)
+{
+	struct mbox *m = (struct mbox *)mbox;
+	return mbox_put_message(m, &msg, sizeof(msg));
+}
+
+static inline struct omap_mbox *__deprecated omap_mbox_get(const char *name,
+		struct notifier_block *nb);
+{
+	return (struct omap_mbox *)mbox_request(NULL, name, nb);
+}
+
+static inline void __deprecated omap_mbox_put(struct omap_mbox *mbox,
+		struct notifier_block *nb)
+{
+	struct mbox *m = (struct mbox *)mbox;
+	mbox_release(m, nb);
+}
+
+static inline void __deprecated omap_mbox_save_ctx(struct omap_mbox *mbox)
+{
+}
+
+static inline void __deprecated omap_mbox_restore_ctx(struct omap_mbox *mbox)
+{
+}
 
-struct omap_mbox *omap_mbox_get(const char *, struct notifier_block *nb);
-void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb);
+static inline void __deprecated omap_mbox_enable_irq(struct omap_mbox *mbox,
+		omap_mbox_irq_t irq)
+{
+}
 
-void omap_mbox_save_ctx(struct omap_mbox *mbox);
-void omap_mbox_restore_ctx(struct omap_mbox *mbox);
-void omap_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq);
-void omap_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+static inline void __deprecated omap_mbox_disable_irq(struct omap_mbox *mbox,
+		omap_mbox_irq_t irq)
+{
+}
 
 #endif /* OMAP_MAILBOX_H */
-- 
1.8.1.5

[RFC 5/6] mailbox: omap1: move to common mbox framework

[Courtney Cavin]

Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
---
 drivers/mailbox/Kconfig         |   1 -
 drivers/mailbox/mailbox-omap1.c | 153 +++++++++++++++++++---------------------
 2 files changed, 73 insertions(+), 81 deletions(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 6befc6e..ae6b09b 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -19,7 +19,6 @@ config PL320_MBOX
 config OMAP1_MBOX
 	tristate "OMAP1 Mailbox framework support"
 	depends on ARCH_OMAP1
-	depends on BROKEN
 	help
 	  Mailbox implementation for OMAP chips with hardware for
 	  interprocessor communication involving DSP in OMAP1. Say Y here
diff --git a/drivers/mailbox/mailbox-omap1.c b/drivers/mailbox/mailbox-omap1.c
index 9001b76..474890d 100644
--- a/drivers/mailbox/mailbox-omap1.c
+++ b/drivers/mailbox/mailbox-omap1.c
@@ -12,10 +12,10 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
+#include <linux/delay.h>
+#include <linux/mbox.h>
 #include <linux/io.h>
 
-#include "omap-mbox.h"
-
 #define MAILBOX_ARM2DSP1		0x00
 #define MAILBOX_ARM2DSP1b		0x04
 #define MAILBOX_DSP2ARM1		0x08
@@ -26,8 +26,6 @@
 #define MAILBOX_DSP2ARM1_Flag		0x1c
 #define MAILBOX_DSP2ARM2_Flag		0x20
 
-static void __iomem *mbox_base;
-
 struct omap_mbox1_fifo {
 	unsigned long cmd;
 	unsigned long data;
@@ -39,90 +37,70 @@ struct omap_mbox1_priv {
 	struct omap_mbox1_fifo rx_fifo;
 };
 
-static inline int mbox_read_reg(size_t ofs)
+struct omap1_mbox {
+	struct mbox_adapter adapter;
+	struct omap_mbox1_priv priv;
+	void __iomem *base;
+	int irq;
+};
+
+static inline int mbox_read_reg(void __iomem *base, size_t ofs)
 {
-	return __raw_readw(mbox_base + ofs);
+	return __raw_readw(base + ofs);
 }
 
-static inline void mbox_write_reg(u32 val, size_t ofs)
+static inline void mbox_write_reg(void __iomem *base, u32 val, size_t ofs)
 {
-	__raw_writew(val, mbox_base + ofs);
+	__raw_writew(val, base + ofs);
 }
 
-/* msg */
-static mbox_msg_t omap1_mbox_fifo_read(struct omap_mbox *mbox)
+static int omap1_mbox_put_message(struct mbox_adapter *adap,
+		struct mbox_channel *chan, const void *data, unsigned int len)
+
 {
-	struct omap_mbox1_fifo *fifo =
-		&((struct omap_mbox1_priv *)mbox->priv)->rx_fifo;
-	mbox_msg_t msg;
+	struct omap_mbox1_fifo *fifo;
+	struct omap1_mbox *mbox;
+	u32 msg;
+	int i;
 
-	msg = mbox_read_reg(fifo->data);
-	msg |= ((mbox_msg_t) mbox_read_reg(fifo->cmd)) << 16;
+	if (len != sizeof(msg))
+		return -EINVAL;
 
-	return msg;
-}
+	msg = ((u32 *)data)[0];
+	mbox = container_of(adap, struct omap1_mbox, adapter);
+	fifo = &mbox->priv.tx_fifo;
 
-static void
-omap1_mbox_fifo_write(struct omap_mbox *mbox, mbox_msg_t msg)
-{
-	struct omap_mbox1_fifo *fifo =
-		&((struct omap_mbox1_priv *)mbox->priv)->tx_fifo;
+	/* wait for available space */
+	for (i = 0; i < 100 && mbox_read_reg(mbox->base, fifo->flag); ++i)
+		usleep_range(10, 20);
+	if (i == 100)
+		return -ETIMEDOUT;
 
-	mbox_write_reg(msg & 0xffff, fifo->data);
-	mbox_write_reg(msg >> 16, fifo->cmd);
-}
+	mbox_write_reg(mbox->base, msg & 0xffff, fifo->data);
+	mbox_write_reg(mbox->base, msg >> 16, fifo->cmd);
 
-static int omap1_mbox_fifo_empty(struct omap_mbox *mbox)
-{
 	return 0;
 }
 
-static int omap1_mbox_fifo_full(struct omap_mbox *mbox)
+static irqreturn_t omap1_mbox_irq(int irq, void *dev)
 {
-	struct omap_mbox1_fifo *fifo =
-		&((struct omap_mbox1_priv *)mbox->priv)->rx_fifo;
+	struct omap1_mbox *mbox = dev;
+	struct omap_mbox1_fifo *fifo;
+	u32 msg;
 
-	return mbox_read_reg(fifo->flag);
-}
+	fifo = &mbox->priv.rx_fifo;
+	msg = mbox_read_reg(mbox->base, fifo->data);
+	msg |= ((u32)mbox_read_reg(mbox->base, fifo->cmd)) << 16;
 
-/* irq */
-static void
-omap1_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	if (irq == IRQ_RX)
-		enable_irq(mbox->irq);
-}
+	mbox_channel_notify(&mbox->adapter.channels[0], &msg, sizeof(msg));
 
-static void
-omap1_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	if (irq == IRQ_RX)
-		disable_irq(mbox->irq);
+	return IRQ_HANDLED;
 }
 
-static int
-omap1_mbox_is_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	if (irq == IRQ_TX)
-		return 0;
-	return 1;
-}
-
-static struct omap_mbox_ops omap1_mbox_ops = {
-	.type		= OMAP_MBOX_TYPE1,
-	.fifo_read	= omap1_mbox_fifo_read,
-	.fifo_write	= omap1_mbox_fifo_write,
-	.fifo_empty	= omap1_mbox_fifo_empty,
-	.fifo_full	= omap1_mbox_fifo_full,
-	.enable_irq	= omap1_mbox_enable_irq,
-	.disable_irq	= omap1_mbox_disable_irq,
-	.is_irq		= omap1_mbox_is_irq,
-};
-
 /* FIXME: the following struct should be created automatically by the user id */
 
 /* DSP */
-static struct omap_mbox1_priv omap1_mbox_dsp_priv = {
+static const struct omap_mbox1_priv omap1_mbox_dsp_priv = {
 	.tx_fifo = {
 		.cmd	= MAILBOX_ARM2DSP1b,
 		.data	= MAILBOX_ARM2DSP1,
@@ -135,44 +113,59 @@ static struct omap_mbox1_priv omap1_mbox_dsp_priv = {
 	},
 };
 
-static struct omap_mbox mbox_dsp_info = {
-	.name	= "dsp",
-	.ops	= &omap1_mbox_ops,
-	.priv	= &omap1_mbox_dsp_priv,
+static const struct mbox_adapter_ops omap1_mbox_ops = {
+	.owner = THIS_MODULE,
+	.put_message = omap1_mbox_put_message,
 };
 
-static struct omap_mbox *omap1_mboxes[] = { &mbox_dsp_info, NULL };
-
 static int omap1_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *mem;
+	struct omap1_mbox *mbox;
 	int ret;
-	struct omap_mbox **list;
 
-	list = omap1_mboxes;
-	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
+	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+	if (mbox == NULL)
+		return -ENOMEM;
+
+	mbox->irq = platform_get_irq_byname(pdev, "dsp");
+	if (mbox->irq < 0)
+		return -EINVAL;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!mem)
 		return -ENOENT;
 
-	mbox_base = ioremap(mem->start, resource_size(mem));
-	if (!mbox_base)
+	mbox->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
+	if (!mbox->base)
 		return -ENOMEM;
 
-	ret = omap_mbox_register(&pdev->dev, list);
-	if (ret) {
-		iounmap(mbox_base);
+	mbox->priv = omap1_mbox_dsp_priv;
+	mbox->adapter.dev = &pdev->dev;
+	mbox->adapter.ops = &omap1_mbox_ops;
+	mbox->adapter.nchannels = 1;
+
+	ret = mbox_adapter_add(&mbox->adapter);
+	if (ret)
+		return ret;
+
+	ret = devm_request_irq(&pdev->dev, mbox->irq, omap1_mbox_irq, 0,
+			dev_name(&pdev->dev), mbox);
+	if (ret < 0) {
+		mbox_adapter_remove(&mbox->adapter);
 		return ret;
 	}
+	platform_set_drvdata(pdev, mbox);
 
 	return 0;
 }
 
 static int omap1_mbox_remove(struct platform_device *pdev)
 {
-	omap_mbox_unregister();
-	iounmap(mbox_base);
+	struct omap1_mbox *mbox;
+
+	mbox = platform_get_drvdata(pdev);
+	mbox_adapter_remove(&mbox->adapter);
 	return 0;
 }
 
-- 
1.8.1.5

[RFC 6/6] mailbox: omap2+: move to common mbox framework

[Courtney Cavin]

Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
---
 drivers/mailbox/Kconfig         |   1 -
 drivers/mailbox/mailbox-omap2.c | 315 +++++++++++++++++-----------------------
 2 files changed, 132 insertions(+), 184 deletions(-)

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index ae6b09b..a592a5a 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -27,7 +27,6 @@ config OMAP1_MBOX
 config OMAP2PLUS_MBOX
 	tristate "OMAP2+ Mailbox framework support"
 	depends on ARCH_OMAP2PLUS
-	depends on BROKEN
 	help
 	  Mailbox implementation for OMAP family chips with hardware for
 	  interprocessor communication involving DSP, IVA1.0 and IVA2 in
diff --git a/drivers/mailbox/mailbox-omap2.c b/drivers/mailbox/mailbox-omap2.c
index 42d2b89..7ddde19 100644
--- a/drivers/mailbox/mailbox-omap2.c
+++ b/drivers/mailbox/mailbox-omap2.c
@@ -18,8 +18,8 @@
 #include <linux/io.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_data/mailbox-omap.h>
-
-#include "omap-mbox.h"
+#include <linux/interrupt.h>
+#include <linux/mbox.h>
 
 #define MAILBOX_REVISION		0x000
 #define MAILBOX_MESSAGE(m)		(0x040 + 4 * (m))
@@ -42,192 +42,165 @@
 #define MBOX_NR_REGS			(MBOX_REG_SIZE / sizeof(u32))
 #define OMAP4_MBOX_NR_REGS		(OMAP4_MBOX_REG_SIZE / sizeof(u32))
 
-static void __iomem *mbox_base;
-
 struct omap_mbox2_fifo {
 	unsigned long msg;
 	unsigned long fifo_stat;
 	unsigned long msg_stat;
 };
 
+struct omap2_mbox;
+
 struct omap_mbox2_priv {
+	struct omap2_mbox *mbox;
+	int irq;
+
 	struct omap_mbox2_fifo tx_fifo;
 	struct omap_mbox2_fifo rx_fifo;
 	unsigned long irqenable;
 	unsigned long irqstatus;
 	u32 newmsg_bit;
 	u32 notfull_bit;
-	u32 ctx[OMAP4_MBOX_NR_REGS];
 	unsigned long irqdisable;
 	u32 intr_type;
 };
 
-static inline unsigned int mbox_read_reg(size_t ofs)
-{
-	return __raw_readl(mbox_base + ofs);
-}
+struct omap2_mbox {
+	struct mbox_adapter adapter;
+	struct completion completion;
+	void __iomem *base;
+	atomic_t active;
+	struct omap_mbox2_priv *priv;
+};
 
-static inline void mbox_write_reg(u32 val, size_t ofs)
+static inline unsigned int mbox_read_reg(void __iomem *base, size_t ofs)
 {
-	__raw_writel(val, mbox_base + ofs);
+	return __raw_readl(base + ofs);
 }
 
-/* Mailbox H/W preparations */
-static int omap2_mbox_startup(struct omap_mbox *mbox)
+static inline void mbox_write_reg(void __iomem *base, u32 val, size_t ofs)
 {
-	u32 l;
-
-	pm_runtime_enable(mbox->dev->parent);
-	pm_runtime_get_sync(mbox->dev->parent);
-
-	l = mbox_read_reg(MAILBOX_REVISION);
-	pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
-
-	return 0;
+	__raw_writel(val, base + ofs);
 }
 
-static void omap2_mbox_shutdown(struct omap_mbox *mbox)
+static int omap2_mbox_request(struct mbox_adapter *adap,
+		struct mbox_channel *chan)
 {
-	pm_runtime_put_sync(mbox->dev->parent);
-	pm_runtime_disable(mbox->dev->parent);
-}
+	struct omap_mbox2_priv *p;
+	struct omap2_mbox *mbox;
+	u32 enable;
 
-/* Mailbox FIFO handle functions */
-static mbox_msg_t omap2_mbox_fifo_read(struct omap_mbox *mbox)
-{
-	struct omap_mbox2_fifo *fifo =
-		&((struct omap_mbox2_priv *)mbox->priv)->rx_fifo;
-	return (mbox_msg_t) mbox_read_reg(fifo->msg);
-}
+	mbox = container_of(adap, struct omap2_mbox, adapter);
+	p = &mbox->priv[chan->id];
 
-static void omap2_mbox_fifo_write(struct omap_mbox *mbox, mbox_msg_t msg)
-{
-	struct omap_mbox2_fifo *fifo =
-		&((struct omap_mbox2_priv *)mbox->priv)->tx_fifo;
-	mbox_write_reg(msg, fifo->msg);
-}
+	if (atomic_inc_return(&mbox->active) == 1) {
+		pm_runtime_enable(adap->dev);
+		pm_runtime_get_sync(adap->dev);
+	}
 
-static int omap2_mbox_fifo_empty(struct omap_mbox *mbox)
-{
-	struct omap_mbox2_fifo *fifo =
-		&((struct omap_mbox2_priv *)mbox->priv)->rx_fifo;
-	return (mbox_read_reg(fifo->msg_stat) == 0);
-}
+	enable = mbox_read_reg(mbox->base, p->irqenable);
+	enable |= p->notfull_bit | p->newmsg_bit;
+	mbox_write_reg(mbox->base, enable, p->irqenable);
 
-static int omap2_mbox_fifo_full(struct omap_mbox *mbox)
-{
-	struct omap_mbox2_fifo *fifo =
-		&((struct omap_mbox2_priv *)mbox->priv)->tx_fifo;
-	return mbox_read_reg(fifo->fifo_stat);
+	return 0;
 }
 
-/* Mailbox IRQ handle functions */
-static void omap2_mbox_enable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
+static int omap2_mbox_release(struct mbox_adapter *adap,
+		struct mbox_channel *chan)
 {
-	struct omap_mbox2_priv *p = mbox->priv;
-	u32 l, bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
+	struct omap_mbox2_priv *p;
+	struct omap2_mbox *mbox;
+	u32 disable;
 
-	l = mbox_read_reg(p->irqenable);
-	l |= bit;
-	mbox_write_reg(l, p->irqenable);
-}
+	mbox = container_of(adap, struct omap2_mbox, adapter);
+	p = &mbox->priv[chan->id];
 
-static void omap2_mbox_disable_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	struct omap_mbox2_priv *p = mbox->priv;
-	u32 bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
+	disable = p->notfull_bit | p->newmsg_bit;
 
 	/*
 	 * Read and update the interrupt configuration register for pre-OMAP4.
 	 * OMAP4 and later SoCs have a dedicated interrupt disabling register.
 	 */
 	if (!p->intr_type)
-		bit = mbox_read_reg(p->irqdisable) & ~bit;
+		disable = mbox_read_reg(mbox->base, p->irqdisable) & ~disable;
+	mbox_write_reg(mbox->base, disable, p->irqdisable);
 
-	mbox_write_reg(bit, p->irqdisable);
+	if (atomic_dec_return(&mbox->active) == 0) {
+		pm_runtime_put_sync(adap->dev);
+		pm_runtime_disable(adap->dev);
+	}
+	return 0;
 }
 
-static void omap2_mbox_ack_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
+static int omap2_mbox_put_message(struct mbox_adapter *adap,
+		struct mbox_channel *chan, const void *data, unsigned int len)
 {
-	struct omap_mbox2_priv *p = mbox->priv;
-	u32 bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
+	struct omap_mbox2_fifo *fifo;
+	struct omap2_mbox *mbox;
+	int ret;
+	u32 msg;
 
-	mbox_write_reg(bit, p->irqstatus);
+	if (len != sizeof(msg))
+		return -EINVAL;
 
-	/* Flush posted write for irq status to avoid spurious interrupts */
-	mbox_read_reg(p->irqstatus);
-}
+	msg = ((u32 *)data)[0];
+	mbox = container_of(adap, struct omap2_mbox, adapter);
+	fifo = &mbox->priv[chan->id].tx_fifo;
 
-static int omap2_mbox_is_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
-{
-	struct omap_mbox2_priv *p = mbox->priv;
-	u32 bit = (irq == IRQ_TX) ? p->notfull_bit : p->newmsg_bit;
-	u32 enable = mbox_read_reg(p->irqenable);
-	u32 status = mbox_read_reg(p->irqstatus);
+	while (mbox_read_reg(mbox->base, fifo->fifo_stat)) {
+		ret = wait_for_completion_timeout(&mbox->completion,
+				msecs_to_jiffies(2));
+		if (!ret)
+			return -ETIMEDOUT;
+	}
 
-	return (int)(enable & status & bit);
-}
+	mbox_write_reg(mbox->base, msg, fifo->msg);
 
-static void omap2_mbox_save_ctx(struct omap_mbox *mbox)
-{
-	int i;
-	struct omap_mbox2_priv *p = mbox->priv;
-	int nr_regs;
-
-	if (p->intr_type)
-		nr_regs = OMAP4_MBOX_NR_REGS;
-	else
-		nr_regs = MBOX_NR_REGS;
-	for (i = 0; i < nr_regs; i++) {
-		p->ctx[i] = mbox_read_reg(i * sizeof(u32));
-
-		dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
-			i, p->ctx[i]);
-	}
+	return 0;
 }
 
-static void omap2_mbox_restore_ctx(struct omap_mbox *mbox)
+static irqreturn_t omap2_mbox_irq(int irq, void *dev)
 {
-	int i;
-	struct omap_mbox2_priv *p = mbox->priv;
-	int nr_regs;
-
-	if (p->intr_type)
-		nr_regs = OMAP4_MBOX_NR_REGS;
-	else
-		nr_regs = MBOX_NR_REGS;
-	for (i = 0; i < nr_regs; i++) {
-		mbox_write_reg(p->ctx[i], i * sizeof(u32));
-
-		dev_dbg(mbox->dev, "%s: [%02x] %08x\n", __func__,
-			i, p->ctx[i]);
+	struct mbox_channel *chan;
+	struct omap_mbox2_priv *p = dev;
+	struct omap2_mbox *mbox;
+	u32 status;
+
+	mbox = p->mbox;
+	status = mbox_read_reg(mbox->base, p->irqstatus);
+	status &= mbox_read_reg(mbox->base, p->irqenable);
+
+	chan = &mbox->adapter.channels[p - mbox->priv];
+
+	if (status & p->notfull_bit) {
+		complete(&mbox->completion);
+		mbox_write_reg(mbox->base, p->newmsg_bit, p->notfull_bit);
+	} else if (status & p->newmsg_bit) {
+		u32 msg = mbox_read_reg(mbox->base, p->rx_fifo.msg);
+		mbox_channel_notify(chan, &msg, sizeof(msg));
+		mbox_write_reg(mbox->base, p->newmsg_bit, p->irqstatus);
 	}
+
+	/* Flush posted write for irq status to avoid spurious interrupts */
+	mbox_read_reg(mbox->base, p->irqstatus);
+
+	return IRQ_HANDLED;
 }
 
-static struct omap_mbox_ops omap2_mbox_ops = {
-	.type		= OMAP_MBOX_TYPE2,
-	.startup	= omap2_mbox_startup,
-	.shutdown	= omap2_mbox_shutdown,
-	.fifo_read	= omap2_mbox_fifo_read,
-	.fifo_write	= omap2_mbox_fifo_write,
-	.fifo_empty	= omap2_mbox_fifo_empty,
-	.fifo_full	= omap2_mbox_fifo_full,
-	.enable_irq	= omap2_mbox_enable_irq,
-	.disable_irq	= omap2_mbox_disable_irq,
-	.ack_irq	= omap2_mbox_ack_irq,
-	.is_irq		= omap2_mbox_is_irq,
-	.save_ctx	= omap2_mbox_save_ctx,
-	.restore_ctx	= omap2_mbox_restore_ctx,
+static const struct mbox_adapter_ops omap2_mbox_ops = {
+	.owner = THIS_MODULE,
+	.request = omap2_mbox_request,
+	.release = omap2_mbox_release,
+	.put_message = omap2_mbox_put_message,
 };
 
 static int omap2_mbox_probe(struct platform_device *pdev)
 {
-	struct resource *mem;
-	int ret;
-	struct omap_mbox **list, *mbox, *mboxblk;
-	struct omap_mbox2_priv *priv, *privblk;
 	struct omap_mbox_pdata *pdata = pdev->dev.platform_data;
+	struct omap_mbox2_priv *priv, *privblk;
 	struct omap_mbox_dev_info *info;
+	struct omap2_mbox *mbox;
+	struct resource *mem;
+	int ret;
 	int i;
 
 	if (!pdata || !pdata->info_cnt || !pdata->info) {
@@ -235,25 +208,22 @@ static int omap2_mbox_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	/* allocate one extra for marking end of list */
-	list = kzalloc((pdata->info_cnt + 1) * sizeof(*list), GFP_KERNEL);
-	if (!list)
+	mbox = devm_kzalloc(&pdev->dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
 		return -ENOMEM;
 
-	mboxblk = mbox = kzalloc(pdata->info_cnt * sizeof(*mbox), GFP_KERNEL);
-	if (!mboxblk) {
-		ret = -ENOMEM;
-		goto free_list;
-	}
+	atomic_set(&mbox->active, 0);
+	init_completion(&mbox->completion);
 
-	privblk = priv = kzalloc(pdata->info_cnt * sizeof(*priv), GFP_KERNEL);
-	if (!privblk) {
-		ret = -ENOMEM;
-		goto free_mboxblk;
-	}
+	privblk = devm_kzalloc(&pdev->dev,
+			pdata->info_cnt * sizeof(*priv), GFP_KERNEL);
+	if (!privblk)
+		return -ENOMEM;
 
+	priv = privblk;
 	info = pdata->info;
 	for (i = 0; i < pdata->info_cnt; i++, info++, priv++) {
+		priv->mbox = mbox;
 		priv->tx_fifo.msg = MAILBOX_MESSAGE(info->tx_id);
 		priv->tx_fifo.fifo_stat = MAILBOX_FIFOSTATUS(info->tx_id);
 		priv->rx_fifo.msg =  MAILBOX_MESSAGE(info->rx_id);
@@ -272,61 +242,40 @@ static int omap2_mbox_probe(struct platform_device *pdev)
 		}
 		priv->intr_type = pdata->intr_type;
 
-		mbox->priv = priv;
-		mbox->name = info->name;
-		mbox->ops = &omap2_mbox_ops;
-		mbox->irq = platform_get_irq(pdev, info->irq_id);
-		if (mbox->irq < 0) {
-			ret = mbox->irq;
-			goto free_privblk;
-		}
-		list[i] = mbox++;
+		priv->irq = platform_get_irq(pdev, info->irq_id);
+		if (priv->irq < 0)
+			return priv->irq;
+
+		ret = devm_request_irq(&pdev->dev, priv->irq,
+				omap2_mbox_irq, IRQF_SHARED, info->name, priv);
+		if (ret < 0)
+			return ret;
 	}
+	mbox->priv = privblk;
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!mem) {
-		ret = -ENOENT;
-		goto free_privblk;
-	}
+	if (!mem)
+		return -ENOENT;
 
-	mbox_base = ioremap(mem->start, resource_size(mem));
-	if (!mbox_base) {
-		ret = -ENOMEM;
-		goto free_privblk;
-	}
+	mbox->base = devm_ioremap(&pdev->dev, mem->start, resource_size(mem));
+	if (!mbox->base)
+		return -ENOMEM;
 
-	ret = omap_mbox_register(&pdev->dev, list);
+	mbox->adapter.nchannels = pdata->info_cnt;
+	mbox->adapter.ops = &omap2_mbox_ops;
+	mbox->adapter.dev = &pdev->dev;
+	ret = mbox_adapter_add(&mbox->adapter);
 	if (ret)
-		goto unmap_mbox;
-	platform_set_drvdata(pdev, list);
+		return ret;
+	platform_set_drvdata(pdev, mbox);
 
 	return 0;
-
-unmap_mbox:
-	iounmap(mbox_base);
-free_privblk:
-	kfree(privblk);
-free_mboxblk:
-	kfree(mboxblk);
-free_list:
-	kfree(list);
-	return ret;
 }
 
 static int omap2_mbox_remove(struct platform_device *pdev)
 {
-	struct omap_mbox2_priv *privblk;
-	struct omap_mbox **list = platform_get_drvdata(pdev);
-	struct omap_mbox *mboxblk = list[0];
-
-	privblk = mboxblk->priv;
-	omap_mbox_unregister();
-	iounmap(mbox_base);
-	kfree(privblk);
-	kfree(mboxblk);
-	kfree(list);
-
-	return 0;
+	struct omap2_mbox *mbox = platform_get_drvdata(pdev);
+	return mbox_adapter_remove(&mbox->adapter);
 }
 
 static struct platform_driver omap2_mbox_driver = {
-- 
1.8.1.5

Re: [RFC 0/6] mailbox: add common framework and port drivers

[Jassi Brar]

Hi,

On 8 February 2014 06:20, Courtney Cavin <courtney.cavin@sonymobile.com> wrote:
> There is currently no common framework for mailbox drivers, so this is my
> attempt to come up with something suitable.  There seems to be a need for
> making this generic, so I have attempted to do just that.  Most of this is
> modeled pretty strongly after the pwm core, with some influences from the clock
> core.
>
> Looking at the existing use-cases, and some new ones, it would appear that the
> requirements here are rather simple. We need essentially two things for
> consumers:
>         - put_message
>         - callback for receiving messages
>
> The code currently uses atomic notifiers for callbacks.  The common omap core
> deals with fifos and work-queues in order to escape atomic contexts, but from
> what I can see, this is unneeded.  I am also of the opinion that the contexts
> can be much better managed in the drivers which are working with these
> contexts, rather than generically.
>
> Hopefully this will be suitable for the plethora of other drivers around the
> kernel which implement mailboxes, as well.  In any case, I'm rather interested
> to see what the rest of the world thinks.
>
> Keep in mind that while the pl320 & omap code should compile, I don't currently
> have a platform on which I can perform proper testing.  I also removed the
> context save/restore code from omap2 mailbox support, because I think it should
> be able to be done via driver suspend/resume, but haven't done a full
> investigation just yet.
>
> I'm also aware that breaking omap, just to fix it again probably isn't the best
> course of action, and I'm open to suggestions.
>
Did you try to look up the history of mailbox api development?  Google
search: 'mailbox common api'

I (Linaro/Fujitsu), Suman Anna (TI), LeyFoon Tan (Intel), Craig
McGeachie(Broadcom) and Loic Pallardy(ST) already worked a generic
Mailbox framework and infact have controller drivers working over
them.
 For some confidentiality and some lazy and some confusion or whatever
reasons the final version of drivers and API wasn't submitted upstream
yet.

 I think the shortest path to have some generic mailbox framework
upstream is for you to adapt your controller driver to that api and
maybe help pushing it upstream. (I should have clearance to push my
controller driver in a couple of weeks).
It might need a bit api update
https://github.com/sumananna/mailbox/commits/jassiv3-3.10-omap

Thanks
Jassi

Re: [RFC 0/6] mailbox: add common framework and port drivers

[Greg Kroah-Hartman]

On Sat, Feb 15, 2014 at 09:02:07AM +0530, Jassi Brar wrote:
> Hi,
> 
> On 8 February 2014 06:20, Courtney Cavin <courtney.cavin@sonymobile.com> wrote:
> > There is currently no common framework for mailbox drivers, so this is my
> > attempt to come up with something suitable.  There seems to be a need for
> > making this generic, so I have attempted to do just that.  Most of this is
> > modeled pretty strongly after the pwm core, with some influences from the clock
> > core.
> >
> > Looking at the existing use-cases, and some new ones, it would appear that the
> > requirements here are rather simple. We need essentially two things for
> > consumers:
> >         - put_message
> >         - callback for receiving messages
> >
> > The code currently uses atomic notifiers for callbacks.  The common omap core
> > deals with fifos and work-queues in order to escape atomic contexts, but from
> > what I can see, this is unneeded.  I am also of the opinion that the contexts
> > can be much better managed in the drivers which are working with these
> > contexts, rather than generically.
> >
> > Hopefully this will be suitable for the plethora of other drivers around the
> > kernel which implement mailboxes, as well.  In any case, I'm rather interested
> > to see what the rest of the world thinks.
> >
> > Keep in mind that while the pl320 & omap code should compile, I don't currently
> > have a platform on which I can perform proper testing.  I also removed the
> > context save/restore code from omap2 mailbox support, because I think it should
> > be able to be done via driver suspend/resume, but haven't done a full
> > investigation just yet.
> >
> > I'm also aware that breaking omap, just to fix it again probably isn't the best
> > course of action, and I'm open to suggestions.
> >
> Did you try to look up the history of mailbox api development?  Google
> search: 'mailbox common api'
> 
> I (Linaro/Fujitsu), Suman Anna (TI), LeyFoon Tan (Intel), Craig
> McGeachie(Broadcom) and Loic Pallardy(ST) already worked a generic
> Mailbox framework and infact have controller drivers working over
> them.
>  For some confidentiality and some lazy and some confusion or whatever
> reasons the final version of drivers and API wasn't submitted upstream
> yet.

Then, in all reality, it doesn't exist at all, and so, we will evaluate
this submission instead.

Just because you all can't send something for merging, doesn't mean you
get to block someone else who has got their act together, that's not
fair.

sorry,

greg k-h

Re: [RFC 0/6] mailbox: add common framework and port drivers

[Jassi Brar]

On 15 February 2014 09:10, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sat, Feb 15, 2014 at 09:02:07AM +0530, Jassi Brar wrote:
>> Hi,
>>
>> On 8 February 2014 06:20, Courtney Cavin <courtney.cavin@sonymobile.com> wrote:
>> > There is currently no common framework for mailbox drivers, so this is my
>> > attempt to come up with something suitable.  There seems to be a need for
>> > making this generic, so I have attempted to do just that.  Most of this is
>> > modeled pretty strongly after the pwm core, with some influences from the clock
>> > core.
>> >
>> > Looking at the existing use-cases, and some new ones, it would appear that the
>> > requirements here are rather simple. We need essentially two things for
>> > consumers:
>> >         - put_message
>> >         - callback for receiving messages
>> >
>> > The code currently uses atomic notifiers for callbacks.  The common omap core
>> > deals with fifos and work-queues in order to escape atomic contexts, but from
>> > what I can see, this is unneeded.  I am also of the opinion that the contexts
>> > can be much better managed in the drivers which are working with these
>> > contexts, rather than generically.
>> >
>> > Hopefully this will be suitable for the plethora of other drivers around the
>> > kernel which implement mailboxes, as well.  In any case, I'm rather interested
>> > to see what the rest of the world thinks.
>> >
>> > Keep in mind that while the pl320 & omap code should compile, I don't currently
>> > have a platform on which I can perform proper testing.  I also removed the
>> > context save/restore code from omap2 mailbox support, because I think it should
>> > be able to be done via driver suspend/resume, but haven't done a full
>> > investigation just yet.
>> >
>> > I'm also aware that breaking omap, just to fix it again probably isn't the best
>> > course of action, and I'm open to suggestions.
>> >
>> Did you try to look up the history of mailbox api development?  Google
>> search: 'mailbox common api'
>>
>> I (Linaro/Fujitsu), Suman Anna (TI), LeyFoon Tan (Intel), Craig
>> McGeachie(Broadcom) and Loic Pallardy(ST) already worked a generic
>> Mailbox framework and infact have controller drivers working over
>> them.
>>  For some confidentiality and some lazy and some confusion or whatever
>> reasons the final version of drivers and API wasn't submitted upstream
>> yet.
>
> Then, in all reality, it doesn't exist at all, and so, we will evaluate
> this submission instead.
>
> Just because you all can't send something for merging, doesn't mean you
> get to block someone else who has got their act together, that's not
> fair.
>
Yup probably not much fair. But then also one usually look for any
early development efforts. IIRC only I and Anna started. Others later
joined us looking at archives. Not to vindicate our gang though.

 Now we could either punish us and have this api tread the same
development path where everyone had their requirements (and the
only-waiting-for-approval controller drivers to convert) .... OR we
could see if our/original/old API just works for the purposes of Sony
as well (which it will most probably) and then we could upstream it
with one more 'works-for-me-too'.

Thanks.

Re: [RFC 0/6] mailbox: add common framework and port drivers

[Greg Kroah-Hartman]

On Sat, Feb 15, 2014 at 09:27:48AM +0530, Jassi Brar wrote:
> On 15 February 2014 09:10, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Sat, Feb 15, 2014 at 09:02:07AM +0530, Jassi Brar wrote:
> >> Hi,
> >>
> >> On 8 February 2014 06:20, Courtney Cavin <courtney.cavin@sonymobile.com> wrote:
> >> > There is currently no common framework for mailbox drivers, so this is my
> >> > attempt to come up with something suitable.  There seems to be a need for
> >> > making this generic, so I have attempted to do just that.  Most of this is
> >> > modeled pretty strongly after the pwm core, with some influences from the clock
> >> > core.
> >> >
> >> > Looking at the existing use-cases, and some new ones, it would appear that the
> >> > requirements here are rather simple. We need essentially two things for
> >> > consumers:
> >> >         - put_message
> >> >         - callback for receiving messages
> >> >
> >> > The code currently uses atomic notifiers for callbacks.  The common omap core
> >> > deals with fifos and work-queues in order to escape atomic contexts, but from
> >> > what I can see, this is unneeded.  I am also of the opinion that the contexts
> >> > can be much better managed in the drivers which are working with these
> >> > contexts, rather than generically.
> >> >
> >> > Hopefully this will be suitable for the plethora of other drivers around the
> >> > kernel which implement mailboxes, as well.  In any case, I'm rather interested
> >> > to see what the rest of the world thinks.
> >> >
> >> > Keep in mind that while the pl320 & omap code should compile, I don't currently
> >> > have a platform on which I can perform proper testing.  I also removed the
> >> > context save/restore code from omap2 mailbox support, because I think it should
> >> > be able to be done via driver suspend/resume, but haven't done a full
> >> > investigation just yet.
> >> >
> >> > I'm also aware that breaking omap, just to fix it again probably isn't the best
> >> > course of action, and I'm open to suggestions.
> >> >
> >> Did you try to look up the history of mailbox api development?  Google
> >> search: 'mailbox common api'
> >>
> >> I (Linaro/Fujitsu), Suman Anna (TI), LeyFoon Tan (Intel), Craig
> >> McGeachie(Broadcom) and Loic Pallardy(ST) already worked a generic
> >> Mailbox framework and infact have controller drivers working over
> >> them.
> >>  For some confidentiality and some lazy and some confusion or whatever
> >> reasons the final version of drivers and API wasn't submitted upstream
> >> yet.
> >
> > Then, in all reality, it doesn't exist at all, and so, we will evaluate
> > this submission instead.
> >
> > Just because you all can't send something for merging, doesn't mean you
> > get to block someone else who has got their act together, that's not
> > fair.
> >
> Yup probably not much fair. But then also one usually look for any
> early development efforts. IIRC only I and Anna started. Others later
> joined us looking at archives. Not to vindicate our gang though.
> 
>  Now we could either punish us and have this api tread the same
> development path where everyone had their requirements (and the
> only-waiting-for-approval controller drivers to convert) .... OR we
> could see if our/original/old API just works for the purposes of Sony
> as well (which it will most probably) and then we could upstream it
> with one more 'works-for-me-too'.

What is stopping you submitting your patches right now?

Re: [RFC 0/6] mailbox: add common framework and port drivers

[Jassi Brar]

On 15 February 2014 09:41, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Sat, Feb 15, 2014 at 09:27:48AM +0530, Jassi Brar wrote:
>> On 15 February 2014 09:10, Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>> > On Sat, Feb 15, 2014 at 09:02:07AM +0530, Jassi Brar wrote:
>> >> Hi,
>> >>
>> >> On 8 February 2014 06:20, Courtney Cavin <courtney.cavin@sonymobile.com> wrote:
>> >> > There is currently no common framework for mailbox drivers, so this is my
>> >> > attempt to come up with something suitable.  There seems to be a need for
>> >> > making this generic, so I have attempted to do just that.  Most of this is
>> >> > modeled pretty strongly after the pwm core, with some influences from the clock
>> >> > core.
>> >> >
>> >> > Looking at the existing use-cases, and some new ones, it would appear that the
>> >> > requirements here are rather simple. We need essentially two things for
>> >> > consumers:
>> >> >         - put_message
>> >> >         - callback for receiving messages
>> >> >
>> >> > The code currently uses atomic notifiers for callbacks.  The common omap core
>> >> > deals with fifos and work-queues in order to escape atomic contexts, but from
>> >> > what I can see, this is unneeded.  I am also of the opinion that the contexts
>> >> > can be much better managed in the drivers which are working with these
>> >> > contexts, rather than generically.
>> >> >
>> >> > Hopefully this will be suitable for the plethora of other drivers around the
>> >> > kernel which implement mailboxes, as well.  In any case, I'm rather interested
>> >> > to see what the rest of the world thinks.
>> >> >
>> >> > Keep in mind that while the pl320 & omap code should compile, I don't currently
>> >> > have a platform on which I can perform proper testing.  I also removed the
>> >> > context save/restore code from omap2 mailbox support, because I think it should
>> >> > be able to be done via driver suspend/resume, but haven't done a full
>> >> > investigation just yet.
>> >> >
>> >> > I'm also aware that breaking omap, just to fix it again probably isn't the best
>> >> > course of action, and I'm open to suggestions.
>> >> >
>> >> Did you try to look up the history of mailbox api development?  Google
>> >> search: 'mailbox common api'
>> >>
>> >> I (Linaro/Fujitsu), Suman Anna (TI), LeyFoon Tan (Intel), Craig
>> >> McGeachie(Broadcom) and Loic Pallardy(ST) already worked a generic
>> >> Mailbox framework and infact have controller drivers working over
>> >> them.
>> >>  For some confidentiality and some lazy and some confusion or whatever
>> >> reasons the final version of drivers and API wasn't submitted upstream
>> >> yet.
>> >
>> > Then, in all reality, it doesn't exist at all, and so, we will evaluate
>> > this submission instead.
>> >
>> > Just because you all can't send something for merging, doesn't mean you
>> > get to block someone else who has got their act together, that's not
>> > fair.
>> >
>> Yup probably not much fair. But then also one usually look for any
>> early development efforts. IIRC only I and Anna started. Others later
>> joined us looking at archives. Not to vindicate our gang though.
>>
>>  Now we could either punish us and have this api tread the same
>> development path where everyone had their requirements (and the
>> only-waiting-for-approval controller drivers to convert) .... OR we
>> could see if our/original/old API just works for the purposes of Sony
>> as well (which it will most probably) and then we could upstream it
>> with one more 'works-for-me-too'.
>
> What is stopping you submitting your patches right now?
>
Nothing. I'll freshen it up and submit today.

Thanks.