* [PATCHv4 0/5] Common Mailbox Framework
@ 2014-03-18 18:42 Jassi Brar
2014-03-18 18:44 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Jassi Brar
` (5 more replies)
0 siblings, 6 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:42 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri, Jassi Brar
Hi,
Here is the next revision of Mailbox code.
Changes since v3:
o Change name of symbols from ipc to mbox
o Return real types instead of void *
o Align structures
o Change some symbol names
rxcb -> rx_callback
txcb -> tx_done
o Added kernel-doc for exported API
o Dropped the cl_id and use clients pointer for callbacks.
o Fixed locking of channel pool
o Return negative error code for unsuccessful ipc_send_message()
o Module referencing during mailbox assignment to a client.
o Made error code symbols specific to mailbox.
Thanks
Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
@ 2014-03-18 18:44 ` Jassi Brar
2014-03-18 18:45 ` [PATCHv4 2/5] mailbox: Introduce framework for mailbox Jassi Brar
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:44 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri
From: Suman Anna <s-anna@ti.com>
The patch 30058677 "ARM / highbank: add support for pl320 IPC"
added a pl320 IPC specific header file as a generic mailbox.h.
This file has been renamed appropriately to allow the
introduction of the generic mailbox API framework.
Acked-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Suman Anna <s-anna@ti.com>
---
drivers/cpufreq/highbank-cpufreq.c | 2 +-
drivers/mailbox/pl320-ipc.c | 2 +-
include/linux/mailbox.h | 17 -----------------
include/linux/pl320-ipc.h | 17 +++++++++++++++++
4 files changed, 19 insertions(+), 19 deletions(-)
delete mode 100644 include/linux/mailbox.h
create mode 100644 include/linux/pl320-ipc.h
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
index bf8902a..b464f29 100644
--- a/drivers/cpufreq/highbank-cpufreq.c
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -19,7 +19,7 @@
#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/of.h>
-#include <linux/mailbox.h>
+#include <linux/pl320-ipc.h>
#include <linux/platform_device.h>
#define HB_CPUFREQ_CHANGE_NOTE 0x80000001
diff --git a/drivers/mailbox/pl320-ipc.c b/drivers/mailbox/pl320-ipc.c
index d873cba..f3755e0 100644
--- a/drivers/mailbox/pl320-ipc.c
+++ b/drivers/mailbox/pl320-ipc.c
@@ -26,7 +26,7 @@
#include <linux/device.h>
#include <linux/amba/bus.h>
-#include <linux/mailbox.h>
+#include <linux/pl320-ipc.h>
#define IPCMxSOURCE(m) ((m) * 0x40)
#define IPCMxDSET(m) (((m) * 0x40) + 0x004)
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
deleted file mode 100644
index 5161f63..0000000
--- a/include/linux/mailbox.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * 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.
- *
- * You should have received a copy of the GNU General Public License along with
- * 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);
diff --git a/include/linux/pl320-ipc.h b/include/linux/pl320-ipc.h
new file mode 100644
index 0000000..5161f63
--- /dev/null
+++ b/include/linux/pl320-ipc.h
@@ -0,0 +1,17 @@
+/*
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * 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);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv4 2/5] mailbox: Introduce framework for mailbox
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
2014-03-18 18:44 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Jassi Brar
@ 2014-03-18 18:45 ` Jassi Brar
2014-03-19 4:00 ` Girish KS
2014-03-28 22:08 ` [PATCHv4,2/5] " Markus Mayer
2014-03-18 18:45 ` [PATCHv4 3/5] mailbox: pl320: Introduce common API driver Jassi Brar
` (3 subsequent siblings)
5 siblings, 2 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:45 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri, Jassi Brar
Introduce common framework for client/protocol drivers and
controller drivers of Inter-Processor-Communication (IPC).
Client driver developers should have a look at
include/linux/mailbox_client.h to understand the part of
the API exposed to client drivers.
Similarly controller driver developers should have a look
at include/linux/mailbox_controller.h
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/mailbox/Makefile | 4 +
drivers/mailbox/mailbox.c | 589 +++++++++++++++++++++++++++++++++++++
include/linux/mailbox.h | 18 ++
include/linux/mailbox_client.h | 48 +++
include/linux/mailbox_controller.h | 85 ++++++
5 files changed, 744 insertions(+)
create mode 100644 drivers/mailbox/mailbox.c
create mode 100644 include/linux/mailbox.h
create mode 100644 include/linux/mailbox_client.h
create mode 100644 include/linux/mailbox_controller.h
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index e0facb3..2fa343a 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -1,3 +1,7 @@
+# Generic MAILBOX API
+
+obj-$(CONFIG_MAILBOX) += mailbox.o
+
obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
obj-$(CONFIG_OMAP_MBOX) += omap-mailbox.o
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
new file mode 100644
index 0000000..79d576e
--- /dev/null
+++ b/drivers/mailbox/mailbox.c
@@ -0,0 +1,589 @@
+/*
+ * Mailbox: Common code for Mailbox controllers and users
+ *
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar <jassisinghbrar@gmail.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.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/spinlock.h>
+#include <linux/mutex.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox_controller.h>
+
+/*
+ * The length of circular buffer for queuing messages from a client.
+ * 'msg_count' tracks the number of buffered messages while 'msg_free'
+ * is the index where the next message would be buffered.
+ * We shouldn't need it too big because every transferr is interrupt
+ * triggered and if we have lots of data to transfer, the interrupt
+ * latencies are going to be the bottleneck, not the buffer length.
+ * Besides, mbox_send_message could be called from atomic context and
+ * the client could also queue another message from the notifier 'tx_done'
+ * of the last transfer done.
+ * REVIST: If too many platforms see the "Try increasing MBOX_TX_QUEUE_LEN"
+ * print, it needs to be taken from config option or somesuch.
+ */
+#define MBOX_TX_QUEUE_LEN 20
+
+#define TXDONE_BY_IRQ (1 << 0) /* controller has remote RTR irq */
+#define TXDONE_BY_POLL (1 << 1) /* controller can read status of last TX */
+#define TXDONE_BY_ACK (1 << 2) /* S/W ACK recevied by Client ticks the TX */
+
+struct mbox_chan {
+ char name[16]; /* Physical link's name */
+ struct mbox_con *con; /* Parent Controller */
+ unsigned txdone_method;
+
+ /* Physical links */
+ struct mbox_link *link;
+ struct mbox_link_ops *link_ops;
+
+ /* client */
+ struct mbox_client *cl;
+ struct completion tx_complete;
+
+ void *active_req;
+ unsigned msg_count, msg_free;
+ void *msg_data[MBOX_TX_QUEUE_LEN];
+ /* Access to the channel */
+ spinlock_t lock;
+ /* Hook to add to the controller's list of channels */
+ struct list_head node;
+ /* Notifier to all clients waiting on aquiring this channel */
+ struct blocking_notifier_head avail;
+} __aligned(32);
+
+/* Internal representation of a controller */
+struct mbox_con {
+ struct device *dev;
+ char name[16]; /* controller_name */
+ struct list_head channels;
+ /*
+ * If the controller supports only TXDONE_BY_POLL,
+ * this timer polls all the links for txdone.
+ */
+ struct timer_list poll;
+ unsigned period;
+ /* Hook to add to the global controller list */
+ struct list_head node;
+} __aligned(32);
+
+static LIST_HEAD(mbox_cons);
+static DEFINE_MUTEX(con_mutex);
+
+static int _add_to_rbuf(struct mbox_chan *chan, void *mssg)
+{
+ int idx;
+ unsigned long flags;
+
+ spin_lock_irqsave(&chan->lock, flags);
+
+ /* See if there is any space left */
+ if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return -ENOMEM;
+ }
+
+ idx = chan->msg_free;
+ chan->msg_data[idx] = mssg;
+ chan->msg_count++;
+
+ if (idx == MBOX_TX_QUEUE_LEN - 1)
+ chan->msg_free = 0;
+ else
+ chan->msg_free++;
+
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ return idx;
+}
+
+static void _msg_submit(struct mbox_chan *chan)
+{
+ struct mbox_link *link = chan->link;
+ unsigned count, idx;
+ unsigned long flags;
+ void *data;
+ int err;
+
+ spin_lock_irqsave(&chan->lock, flags);
+
+ if (!chan->msg_count || chan->active_req) {
+ spin_unlock_irqrestore(&chan->lock, flags);
+ return;
+ }
+
+ count = chan->msg_count;
+ idx = chan->msg_free;
+ if (idx >= count)
+ idx -= count;
+ else
+ idx += MBOX_TX_QUEUE_LEN - count;
+
+ data = chan->msg_data[idx];
+
+ /* Try to submit a message to the MBOX controller */
+ err = chan->link_ops->send_data(link, data);
+ if (!err) {
+ chan->active_req = data;
+ chan->msg_count--;
+ }
+
+ spin_unlock_irqrestore(&chan->lock, flags);
+}
+
+static void tx_tick(struct mbox_chan *chan, enum mbox_result r)
+{
+ unsigned long flags;
+ void *mssg;
+
+ spin_lock_irqsave(&chan->lock, flags);
+ mssg = chan->active_req;
+ chan->active_req = NULL;
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ /* Submit next message */
+ _msg_submit(chan);
+
+ /* Notify the client */
+ if (chan->cl->tx_block)
+ complete(&chan->tx_complete);
+ else if (mssg && chan->cl->tx_done)
+ chan->cl->tx_done(chan->cl, mssg, r);
+}
+
+static void poll_txdone(unsigned long data)
+{
+ struct mbox_con *con = (struct mbox_con *)data;
+ bool txdone, resched = false;
+ struct mbox_chan *chan;
+
+ list_for_each_entry(chan, &con->channels, node) {
+ if (chan->active_req && chan->cl) {
+ resched = true;
+ txdone = chan->link_ops->last_tx_done(chan->link);
+ if (txdone)
+ tx_tick(chan, MBOX_OK);
+ }
+ }
+
+ if (resched)
+ mod_timer(&con->poll,
+ jiffies + msecs_to_jiffies(con->period));
+}
+
+/**
+ * mbox_link_received_data - A way for controller driver to push data
+ * received from remote to the upper layer.
+ * @link: Pointer to the mailbox link on which RX happened.
+ * @data: Client specific message typecasted as void *
+ *
+ * After startup and before shutdown any data received on the link
+ * is passed on to the API via atomic mbox_link_received_data().
+ * The controller should ACK the RX only after this call returns.
+ */
+void mbox_link_received_data(struct mbox_link *link, void *mssg)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
+
+ /* No buffering the received data */
+ if (chan->cl->rx_callback)
+ chan->cl->rx_callback(chan->cl, mssg);
+}
+EXPORT_SYMBOL_GPL(mbox_link_received_data);
+
+/**
+ * mbox_link_txdone - A way for controller driver to notify the
+ * framework that the last TX has completed.
+ * @link: Pointer to the mailbox link on which TX happened.
+ * @r: Status of last TX - OK or ERROR
+ *
+ * The controller that has IRQ for TX ACK calls this atomic API
+ * to tick the TX state machine. It works only if txdone_irq
+ * is set by the controller.
+ */
+void mbox_link_txdone(struct mbox_link *link, enum mbox_result r)
+{
+ struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
+
+ if (unlikely(!(chan->txdone_method & TXDONE_BY_IRQ))) {
+ pr_err("Controller can't run the TX ticker\n");
+ return;
+ }
+
+ tx_tick(chan, r);
+}
+EXPORT_SYMBOL_GPL(mbox_link_txdone);
+
+/**
+ * mbox_client_txdone - The way for a client to run the TX state machine.
+ * @chan: Mailbox channel assigned to this client.
+ * @r: Success status of last transmission.
+ *
+ * The client/protocol had received some 'ACK' packet and it notifies
+ * the API that the last packet was sent successfully. This only works
+ * if the controller can't sense TX-Done.
+ */
+void mbox_client_txdone(struct mbox_chan *chan, enum mbox_result r)
+{
+ if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
+ pr_err("Client can't run the TX ticker\n");
+ return;
+ }
+
+ tx_tick(chan, r);
+}
+EXPORT_SYMBOL_GPL(mbox_client_txdone);
+
+/**
+ * mbox_send_message - For client to submit a message to be
+ * sent to the remote.
+ * @chan: Mailbox channel assigned to this client.
+ * @mssg: Client specific message typecasted.
+ *
+ * For client to submit data to the controller destined for a remote
+ * processor. If the client had set 'tx_block', the call will return
+ * either when the remote receives the data or when 'tx_tout' millisecs
+ * run out.
+ * In non-blocking mode, the requests are buffered by the API and a
+ * non-negative token is returned for each queued request. If the request
+ * is not queued, a negative token is returned. Upon failure or successful
+ * TX, the API calls 'tx_done' from atomic context, from which the client
+ * could submit yet another request.
+ * In blocking mode, 'tx_done' is not called, effectively making the
+ * queue length 1.
+ * The pointer to message should be preserved until it is sent
+ * over the link, i.e, tx_done() is made.
+ * This function could be called from atomic context as it simply
+ * queues the data and returns a token against the request.
+ *
+ * Return: Non-negative integer for successful submission (non-blocking mode)
+ * or transmission over link (blocking mode).
+ * Negative value denotes failure.
+ */
+int mbox_send_message(struct mbox_chan *chan, void *mssg)
+{
+ int t;
+
+ if (!chan || !chan->cl)
+ return -EINVAL;
+
+ t = _add_to_rbuf(chan, mssg);
+ if (t < 0) {
+ pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
+ return t;
+ }
+
+ _msg_submit(chan);
+
+ if (chan->txdone_method == TXDONE_BY_POLL)
+ poll_txdone((unsigned long)chan->con);
+
+ if (chan->cl->tx_block && chan->active_req) {
+ int ret;
+ init_completion(&chan->tx_complete);
+ ret = wait_for_completion_timeout(&chan->tx_complete,
+ chan->cl->tx_tout);
+ if (ret == 0) {
+ t = -EIO;
+ tx_tick(chan, MBOX_ERR);
+ }
+ }
+
+ return t;
+}
+EXPORT_SYMBOL_GPL(mbox_send_message);
+
+/**
+ * mbox_request_channel - Request a mailbox channel.
+ * @cl: Identity of the client requesting the channel.
+ *
+ * The Client specifies its requirements and capabilities while asking for
+ * a mailbox channel by name. It can't be called from atomic context.
+ * The channel is exclusively allocated and can't be used by another
+ * client before the owner calls mbox_free_channel.
+ * After assignment, any packet received on this channel will be
+ * handed over to the client via the 'rx_callback'.
+ *
+ * Return: Pointer to the channel assigned to the client if successful.
+ * ERR_PTR for request failure.
+ */
+struct mbox_chan *mbox_request_channel(struct mbox_client *cl)
+{
+ struct mbox_chan *chan;
+ struct mbox_con *con;
+ unsigned long flags;
+ char *con_name;
+ int len, ret;
+
+ con_name = cl->chan_name;
+ len = strcspn(cl->chan_name, ":");
+
+ ret = 0;
+ mutex_lock(&con_mutex);
+ list_for_each_entry(con, &mbox_cons, node)
+ if (!strncmp(con->name, con_name, len)) {
+ ret = 1;
+ break;
+ }
+ mutex_unlock(&con_mutex);
+
+ if (!ret) {
+ pr_info("Channel(%s) not found!\n", cl->chan_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ ret = 0;
+ list_for_each_entry(chan, &con->channels, node) {
+ if (!chan->cl &&
+ !strcmp(con_name + len + 1, chan->name) &&
+ try_module_get(con->dev->driver->owner)) {
+ spin_lock_irqsave(&chan->lock, flags);
+ chan->msg_free = 0;
+ chan->msg_count = 0;
+ chan->active_req = NULL;
+ chan->cl = cl;
+ if (!cl->tx_tout) /* wait for ever */
+ cl->tx_tout = msecs_to_jiffies(3600000);
+ else
+ cl->tx_tout = msecs_to_jiffies(cl->tx_tout);
+ if (chan->txdone_method == TXDONE_BY_POLL
+ && cl->knows_txdone)
+ chan->txdone_method |= TXDONE_BY_ACK;
+ spin_unlock_irqrestore(&chan->lock, flags);
+ ret = 1;
+ break;
+ }
+ }
+
+ if (!ret) {
+ pr_err("Unable to assign mailbox(%s)\n", cl->chan_name);
+ return ERR_PTR(-EBUSY);
+ }
+
+ ret = chan->link_ops->startup(chan->link, cl->link_data);
+ if (ret) {
+ pr_err("Unable to startup the link\n");
+ mbox_free_channel(chan);
+ return ERR_PTR(ret);
+ }
+
+ return chan;
+}
+EXPORT_SYMBOL_GPL(mbox_request_channel);
+
+/**
+ * mbox_free_channel - The client relinquishes control of a mailbox
+ * channel by this call.
+ * @chan: The mailbox channel to be freed.
+ */
+void mbox_free_channel(struct mbox_chan *chan)
+{
+ unsigned long flags;
+
+ if (!chan || !chan->cl)
+ return;
+
+ chan->link_ops->shutdown(chan->link);
+
+ /* The queued TX requests are simply aborted, no callbacks are made */
+ spin_lock_irqsave(&chan->lock, flags);
+ chan->cl = NULL;
+ chan->active_req = NULL;
+ if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
+ chan->txdone_method = TXDONE_BY_POLL;
+
+ module_put(chan->con->dev->driver->owner);
+
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ blocking_notifier_call_chain(&chan->avail, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(mbox_free_channel);
+
+static struct mbox_chan *name_to_chan(const char *name)
+{
+ struct mbox_chan *chan = NULL;
+ struct mbox_con *con;
+ int len, found = 0;
+
+ len = strcspn(name, ":");
+
+ mutex_lock(&con_mutex);
+
+ list_for_each_entry(con, &mbox_cons, node) {
+ if (!strncmp(con->name, name, len)) {
+ list_for_each_entry(chan, &con->channels, node) {
+ if (!strcmp(name + len + 1, chan->name)) {
+ found = 1;
+ goto done;
+ }
+ }
+ }
+ }
+done:
+ mutex_unlock(&con_mutex);
+
+ if (!found)
+ return NULL;
+
+ return chan;
+}
+
+/**
+ * mbox_notify_chan_register - The client may ask the framework to be
+ * notified when a particular channel becomes available
+ * to be acquired again.
+ * @name: Name of the mailbox channel the client is interested in.
+ * @nb: Pointer to the notifier.
+ */
+int mbox_notify_chan_register(const char *name, struct notifier_block *nb)
+{
+ struct mbox_chan *chan = name_to_chan(name);
+
+ if (chan && nb)
+ return blocking_notifier_chain_register(&chan->avail, nb);
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL_GPL(mbox_notify_chan_register);
+
+/**
+ * mbox_notify_chan_unregister - The client is no more interested in channel.
+ *
+ * @name: Name of the mailbox channel the client was interested in.
+ * @nb: Pointer to the notifier.
+ */
+void mbox_notify_chan_unregister(const char *name, struct notifier_block *nb)
+{
+ struct mbox_chan *chan = name_to_chan(name);
+
+ if (chan && nb)
+ blocking_notifier_chain_unregister(&chan->avail, nb);
+}
+EXPORT_SYMBOL_GPL(mbox_notify_chan_unregister);
+
+/**
+ * mbox_controller_register - Register the mailbox controller
+ * @mbox_con: Pointer to the mailbox controller.
+ *
+ * The controller driver registers its communication links to the
+ * global pool managed by the common framework.
+ */
+int mbox_controller_register(struct mbox_controller *mbox)
+{
+ int i, num_links, txdone;
+ struct mbox_chan *chan;
+ struct mbox_con *con;
+
+ /* Sanity check */
+ if (!mbox || !mbox->ops)
+ return -EINVAL;
+
+ for (i = 0; mbox->links[i]; i++)
+ ;
+ if (!i)
+ return -EINVAL;
+ num_links = i;
+
+ mutex_lock(&con_mutex);
+ /* Check if already populated */
+ list_for_each_entry(con, &mbox_cons, node)
+ if (!strcmp(mbox->controller_name, con->name)) {
+ mutex_unlock(&con_mutex);
+ return -EINVAL;
+ }
+
+ con = kzalloc(sizeof(struct mbox_con), GFP_KERNEL);
+ if (!con)
+ return -ENOMEM;
+
+ chan = kzalloc(sizeof(struct mbox_chan) * num_links, GFP_KERNEL);
+ if (!chan) {
+ kfree(con);
+ return -ENOMEM;
+ }
+
+ con->dev = mbox->dev;
+ INIT_LIST_HEAD(&con->channels);
+ snprintf(con->name, 16, "%s", mbox->controller_name);
+
+ if (mbox->txdone_irq)
+ txdone = TXDONE_BY_IRQ;
+ else if (mbox->txdone_poll)
+ txdone = TXDONE_BY_POLL;
+ else /* It has to be ACK then */
+ txdone = TXDONE_BY_ACK;
+
+ if (txdone == TXDONE_BY_POLL) {
+ con->period = mbox->txpoll_period;
+ con->poll.function = &poll_txdone;
+ con->poll.data = (unsigned long)con;
+ init_timer(&con->poll);
+ }
+
+ for (i = 0; i < num_links; i++) {
+ chan[i].con = con;
+ chan[i].cl = NULL;
+ chan[i].link_ops = mbox->ops;
+ chan[i].link = mbox->links[i];
+ chan[i].txdone_method = txdone;
+ chan[i].link->api_priv = &chan[i];
+ spin_lock_init(&chan[i].lock);
+ BLOCKING_INIT_NOTIFIER_HEAD(&chan[i].avail);
+ list_add_tail(&chan[i].node, &con->channels);
+ snprintf(chan[i].name, 16, "%s", mbox->links[i]->link_name);
+ }
+
+ list_add_tail(&con->node, &mbox_cons);
+ mutex_unlock(&con_mutex);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mbox_controller_register);
+
+/**
+ * mbox_controller_unregister - UnRegister the mailbox controller
+ * @mbox_con: Pointer to the mailbox controller.
+ *
+ * Purge the mailbox links from the global pool maintained by the framework.
+ */
+void mbox_controller_unregister(struct mbox_controller *mbox)
+{
+ struct mbox_con *t, *con = NULL;
+ struct mbox_chan *chan;
+
+ mutex_lock(&con_mutex);
+
+ list_for_each_entry(t, &mbox_cons, node)
+ if (!strcmp(mbox->controller_name, t->name)) {
+ con = t;
+ break;
+ }
+
+ if (con)
+ list_del(&con->node);
+
+ mutex_unlock(&con_mutex);
+
+ if (!con)
+ return;
+
+ list_for_each_entry(chan, &con->channels, node)
+ mbox_free_channel(chan);
+
+ del_timer_sync(&con->poll);
+
+ kfree(con);
+}
+EXPORT_SYMBOL_GPL(mbox_controller_unregister);
diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
new file mode 100644
index 0000000..44bcb2b
--- /dev/null
+++ b/include/linux/mailbox.h
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar <jassisinghbrar@gmail.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.
+ */
+
+#ifndef __MAILBOX_H
+#define __MAILBOX_H
+
+enum mbox_result {
+ MBOX_OK = 0,
+ MBOX_ERR,
+};
+
+#endif /* __MAILBOX_H */
diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
new file mode 100644
index 0000000..4954378
--- /dev/null
+++ b/include/linux/mailbox_client.h
@@ -0,0 +1,48 @@
+/*
+ * Copyright (C) 2014 Linaro Ltd.
+ * Author: Jassi Brar <jassisinghbrar@gmail.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.
+ */
+
+#ifndef __MAILBOX_CLIENT_H
+#define __MAILBOX_CLIENT_H
+
+#include <linux/mailbox.h>
+
+struct mbox_chan;
+
+/**
+ * struct mbox_client - User of a mailbox
+ * @chan_name: The "controller:channel" this client wants
+ * @rx_callback: Atomic callback to provide client the data received
+ * @tx_done: Atomic callback to tell client of data transmission
+ * @tx_block: If the mbox_send_message should block until data is
+ * transmitted.
+ * @tx_tout: Max block period in ms before TX is assumed failure
+ * @knows_txdone: if the client could run the TX state machine. Usually
+ * if the client receives some ACK packet for transmission.
+ * Unused if the controller already has TX_Done/RTR IRQ.
+ * @link_data: Optional controller specific parameters during channel
+ * request.
+ */
+struct mbox_client {
+ char *chan_name;
+ void (*rx_callback)(struct mbox_client *cl, void *mssg);
+ void (*tx_done)(struct mbox_client *cl, void *mssg, enum mbox_result r);
+ bool tx_block;
+ unsigned long tx_tout;
+ bool knows_txdone;
+ void *link_data;
+};
+
+struct mbox_chan *mbox_request_channel(struct mbox_client *cl);
+int mbox_send_message(struct mbox_chan *chan, void *mssg);
+void mbox_client_txdone(struct mbox_chan *chan, enum mbox_result r);
+void mbox_free_channel(struct mbox_chan *chan);
+int mbox_notify_chan_register(const char *name, struct notifier_block *nb);
+void mbox_notify_chan_unregister(const char *name, struct notifier_block *nb);
+
+#endif /* __MAILBOX_CLIENT_H */
diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
new file mode 100644
index 0000000..5ff22c6
--- /dev/null
+++ b/include/linux/mailbox_controller.h
@@ -0,0 +1,85 @@
+/*
+ * 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 __MAILBOX_CONTROLLER_H
+#define __MAILBOX_CONTROLLER_H
+
+#include <linux/mailbox.h>
+
+/**
+ * struct mbox_link - s/w representation of a communication link
+ * @link_name: Literal name assigned to the link. Physically
+ * identical channels may have the same name.
+ * @api_priv: Hook for the API to map its private data on the link
+ * Controller driver must not touch it.
+ */
+struct mbox_link {
+ char link_name[16];
+ void *api_priv;
+};
+
+/**
+ * struct mbox_link - s/w representation of a communication link
+ * @send_data: The API asks the MBOX controller driver, in atomic
+ * context try to transmit a message on the bus. Returns 0 if
+ * data is accepted for transmission, -EBUSY while rejecting
+ * if the remote hasn't yet read the last data sent. Actual
+ * transmission of data is reported by the controller via
+ * mbox_link_txdone (if it has some TX ACK irq). It must not
+ * block.
+ * @startup: Called when a client requests the link. The controller
+ * could ask clients for additional parameters of communication
+ * to be provided via client's link_data. This call may
+ * block. After this call the Controller must forward any
+ * data received on the link by calling mbox_link_received_data.
+ * @shutdown: Called when a client relinquishes control of a link.
+ * This call may block too. The controller must not forwared
+ * any received data anymore.
+ * @last_tx_done: If the controller sets 'txdone_poll', the API calls
+ * this to poll status of last TX. The controller must
+ * give priority to IRQ method over polling and never
+ * set both txdone_poll and txdone_irq. Only in polling
+ * mode 'send_data' is expected to return -EBUSY.
+ * Used only if txdone_poll:=true && txdone_irq:=false
+ */
+struct mbox_link_ops {
+ int (*send_data)(struct mbox_link *link, void *data);
+ int (*startup)(struct mbox_link *link, void *params);
+ void (*shutdown)(struct mbox_link *link);
+ bool (*last_tx_done)(struct mbox_link *link);
+};
+
+/**
+ * struct mbox_controller - Controller of a class of communication links
+ * @dev: Device backing this controller
+ * @controller_name: Literal name of the controller.
+ * @ops: Operators that work on each communication link
+ * @links: Null terminated array of links.
+ * @txdone_irq: Indicates if the controller can report to API when
+ * the last transmitted data was read by the remote.
+ * Eg, if it has some TX ACK irq.
+ * @txdone_poll: If the controller can read but not report the TX
+ * done. Ex, some register shows the TX status but
+ * no interrupt rises. Ignored if 'txdone_irq' is set.
+ * @txpoll_period: If 'txdone_poll' is in effect, the API polls for
+ * last TX's status after these many millisecs
+ */
+struct mbox_controller {
+ struct device *dev;
+ char controller_name[16];
+ struct mbox_link_ops *ops;
+ struct mbox_link **links;
+ bool txdone_irq;
+ bool txdone_poll;
+ unsigned txpoll_period;
+};
+
+int mbox_controller_register(struct mbox_controller *mbox);
+void mbox_link_received_data(struct mbox_link *link, void *data);
+void mbox_link_txdone(struct mbox_link *link, enum mbox_result r);
+void mbox_controller_unregister(struct mbox_controller *mbox);
+
+#endif /* __MAILBOX_CONTROLLER_H */
--
1.8.1.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv4 3/5] mailbox: pl320: Introduce common API driver
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
2014-03-18 18:44 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Jassi Brar
2014-03-18 18:45 ` [PATCHv4 2/5] mailbox: Introduce framework for mailbox Jassi Brar
@ 2014-03-18 18:45 ` Jassi Brar
2014-03-18 19:10 ` Rob Herring
2014-03-18 18:46 ` [PATCHv4 4/5] mailbox: Fix TX completion init Jassi Brar
` (2 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:45 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri, Jassi Brar
Convert the PL320 controller driver to work with the common
mailbox API. Also convert the only user of PL320, highbank-cpufreq.c
to work with thee API. Drop the obsoleted driver pl320-ipc.c
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/cpufreq/highbank-cpufreq.c | 24 ++++-
drivers/mailbox/Makefile | 2 +-
drivers/mailbox/pl320-ipc.c | 198 ----------------------------------
drivers/mailbox/pl320.c | 214 +++++++++++++++++++++++++++++++++++++
include/linux/pl320-ipc.h | 17 ---
5 files changed, 237 insertions(+), 218 deletions(-)
delete mode 100644 drivers/mailbox/pl320-ipc.c
create mode 100644 drivers/mailbox/pl320.c
delete mode 100644 include/linux/pl320-ipc.h
diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
index b464f29..4846734 100644
--- a/drivers/cpufreq/highbank-cpufreq.c
+++ b/drivers/cpufreq/highbank-cpufreq.c
@@ -19,7 +19,7 @@
#include <linux/cpu.h>
#include <linux/err.h>
#include <linux/of.h>
-#include <linux/pl320-ipc.h>
+#include <linux/mailbox_client.h>
#include <linux/platform_device.h>
#define HB_CPUFREQ_CHANGE_NOTE 0x80000001
@@ -29,8 +29,28 @@
static int hb_voltage_change(unsigned int freq)
{
u32 msg[HB_CPUFREQ_IPC_LEN] = {HB_CPUFREQ_CHANGE_NOTE, freq / 1000000};
+ struct mbox_client cl;
+ int ret = -ETIMEDOUT;
+ struct mbox_chan *chan;
- return pl320_ipc_transmit(msg);
+ cl.rx_callback = NULL;
+ cl.tx_done = NULL;
+ cl.tx_block = true;
+ cl.tx_tout = 1000; /* 1 sec */
+ cl.link_data = NULL;
+ cl.knows_txdone = false;
+ cl.chan_name = "pl320:A9_to_M3";
+
+ chan = mbox_request_channel(&cl);
+ if (IS_ERR(chan))
+ return PTR_ERR(chan);
+
+ if (mbox_send_message(chan, (void *)msg))
+ ret = msg[1]; /* PL320 updates buffer with FIFO after ACK */
+
+ mbox_free_channel(chan);
+
+ return ret;
}
static int hb_cpufreq_clk_notify(struct notifier_block *nb,
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index 2fa343a..f3f4ab7 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -2,7 +2,7 @@
obj-$(CONFIG_MAILBOX) += mailbox.o
-obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
+obj-$(CONFIG_PL320_MBOX) += pl320.o
obj-$(CONFIG_OMAP_MBOX) += omap-mailbox.o
obj-$(CONFIG_OMAP1_MBOX) += mailbox_omap1.o
diff --git a/drivers/mailbox/pl320-ipc.c b/drivers/mailbox/pl320-ipc.c
deleted file mode 100644
index f3755e0..0000000
--- a/drivers/mailbox/pl320-ipc.c
+++ /dev/null
@@ -1,198 +0,0 @@
-/*
- * Copyright 2012 Calxeda, Inc.
- *
- * 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.
- *
- * You should have received a copy of the GNU General Public License along with
- * this program. If not, see <http://www.gnu.org/licenses/>.
- */
-#include <linux/types.h>
-#include <linux/err.h>
-#include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/io.h>
-#include <linux/interrupt.h>
-#include <linux/completion.h>
-#include <linux/mutex.h>
-#include <linux/notifier.h>
-#include <linux/spinlock.h>
-#include <linux/device.h>
-#include <linux/amba/bus.h>
-
-#include <linux/pl320-ipc.h>
-
-#define IPCMxSOURCE(m) ((m) * 0x40)
-#define IPCMxDSET(m) (((m) * 0x40) + 0x004)
-#define IPCMxDCLEAR(m) (((m) * 0x40) + 0x008)
-#define IPCMxDSTATUS(m) (((m) * 0x40) + 0x00C)
-#define IPCMxMODE(m) (((m) * 0x40) + 0x010)
-#define IPCMxMSET(m) (((m) * 0x40) + 0x014)
-#define IPCMxMCLEAR(m) (((m) * 0x40) + 0x018)
-#define IPCMxMSTATUS(m) (((m) * 0x40) + 0x01C)
-#define IPCMxSEND(m) (((m) * 0x40) + 0x020)
-#define IPCMxDR(m, dr) (((m) * 0x40) + ((dr) * 4) + 0x024)
-
-#define IPCMMIS(irq) (((irq) * 8) + 0x800)
-#define IPCMRIS(irq) (((irq) * 8) + 0x804)
-
-#define MBOX_MASK(n) (1 << (n))
-#define IPC_TX_MBOX 1
-#define IPC_RX_MBOX 2
-
-#define CHAN_MASK(n) (1 << (n))
-#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);
-
-static inline void set_destination(int source, int mbox)
-{
- __raw_writel(CHAN_MASK(source), ipc_base + IPCMxDSET(mbox));
- __raw_writel(CHAN_MASK(source), ipc_base + IPCMxMSET(mbox));
-}
-
-static inline void clear_destination(int source, int mbox)
-{
- __raw_writel(CHAN_MASK(source), ipc_base + IPCMxDCLEAR(mbox));
- __raw_writel(CHAN_MASK(source), ipc_base + IPCMxMCLEAR(mbox));
-}
-
-static void __ipc_send(int mbox, 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));
-}
-
-static u32 __ipc_rcv(int mbox, u32 *data)
-{
- int i;
- for (i = 0; i < 7; i++)
- data[i] = __raw_readl(ipc_base + IPCMxDR(mbox, i));
- return data[1];
-}
-
-/* blocking implmentation from the A9 side, not usuable in interrupts! */
-int pl320_ipc_transmit(u32 *data)
-{
- int ret;
-
- mutex_lock(&ipc_m1_lock);
-
- init_completion(&ipc_completion);
- __ipc_send(IPC_TX_MBOX, data);
- ret = wait_for_completion_timeout(&ipc_completion,
- msecs_to_jiffies(1000));
- if (ret == 0) {
- ret = -ETIMEDOUT;
- goto out;
- }
-
- 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)
-{
- u32 irq_stat;
- u32 data[7];
-
- irq_stat = __raw_readl(ipc_base + IPCMMIS(1));
- if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
- __raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
- complete(&ipc_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));
- }
-
- 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 int pl320_probe(struct amba_device *adev, const struct amba_id *id)
-{
- int ret;
-
- ipc_base = ioremap(adev->res.start, resource_size(&adev->res));
- if (ipc_base == NULL)
- return -ENOMEM;
-
- __raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
-
- ipc_irq = adev->irq[0];
- ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
- if (ret < 0)
- goto err;
-
- /* Init slow mailbox */
- __raw_writel(CHAN_MASK(A9_SOURCE),
- ipc_base + IPCMxSOURCE(IPC_TX_MBOX));
- __raw_writel(CHAN_MASK(M3_SOURCE),
- ipc_base + IPCMxDSET(IPC_TX_MBOX));
- __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
- ipc_base + IPCMxMSET(IPC_TX_MBOX));
-
- /* Init receive mailbox */
- __raw_writel(CHAN_MASK(M3_SOURCE),
- ipc_base + IPCMxSOURCE(IPC_RX_MBOX));
- __raw_writel(CHAN_MASK(A9_SOURCE),
- ipc_base + IPCMxDSET(IPC_RX_MBOX));
- __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
- ipc_base + IPCMxMSET(IPC_RX_MBOX));
-
- return 0;
-err:
- iounmap(ipc_base);
- return ret;
-}
-
-static struct amba_id pl320_ids[] = {
- {
- .id = 0x00041320,
- .mask = 0x000fffff,
- },
- { 0, 0 },
-};
-
-static struct amba_driver pl320_driver = {
- .drv = {
- .name = "pl320",
- },
- .id_table = pl320_ids,
- .probe = pl320_probe,
-};
-
-static int __init ipc_init(void)
-{
- return amba_driver_register(&pl320_driver);
-}
-module_init(ipc_init);
diff --git a/drivers/mailbox/pl320.c b/drivers/mailbox/pl320.c
new file mode 100644
index 0000000..1121ca8
--- /dev/null
+++ b/drivers/mailbox/pl320.c
@@ -0,0 +1,214 @@
+/*
+ * Copyright 2012 Calxeda, Inc.
+ *
+ * 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <linux/types.h>
+#include <linux/err.h>
+#include <linux/delay.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/completion.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/spinlock.h>
+#include <linux/device.h>
+#include <linux/amba/bus.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mailbox_controller.h>
+
+#define IPCMSOURCE(m) ((m) * 0x40)
+#define IPCMDSET(m) (((m) * 0x40) + 0x004)
+#define IPCMDCLEAR(m) (((m) * 0x40) + 0x008)
+#define IPCMDSTATUS(m) (((m) * 0x40) + 0x00C)
+#define IPCMMODE(m) (((m) * 0x40) + 0x010)
+#define IPCMMSET(m) (((m) * 0x40) + 0x014)
+#define IPCMMCLEAR(m) (((m) * 0x40) + 0x018)
+#define IPCMMSTATUS(m) (((m) * 0x40) + 0x01C)
+#define IPCMSEND(m) (((m) * 0x40) + 0x020)
+#define IPCMDR(m, dr) (((m) * 0x40) + ((dr) * 4) + 0x024)
+
+#define IPCMMIS(irq) (((irq) * 8) + 0x800)
+#define IPCMRIS(irq) (((irq) * 8) + 0x804)
+
+#define MBOX_MASK(n) (1 << (n))
+#define IPC_TX_MBOX 1
+
+#define CHAN_MASK(n) (1 << (n))
+#define A9_SOURCE 1
+#define M3_SOURCE 0
+
+struct pl320_con {
+ u32 *data;
+ int mbox_irq;
+ struct device *dev;
+ struct mbox_link link;
+ void __iomem *mbox_base;
+ struct mbox_controller mbox_con;
+};
+
+static inline struct pl320_con *to_pl320(struct mbox_link *l)
+{
+ if (!l)
+ return NULL;
+
+ return container_of(l, struct pl320_con, link);
+}
+
+static irqreturn_t mbox_handler(int irq, void *p)
+{
+ struct mbox_link *link = (struct mbox_link *)p;
+ struct pl320_con *pl320 = to_pl320(link);
+ void __iomem *mbox_base = pl320->mbox_base;
+ u32 irq_stat;
+
+ irq_stat = __raw_readl(mbox_base + IPCMMIS(1));
+ if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
+ u32 *data = pl320->data;
+ int i;
+
+ __raw_writel(0, mbox_base + IPCMSEND(IPC_TX_MBOX));
+
+ /*
+ * The PL320 driver specifies that the send buffer
+ * will be overwritten by same fifo upon TX ACK.
+ */
+ for (i = 0; i < 7; i++)
+ data[i] = __raw_readl(mbox_base
+ + IPCMDR(IPC_TX_MBOX, i));
+
+ mbox_link_txdone(link, MBOX_OK);
+
+ pl320->data = NULL;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int pl320_send_data(struct mbox_link *link, void *msg)
+{
+ struct pl320_con *pl320 = to_pl320(link);
+ void __iomem *mbox_base = pl320->mbox_base;
+ u32 *data = (u32 *)msg;
+ int i;
+
+ pl320->data = data;
+
+ for (i = 0; i < 7; i++)
+ __raw_writel(data[i], mbox_base + IPCMDR(IPC_TX_MBOX, i));
+
+ __raw_writel(0x1, mbox_base + IPCMSEND(IPC_TX_MBOX));
+
+ return 0;
+}
+
+static int pl320_startup(struct mbox_link *link, void *ignored)
+{
+ struct pl320_con *pl320 = to_pl320(link);
+ void __iomem *mbox_base = pl320->mbox_base;
+ int err, mbox_irq = pl320->mbox_irq;
+
+ __raw_writel(0, mbox_base + IPCMSEND(IPC_TX_MBOX));
+
+ err = request_irq(mbox_irq, mbox_handler,
+ 0, dev_name(pl320->dev), link);
+ if (err)
+ return err;
+
+ /* Init slow mailbox */
+ __raw_writel(CHAN_MASK(A9_SOURCE),
+ mbox_base + IPCMSOURCE(IPC_TX_MBOX));
+ __raw_writel(CHAN_MASK(M3_SOURCE),
+ mbox_base + IPCMDSET(IPC_TX_MBOX));
+ __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
+ mbox_base + IPCMMSET(IPC_TX_MBOX));
+
+ pl320->data = NULL;
+ return 0;
+}
+
+static void pl320_shutdown(struct mbox_link *link)
+{
+ struct pl320_con *pl320 = to_pl320(link);
+
+ pl320->data = NULL;
+ free_irq(pl320->mbox_irq, link);
+}
+
+static struct mbox_link_ops pl320_ops = {
+ .send_data = pl320_send_data,
+ .startup = pl320_startup,
+ .shutdown = pl320_shutdown,
+};
+
+static int pl320_probe(struct amba_device *adev, const struct amba_id *id)
+{
+ struct pl320_con *pl320;
+ struct mbox_link *l[2];
+ int ret;
+
+ pl320 = kzalloc(sizeof(struct pl320_con), GFP_KERNEL);
+ if (!pl320)
+ return -ENOMEM;
+
+ pl320->mbox_base = ioremap(adev->res.start, resource_size(&adev->res));
+ if (pl320->mbox_base == NULL) {
+ kfree(pl320);
+ return -ENOMEM;
+ }
+
+ pl320->dev = &adev->dev;
+ pl320->mbox_irq = adev->irq[0];
+ amba_set_drvdata(adev, pl320);
+
+ l[0] = &pl320->link;
+ l[1] = NULL;
+ pl320->mbox_con.links = l;
+ pl320->mbox_con.txdone_irq = true;
+ pl320->mbox_con.ops = &pl320_ops;
+ snprintf(pl320->link.link_name, 16, "A9_to_M3");
+ snprintf(pl320->mbox_con.controller_name, 16, "pl320");
+ pl320->mbox_con.dev = &adev->dev;
+
+ ret = mbox_controller_register(&pl320->mbox_con);
+ if (ret) {
+ iounmap(pl320->mbox_base);
+ kfree(pl320);
+ }
+
+ return ret;
+}
+
+static struct amba_id pl320_ids[] = {
+ {
+ .id = 0x00041320,
+ .mask = 0x000fffff,
+ },
+ { 0, 0 },
+};
+
+static struct amba_driver pl320_driver = {
+ .drv = {
+ .name = "pl320",
+ },
+ .id_table = pl320_ids,
+ .probe = pl320_probe,
+};
+
+static int __init mbox_init(void)
+{
+ return amba_driver_register(&pl320_driver);
+}
+module_init(mbox_init);
diff --git a/include/linux/pl320-ipc.h b/include/linux/pl320-ipc.h
deleted file mode 100644
index 5161f63..0000000
--- a/include/linux/pl320-ipc.h
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * 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.
- *
- * You should have received a copy of the GNU General Public License along with
- * 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);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv4 4/5] mailbox: Fix TX completion init
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
` (2 preceding siblings ...)
2014-03-18 18:45 ` [PATCHv4 3/5] mailbox: pl320: Introduce common API driver Jassi Brar
@ 2014-03-18 18:46 ` Jassi Brar
2014-03-18 18:46 ` [PATCHv4 5/5] mailbox: Fix deleteing poll timer Jassi Brar
2014-03-27 22:51 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Markus Mayer
5 siblings, 0 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri, Jassi Brar
From: LeyFoon Tan <lftan.linux@gmail.com>
For fast TX the complete could be called before being initialized as follows
mbox_send_message --> poll_txdone --> tx_tick --> complete(&chan->tx_complete)
Init the completion early enough to fix the race.
Signed-off-by: LeyFoon Tan <lftan.linux@gmail.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/mailbox/mailbox.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index 79d576e..cdf7d45 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -278,6 +278,9 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
if (!chan || !chan->cl)
return -EINVAL;
+ if (chan->cl->tx_block)
+ init_completion(&chan->tx_complete);
+
t = _add_to_rbuf(chan, mssg);
if (t < 0) {
pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
@@ -291,7 +294,6 @@ int mbox_send_message(struct mbox_chan *chan, void *mssg)
if (chan->cl->tx_block && chan->active_req) {
int ret;
- init_completion(&chan->tx_complete);
ret = wait_for_completion_timeout(&chan->tx_complete,
chan->cl->tx_tout);
if (ret == 0) {
--
1.8.1.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCHv4 5/5] mailbox: Fix deleteing poll timer
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
` (3 preceding siblings ...)
2014-03-18 18:46 ` [PATCHv4 4/5] mailbox: Fix TX completion init Jassi Brar
@ 2014-03-18 18:46 ` Jassi Brar
2014-03-27 22:51 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Markus Mayer
5 siblings, 0 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-18 18:46 UTC (permalink / raw)
To: linux-kernel
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri, Jassi Brar
From: LeyFoon Tan <lftan.linux@gmail.com>
Try to delete the timer only if it was init/used.
Signed-off-by: LeyFoon Tan <lftan.linux@gmail.com>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
drivers/mailbox/mailbox.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
index cdf7d45..d080fc3 100644
--- a/drivers/mailbox/mailbox.c
+++ b/drivers/mailbox/mailbox.c
@@ -584,7 +584,8 @@ void mbox_controller_unregister(struct mbox_controller *mbox)
list_for_each_entry(chan, &con->channels, node)
mbox_free_channel(chan);
- del_timer_sync(&con->poll);
+ if (mbox->txdone_poll)
+ del_timer_sync(&con->poll);
kfree(con);
}
--
1.8.1.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCHv4 3/5] mailbox: pl320: Introduce common API driver
2014-03-18 18:45 ` [PATCHv4 3/5] mailbox: pl320: Introduce common API driver Jassi Brar
@ 2014-03-18 19:10 ` Rob Herring
2014-03-19 5:40 ` Jassi Brar
0 siblings, 1 reply; 16+ messages in thread
From: Rob Herring @ 2014-03-18 19:10 UTC (permalink / raw)
To: Jassi Brar
Cc: linux-kernel, Greg Kroah-Hartman, s-anna, Tony Lindgren,
omar.ramirez, loic.pallardy, lftan.linux, slapdau,
Courtney Cavin, Wysocki, Rafael J, Arnd Bergmann,
Josh Cartwright, Linus Walleij, Kumar Gala, ks.giri, Jassi Brar
On Tue, Mar 18, 2014 at 1:45 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> Convert the PL320 controller driver to work with the common
> mailbox API. Also convert the only user of PL320, highbank-cpufreq.c
> to work with thee API. Drop the obsoleted driver pl320-ipc.c
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> drivers/cpufreq/highbank-cpufreq.c | 24 ++++-
> drivers/mailbox/Makefile | 2 +-
> drivers/mailbox/pl320-ipc.c | 198 ----------------------------------
> drivers/mailbox/pl320.c | 214 +++++++++++++++++++++++++++++++++++++
Would you mind resending with rename detection enabled.
You can make that the default by adding this to your config:
[diff]
renames = true
Rob
> include/linux/pl320-ipc.h | 17 ---
> 5 files changed, 237 insertions(+), 218 deletions(-)
> delete mode 100644 drivers/mailbox/pl320-ipc.c
> create mode 100644 drivers/mailbox/pl320.c
> delete mode 100644 include/linux/pl320-ipc.h
>
> diff --git a/drivers/cpufreq/highbank-cpufreq.c b/drivers/cpufreq/highbank-cpufreq.c
> index b464f29..4846734 100644
> --- a/drivers/cpufreq/highbank-cpufreq.c
> +++ b/drivers/cpufreq/highbank-cpufreq.c
> @@ -19,7 +19,7 @@
> #include <linux/cpu.h>
> #include <linux/err.h>
> #include <linux/of.h>
> -#include <linux/pl320-ipc.h>
> +#include <linux/mailbox_client.h>
> #include <linux/platform_device.h>
>
> #define HB_CPUFREQ_CHANGE_NOTE 0x80000001
> @@ -29,8 +29,28 @@
> static int hb_voltage_change(unsigned int freq)
> {
> u32 msg[HB_CPUFREQ_IPC_LEN] = {HB_CPUFREQ_CHANGE_NOTE, freq / 1000000};
> + struct mbox_client cl;
> + int ret = -ETIMEDOUT;
> + struct mbox_chan *chan;
>
> - return pl320_ipc_transmit(msg);
> + cl.rx_callback = NULL;
> + cl.tx_done = NULL;
> + cl.tx_block = true;
> + cl.tx_tout = 1000; /* 1 sec */
> + cl.link_data = NULL;
> + cl.knows_txdone = false;
> + cl.chan_name = "pl320:A9_to_M3";
> +
> + chan = mbox_request_channel(&cl);
> + if (IS_ERR(chan))
> + return PTR_ERR(chan);
> +
> + if (mbox_send_message(chan, (void *)msg))
> + ret = msg[1]; /* PL320 updates buffer with FIFO after ACK */
> +
> + mbox_free_channel(chan);
> +
> + return ret;
> }
>
> static int hb_cpufreq_clk_notify(struct notifier_block *nb,
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index 2fa343a..f3f4ab7 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -2,7 +2,7 @@
>
> obj-$(CONFIG_MAILBOX) += mailbox.o
>
> -obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
> +obj-$(CONFIG_PL320_MBOX) += pl320.o
>
> obj-$(CONFIG_OMAP_MBOX) += omap-mailbox.o
> obj-$(CONFIG_OMAP1_MBOX) += mailbox_omap1.o
> diff --git a/drivers/mailbox/pl320-ipc.c b/drivers/mailbox/pl320-ipc.c
> deleted file mode 100644
> index f3755e0..0000000
> --- a/drivers/mailbox/pl320-ipc.c
> +++ /dev/null
> @@ -1,198 +0,0 @@
> -/*
> - * Copyright 2012 Calxeda, Inc.
> - *
> - * 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.
> - *
> - * You should have received a copy of the GNU General Public License along with
> - * this program. If not, see <http://www.gnu.org/licenses/>.
> - */
> -#include <linux/types.h>
> -#include <linux/err.h>
> -#include <linux/delay.h>
> -#include <linux/export.h>
> -#include <linux/io.h>
> -#include <linux/interrupt.h>
> -#include <linux/completion.h>
> -#include <linux/mutex.h>
> -#include <linux/notifier.h>
> -#include <linux/spinlock.h>
> -#include <linux/device.h>
> -#include <linux/amba/bus.h>
> -
> -#include <linux/pl320-ipc.h>
> -
> -#define IPCMxSOURCE(m) ((m) * 0x40)
> -#define IPCMxDSET(m) (((m) * 0x40) + 0x004)
> -#define IPCMxDCLEAR(m) (((m) * 0x40) + 0x008)
> -#define IPCMxDSTATUS(m) (((m) * 0x40) + 0x00C)
> -#define IPCMxMODE(m) (((m) * 0x40) + 0x010)
> -#define IPCMxMSET(m) (((m) * 0x40) + 0x014)
> -#define IPCMxMCLEAR(m) (((m) * 0x40) + 0x018)
> -#define IPCMxMSTATUS(m) (((m) * 0x40) + 0x01C)
> -#define IPCMxSEND(m) (((m) * 0x40) + 0x020)
> -#define IPCMxDR(m, dr) (((m) * 0x40) + ((dr) * 4) + 0x024)
> -
> -#define IPCMMIS(irq) (((irq) * 8) + 0x800)
> -#define IPCMRIS(irq) (((irq) * 8) + 0x804)
> -
> -#define MBOX_MASK(n) (1 << (n))
> -#define IPC_TX_MBOX 1
> -#define IPC_RX_MBOX 2
> -
> -#define CHAN_MASK(n) (1 << (n))
> -#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);
> -
> -static inline void set_destination(int source, int mbox)
> -{
> - __raw_writel(CHAN_MASK(source), ipc_base + IPCMxDSET(mbox));
> - __raw_writel(CHAN_MASK(source), ipc_base + IPCMxMSET(mbox));
> -}
> -
> -static inline void clear_destination(int source, int mbox)
> -{
> - __raw_writel(CHAN_MASK(source), ipc_base + IPCMxDCLEAR(mbox));
> - __raw_writel(CHAN_MASK(source), ipc_base + IPCMxMCLEAR(mbox));
> -}
> -
> -static void __ipc_send(int mbox, 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));
> -}
> -
> -static u32 __ipc_rcv(int mbox, u32 *data)
> -{
> - int i;
> - for (i = 0; i < 7; i++)
> - data[i] = __raw_readl(ipc_base + IPCMxDR(mbox, i));
> - return data[1];
> -}
> -
> -/* blocking implmentation from the A9 side, not usuable in interrupts! */
> -int pl320_ipc_transmit(u32 *data)
> -{
> - int ret;
> -
> - mutex_lock(&ipc_m1_lock);
> -
> - init_completion(&ipc_completion);
> - __ipc_send(IPC_TX_MBOX, data);
> - ret = wait_for_completion_timeout(&ipc_completion,
> - msecs_to_jiffies(1000));
> - if (ret == 0) {
> - ret = -ETIMEDOUT;
> - goto out;
> - }
> -
> - 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)
> -{
> - u32 irq_stat;
> - u32 data[7];
> -
> - irq_stat = __raw_readl(ipc_base + IPCMMIS(1));
> - if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
> - __raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
> - complete(&ipc_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));
> - }
> -
> - 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 int pl320_probe(struct amba_device *adev, const struct amba_id *id)
> -{
> - int ret;
> -
> - ipc_base = ioremap(adev->res.start, resource_size(&adev->res));
> - if (ipc_base == NULL)
> - return -ENOMEM;
> -
> - __raw_writel(0, ipc_base + IPCMxSEND(IPC_TX_MBOX));
> -
> - ipc_irq = adev->irq[0];
> - ret = request_irq(ipc_irq, ipc_handler, 0, dev_name(&adev->dev), NULL);
> - if (ret < 0)
> - goto err;
> -
> - /* Init slow mailbox */
> - __raw_writel(CHAN_MASK(A9_SOURCE),
> - ipc_base + IPCMxSOURCE(IPC_TX_MBOX));
> - __raw_writel(CHAN_MASK(M3_SOURCE),
> - ipc_base + IPCMxDSET(IPC_TX_MBOX));
> - __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
> - ipc_base + IPCMxMSET(IPC_TX_MBOX));
> -
> - /* Init receive mailbox */
> - __raw_writel(CHAN_MASK(M3_SOURCE),
> - ipc_base + IPCMxSOURCE(IPC_RX_MBOX));
> - __raw_writel(CHAN_MASK(A9_SOURCE),
> - ipc_base + IPCMxDSET(IPC_RX_MBOX));
> - __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
> - ipc_base + IPCMxMSET(IPC_RX_MBOX));
> -
> - return 0;
> -err:
> - iounmap(ipc_base);
> - return ret;
> -}
> -
> -static struct amba_id pl320_ids[] = {
> - {
> - .id = 0x00041320,
> - .mask = 0x000fffff,
> - },
> - { 0, 0 },
> -};
> -
> -static struct amba_driver pl320_driver = {
> - .drv = {
> - .name = "pl320",
> - },
> - .id_table = pl320_ids,
> - .probe = pl320_probe,
> -};
> -
> -static int __init ipc_init(void)
> -{
> - return amba_driver_register(&pl320_driver);
> -}
> -module_init(ipc_init);
> diff --git a/drivers/mailbox/pl320.c b/drivers/mailbox/pl320.c
> new file mode 100644
> index 0000000..1121ca8
> --- /dev/null
> +++ b/drivers/mailbox/pl320.c
> @@ -0,0 +1,214 @@
> +/*
> + * Copyright 2012 Calxeda, Inc.
> + *
> + * 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.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program. If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <linux/types.h>
> +#include <linux/err.h>
> +#include <linux/delay.h>
> +#include <linux/export.h>
> +#include <linux/io.h>
> +#include <linux/interrupt.h>
> +#include <linux/completion.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/spinlock.h>
> +#include <linux/device.h>
> +#include <linux/amba/bus.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <linux/mailbox_controller.h>
> +
> +#define IPCMSOURCE(m) ((m) * 0x40)
> +#define IPCMDSET(m) (((m) * 0x40) + 0x004)
> +#define IPCMDCLEAR(m) (((m) * 0x40) + 0x008)
> +#define IPCMDSTATUS(m) (((m) * 0x40) + 0x00C)
> +#define IPCMMODE(m) (((m) * 0x40) + 0x010)
> +#define IPCMMSET(m) (((m) * 0x40) + 0x014)
> +#define IPCMMCLEAR(m) (((m) * 0x40) + 0x018)
> +#define IPCMMSTATUS(m) (((m) * 0x40) + 0x01C)
> +#define IPCMSEND(m) (((m) * 0x40) + 0x020)
> +#define IPCMDR(m, dr) (((m) * 0x40) + ((dr) * 4) + 0x024)
> +
> +#define IPCMMIS(irq) (((irq) * 8) + 0x800)
> +#define IPCMRIS(irq) (((irq) * 8) + 0x804)
> +
> +#define MBOX_MASK(n) (1 << (n))
> +#define IPC_TX_MBOX 1
> +
> +#define CHAN_MASK(n) (1 << (n))
> +#define A9_SOURCE 1
> +#define M3_SOURCE 0
> +
> +struct pl320_con {
> + u32 *data;
> + int mbox_irq;
> + struct device *dev;
> + struct mbox_link link;
> + void __iomem *mbox_base;
> + struct mbox_controller mbox_con;
> +};
> +
> +static inline struct pl320_con *to_pl320(struct mbox_link *l)
> +{
> + if (!l)
> + return NULL;
> +
> + return container_of(l, struct pl320_con, link);
> +}
> +
> +static irqreturn_t mbox_handler(int irq, void *p)
> +{
> + struct mbox_link *link = (struct mbox_link *)p;
> + struct pl320_con *pl320 = to_pl320(link);
> + void __iomem *mbox_base = pl320->mbox_base;
> + u32 irq_stat;
> +
> + irq_stat = __raw_readl(mbox_base + IPCMMIS(1));
> + if (irq_stat & MBOX_MASK(IPC_TX_MBOX)) {
> + u32 *data = pl320->data;
> + int i;
> +
> + __raw_writel(0, mbox_base + IPCMSEND(IPC_TX_MBOX));
> +
> + /*
> + * The PL320 driver specifies that the send buffer
> + * will be overwritten by same fifo upon TX ACK.
> + */
> + for (i = 0; i < 7; i++)
> + data[i] = __raw_readl(mbox_base
> + + IPCMDR(IPC_TX_MBOX, i));
> +
> + mbox_link_txdone(link, MBOX_OK);
> +
> + pl320->data = NULL;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int pl320_send_data(struct mbox_link *link, void *msg)
> +{
> + struct pl320_con *pl320 = to_pl320(link);
> + void __iomem *mbox_base = pl320->mbox_base;
> + u32 *data = (u32 *)msg;
> + int i;
> +
> + pl320->data = data;
> +
> + for (i = 0; i < 7; i++)
> + __raw_writel(data[i], mbox_base + IPCMDR(IPC_TX_MBOX, i));
> +
> + __raw_writel(0x1, mbox_base + IPCMSEND(IPC_TX_MBOX));
> +
> + return 0;
> +}
> +
> +static int pl320_startup(struct mbox_link *link, void *ignored)
> +{
> + struct pl320_con *pl320 = to_pl320(link);
> + void __iomem *mbox_base = pl320->mbox_base;
> + int err, mbox_irq = pl320->mbox_irq;
> +
> + __raw_writel(0, mbox_base + IPCMSEND(IPC_TX_MBOX));
> +
> + err = request_irq(mbox_irq, mbox_handler,
> + 0, dev_name(pl320->dev), link);
> + if (err)
> + return err;
> +
> + /* Init slow mailbox */
> + __raw_writel(CHAN_MASK(A9_SOURCE),
> + mbox_base + IPCMSOURCE(IPC_TX_MBOX));
> + __raw_writel(CHAN_MASK(M3_SOURCE),
> + mbox_base + IPCMDSET(IPC_TX_MBOX));
> + __raw_writel(CHAN_MASK(M3_SOURCE) | CHAN_MASK(A9_SOURCE),
> + mbox_base + IPCMMSET(IPC_TX_MBOX));
> +
> + pl320->data = NULL;
> + return 0;
> +}
> +
> +static void pl320_shutdown(struct mbox_link *link)
> +{
> + struct pl320_con *pl320 = to_pl320(link);
> +
> + pl320->data = NULL;
> + free_irq(pl320->mbox_irq, link);
> +}
> +
> +static struct mbox_link_ops pl320_ops = {
> + .send_data = pl320_send_data,
> + .startup = pl320_startup,
> + .shutdown = pl320_shutdown,
> +};
> +
> +static int pl320_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> + struct pl320_con *pl320;
> + struct mbox_link *l[2];
> + int ret;
> +
> + pl320 = kzalloc(sizeof(struct pl320_con), GFP_KERNEL);
> + if (!pl320)
> + return -ENOMEM;
> +
> + pl320->mbox_base = ioremap(adev->res.start, resource_size(&adev->res));
> + if (pl320->mbox_base == NULL) {
> + kfree(pl320);
> + return -ENOMEM;
> + }
> +
> + pl320->dev = &adev->dev;
> + pl320->mbox_irq = adev->irq[0];
> + amba_set_drvdata(adev, pl320);
> +
> + l[0] = &pl320->link;
> + l[1] = NULL;
> + pl320->mbox_con.links = l;
> + pl320->mbox_con.txdone_irq = true;
> + pl320->mbox_con.ops = &pl320_ops;
> + snprintf(pl320->link.link_name, 16, "A9_to_M3");
> + snprintf(pl320->mbox_con.controller_name, 16, "pl320");
> + pl320->mbox_con.dev = &adev->dev;
> +
> + ret = mbox_controller_register(&pl320->mbox_con);
> + if (ret) {
> + iounmap(pl320->mbox_base);
> + kfree(pl320);
> + }
> +
> + return ret;
> +}
> +
> +static struct amba_id pl320_ids[] = {
> + {
> + .id = 0x00041320,
> + .mask = 0x000fffff,
> + },
> + { 0, 0 },
> +};
> +
> +static struct amba_driver pl320_driver = {
> + .drv = {
> + .name = "pl320",
> + },
> + .id_table = pl320_ids,
> + .probe = pl320_probe,
> +};
> +
> +static int __init mbox_init(void)
> +{
> + return amba_driver_register(&pl320_driver);
> +}
> +module_init(mbox_init);
> diff --git a/include/linux/pl320-ipc.h b/include/linux/pl320-ipc.h
> deleted file mode 100644
> index 5161f63..0000000
> --- a/include/linux/pl320-ipc.h
> +++ /dev/null
> @@ -1,17 +0,0 @@
> -/*
> - * 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.
> - *
> - * You should have received a copy of the GNU General Public License along with
> - * 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);
> --
> 1.8.1.2
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 2/5] mailbox: Introduce framework for mailbox
2014-03-18 18:45 ` [PATCHv4 2/5] mailbox: Introduce framework for mailbox Jassi Brar
@ 2014-03-19 4:00 ` Girish KS
2014-03-19 5:20 ` Jassi Brar
2014-03-28 22:08 ` [PATCHv4,2/5] " Markus Mayer
1 sibling, 1 reply; 16+ messages in thread
From: Girish KS @ 2014-03-19 4:00 UTC (permalink / raw)
To: Jassi Brar
Cc: Linux Kernel Mailing List, gregkh, Anna, Suman, tony,
omar.ramirez, loic.pallardy, lftan.linux, slapdau,
courtney.cavin, rafael.j.wysocki, robherring2, arnd, joshc,
linus.walleij, galak, ks.giri, Jassi Brar
On Wed, Mar 19, 2014 at 12:15 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> Introduce common framework for client/protocol drivers and
> controller drivers of Inter-Processor-Communication (IPC).
>
> Client driver developers should have a look at
> include/linux/mailbox_client.h to understand the part of
> the API exposed to client drivers.
> Similarly controller driver developers should have a look
> at include/linux/mailbox_controller.h
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
> drivers/mailbox/Makefile | 4 +
> drivers/mailbox/mailbox.c | 589 +++++++++++++++++++++++++++++++++++++
> include/linux/mailbox.h | 18 ++
> include/linux/mailbox_client.h | 48 +++
> include/linux/mailbox_controller.h | 85 ++++++
> 5 files changed, 744 insertions(+)
> create mode 100644 drivers/mailbox/mailbox.c
> create mode 100644 include/linux/mailbox.h
> create mode 100644 include/linux/mailbox_client.h
> create mode 100644 include/linux/mailbox_controller.h
>
> diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
> index e0facb3..2fa343a 100644
> --- a/drivers/mailbox/Makefile
> +++ b/drivers/mailbox/Makefile
> @@ -1,3 +1,7 @@
> +# Generic MAILBOX API
> +
> +obj-$(CONFIG_MAILBOX) += mailbox.o
> +
> obj-$(CONFIG_PL320_MBOX) += pl320-ipc.o
>
> obj-$(CONFIG_OMAP_MBOX) += omap-mailbox.o
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> new file mode 100644
> index 0000000..79d576e
> --- /dev/null
> +++ b/drivers/mailbox/mailbox.c
> @@ -0,0 +1,589 @@
> +/*
> + * Mailbox: Common code for Mailbox controllers and users
> + *
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Jassi Brar <jassisinghbrar@gmail.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.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/mailbox_controller.h>
> +
> +/*
> + * The length of circular buffer for queuing messages from a client.
> + * 'msg_count' tracks the number of buffered messages while 'msg_free'
> + * is the index where the next message would be buffered.
> + * We shouldn't need it too big because every transferr is interrupt
> + * triggered and if we have lots of data to transfer, the interrupt
> + * latencies are going to be the bottleneck, not the buffer length.
> + * Besides, mbox_send_message could be called from atomic context and
> + * the client could also queue another message from the notifier 'tx_done'
> + * of the last transfer done.
> + * REVIST: If too many platforms see the "Try increasing MBOX_TX_QUEUE_LEN"
> + * print, it needs to be taken from config option or somesuch.
> + */
> +#define MBOX_TX_QUEUE_LEN 20
> +
> +#define TXDONE_BY_IRQ (1 << 0) /* controller has remote RTR irq */
> +#define TXDONE_BY_POLL (1 << 1) /* controller can read status of last TX */
> +#define TXDONE_BY_ACK (1 << 2) /* S/W ACK recevied by Client ticks the TX */
> +
> +struct mbox_chan {
> + char name[16]; /* Physical link's name */
> + struct mbox_con *con; /* Parent Controller */
> + unsigned txdone_method;
> +
> + /* Physical links */
> + struct mbox_link *link;
> + struct mbox_link_ops *link_ops;
> +
> + /* client */
> + struct mbox_client *cl;
> + struct completion tx_complete;
> +
> + void *active_req;
> + unsigned msg_count, msg_free;
> + void *msg_data[MBOX_TX_QUEUE_LEN];
> + /* Access to the channel */
> + spinlock_t lock;
> + /* Hook to add to the controller's list of channels */
> + struct list_head node;
> + /* Notifier to all clients waiting on aquiring this channel */
> + struct blocking_notifier_head avail;
> +} __aligned(32);
> +
> +/* Internal representation of a controller */
> +struct mbox_con {
> + struct device *dev;
> + char name[16]; /* controller_name */
> + struct list_head channels;
> + /*
> + * If the controller supports only TXDONE_BY_POLL,
> + * this timer polls all the links for txdone.
> + */
> + struct timer_list poll;
> + unsigned period;
> + /* Hook to add to the global controller list */
> + struct list_head node;
> +} __aligned(32);
> +
> +static LIST_HEAD(mbox_cons);
> +static DEFINE_MUTEX(con_mutex);
> +
> +static int _add_to_rbuf(struct mbox_chan *chan, void *mssg)
> +{
> + int idx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* See if there is any space left */
> + if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return -ENOMEM;
> + }
> +
> + idx = chan->msg_free;
> + chan->msg_data[idx] = mssg;
> + chan->msg_count++;
> +
> + if (idx == MBOX_TX_QUEUE_LEN - 1)
> + chan->msg_free = 0;
> + else
> + chan->msg_free++;
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + return idx;
> +}
> +
> +static void _msg_submit(struct mbox_chan *chan)
> +{
> + struct mbox_link *link = chan->link;
> + unsigned count, idx;
> + unsigned long flags;
> + void *data;
> + int err;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + if (!chan->msg_count || chan->active_req) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return;
> + }
> +
> + count = chan->msg_count;
> + idx = chan->msg_free;
> + if (idx >= count)
> + idx -= count;
> + else
> + idx += MBOX_TX_QUEUE_LEN - count;
> +
> + data = chan->msg_data[idx];
> +
> + /* Try to submit a message to the MBOX controller */
> + err = chan->link_ops->send_data(link, data);
> + if (!err) {
> + chan->active_req = data;
> + chan->msg_count--;
> + }
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +
> +static void tx_tick(struct mbox_chan *chan, enum mbox_result r)
> +{
> + unsigned long flags;
> + void *mssg;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> + mssg = chan->active_req;
> + chan->active_req = NULL;
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + /* Submit next message */
> + _msg_submit(chan);
> +
> + /* Notify the client */
> + if (chan->cl->tx_block)
> + complete(&chan->tx_complete);
> + else if (mssg && chan->cl->tx_done)
> + chan->cl->tx_done(chan->cl, mssg, r);
> +}
> +
> +static void poll_txdone(unsigned long data)
> +{
> + struct mbox_con *con = (struct mbox_con *)data;
> + bool txdone, resched = false;
> + struct mbox_chan *chan;
> +
> + list_for_each_entry(chan, &con->channels, node) {
> + if (chan->active_req && chan->cl) {
> + resched = true;
> + txdone = chan->link_ops->last_tx_done(chan->link);
> + if (txdone)
> + tx_tick(chan, MBOX_OK);
> + }
> + }
> +
> + if (resched)
> + mod_timer(&con->poll,
> + jiffies + msecs_to_jiffies(con->period));
> +}
> +
> +/**
> + * mbox_link_received_data - A way for controller driver to push data
> + * received from remote to the upper layer.
> + * @link: Pointer to the mailbox link on which RX happened.
> + * @data: Client specific message typecasted as void *
> + *
> + * After startup and before shutdown any data received on the link
> + * is passed on to the API via atomic mbox_link_received_data().
> + * The controller should ACK the RX only after this call returns.
> + */
> +void mbox_link_received_data(struct mbox_link *link, void *mssg)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
> +
> + /* No buffering the received data */
> + if (chan->cl->rx_callback)
> + chan->cl->rx_callback(chan->cl, mssg);
> +}
> +EXPORT_SYMBOL_GPL(mbox_link_received_data);
> +
> +/**
> + * mbox_link_txdone - A way for controller driver to notify the
> + * framework that the last TX has completed.
> + * @link: Pointer to the mailbox link on which TX happened.
> + * @r: Status of last TX - OK or ERROR
> + *
> + * The controller that has IRQ for TX ACK calls this atomic API
> + * to tick the TX state machine. It works only if txdone_irq
> + * is set by the controller.
> + */
> +void mbox_link_txdone(struct mbox_link *link, enum mbox_result r)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
> +
> + if (unlikely(!(chan->txdone_method & TXDONE_BY_IRQ))) {
> + pr_err("Controller can't run the TX ticker\n");
> + return;
> + }
> +
> + tx_tick(chan, r);
> +}
> +EXPORT_SYMBOL_GPL(mbox_link_txdone);
> +
> +/**
> + * mbox_client_txdone - The way for a client to run the TX state machine.
> + * @chan: Mailbox channel assigned to this client.
> + * @r: Success status of last transmission.
> + *
> + * The client/protocol had received some 'ACK' packet and it notifies
> + * the API that the last packet was sent successfully. This only works
> + * if the controller can't sense TX-Done.
> + */
> +void mbox_client_txdone(struct mbox_chan *chan, enum mbox_result r)
> +{
> + if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
> + pr_err("Client can't run the TX ticker\n");
> + return;
> + }
> +
> + tx_tick(chan, r);
> +}
> +EXPORT_SYMBOL_GPL(mbox_client_txdone);
> +
> +/**
> + * mbox_send_message - For client to submit a message to be
> + * sent to the remote.
> + * @chan: Mailbox channel assigned to this client.
> + * @mssg: Client specific message typecasted.
> + *
> + * For client to submit data to the controller destined for a remote
> + * processor. If the client had set 'tx_block', the call will return
> + * either when the remote receives the data or when 'tx_tout' millisecs
> + * run out.
> + * In non-blocking mode, the requests are buffered by the API and a
> + * non-negative token is returned for each queued request. If the request
> + * is not queued, a negative token is returned. Upon failure or successful
> + * TX, the API calls 'tx_done' from atomic context, from which the client
> + * could submit yet another request.
> + * In blocking mode, 'tx_done' is not called, effectively making the
> + * queue length 1.
> + * The pointer to message should be preserved until it is sent
> + * over the link, i.e, tx_done() is made.
> + * This function could be called from atomic context as it simply
> + * queues the data and returns a token against the request.
> + *
> + * Return: Non-negative integer for successful submission (non-blocking mode)
> + * or transmission over link (blocking mode).
> + * Negative value denotes failure.
> + */
> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
> +{
> + int t;
> +
> + if (!chan || !chan->cl)
> + return -EINVAL;
> +
> + t = _add_to_rbuf(chan, mssg);
> + if (t < 0) {
> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
> + return t;
> + }
> +
> + _msg_submit(chan);
> +
> + if (chan->txdone_method == TXDONE_BY_POLL)
> + poll_txdone((unsigned long)chan->con);
I came across a panic in the complete function. When i traced bact the
call sequence it was
When a client sets chan->cl->tx_block = true, and polling is enabled
for controller.
1.Client sends the message with mbox_send_message. This function as
seen above will call __msg_submit (this calls the controller specific
send function).
2. Since the tx method is polling the above condition is satisfied and
calls the poll_txdone function.
3. In this poll function, the tx_tick function is invoked.
4. In this tick function since the client has enabled the tx_block it
calls the notify function complete(&chan->tx_complete);
5. Here there is a panic. because the complete is called before
initialization. init_completion needs to be called but not called.
> +
> + if (chan->cl->tx_block && chan->active_req) {
> + int ret;
> + init_completion(&chan->tx_complete);
> + ret = wait_for_completion_timeout(&chan->tx_complete,
> + chan->cl->tx_tout);
> + if (ret == 0) {
> + t = -EIO;
> + tx_tick(chan, MBOX_ERR);
> + }
> + }
> +
> + return t;
> +}
> +EXPORT_SYMBOL_GPL(mbox_send_message);
> +
> +/**
> + * mbox_request_channel - Request a mailbox channel.
> + * @cl: Identity of the client requesting the channel.
> + *
> + * The Client specifies its requirements and capabilities while asking for
> + * a mailbox channel by name. It can't be called from atomic context.
> + * The channel is exclusively allocated and can't be used by another
> + * client before the owner calls mbox_free_channel.
> + * After assignment, any packet received on this channel will be
> + * handed over to the client via the 'rx_callback'.
> + *
> + * Return: Pointer to the channel assigned to the client if successful.
> + * ERR_PTR for request failure.
> + */
> +struct mbox_chan *mbox_request_channel(struct mbox_client *cl)
> +{
> + struct mbox_chan *chan;
> + struct mbox_con *con;
> + unsigned long flags;
> + char *con_name;
> + int len, ret;
> +
> + con_name = cl->chan_name;
> + len = strcspn(cl->chan_name, ":");
> +
> + ret = 0;
> + mutex_lock(&con_mutex);
> + list_for_each_entry(con, &mbox_cons, node)
> + if (!strncmp(con->name, con_name, len)) {
> + ret = 1;
> + break;
> + }
> + mutex_unlock(&con_mutex);
> +
> + if (!ret) {
> + pr_info("Channel(%s) not found!\n", cl->chan_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + ret = 0;
> + list_for_each_entry(chan, &con->channels, node) {
> + if (!chan->cl &&
> + !strcmp(con_name + len + 1, chan->name) &&
> + try_module_get(con->dev->driver->owner)) {
> + spin_lock_irqsave(&chan->lock, flags);
> + chan->msg_free = 0;
> + chan->msg_count = 0;
> + chan->active_req = NULL;
> + chan->cl = cl;
> + if (!cl->tx_tout) /* wait for ever */
> + cl->tx_tout = msecs_to_jiffies(3600000);
> + else
> + cl->tx_tout = msecs_to_jiffies(cl->tx_tout);
> + if (chan->txdone_method == TXDONE_BY_POLL
> + && cl->knows_txdone)
> + chan->txdone_method |= TXDONE_BY_ACK;
> + spin_unlock_irqrestore(&chan->lock, flags);
> + ret = 1;
> + break;
> + }
> + }
> +
> + if (!ret) {
> + pr_err("Unable to assign mailbox(%s)\n", cl->chan_name);
> + return ERR_PTR(-EBUSY);
> + }
> +
> + ret = chan->link_ops->startup(chan->link, cl->link_data);
> + if (ret) {
> + pr_err("Unable to startup the link\n");
> + mbox_free_channel(chan);
> + return ERR_PTR(ret);
> + }
> +
> + return chan;
> +}
> +EXPORT_SYMBOL_GPL(mbox_request_channel);
> +
> +/**
> + * mbox_free_channel - The client relinquishes control of a mailbox
> + * channel by this call.
> + * @chan: The mailbox channel to be freed.
> + */
> +void mbox_free_channel(struct mbox_chan *chan)
> +{
> + unsigned long flags;
> +
> + if (!chan || !chan->cl)
> + return;
> +
> + chan->link_ops->shutdown(chan->link);
> +
> + /* The queued TX requests are simply aborted, no callbacks are made */
> + spin_lock_irqsave(&chan->lock, flags);
> + chan->cl = NULL;
> + chan->active_req = NULL;
> + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
> + chan->txdone_method = TXDONE_BY_POLL;
> +
> + module_put(chan->con->dev->driver->owner);
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + blocking_notifier_call_chain(&chan->avail, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(mbox_free_channel);
> +
> +static struct mbox_chan *name_to_chan(const char *name)
> +{
> + struct mbox_chan *chan = NULL;
> + struct mbox_con *con;
> + int len, found = 0;
> +
> + len = strcspn(name, ":");
> +
> + mutex_lock(&con_mutex);
> +
> + list_for_each_entry(con, &mbox_cons, node) {
> + if (!strncmp(con->name, name, len)) {
> + list_for_each_entry(chan, &con->channels, node) {
> + if (!strcmp(name + len + 1, chan->name)) {
> + found = 1;
> + goto done;
> + }
> + }
> + }
> + }
> +done:
> + mutex_unlock(&con_mutex);
> +
> + if (!found)
> + return NULL;
> +
> + return chan;
> +}
> +
> +/**
> + * mbox_notify_chan_register - The client may ask the framework to be
> + * notified when a particular channel becomes available
> + * to be acquired again.
> + * @name: Name of the mailbox channel the client is interested in.
> + * @nb: Pointer to the notifier.
> + */
> +int mbox_notify_chan_register(const char *name, struct notifier_block *nb)
> +{
> + struct mbox_chan *chan = name_to_chan(name);
> +
> + if (chan && nb)
> + return blocking_notifier_chain_register(&chan->avail, nb);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(mbox_notify_chan_register);
> +
> +/**
> + * mbox_notify_chan_unregister - The client is no more interested in channel.
> + *
> + * @name: Name of the mailbox channel the client was interested in.
> + * @nb: Pointer to the notifier.
> + */
> +void mbox_notify_chan_unregister(const char *name, struct notifier_block *nb)
> +{
> + struct mbox_chan *chan = name_to_chan(name);
> +
> + if (chan && nb)
> + blocking_notifier_chain_unregister(&chan->avail, nb);
> +}
> +EXPORT_SYMBOL_GPL(mbox_notify_chan_unregister);
> +
> +/**
> + * mbox_controller_register - Register the mailbox controller
> + * @mbox_con: Pointer to the mailbox controller.
> + *
> + * The controller driver registers its communication links to the
> + * global pool managed by the common framework.
> + */
> +int mbox_controller_register(struct mbox_controller *mbox)
> +{
> + int i, num_links, txdone;
> + struct mbox_chan *chan;
> + struct mbox_con *con;
> +
> + /* Sanity check */
> + if (!mbox || !mbox->ops)
> + return -EINVAL;
> +
> + for (i = 0; mbox->links[i]; i++)
> + ;
> + if (!i)
> + return -EINVAL;
> + num_links = i;
> +
> + mutex_lock(&con_mutex);
> + /* Check if already populated */
> + list_for_each_entry(con, &mbox_cons, node)
> + if (!strcmp(mbox->controller_name, con->name)) {
> + mutex_unlock(&con_mutex);
> + return -EINVAL;
> + }
> +
> + con = kzalloc(sizeof(struct mbox_con), GFP_KERNEL);
> + if (!con)
> + return -ENOMEM;
> +
> + chan = kzalloc(sizeof(struct mbox_chan) * num_links, GFP_KERNEL);
> + if (!chan) {
> + kfree(con);
> + return -ENOMEM;
> + }
> +
> + con->dev = mbox->dev;
> + INIT_LIST_HEAD(&con->channels);
> + snprintf(con->name, 16, "%s", mbox->controller_name);
> +
> + if (mbox->txdone_irq)
> + txdone = TXDONE_BY_IRQ;
> + else if (mbox->txdone_poll)
> + txdone = TXDONE_BY_POLL;
> + else /* It has to be ACK then */
> + txdone = TXDONE_BY_ACK;
> +
> + if (txdone == TXDONE_BY_POLL) {
> + con->period = mbox->txpoll_period;
> + con->poll.function = &poll_txdone;
> + con->poll.data = (unsigned long)con;
> + init_timer(&con->poll);
> + }
> +
> + for (i = 0; i < num_links; i++) {
> + chan[i].con = con;
> + chan[i].cl = NULL;
> + chan[i].link_ops = mbox->ops;
> + chan[i].link = mbox->links[i];
> + chan[i].txdone_method = txdone;
> + chan[i].link->api_priv = &chan[i];
> + spin_lock_init(&chan[i].lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&chan[i].avail);
> + list_add_tail(&chan[i].node, &con->channels);
> + snprintf(chan[i].name, 16, "%s", mbox->links[i]->link_name);
> + }
> +
> + list_add_tail(&con->node, &mbox_cons);
> + mutex_unlock(&con_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mbox_controller_register);
> +
> +/**
> + * mbox_controller_unregister - UnRegister the mailbox controller
> + * @mbox_con: Pointer to the mailbox controller.
> + *
> + * Purge the mailbox links from the global pool maintained by the framework.
> + */
> +void mbox_controller_unregister(struct mbox_controller *mbox)
> +{
> + struct mbox_con *t, *con = NULL;
> + struct mbox_chan *chan;
> +
> + mutex_lock(&con_mutex);
> +
> + list_for_each_entry(t, &mbox_cons, node)
> + if (!strcmp(mbox->controller_name, t->name)) {
> + con = t;
> + break;
> + }
> +
> + if (con)
> + list_del(&con->node);
> +
> + mutex_unlock(&con_mutex);
> +
> + if (!con)
> + return;
> +
> + list_for_each_entry(chan, &con->channels, node)
> + mbox_free_channel(chan);
> +
> + del_timer_sync(&con->poll);
> +
> + kfree(con);
> +}
> +EXPORT_SYMBOL_GPL(mbox_controller_unregister);
> diff --git a/include/linux/mailbox.h b/include/linux/mailbox.h
> new file mode 100644
> index 0000000..44bcb2b
> --- /dev/null
> +++ b/include/linux/mailbox.h
> @@ -0,0 +1,18 @@
> +/*
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Jassi Brar <jassisinghbrar@gmail.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.
> + */
> +
> +#ifndef __MAILBOX_H
> +#define __MAILBOX_H
> +
> +enum mbox_result {
> + MBOX_OK = 0,
> + MBOX_ERR,
> +};
> +
> +#endif /* __MAILBOX_H */
> diff --git a/include/linux/mailbox_client.h b/include/linux/mailbox_client.h
> new file mode 100644
> index 0000000..4954378
> --- /dev/null
> +++ b/include/linux/mailbox_client.h
> @@ -0,0 +1,48 @@
> +/*
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Jassi Brar <jassisinghbrar@gmail.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.
> + */
> +
> +#ifndef __MAILBOX_CLIENT_H
> +#define __MAILBOX_CLIENT_H
> +
> +#include <linux/mailbox.h>
> +
> +struct mbox_chan;
> +
> +/**
> + * struct mbox_client - User of a mailbox
> + * @chan_name: The "controller:channel" this client wants
> + * @rx_callback: Atomic callback to provide client the data received
> + * @tx_done: Atomic callback to tell client of data transmission
> + * @tx_block: If the mbox_send_message should block until data is
> + * transmitted.
> + * @tx_tout: Max block period in ms before TX is assumed failure
> + * @knows_txdone: if the client could run the TX state machine. Usually
> + * if the client receives some ACK packet for transmission.
> + * Unused if the controller already has TX_Done/RTR IRQ.
> + * @link_data: Optional controller specific parameters during channel
> + * request.
> + */
> +struct mbox_client {
> + char *chan_name;
> + void (*rx_callback)(struct mbox_client *cl, void *mssg);
> + void (*tx_done)(struct mbox_client *cl, void *mssg, enum mbox_result r);
> + bool tx_block;
> + unsigned long tx_tout;
> + bool knows_txdone;
> + void *link_data;
> +};
> +
> +struct mbox_chan *mbox_request_channel(struct mbox_client *cl);
> +int mbox_send_message(struct mbox_chan *chan, void *mssg);
> +void mbox_client_txdone(struct mbox_chan *chan, enum mbox_result r);
> +void mbox_free_channel(struct mbox_chan *chan);
> +int mbox_notify_chan_register(const char *name, struct notifier_block *nb);
> +void mbox_notify_chan_unregister(const char *name, struct notifier_block *nb);
> +
> +#endif /* __MAILBOX_CLIENT_H */
> diff --git a/include/linux/mailbox_controller.h b/include/linux/mailbox_controller.h
> new file mode 100644
> index 0000000..5ff22c6
> --- /dev/null
> +++ b/include/linux/mailbox_controller.h
> @@ -0,0 +1,85 @@
> +/*
> + * 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 __MAILBOX_CONTROLLER_H
> +#define __MAILBOX_CONTROLLER_H
> +
> +#include <linux/mailbox.h>
> +
> +/**
> + * struct mbox_link - s/w representation of a communication link
> + * @link_name: Literal name assigned to the link. Physically
> + * identical channels may have the same name.
> + * @api_priv: Hook for the API to map its private data on the link
> + * Controller driver must not touch it.
> + */
> +struct mbox_link {
> + char link_name[16];
> + void *api_priv;
> +};
> +
> +/**
> + * struct mbox_link - s/w representation of a communication link
> + * @send_data: The API asks the MBOX controller driver, in atomic
> + * context try to transmit a message on the bus. Returns 0 if
> + * data is accepted for transmission, -EBUSY while rejecting
> + * if the remote hasn't yet read the last data sent. Actual
> + * transmission of data is reported by the controller via
> + * mbox_link_txdone (if it has some TX ACK irq). It must not
> + * block.
> + * @startup: Called when a client requests the link. The controller
> + * could ask clients for additional parameters of communication
> + * to be provided via client's link_data. This call may
> + * block. After this call the Controller must forward any
> + * data received on the link by calling mbox_link_received_data.
> + * @shutdown: Called when a client relinquishes control of a link.
> + * This call may block too. The controller must not forwared
> + * any received data anymore.
> + * @last_tx_done: If the controller sets 'txdone_poll', the API calls
> + * this to poll status of last TX. The controller must
> + * give priority to IRQ method over polling and never
> + * set both txdone_poll and txdone_irq. Only in polling
> + * mode 'send_data' is expected to return -EBUSY.
> + * Used only if txdone_poll:=true && txdone_irq:=false
> + */
> +struct mbox_link_ops {
> + int (*send_data)(struct mbox_link *link, void *data);
> + int (*startup)(struct mbox_link *link, void *params);
> + void (*shutdown)(struct mbox_link *link);
> + bool (*last_tx_done)(struct mbox_link *link);
> +};
> +
> +/**
> + * struct mbox_controller - Controller of a class of communication links
> + * @dev: Device backing this controller
> + * @controller_name: Literal name of the controller.
> + * @ops: Operators that work on each communication link
> + * @links: Null terminated array of links.
> + * @txdone_irq: Indicates if the controller can report to API when
> + * the last transmitted data was read by the remote.
> + * Eg, if it has some TX ACK irq.
> + * @txdone_poll: If the controller can read but not report the TX
> + * done. Ex, some register shows the TX status but
> + * no interrupt rises. Ignored if 'txdone_irq' is set.
> + * @txpoll_period: If 'txdone_poll' is in effect, the API polls for
> + * last TX's status after these many millisecs
> + */
> +struct mbox_controller {
> + struct device *dev;
> + char controller_name[16];
> + struct mbox_link_ops *ops;
> + struct mbox_link **links;
> + bool txdone_irq;
> + bool txdone_poll;
> + unsigned txpoll_period;
> +};
> +
> +int mbox_controller_register(struct mbox_controller *mbox);
> +void mbox_link_received_data(struct mbox_link *link, void *data);
> +void mbox_link_txdone(struct mbox_link *link, enum mbox_result r);
> +void mbox_controller_unregister(struct mbox_controller *mbox);
> +
> +#endif /* __MAILBOX_CONTROLLER_H */
> --
> 1.8.1.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 2/5] mailbox: Introduce framework for mailbox
2014-03-19 4:00 ` Girish KS
@ 2014-03-19 5:20 ` Jassi Brar
2014-03-19 6:02 ` Girish KS
0 siblings, 1 reply; 16+ messages in thread
From: Jassi Brar @ 2014-03-19 5:20 UTC (permalink / raw)
To: Girish KS
Cc: Jassi Brar, Linux Kernel Mailing List, Greg Kroah-Hartman, Anna,
Suman, Tony Lindgren, Omar Ramirez Luna (omar.ramirez@copitl.com),
Loic Pallardy, LeyFoon Tan, Craig McGeachie, Courtney Cavin,
Rafael J. Wysocki, Rob Herring, Arnd Bergmann, Josh Cartwright,
Linus Walleij, Kumar Gala, ks.giri
On 19 March 2014 09:30, Girish KS <girishks2000@gmail.com> wrote:
> On Wed, Mar 19, 2014 at 12:15 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
>> +{
>> + int t;
>> +
>> + if (!chan || !chan->cl)
>> + return -EINVAL;
>> +
>> + t = _add_to_rbuf(chan, mssg);
>> + if (t < 0) {
>> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
>> + return t;
>> + }
>> +
>> + _msg_submit(chan);
>> +
>> + if (chan->txdone_method == TXDONE_BY_POLL)
>> + poll_txdone((unsigned long)chan->con);
>
> I came across a panic in the complete function. When i traced bact the
> call sequence it was
> When a client sets chan->cl->tx_block = true, and polling is enabled
> for controller.
> 1.Client sends the message with mbox_send_message. This function as
> seen above will call __msg_submit (this calls the controller specific
> send function).
> 2. Since the tx method is polling the above condition is satisfied and
> calls the poll_txdone function.
> 3. In this poll function, the tx_tick function is invoked.
> 4. In this tick function since the client has enabled the tx_block it
> calls the notify function complete(&chan->tx_complete);
> 5. Here there is a panic. because the complete is called before
> initialization. init_completion needs to be called but not called.
>
Are you sure you have applied the patch "[PATCHv4 4/5] mailbox: Fix TX
completion init" ?
Thanks
Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 3/5] mailbox: pl320: Introduce common API driver
2014-03-18 19:10 ` Rob Herring
@ 2014-03-19 5:40 ` Jassi Brar
0 siblings, 0 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-19 5:40 UTC (permalink / raw)
To: Rob Herring
Cc: Jassi Brar, linux-kernel, Greg Kroah-Hartman, s-anna,
Tony Lindgren, omar.ramirez, Loic Pallardy, LeyFoon Tan,
Craig McGeachie, Courtney Cavin, Wysocki, Rafael J,
Arnd Bergmann, Josh Cartwright, Linus Walleij, Kumar Gala,
ks.giri
On 19 March 2014 00:40, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Mar 18, 2014 at 1:45 PM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>> Convert the PL320 controller driver to work with the common
>> mailbox API. Also convert the only user of PL320, highbank-cpufreq.c
>> to work with thee API. Drop the obsoleted driver pl320-ipc.c
>>
>> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
>> ---
>> drivers/cpufreq/highbank-cpufreq.c | 24 ++++-
>> drivers/mailbox/Makefile | 2 +-
>> drivers/mailbox/pl320-ipc.c | 198 ----------------------------------
>> drivers/mailbox/pl320.c | 214 +++++++++++++++++++++++++++++++++++++
>
> Would you mind resending with rename detection enabled.
>
> You can make that the default by adding this to your config:
>
> [diff]
> renames = true
>
The driver isn't just renamed. The new driver is quite different from
original. So '-M' option to git format-patch doesn't make it simpler.
Btw I do have "git config --global diff.renames true" as well.
-Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 2/5] mailbox: Introduce framework for mailbox
2014-03-19 5:20 ` Jassi Brar
@ 2014-03-19 6:02 ` Girish KS
0 siblings, 0 replies; 16+ messages in thread
From: Girish KS @ 2014-03-19 6:02 UTC (permalink / raw)
To: Jassi Brar
Cc: Jassi Brar, Linux Kernel Mailing List, Greg Kroah-Hartman, Anna,
Suman, Tony Lindgren, Omar Ramirez Luna (omar.ramirez@copitl.com),
Loic Pallardy, LeyFoon Tan, Craig McGeachie, Courtney Cavin,
Rafael J. Wysocki, Rob Herring, Arnd Bergmann, Josh Cartwright,
Linus Walleij, Kumar Gala, ks.giri
On Wed, Mar 19, 2014 at 10:50 AM, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> On 19 March 2014 09:30, Girish KS <girishks2000@gmail.com> wrote:
>> On Wed, Mar 19, 2014 at 12:15 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
>>> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
>>> +{
>>> + int t;
>>> +
>>> + if (!chan || !chan->cl)
>>> + return -EINVAL;
>>> +
>>> + t = _add_to_rbuf(chan, mssg);
>>> + if (t < 0) {
>>> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
>>> + return t;
>>> + }
>>> +
>>> + _msg_submit(chan);
>>> +
>>> + if (chan->txdone_method == TXDONE_BY_POLL)
>>> + poll_txdone((unsigned long)chan->con);
>>
>> I came across a panic in the complete function. When i traced bact the
>> call sequence it was
>> When a client sets chan->cl->tx_block = true, and polling is enabled
>> for controller.
>> 1.Client sends the message with mbox_send_message. This function as
>> seen above will call __msg_submit (this calls the controller specific
>> send function).
>> 2. Since the tx method is polling the above condition is satisfied and
>> calls the poll_txdone function.
>> 3. In this poll function, the tx_tick function is invoked.
>> 4. In this tick function since the client has enabled the tx_block it
>> calls the notify function complete(&chan->tx_complete);
>> 5. Here there is a panic. because the complete is called before
>> initialization. init_completion needs to be called but not called.
>>
> Are you sure you have applied the patch "[PATCHv4 4/5] mailbox: Fix TX
> completion init" ?
I was missing it. I was using your old patch set
>
> Thanks
> Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
` (4 preceding siblings ...)
2014-03-18 18:46 ` [PATCHv4 5/5] mailbox: Fix deleteing poll timer Jassi Brar
@ 2014-03-27 22:51 ` Markus Mayer
2014-03-29 3:55 ` Jassi Brar
5 siblings, 1 reply; 16+ messages in thread
From: Markus Mayer @ 2014-03-27 22:51 UTC (permalink / raw)
To: Linux Kernel Mailing list, Jassi Brar
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd
> The patch 30058677 "ARM / highbank: add support for pl320 IPC"
> added a pl320 IPC specific header file as a generic mailbox.h.
> This file has been renamed appropriately to allow the
> introduction of the generic mailbox API framework.
>
> Acked-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Signed-off-by: Suman Anna <s-anna@ti.com>
>
> ---
> drivers/cpufreq/highbank-cpufreq.c | 2 +-
> drivers/mailbox/pl320-ipc.c | 2 +-
> include/linux/mailbox.h | 17 -----------------
> include/linux/pl320-ipc.h | 17 +++++++++++++++++
> 4 files changed, 19 insertions(+), 19 deletions(-)
> delete mode 100644 include/linux/mailbox.h
> create mode 100644 include/linux/pl320-ipc.h
This does not look like GIT realized you were just renaming these files.
When I apply this patch and re-generate the patch file, it looks like this:
...
drivers/cpufreq/highbank-cpufreq.c | 2 +-
drivers/mailbox/pl320-ipc.c | 2 +-
include/linux/{mailbox.h => pl320-ipc.h} | 0
3 files changed, 2 insertions(+), 2 deletions(-)
rename include/linux/{mailbox.h => pl320-ipc.h} (100%)
...
Regards,
-Markus
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4,2/5] mailbox: Introduce framework for mailbox
2014-03-18 18:45 ` [PATCHv4 2/5] mailbox: Introduce framework for mailbox Jassi Brar
2014-03-19 4:00 ` Girish KS
@ 2014-03-28 22:08 ` Markus Mayer
2014-03-29 3:54 ` Jassi Brar
1 sibling, 1 reply; 16+ messages in thread
From: Markus Mayer @ 2014-03-28 22:08 UTC (permalink / raw)
To: Linux Kernel Mailing list, Jassi Brar
Cc: gregkh, s-anna, tony, omar.ramirez, loic.pallardy, lftan.linux,
slapdau, courtney.cavin, rafael.j.wysocki, robherring2, arnd,
joshc, linus.walleij, galak, ks.giri
> diff --git a/drivers/mailbox/mailbox.c b/drivers/mailbox/mailbox.c
> new file mode 100644
> index 0000000..79d576e
> --- /dev/null
> +++ b/drivers/mailbox/mailbox.c
> @@ -0,0 +1,589 @@
> +/*
> + * Mailbox: Common code for Mailbox controllers and users
> + *
> + * Copyright (C) 2014 Linaro Ltd.
> + * Author: Jassi Brar <jassisinghbrar@gmail.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.
> + */
> +
> +#include <linux/interrupt.h>
> +#include <linux/spinlock.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/mailbox_client.h>
> +#include <linux/mailbox_controller.h>
> +
> +/*
> + * The length of circular buffer for queuing messages from a client.
> + * 'msg_count' tracks the number of buffered messages while 'msg_free'
> + * is the index where the next message would be buffered.
> + * We shouldn't need it too big because every transferr is interrupt
> + * triggered and if we have lots of data to transfer, the interrupt
> + * latencies are going to be the bottleneck, not the buffer length.
> + * Besides, mbox_send_message could be called from atomic context and
> + * the client could also queue another message from the notifier 'tx_done'
> + * of the last transfer done.
> + * REVIST: If too many platforms see the "Try increasing MBOX_TX_QUEUE_LEN"
> + * print, it needs to be taken from config option or somesuch.
> + */
> +#define MBOX_TX_QUEUE_LEN 20
> +
> +#define TXDONE_BY_IRQ (1 << 0) /* controller has remote RTR irq */
> +#define TXDONE_BY_POLL (1 << 1) /* controller can read status of last TX */
> +#define TXDONE_BY_ACK (1 << 2) /* S/W ACK recevied by Client ticks the TX */
> +
> +struct mbox_chan {
> + char name[16]; /* Physical link's name */
> + struct mbox_con *con; /* Parent Controller */
> + unsigned txdone_method;
> +
> + /* Physical links */
> + struct mbox_link *link;
> + struct mbox_link_ops *link_ops;
> +
> + /* client */
> + struct mbox_client *cl;
> + struct completion tx_complete;
> +
> + void *active_req;
> + unsigned msg_count, msg_free;
> + void *msg_data[MBOX_TX_QUEUE_LEN];
> + /* Access to the channel */
> + spinlock_t lock;
> + /* Hook to add to the controller's list of channels */
> + struct list_head node;
> + /* Notifier to all clients waiting on aquiring this channel */
> + struct blocking_notifier_head avail;
> +} __aligned(32);
> +
> +/* Internal representation of a controller */
> +struct mbox_con {
> + struct device *dev;
> + char name[16]; /* controller_name */
> + struct list_head channels;
> + /*
> + * If the controller supports only TXDONE_BY_POLL,
> + * this timer polls all the links for txdone.
> + */
> + struct timer_list poll;
> + unsigned period;
> + /* Hook to add to the global controller list */
> + struct list_head node;
> +} __aligned(32);
> +
> +static LIST_HEAD(mbox_cons);
> +static DEFINE_MUTEX(con_mutex);
> +
> +static int _add_to_rbuf(struct mbox_chan *chan, void *mssg)
> +{
> + int idx;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + /* See if there is any space left */
> + if (chan->msg_count == MBOX_TX_QUEUE_LEN) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return -ENOMEM;
> + }
> +
> + idx = chan->msg_free;
> + chan->msg_data[idx] = mssg;
> + chan->msg_count++;
> +
> + if (idx == MBOX_TX_QUEUE_LEN - 1)
> + chan->msg_free = 0;
> + else
> + chan->msg_free++;
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + return idx;
> +}
> +
> +static void _msg_submit(struct mbox_chan *chan)
> +{
> + struct mbox_link *link = chan->link;
> + unsigned count, idx;
> + unsigned long flags;
> + void *data;
> + int err;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> +
> + if (!chan->msg_count || chan->active_req) {
> + spin_unlock_irqrestore(&chan->lock, flags);
> + return;
> + }
> +
> + count = chan->msg_count;
> + idx = chan->msg_free;
> + if (idx >= count)
> + idx -= count;
> + else
> + idx += MBOX_TX_QUEUE_LEN - count;
> +
> + data = chan->msg_data[idx];
> +
> + /* Try to submit a message to the MBOX controller */
> + err = chan->link_ops->send_data(link, data);
> + if (!err) {
> + chan->active_req = data;
> + chan->msg_count--;
> + }
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +}
> +
> +static void tx_tick(struct mbox_chan *chan, enum mbox_result r)
> +{
> + unsigned long flags;
> + void *mssg;
> +
> + spin_lock_irqsave(&chan->lock, flags);
> + mssg = chan->active_req;
> + chan->active_req = NULL;
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + /* Submit next message */
> + _msg_submit(chan);
> +
> + /* Notify the client */
> + if (chan->cl->tx_block)
> + complete(&chan->tx_complete);
> + else if (mssg && chan->cl->tx_done)
> + chan->cl->tx_done(chan->cl, mssg, r);
> +}
> +
> +static void poll_txdone(unsigned long data)
> +{
> + struct mbox_con *con = (struct mbox_con *)data;
> + bool txdone, resched = false;
> + struct mbox_chan *chan;
> +
> + list_for_each_entry(chan, &con->channels, node) {
> + if (chan->active_req && chan->cl) {
> + resched = true;
> + txdone = chan->link_ops->last_tx_done(chan->link);
> + if (txdone)
> + tx_tick(chan, MBOX_OK);
> + }
> + }
> +
> + if (resched)
> + mod_timer(&con->poll,
> + jiffies + msecs_to_jiffies(con->period));
> +}
> +
> +/**
> + * mbox_link_received_data - A way for controller driver to push data
> + * received from remote to the upper layer.
> + * @link: Pointer to the mailbox link on which RX happened.
> + * @data: Client specific message typecasted as void *
> + *
> + * After startup and before shutdown any data received on the link
> + * is passed on to the API via atomic mbox_link_received_data().
> + * The controller should ACK the RX only after this call returns.
> + */
> +void mbox_link_received_data(struct mbox_link *link, void *mssg)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
> +
> + /* No buffering the received data */
> + if (chan->cl->rx_callback)
> + chan->cl->rx_callback(chan->cl, mssg);
> +}
> +EXPORT_SYMBOL_GPL(mbox_link_received_data);
> +
> +/**
> + * mbox_link_txdone - A way for controller driver to notify the
> + * framework that the last TX has completed.
> + * @link: Pointer to the mailbox link on which TX happened.
> + * @r: Status of last TX - OK or ERROR
> + *
> + * The controller that has IRQ for TX ACK calls this atomic API
> + * to tick the TX state machine. It works only if txdone_irq
> + * is set by the controller.
> + */
> +void mbox_link_txdone(struct mbox_link *link, enum mbox_result r)
> +{
> + struct mbox_chan *chan = (struct mbox_chan *)link->api_priv;
> +
> + if (unlikely(!(chan->txdone_method & TXDONE_BY_IRQ))) {
> + pr_err("Controller can't run the TX ticker\n");
> + return;
> + }
> +
> + tx_tick(chan, r);
> +}
> +EXPORT_SYMBOL_GPL(mbox_link_txdone);
> +
> +/**
> + * mbox_client_txdone - The way for a client to run the TX state machine.
> + * @chan: Mailbox channel assigned to this client.
> + * @r: Success status of last transmission.
> + *
> + * The client/protocol had received some 'ACK' packet and it notifies
> + * the API that the last packet was sent successfully. This only works
> + * if the controller can't sense TX-Done.
> + */
> +void mbox_client_txdone(struct mbox_chan *chan, enum mbox_result r)
> +{
> + if (unlikely(!(chan->txdone_method & TXDONE_BY_ACK))) {
> + pr_err("Client can't run the TX ticker\n");
> + return;
> + }
> +
> + tx_tick(chan, r);
> +}
> +EXPORT_SYMBOL_GPL(mbox_client_txdone);
> +
> +/**
> + * mbox_send_message - For client to submit a message to be
> + * sent to the remote.
> + * @chan: Mailbox channel assigned to this client.
> + * @mssg: Client specific message typecasted.
> + *
> + * For client to submit data to the controller destined for a remote
> + * processor. If the client had set 'tx_block', the call will return
> + * either when the remote receives the data or when 'tx_tout' millisecs
> + * run out.
> + * In non-blocking mode, the requests are buffered by the API and a
> + * non-negative token is returned for each queued request. If the request
> + * is not queued, a negative token is returned. Upon failure or successful
> + * TX, the API calls 'tx_done' from atomic context, from which the client
> + * could submit yet another request.
> + * In blocking mode, 'tx_done' is not called, effectively making the
> + * queue length 1.
> + * The pointer to message should be preserved until it is sent
> + * over the link, i.e, tx_done() is made.
> + * This function could be called from atomic context as it simply
> + * queues the data and returns a token against the request.
> + *
> + * Return: Non-negative integer for successful submission (non-blocking mode)
> + * or transmission over link (blocking mode).
> + * Negative value denotes failure.
> + */
> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
> +{
> + int t;
> +
> + if (!chan || !chan->cl)
> + return -EINVAL;
> +
> + t = _add_to_rbuf(chan, mssg);
> + if (t < 0) {
> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
> + return t;
> + }
> +
> + _msg_submit(chan);
> +
> + if (chan->txdone_method == TXDONE_BY_POLL)
> + poll_txdone((unsigned long)chan->con);
Wouldn't it be cleaner to use
poll_txdone((unsigned long)&chan->con);
?
> +
> + if (chan->cl->tx_block && chan->active_req) {
> + int ret;
> + init_completion(&chan->tx_complete);
> + ret = wait_for_completion_timeout(&chan->tx_complete,
> + chan->cl->tx_tout);
> + if (ret == 0) {
> + t = -EIO;
> + tx_tick(chan, MBOX_ERR);
> + }
> + }
> +
> + return t;
> +}
> +EXPORT_SYMBOL_GPL(mbox_send_message);
> +
> +/**
> + * mbox_request_channel - Request a mailbox channel.
> + * @cl: Identity of the client requesting the channel.
> + *
> + * The Client specifies its requirements and capabilities while asking for
> + * a mailbox channel by name. It can't be called from atomic context.
> + * The channel is exclusively allocated and can't be used by another
> + * client before the owner calls mbox_free_channel.
> + * After assignment, any packet received on this channel will be
> + * handed over to the client via the 'rx_callback'.
> + *
> + * Return: Pointer to the channel assigned to the client if successful.
> + * ERR_PTR for request failure.
> + */
> +struct mbox_chan *mbox_request_channel(struct mbox_client *cl)
> +{
> + struct mbox_chan *chan;
> + struct mbox_con *con;
> + unsigned long flags;
> + char *con_name;
> + int len, ret;
> +
> + con_name = cl->chan_name;
> + len = strcspn(cl->chan_name, ":");
> +
> + ret = 0;
> + mutex_lock(&con_mutex);
> + list_for_each_entry(con, &mbox_cons, node)
> + if (!strncmp(con->name, con_name, len)) {
> + ret = 1;
> + break;
> + }
> + mutex_unlock(&con_mutex);
> +
> + if (!ret) {
> + pr_info("Channel(%s) not found!\n", cl->chan_name);
> + return ERR_PTR(-ENODEV);
> + }
> +
> + ret = 0;
> + list_for_each_entry(chan, &con->channels, node) {
> + if (!chan->cl &&
> + !strcmp(con_name + len + 1, chan->name) &&
> + try_module_get(con->dev->driver->owner)) {
> + spin_lock_irqsave(&chan->lock, flags);
> + chan->msg_free = 0;
> + chan->msg_count = 0;
> + chan->active_req = NULL;
> + chan->cl = cl;
> + if (!cl->tx_tout) /* wait for ever */
> + cl->tx_tout = msecs_to_jiffies(3600000);
> + else
> + cl->tx_tout = msecs_to_jiffies(cl->tx_tout);
> + if (chan->txdone_method == TXDONE_BY_POLL
> + && cl->knows_txdone)
> + chan->txdone_method |= TXDONE_BY_ACK;
> + spin_unlock_irqrestore(&chan->lock, flags);
> + ret = 1;
> + break;
> + }
> + }
> +
> + if (!ret) {
> + pr_err("Unable to assign mailbox(%s)\n", cl->chan_name);
> + return ERR_PTR(-EBUSY);
> + }
> +
> + ret = chan->link_ops->startup(chan->link, cl->link_data);
> + if (ret) {
> + pr_err("Unable to startup the link\n");
> + mbox_free_channel(chan);
> + return ERR_PTR(ret);
> + }
> +
> + return chan;
> +}
> +EXPORT_SYMBOL_GPL(mbox_request_channel);
> +
> +/**
> + * mbox_free_channel - The client relinquishes control of a mailbox
> + * channel by this call.
> + * @chan: The mailbox channel to be freed.
> + */
> +void mbox_free_channel(struct mbox_chan *chan)
> +{
> + unsigned long flags;
> +
> + if (!chan || !chan->cl)
> + return;
> +
> + chan->link_ops->shutdown(chan->link);
> +
> + /* The queued TX requests are simply aborted, no callbacks are made */
> + spin_lock_irqsave(&chan->lock, flags);
> + chan->cl = NULL;
> + chan->active_req = NULL;
> + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK))
> + chan->txdone_method = TXDONE_BY_POLL;
> +
> + module_put(chan->con->dev->driver->owner);
> +
> + spin_unlock_irqrestore(&chan->lock, flags);
> +
> + blocking_notifier_call_chain(&chan->avail, 0, NULL);
> +}
> +EXPORT_SYMBOL_GPL(mbox_free_channel);
> +
> +static struct mbox_chan *name_to_chan(const char *name)
> +{
> + struct mbox_chan *chan = NULL;
> + struct mbox_con *con;
> + int len, found = 0;
> +
> + len = strcspn(name, ":");
> +
> + mutex_lock(&con_mutex);
> +
> + list_for_each_entry(con, &mbox_cons, node) {
> + if (!strncmp(con->name, name, len)) {
> + list_for_each_entry(chan, &con->channels, node) {
> + if (!strcmp(name + len + 1, chan->name)) {
> + found = 1;
> + goto done;
> + }
> + }
> + }
> + }
> +done:
> + mutex_unlock(&con_mutex);
> +
> + if (!found)
> + return NULL;
> +
> + return chan;
> +}
> +
> +/**
> + * mbox_notify_chan_register - The client may ask the framework to be
> + * notified when a particular channel becomes available
> + * to be acquired again.
> + * @name: Name of the mailbox channel the client is interested in.
> + * @nb: Pointer to the notifier.
> + */
> +int mbox_notify_chan_register(const char *name, struct notifier_block *nb)
> +{
> + struct mbox_chan *chan = name_to_chan(name);
> +
> + if (chan && nb)
> + return blocking_notifier_chain_register(&chan->avail, nb);
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(mbox_notify_chan_register);
> +
> +/**
> + * mbox_notify_chan_unregister - The client is no more interested in channel.
> + *
> + * @name: Name of the mailbox channel the client was interested in.
> + * @nb: Pointer to the notifier.
> + */
> +void mbox_notify_chan_unregister(const char *name, struct notifier_block *nb)
> +{
> + struct mbox_chan *chan = name_to_chan(name);
> +
> + if (chan && nb)
> + blocking_notifier_chain_unregister(&chan->avail, nb);
> +}
> +EXPORT_SYMBOL_GPL(mbox_notify_chan_unregister);
> +
> +/**
> + * mbox_controller_register - Register the mailbox controller
> + * @mbox_con: Pointer to the mailbox controller.
> + *
> + * The controller driver registers its communication links to the
> + * global pool managed by the common framework.
> + */
> +int mbox_controller_register(struct mbox_controller *mbox)
> +{
> + int i, num_links, txdone;
> + struct mbox_chan *chan;
> + struct mbox_con *con;
> +
> + /* Sanity check */
> + if (!mbox || !mbox->ops)
> + return -EINVAL;
> +
> + for (i = 0; mbox->links[i]; i++)
> + ;
> + if (!i)
> + return -EINVAL;
> + num_links = i;
> +
> + mutex_lock(&con_mutex);
> + /* Check if already populated */
> + list_for_each_entry(con, &mbox_cons, node)
> + if (!strcmp(mbox->controller_name, con->name)) {
> + mutex_unlock(&con_mutex);
> + return -EINVAL;
> + }
> +
> + con = kzalloc(sizeof(struct mbox_con), GFP_KERNEL);
> + if (!con)
> + return -ENOMEM;
The mutex is not freed here.
> +
> + chan = kzalloc(sizeof(struct mbox_chan) * num_links, GFP_KERNEL);
> + if (!chan) {
> + kfree(con);
> + return -ENOMEM;
Again, the mutex is not freed.
You could move both allocations above the mutex. Then you won't have to
worry about it.
> + }
> +
> + con->dev = mbox->dev;
> + INIT_LIST_HEAD(&con->channels);
> + snprintf(con->name, 16, "%s", mbox->controller_name);
> +
> + if (mbox->txdone_irq)
> + txdone = TXDONE_BY_IRQ;
> + else if (mbox->txdone_poll)
> + txdone = TXDONE_BY_POLL;
> + else /* It has to be ACK then */
> + txdone = TXDONE_BY_ACK;
> +
> + if (txdone == TXDONE_BY_POLL) {
> + con->period = mbox->txpoll_period;
> + con->poll.function = &poll_txdone;
> + con->poll.data = (unsigned long)con;
How about
con->poll.data = (unsigned long)&con;
> + init_timer(&con->poll);
> + }
> +
> + for (i = 0; i < num_links; i++) {
> + chan[i].con = con;
> + chan[i].cl = NULL;
> + chan[i].link_ops = mbox->ops;
> + chan[i].link = mbox->links[i];
> + chan[i].txdone_method = txdone;
> + chan[i].link->api_priv = &chan[i];
> + spin_lock_init(&chan[i].lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&chan[i].avail);
> + list_add_tail(&chan[i].node, &con->channels);
> + snprintf(chan[i].name, 16, "%s", mbox->links[i]->link_name);
> + }
> +
> + list_add_tail(&con->node, &mbox_cons);
> + mutex_unlock(&con_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(mbox_controller_register);
> +
> +/**
> + * mbox_controller_unregister - UnRegister the mailbox controller
> + * @mbox_con: Pointer to the mailbox controller.
> + *
> + * Purge the mailbox links from the global pool maintained by the framework.
> + */
> +void mbox_controller_unregister(struct mbox_controller *mbox)
> +{
> + struct mbox_con *t, *con = NULL;
> + struct mbox_chan *chan;
> +
> + mutex_lock(&con_mutex);
> +
> + list_for_each_entry(t, &mbox_cons, node)
> + if (!strcmp(mbox->controller_name, t->name)) {
> + con = t;
> + break;
> + }
> +
> + if (con)
> + list_del(&con->node);
> +
> + mutex_unlock(&con_mutex);
> +
> + if (!con)
> + return;
> +
> + list_for_each_entry(chan, &con->channels, node)
> + mbox_free_channel(chan);
> +
> + del_timer_sync(&con->poll);
> +
> + kfree(con);
> +}
> +EXPORT_SYMBOL_GPL(mbox_controller_unregister);
Regards,
-Markus
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4,2/5] mailbox: Introduce framework for mailbox
2014-03-28 22:08 ` [PATCHv4,2/5] " Markus Mayer
@ 2014-03-29 3:54 ` Jassi Brar
2014-03-31 22:05 ` Markus Mayer
0 siblings, 1 reply; 16+ messages in thread
From: Jassi Brar @ 2014-03-29 3:54 UTC (permalink / raw)
To: Markus Mayer
Cc: Linux Kernel Mailing list, Greg KH, Suman Anna, Tony Lindgren,
Omar Ramirez Luna (omar.ramirez@copitl.com),
Loic Pallardy, LeyFoon Tan, Craig McGeachie, Courtney Cavin,
Rafael J. Wysocki, Rob Herring, Arnd Bergmann, Josh Cartwright,
Linus Walleij, Kumar Gala, Girish K S
On Sat, Mar 29, 2014 at 3:38 AM, Markus Mayer <markus.mayer@linaro.org> wrote:
.....
>> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
>> +{
>> + int t;
>> +
>> + if (!chan || !chan->cl)
>> + return -EINVAL;
>> +
>> + t = _add_to_rbuf(chan, mssg);
>> + if (t < 0) {
>> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
>> + return t;
>> + }
>> +
>> + _msg_submit(chan);
>> +
>> + if (chan->txdone_method == TXDONE_BY_POLL)
>> + poll_txdone((unsigned long)chan->con);
>
> Wouldn't it be cleaner to use
> poll_txdone((unsigned long)&chan->con);
> ?
>
Here's how we use it ...
static void poll_txdone(unsigned long data)
{
struct mbox_con *con = (struct mbox_con *)data;
.....
}
To me, unnecessarily passing a pointer to a pointer seems unclean.
>> +int mbox_controller_register(struct mbox_controller *mbox)
>> +{
>> + int i, num_links, txdone;
>> + struct mbox_chan *chan;
>> + struct mbox_con *con;
>> +
>> + /* Sanity check */
>> + if (!mbox || !mbox->ops)
>> + return -EINVAL;
>> +
>> + for (i = 0; mbox->links[i]; i++)
>> + ;
>> + if (!i)
>> + return -EINVAL;
>> + num_links = i;
>> +
>> + mutex_lock(&con_mutex);
>> + /* Check if already populated */
>> + list_for_each_entry(con, &mbox_cons, node)
>> + if (!strcmp(mbox->controller_name, con->name)) {
>> + mutex_unlock(&con_mutex);
>> + return -EINVAL;
>> + }
>> +
>> + con = kzalloc(sizeof(struct mbox_con), GFP_KERNEL);
>> + if (!con)
>> + return -ENOMEM;
>
> The mutex is not freed here.
>
>> +
>> + chan = kzalloc(sizeof(struct mbox_chan) * num_links, GFP_KERNEL);
>> + if (!chan) {
>> + kfree(con);
>> + return -ENOMEM;
>
> Again, the mutex is not freed.
>
> You could move both allocations above the mutex. Then you won't have to
> worry about it.
>
Yes thanks. I overlooked. Will fix it.
Regards,
-Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h
2014-03-27 22:51 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Markus Mayer
@ 2014-03-29 3:55 ` Jassi Brar
0 siblings, 0 replies; 16+ messages in thread
From: Jassi Brar @ 2014-03-29 3:55 UTC (permalink / raw)
To: Markus Mayer
Cc: Linux Kernel Mailing list, Greg KH, Suman Anna, Tony Lindgren,
Omar Ramirez Luna (omar.ramirez@copitl.com),
Loic Pallardy, LeyFoon Tan, Craig McGeachie, Courtney Cavin,
Rafael J. Wysocki, Rob Herring, Arnd Bergmann
On Fri, Mar 28, 2014 at 4:21 AM, Markus Mayer <markus.mayer@linaro.org> wrote:
>> The patch 30058677 "ARM / highbank: add support for pl320 IPC"
>> added a pl320 IPC specific header file as a generic mailbox.h.
>> This file has been renamed appropriately to allow the
>> introduction of the generic mailbox API framework.
>>
>> Acked-by: Mark Langsdorf <mark.langsdorf@calxeda.com>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>
>> ---
>> drivers/cpufreq/highbank-cpufreq.c | 2 +-
>> drivers/mailbox/pl320-ipc.c | 2 +-
>> include/linux/mailbox.h | 17 -----------------
>> include/linux/pl320-ipc.h | 17 +++++++++++++++++
>> 4 files changed, 19 insertions(+), 19 deletions(-)
>> delete mode 100644 include/linux/mailbox.h
>> create mode 100644 include/linux/pl320-ipc.h
>
> This does not look like GIT realized you were just renaming these files.
> When I apply this patch and re-generate the patch file, it looks like this:
>
> ...
> drivers/cpufreq/highbank-cpufreq.c | 2 +-
> drivers/mailbox/pl320-ipc.c | 2 +-
> include/linux/{mailbox.h => pl320-ipc.h} | 0
> 3 files changed, 2 insertions(+), 2 deletions(-)
> rename include/linux/{mailbox.h => pl320-ipc.h} (100%)
> ...
>
OK, will generate the patch with -M option.
Thanks
Jassi
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCHv4,2/5] mailbox: Introduce framework for mailbox
2014-03-29 3:54 ` Jassi Brar
@ 2014-03-31 22:05 ` Markus Mayer
0 siblings, 0 replies; 16+ messages in thread
From: Markus Mayer @ 2014-03-31 22:05 UTC (permalink / raw)
To: Jassi Brar
Cc: Linux Kernel Mailing list, Greg KH, Suman Anna, Tony Lindgren,
Omar Ramirez Luna (omar.ramirez@copitl.com),
Loic Pallardy, LeyFoon Tan, Craig McGeachie, Courtney Cavin,
Rafael J. Wysocki, Rob Herring, Arnd Bergmann, Josh Cartwright,
Linus Walleij, Kumar Gala, Girish K S
On 28 March 2014 20:54, Jassi Brar <jassisinghbrar@gmail.com> wrote:
> On Sat, Mar 29, 2014 at 3:38 AM, Markus Mayer <markus.mayer@linaro.org> wrote:
>
> .....
>
>>> +int mbox_send_message(struct mbox_chan *chan, void *mssg)
>>> +{
>>> + int t;
>>> +
>>> + if (!chan || !chan->cl)
>>> + return -EINVAL;
>>> +
>>> + t = _add_to_rbuf(chan, mssg);
>>> + if (t < 0) {
>>> + pr_err("Try increasing MBOX_TX_QUEUE_LEN\n");
>>> + return t;
>>> + }
>>> +
>>> + _msg_submit(chan);
>>> +
>>> + if (chan->txdone_method == TXDONE_BY_POLL)
>>> + poll_txdone((unsigned long)chan->con);
>>
>> Wouldn't it be cleaner to use
>> poll_txdone((unsigned long)&chan->con);
>> ?
>>
> Here's how we use it ...
>
> static void poll_txdone(unsigned long data)
> {
> struct mbox_con *con = (struct mbox_con *)data;
> .....
> }
>
> To me, unnecessarily passing a pointer to a pointer seems unclean.
You are right. I didn't look closely enough.
Regards,
-Markus
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2014-03-31 22:05 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-18 18:42 [PATCHv4 0/5] Common Mailbox Framework Jassi Brar
2014-03-18 18:44 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Jassi Brar
2014-03-18 18:45 ` [PATCHv4 2/5] mailbox: Introduce framework for mailbox Jassi Brar
2014-03-19 4:00 ` Girish KS
2014-03-19 5:20 ` Jassi Brar
2014-03-19 6:02 ` Girish KS
2014-03-28 22:08 ` [PATCHv4,2/5] " Markus Mayer
2014-03-29 3:54 ` Jassi Brar
2014-03-31 22:05 ` Markus Mayer
2014-03-18 18:45 ` [PATCHv4 3/5] mailbox: pl320: Introduce common API driver Jassi Brar
2014-03-18 19:10 ` Rob Herring
2014-03-19 5:40 ` Jassi Brar
2014-03-18 18:46 ` [PATCHv4 4/5] mailbox: Fix TX completion init Jassi Brar
2014-03-18 18:46 ` [PATCHv4 5/5] mailbox: Fix deleteing poll timer Jassi Brar
2014-03-27 22:51 ` [PATCHv4 1/5] mailbox: rename pl320-ipc specific mailbox.h Markus Mayer
2014-03-29 3:55 ` Jassi Brar
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.