* [PATCH 0/4] omap: mailbox: cleanup & simplify
@ 2010-04-27 17:56 Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-27 17:56 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
A few simple patches that cleanup and simplifies mailbox.
The last patch is the most interesting -
It converts mailbox to use kfifo as the underlying
queueing implementation instead of using the block API.
There're also additional performance patches on the way, we
are internally testing them now.
Please review and let me know your comments.
Thanks,
Ohad Ben-Cohen (4):
omap: mailbox cleanup: convert rwlocks to spinlock
omap: mailbox cleanup: split MODULE_AUTHOR line
omap: mailbox: fix reverse likeliness
omap: mailbox: convert block api to kfifo
arch/arm/mach-omap2/mailbox.c | 3 +-
arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
arch/arm/plat-omap/mailbox.c | 135 +++++++++++++----------------
3 files changed, 68 insertions(+), 75 deletions(-)
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-27 17:56 [PATCH 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
@ 2010-04-27 17:56 ` Ohad Ben-Cohen
2010-04-28 7:50 ` Hiroshi DOYU
2010-04-29 12:44 ` Kanigeri, Hari
2010-04-27 17:56 ` [PATCH 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line Ohad Ben-Cohen
` (2 subsequent siblings)
3 siblings, 2 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-27 17:56 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
rwlocks are slower and have potential starvation issues so spinlocks are
generally preferred
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
arch/arm/plat-omap/mailbox.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 08a2df7..d73d51a 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
static struct workqueue_struct *mboxd;
static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const char *name)
struct omap_mbox *mbox;
int ret;
- read_lock(&mboxes_lock);
+ spin_lock(&mboxes_lock);
mbox = *(find_mboxes(name));
if (mbox == NULL) {
- read_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
return ERR_PTR(-ENOENT);
}
- read_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
ret = omap_mbox_startup(mbox);
if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
if (mbox->next)
return -EBUSY;
- write_lock(&mboxes_lock);
+ spin_lock(&mboxes_lock);
tmp = find_mboxes(mbox->name);
if (*tmp) {
ret = -EBUSY;
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
goto err_find;
}
*tmp = mbox;
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
return 0;
@@ -384,18 +384,18 @@ int omap_mbox_unregister(struct omap_mbox *mbox)
{
struct omap_mbox **tmp;
- write_lock(&mboxes_lock);
+ spin_lock(&mboxes_lock);
tmp = &mboxes;
while (*tmp) {
if (mbox == *tmp) {
*tmp = mbox->next;
mbox->next = NULL;
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
return 0;
}
tmp = &(*tmp)->next;
}
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
return -EINVAL;
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line
2010-04-27 17:56 [PATCH 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
@ 2010-04-27 17:56 ` Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 3/4] omap: mailbox: fix reverse likeliness Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
3 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-27 17:56 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
use multiple MODULE_AUTHOR lines for multiple authors
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
arch/arm/mach-omap2/mailbox.c | 3 ++-
arch/arm/plat-omap/mailbox.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 318f363..763272c 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -486,5 +486,6 @@ module_exit(omap2_mbox_exit);
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("omap mailbox: omap2/3/4 architecture specific functions");
-MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>, Paul Mundt");
+MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>");
+MODULE_AUTHOR("Paul Mundt");
MODULE_ALIAS("platform:"DRV_NAME);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index d73d51a..8abf0e8 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -419,4 +419,5 @@ module_exit(omap_mbox_exit);
MODULE_LICENSE("GPL v2");
MODULE_DESCRIPTION("omap mailbox: interrupt driven messaging");
-MODULE_AUTHOR("Toshihiro Kobayashi and Hiroshi DOYU");
+MODULE_AUTHOR("Toshihiro Kobayashi");
+MODULE_AUTHOR("Hiroshi DOYU");
--
1.6.3.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 3/4] omap: mailbox: fix reverse likeliness
2010-04-27 17:56 [PATCH 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line Ohad Ben-Cohen
@ 2010-04-27 17:56 ` Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
3 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-27 17:56 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
Fix reverse likeliness
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
arch/arm/plat-omap/mailbox.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 8abf0e8..72b17ad 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
fail_alloc_txq:
free_irq(mbox->irq, mbox);
fail_request_irq:
- if (unlikely(mbox->ops->shutdown))
+ if (likely(mbox->ops->shutdown))
mbox->ops->shutdown(mbox);
return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox->irq, mbox);
- if (unlikely(mbox->ops->shutdown)) {
+ if (likely(mbox->ops->shutdown)) {
write_lock(&mboxes_lock);
if (mbox_configured > 0)
mbox_configured--;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-27 17:56 [PATCH 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
` (2 preceding siblings ...)
2010-04-27 17:56 ` [PATCH 3/4] omap: mailbox: fix reverse likeliness Ohad Ben-Cohen
@ 2010-04-27 17:56 ` Ohad Ben-Cohen
2010-04-28 5:52 ` Hiroshi DOYU
2010-04-28 5:56 ` Hiroshi DOYU
3 siblings, 2 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-27 17:56 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
arch/arm/plat-omap/mailbox.c | 108 +++++++++++++----------------
2 files changed, 52 insertions(+), 61 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..014cc58 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -7,6 +7,7 @@
#include <linux/workqueue.h>
#include <linux/blkdev.h>
#include <linux/interrupt.h>
+#include <linux/kfifo.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -19,6 +20,8 @@ typedef int __bitwise omap_mbox_type_t;
#define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
#define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
+#define MBOX_KFIFO_SIZE (256)
+
struct omap_mbox_ops {
omap_mbox_type_t type;
int (*startup)(struct omap_mbox *mbox);
@@ -42,7 +45,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue *queue;
+ struct kfifo fifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 72b17ad..b1324f3 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,7 @@
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/kfifo.h>
#include <plat/mailbox.h>
@@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox->txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox->txq->queue;
+ spin_lock(&mq->lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg))) {
+ pr_err("%s: kfifo_in anomaly\n", __func__);
+ ret = -ENOMEM;
+ }
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(&mbox->txq->tasklet);
- return 0;
+out:
+ spin_unlock(&mq->lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox->txq->queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox->txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
- if (ret) {
+ while (kfifo_len(&mq->fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
+ sizeof(msg));
+ if (unlikely(ret != sizeof(msg)))
+ pr_err("%s: kfifo_out anomaly\n", __func__);
+
+ mbox_fifo_write(mbox, msg);
}
}
@@ -131,36 +137,22 @@ static void mbox_rx_work(struct work_struct *work)
{
struct omap_mbox_queue *mq =
container_of(work, struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
- struct request_queue *q = mbox->rxq->queue;
- struct request *rq;
mbox_msg_t msg;
- unsigned long flags;
+ int len;
- while (1) {
- spin_lock_irqsave(q->queue_lock, flags);
- rq = blk_fetch_request(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
- if (!rq)
- break;
+ while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+ len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg)))
+ pr_err("%s: kfifo_out anomaly detected\n", __func__);
- msg = (mbox_msg_t)rq->special;
- blk_end_request_all(rq, 0);
- mbox->rxq->callback((void *)msg);
+ if (mq->callback)
+ mq->callback((void *)msg);
}
}
/*
* Mailbox interrupt handler
*/
-static void mbox_txq_fn(struct request_queue *q)
-{
-}
-
-static void mbox_rxq_fn(struct request_queue *q)
-{
-}
-
static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
@@ -170,19 +162,20 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
{
- struct request *rq;
+ struct omap_mbox_queue *mq = mbox->rxq;
mbox_msg_t msg;
- struct request_queue *q = mbox->rxq->queue;
+ int len;
while (!mbox_fifo_empty(mbox)) {
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
+ if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
goto nomem;
msg = mbox_fifo_read(mbox);
- blk_insert_request(q, rq, 0, (void *)msg);
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ if (unlikely(len != sizeof(msg)))
+ pr_err("%s: kfifo_in anomaly detected\n", __func__);
if (mbox->ops->type == OMAP_MBOX_TYPE1)
break;
}
@@ -207,11 +200,9 @@ static irqreturn_t mbox_interrupt(int irq, void *p)
}
static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
- request_fn_proc *proc,
void (*work) (struct work_struct *),
void (*tasklet)(unsigned long))
{
- struct request_queue *q;
struct omap_mbox_queue *mq;
mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
@@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
spin_lock_init(&mq->lock);
- q = blk_init_queue(proc, &mq->lock);
- if (!q)
+ if (kfifo_alloc(&mq->fifo, MBOX_KFIFO_SIZE, GFP_KERNEL))
goto error;
- q->queuedata = mbox;
- mq->queue = q;
if (work)
INIT_WORK(&mq->work, work);
@@ -239,7 +227,7 @@ error:
static void mbox_queue_free(struct omap_mbox_queue *q)
{
- blk_cleanup_queue(q->queue);
+ kfifo_free(&q->fifo);
kfree(q);
}
@@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
+ mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_txq;
}
mbox->txq = mq;
- mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
+ mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-27 17:56 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
@ 2010-04-28 5:52 ` Hiroshi DOYU
2010-04-28 11:02 ` Ohad Ben-Cohen
2010-04-28 5:56 ` Hiroshi DOYU
1 sibling, 1 reply; 20+ messages in thread
From: Hiroshi DOYU @ 2010-04-28 5:52 UTC (permalink / raw)
To: ohad; +Cc: linux-omap, h-kanigeri2
Hi Ohad,
From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Tue, 27 Apr 2010 19:56:22 +0200
> The underlying buffering implementation of mailbox
> is converted from block API to kfifo due to the simplicity
> and speed of kfifo.
>
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
> ---
> arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
> arch/arm/plat-omap/mailbox.c | 108 +++++++++++++----------------
> 2 files changed, 52 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
> index 729166b..014cc58 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -7,6 +7,7 @@
> #include <linux/workqueue.h>
> #include <linux/blkdev.h>
> #include <linux/interrupt.h>
> +#include <linux/kfifo.h>
>
> typedef u32 mbox_msg_t;
> struct omap_mbox;
> @@ -19,6 +20,8 @@ typedef int __bitwise omap_mbox_type_t;
> #define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
> #define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
>
> +#define MBOX_KFIFO_SIZE (256)
Can this be a module parameter? Then, OEM could set their optimized value.
> +
> struct omap_mbox_ops {
> omap_mbox_type_t type;
> int (*startup)(struct omap_mbox *mbox);
> @@ -42,7 +45,7 @@ struct omap_mbox_ops {
>
> struct omap_mbox_queue {
> spinlock_t lock;
> - struct request_queue *queue;
> + struct kfifo fifo;
> struct work_struct work;
> struct tasklet_struct tasklet;
> int (*callback)(void *);
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 72b17ad..b1324f3 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -26,6 +26,7 @@
> #include <linux/device.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> +#include <linux/kfifo.h>
>
> #include <plat/mailbox.h>
>
> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
> /*
> * message sender
> */
> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
> {
> int ret = 0, i = 1000;
>
> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
> return -1;
> udelay(1);
> }
> - mbox_fifo_write(mbox, msg);
> return ret;
> }
>
> -
> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
> {
> + struct omap_mbox_queue *mq = mbox->txq;
> + int ret = 0, len;
>
> - struct request *rq;
> - struct request_queue *q = mbox->txq->queue;
> + spin_lock(&mq->lock);
>
> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> - if (unlikely(!rq))
> - return -ENOMEM;
> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
> + if (unlikely(len != sizeof(msg))) {
> + pr_err("%s: kfifo_in anomaly\n", __func__);
> + ret = -ENOMEM;
> + }
>
> - blk_insert_request(q, rq, 0, (void *) msg);
> tasklet_schedule(&mbox->txq->tasklet);
>
> - return 0;
> +out:
> + spin_unlock(&mq->lock);
> + return ret;
> }
> EXPORT_SYMBOL(omap_mbox_msg_send);
>
> static void mbox_tx_tasklet(unsigned long tx_data)
> {
> - int ret;
> - struct request *rq;
> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
> - struct request_queue *q = mbox->txq->queue;
> -
> - while (1) {
> -
> - rq = blk_fetch_request(q);
> -
> - if (!rq)
> - break;
> + struct omap_mbox_queue *mq = mbox->txq;
> + mbox_msg_t msg;
> + int ret;
>
> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
> - if (ret) {
> + while (kfifo_len(&mq->fifo)) {
> + if (__mbox_poll_for_space(mbox)) {
> omap_mbox_enable_irq(mbox, IRQ_TX);
> - blk_requeue_request(q, rq);
> - return;
> + break;
> }
> - blk_end_request_all(rq, 0);
> +
> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
> + sizeof(msg));
> + if (unlikely(ret != sizeof(msg)))
> + pr_err("%s: kfifo_out anomaly\n", __func__);
No error recovery? same for other anomalies.
> +
> + mbox_fifo_write(mbox, msg);
> }
> }
>
> @@ -131,36 +137,22 @@ static void mbox_rx_work(struct work_struct *work)
> {
> struct omap_mbox_queue *mq =
> container_of(work, struct omap_mbox_queue, work);
> - struct omap_mbox *mbox = mq->queue->queuedata;
> - struct request_queue *q = mbox->rxq->queue;
> - struct request *rq;
> mbox_msg_t msg;
> - unsigned long flags;
> + int len;
>
> - while (1) {
> - spin_lock_irqsave(q->queue_lock, flags);
> - rq = blk_fetch_request(q);
> - spin_unlock_irqrestore(q->queue_lock, flags);
> - if (!rq)
> - break;
> + while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
> + len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
> + if (unlikely(len != sizeof(msg)))
> + pr_err("%s: kfifo_out anomaly detected\n", __func__);
>
> - msg = (mbox_msg_t)rq->special;
> - blk_end_request_all(rq, 0);
> - mbox->rxq->callback((void *)msg);
> + if (mq->callback)
> + mq->callback((void *)msg);
> }
> }
>
> /*
> * Mailbox interrupt handler
> */
> -static void mbox_txq_fn(struct request_queue *q)
> -{
> -}
> -
> -static void mbox_rxq_fn(struct request_queue *q)
> -{
> -}
> -
> static void __mbox_tx_interrupt(struct omap_mbox *mbox)
> {
> omap_mbox_disable_irq(mbox, IRQ_TX);
> @@ -170,19 +162,20 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
>
> static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> {
> - struct request *rq;
> + struct omap_mbox_queue *mq = mbox->rxq;
> mbox_msg_t msg;
> - struct request_queue *q = mbox->rxq->queue;
> + int len;
>
> while (!mbox_fifo_empty(mbox)) {
> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> - if (unlikely(!rq))
> + if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
> goto nomem;
>
> msg = mbox_fifo_read(mbox);
>
>
> - blk_insert_request(q, rq, 0, (void *)msg);
> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
> + if (unlikely(len != sizeof(msg)))
> + pr_err("%s: kfifo_in anomaly detected\n", __func__);
> if (mbox->ops->type == OMAP_MBOX_TYPE1)
> break;
> }
> @@ -207,11 +200,9 @@ static irqreturn_t mbox_interrupt(int irq, void *p)
> }
>
> static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
> - request_fn_proc *proc,
> void (*work) (struct work_struct *),
> void (*tasklet)(unsigned long))
> {
> - struct request_queue *q;
> struct omap_mbox_queue *mq;
>
> mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
> @@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
>
> spin_lock_init(&mq->lock);
>
> - q = blk_init_queue(proc, &mq->lock);
> - if (!q)
> + if (kfifo_alloc(&mq->fifo, MBOX_KFIFO_SIZE, GFP_KERNEL))
> goto error;
> - q->queuedata = mbox;
> - mq->queue = q;
>
> if (work)
> INIT_WORK(&mq->work, work);
> @@ -239,7 +227,7 @@ error:
>
> static void mbox_queue_free(struct omap_mbox_queue *q)
> {
> - blk_cleanup_queue(q->queue);
> + kfifo_free(&q->fifo);
> kfree(q);
> }
>
> @@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> goto fail_request_irq;
> }
>
> - mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
> + mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_txq;
> }
> mbox->txq = mq;
>
> - mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
> + mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_rxq;
> --
> 1.6.3.3
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-27 17:56 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
2010-04-28 5:52 ` Hiroshi DOYU
@ 2010-04-28 5:56 ` Hiroshi DOYU
2010-04-28 11:02 ` Ohad Ben-Cohen
1 sibling, 1 reply; 20+ messages in thread
From: Hiroshi DOYU @ 2010-04-28 5:56 UTC (permalink / raw)
To: ohad; +Cc: linux-omap, h-kanigeri2
From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Tue, 27 Apr 2010 19:56:22 +0200
> The underlying buffering implementation of mailbox
> is converted from block API to kfifo due to the simplicity
> and speed of kfifo.
>
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
> ---
> arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
> arch/arm/plat-omap/mailbox.c | 108 +++++++++++++----------------
> 2 files changed, 52 insertions(+), 61 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
> index 729166b..014cc58 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -7,6 +7,7 @@
> #include <linux/workqueue.h>
> #include <linux/blkdev.h>
Also the above is not necessary?
> #include <linux/interrupt.h>
> +#include <linux/kfifo.h>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
@ 2010-04-28 7:50 ` Hiroshi DOYU
2010-04-28 10:41 ` Ohad Ben-Cohen
2010-04-29 12:44 ` Kanigeri, Hari
1 sibling, 1 reply; 20+ messages in thread
From: Hiroshi DOYU @ 2010-04-28 7:50 UTC (permalink / raw)
To: ohad; +Cc: linux-omap, h-kanigeri2
Hi Ohad,
From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
Date: Tue, 27 Apr 2010 19:56:19 +0200
> rwlocks are slower and have potential starvation issues so spinlocks are
> generally preferred
Would it be possible to explain the above a bit more?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-28 7:50 ` Hiroshi DOYU
@ 2010-04-28 10:41 ` Ohad Ben-Cohen
0 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-28 10:41 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2
Hi Hiroshi,
On Wed, Apr 28, 2010 at 10:50 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Ohad,
>
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
> Date: Tue, 27 Apr 2010 19:56:19 +0200
>
>> rwlocks are slower and have potential starvation issues so spinlocks are
>> generally preferred
>
> Would it be possible to explain the above a bit more?
Sure, sorry for the laconic description.
Jonathan Corbet wrote a nice summary about this:
http://lwn.net/Articles/364583/
We could switch to rcu, but it's really an overkill because we don't really
have a high bandwidth of readers (omap_mbox_get is not being called
so much).
The only disadvantage of a plain spinlock is that readers now will have
to wait in the line, but since omap_mbox_get isn't called so frequently,
I guess that by moving to spinlocks the average performance will actually
increase (since spinlocks are faster and most likely there will not be
multiple concurrent calls to omap_mbox_get).
Anyway I only consider this as a cleanup and not really a performance
issue, as mboxes_lock is not really on a hot path.
Thanks,
Ohad.
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 5:52 ` Hiroshi DOYU
@ 2010-04-28 11:02 ` Ohad Ben-Cohen
2010-04-28 11:16 ` Hiroshi DOYU
0 siblings, 1 reply; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-28 11:02 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2
Hi Hiroshi,
On Wed, Apr 28, 2010 at 8:52 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Ohad,
>
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo
> Date: Tue, 27 Apr 2010 19:56:22 +0200
>
>> The underlying buffering implementation of mailbox
>> is converted from block API to kfifo due to the simplicity
>> and speed of kfifo.
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>> ---
>> arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
>> arch/arm/plat-omap/mailbox.c | 108 +++++++++++++----------------
>> 2 files changed, 52 insertions(+), 61 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
>> index 729166b..014cc58 100644
>> --- a/arch/arm/plat-omap/include/plat/mailbox.h
>> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
>> @@ -7,6 +7,7 @@
>> #include <linux/workqueue.h>
>> #include <linux/blkdev.h>
>> #include <linux/interrupt.h>
>> +#include <linux/kfifo.h>
>>
>> typedef u32 mbox_msg_t;
>> struct omap_mbox;
>> @@ -19,6 +20,8 @@ typedef int __bitwise omap_mbox_type_t;
>> #define OMAP_MBOX_TYPE1 ((__force omap_mbox_type_t) 1)
>> #define OMAP_MBOX_TYPE2 ((__force omap_mbox_type_t) 2)
>>
>> +#define MBOX_KFIFO_SIZE (256)
>
> Can this be a module parameter? Then, OEM could set their optimized value.
Sure.
>
>> +
>> struct omap_mbox_ops {
>> omap_mbox_type_t type;
>> int (*startup)(struct omap_mbox *mbox);
>> @@ -42,7 +45,7 @@ struct omap_mbox_ops {
>>
>> struct omap_mbox_queue {
>> spinlock_t lock;
>> - struct request_queue *queue;
>> + struct kfifo fifo;
>> struct work_struct work;
>> struct tasklet_struct tasklet;
>> int (*callback)(void *);
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index 72b17ad..b1324f3 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -26,6 +26,7 @@
>> #include <linux/device.h>
>> #include <linux/delay.h>
>> #include <linux/slab.h>
>> +#include <linux/kfifo.h>
>>
>> #include <plat/mailbox.h>
>>
>> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
>> /*
>> * message sender
>> */
>> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
>> {
>> int ret = 0, i = 1000;
>>
>> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>> return -1;
>> udelay(1);
>> }
>> - mbox_fifo_write(mbox, msg);
>> return ret;
>> }
>>
>> -
>> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>> {
>> + struct omap_mbox_queue *mq = mbox->txq;
>> + int ret = 0, len;
>>
>> - struct request *rq;
>> - struct request_queue *q = mbox->txq->queue;
>> + spin_lock(&mq->lock);
>>
>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>> - if (unlikely(!rq))
>> - return -ENOMEM;
>> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>> + if (unlikely(len != sizeof(msg))) {
>> + pr_err("%s: kfifo_in anomaly\n", __func__);
>> + ret = -ENOMEM;
>> + }
>>
>> - blk_insert_request(q, rq, 0, (void *) msg);
>> tasklet_schedule(&mbox->txq->tasklet);
>>
>> - return 0;
>> +out:
>> + spin_unlock(&mq->lock);
>> + return ret;
>> }
>> EXPORT_SYMBOL(omap_mbox_msg_send);
>>
>> static void mbox_tx_tasklet(unsigned long tx_data)
>> {
>> - int ret;
>> - struct request *rq;
>> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
>> - struct request_queue *q = mbox->txq->queue;
>> -
>> - while (1) {
>> -
>> - rq = blk_fetch_request(q);
>> -
>> - if (!rq)
>> - break;
>> + struct omap_mbox_queue *mq = mbox->txq;
>> + mbox_msg_t msg;
>> + int ret;
>>
>> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
>> - if (ret) {
>> + while (kfifo_len(&mq->fifo)) {
>> + if (__mbox_poll_for_space(mbox)) {
>> omap_mbox_enable_irq(mbox, IRQ_TX);
>> - blk_requeue_request(q, rq);
>> - return;
>> + break;
>> }
>> - blk_end_request_all(rq, 0);
>> +
>> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
>> + sizeof(msg));
>> + if (unlikely(ret != sizeof(msg)))
>> + pr_err("%s: kfifo_out anomaly\n", __func__);
>
> No error recovery? same for other anomalies.
I thought about it too, but eventually I think it doesn't really make
a lot of sense:
The only reason that kfifo_out/kfifo_in can fail here is if there's
not enough data/space (respectively).
Since we are checking for the availability of data/space before calling
kfifo_out/kfifo_in, it cannot really fail.
If it does fail, that's a critical bug that we cannot really recover from.
Only reasonable explanation can be a bug in the code (either ours or kfifo's),
and with such a bug it really feels futile to put some recovery.
So maybe we should really make this a WARN_ON.
What do you think ?
Thanks for the review!
Ohad.
>
>> +
>> + mbox_fifo_write(mbox, msg);
>> }
>> }
>>
>> @@ -131,36 +137,22 @@ static void mbox_rx_work(struct work_struct *work)
>> {
>> struct omap_mbox_queue *mq =
>> container_of(work, struct omap_mbox_queue, work);
>> - struct omap_mbox *mbox = mq->queue->queuedata;
>> - struct request_queue *q = mbox->rxq->queue;
>> - struct request *rq;
>> mbox_msg_t msg;
>> - unsigned long flags;
>> + int len;
>>
>> - while (1) {
>> - spin_lock_irqsave(q->queue_lock, flags);
>> - rq = blk_fetch_request(q);
>> - spin_unlock_irqrestore(q->queue_lock, flags);
>> - if (!rq)
>> - break;
>> + while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
>> + len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>> + if (unlikely(len != sizeof(msg)))
>> + pr_err("%s: kfifo_out anomaly detected\n", __func__);
>>
>> - msg = (mbox_msg_t)rq->special;
>> - blk_end_request_all(rq, 0);
>> - mbox->rxq->callback((void *)msg);
>> + if (mq->callback)
>> + mq->callback((void *)msg);
>> }
>> }
>>
>> /*
>> * Mailbox interrupt handler
>> */
>> -static void mbox_txq_fn(struct request_queue *q)
>> -{
>> -}
>> -
>> -static void mbox_rxq_fn(struct request_queue *q)
>> -{
>> -}
>> -
>> static void __mbox_tx_interrupt(struct omap_mbox *mbox)
>> {
>> omap_mbox_disable_irq(mbox, IRQ_TX);
>> @@ -170,19 +162,20 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
>>
>> static void __mbox_rx_interrupt(struct omap_mbox *mbox)
>> {
>> - struct request *rq;
>> + struct omap_mbox_queue *mq = mbox->rxq;
>> mbox_msg_t msg;
>> - struct request_queue *q = mbox->rxq->queue;
>> + int len;
>>
>> while (!mbox_fifo_empty(mbox)) {
>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>> - if (unlikely(!rq))
>> + if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
>> goto nomem;
>>
>> msg = mbox_fifo_read(mbox);
>>
>>
>> - blk_insert_request(q, rq, 0, (void *)msg);
>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>> + if (unlikely(len != sizeof(msg)))
>> + pr_err("%s: kfifo_in anomaly detected\n", __func__);
>> if (mbox->ops->type == OMAP_MBOX_TYPE1)
>> break;
>> }
>> @@ -207,11 +200,9 @@ static irqreturn_t mbox_interrupt(int irq, void *p)
>> }
>>
>> static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
>> - request_fn_proc *proc,
>> void (*work) (struct work_struct *),
>> void (*tasklet)(unsigned long))
>> {
>> - struct request_queue *q;
>> struct omap_mbox_queue *mq;
>>
>> mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
>> @@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
>>
>> spin_lock_init(&mq->lock);
>>
>> - q = blk_init_queue(proc, &mq->lock);
>> - if (!q)
>> + if (kfifo_alloc(&mq->fifo, MBOX_KFIFO_SIZE, GFP_KERNEL))
>> goto error;
>> - q->queuedata = mbox;
>> - mq->queue = q;
>>
>> if (work)
>> INIT_WORK(&mq->work, work);
>> @@ -239,7 +227,7 @@ error:
>>
>> static void mbox_queue_free(struct omap_mbox_queue *q)
>> {
>> - blk_cleanup_queue(q->queue);
>> + kfifo_free(&q->fifo);
>> kfree(q);
>> }
>>
>> @@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>> goto fail_request_irq;
>> }
>>
>> - mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
>> + mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
>> if (!mq) {
>> ret = -ENOMEM;
>> goto fail_alloc_txq;
>> }
>> mbox->txq = mq;
>>
>> - mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
>> + mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
>> if (!mq) {
>> ret = -ENOMEM;
>> goto fail_alloc_rxq;
>> --
>> 1.6.3.3
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 5:56 ` Hiroshi DOYU
@ 2010-04-28 11:02 ` Ohad Ben-Cohen
0 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-28 11:02 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2
On Wed, Apr 28, 2010 at 8:56 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: [PATCH 4/4] omap: mailbox: convert block api to kfifo
> Date: Tue, 27 Apr 2010 19:56:22 +0200
>
>> The underlying buffering implementation of mailbox
>> is converted from block API to kfifo due to the simplicity
>> and speed of kfifo.
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>> ---
>> arch/arm/plat-omap/include/plat/mailbox.h | 5 +-
>> arch/arm/plat-omap/mailbox.c | 108 +++++++++++++----------------
>> 2 files changed, 52 insertions(+), 61 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
>> index 729166b..014cc58 100644
>> --- a/arch/arm/plat-omap/include/plat/mailbox.h
>> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
>> @@ -7,6 +7,7 @@
>> #include <linux/workqueue.h>
>> #include <linux/blkdev.h>
>
> Also the above is not necessary?
Good point. It will be removed.
Thanks,
Ohad.
>
>> #include <linux/interrupt.h>
>> +#include <linux/kfifo.h>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 11:02 ` Ohad Ben-Cohen
@ 2010-04-28 11:16 ` Hiroshi DOYU
2010-04-28 11:25 ` Ohad Ben-Cohen
0 siblings, 1 reply; 20+ messages in thread
From: Hiroshi DOYU @ 2010-04-28 11:16 UTC (permalink / raw)
To: ohad; +Cc: linux-omap, h-kanigeri2
From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Wed, 28 Apr 2010 13:02:06 +0200
>>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>>> index 72b17ad..b1324f3 100644
>>> --- a/arch/arm/plat-omap/mailbox.c
>>> +++ b/arch/arm/plat-omap/mailbox.c
>>> @@ -26,6 +26,7 @@
>>> #include <linux/device.h>
>>> #include <linux/delay.h>
>>> #include <linux/slab.h>
>>> +#include <linux/kfifo.h>
>>>
>>> #include <plat/mailbox.h>
>>>
>>> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
>>> /*
>>> * message sender
>>> */
>>> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
>>> {
>>> int ret = 0, i = 1000;
>>>
>>> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>> return -1;
>>> udelay(1);
>>> }
>>> - mbox_fifo_write(mbox, msg);
>>> return ret;
>>> }
>>>
>>> -
>>> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>> {
>>> + struct omap_mbox_queue *mq = mbox->txq;
>>> + int ret = 0, len;
>>>
>>> - struct request *rq;
>>> - struct request_queue *q = mbox->txq->queue;
>>> + spin_lock(&mq->lock);
>>>
>>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>>> - if (unlikely(!rq))
>>> - return -ENOMEM;
>>> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>> + ret = -ENOMEM;
>>> + goto out;
>>> + }
>>> +
>>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>>> + if (unlikely(len != sizeof(msg))) {
>>> + pr_err("%s: kfifo_in anomaly\n", __func__);
>>> + ret = -ENOMEM;
>>> + }
>>>
>>> - blk_insert_request(q, rq, 0, (void *) msg);
>>> tasklet_schedule(&mbox->txq->tasklet);
>>>
>>> - return 0;
>>> +out:
>>> + spin_unlock(&mq->lock);
>>> + return ret;
>>> }
>>> EXPORT_SYMBOL(omap_mbox_msg_send);
>>>
>>> static void mbox_tx_tasklet(unsigned long tx_data)
>>> {
>>> - int ret;
>>> - struct request *rq;
>>> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
>>> - struct request_queue *q = mbox->txq->queue;
>>> -
>>> - while (1) {
>>> -
>>> - rq = blk_fetch_request(q);
>>> -
>>> - if (!rq)
>>> - break;
>>> + struct omap_mbox_queue *mq = mbox->txq;
>>> + mbox_msg_t msg;
>>> + int ret;
>>>
>>> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
>>> - if (ret) {
>>> + while (kfifo_len(&mq->fifo)) {
>>> + if (__mbox_poll_for_space(mbox)) {
>>> omap_mbox_enable_irq(mbox, IRQ_TX);
>>> - blk_requeue_request(q, rq);
>>> - return;
>>> + break;
>>> }
>>> - blk_end_request_all(rq, 0);
>>> +
>>> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
>>> + sizeof(msg));
>>> + if (unlikely(ret != sizeof(msg)))
>>> + pr_err("%s: kfifo_out anomaly\n", __func__);
>>
>> No error recovery? same for other anomalies.
>
> I thought about it too, but eventually I think it doesn't really make
> a lot of sense:
> The only reason that kfifo_out/kfifo_in can fail here is if there's
> not enough data/space (respectively).
> Since we are checking for the availability of data/space before calling
> kfifo_out/kfifo_in, it cannot really fail.
> If it does fail, that's a critical bug that we cannot really recover from.
> Only reasonable explanation can be a bug in the code (either ours or kfifo's),
> and with such a bug it really feels futile to put some recovery.
> So maybe we should really make this a WARN_ON.
> What do you think ?
Makes sense. What about BUG_ON if it shouldn't happen theoretically?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 11:16 ` Hiroshi DOYU
@ 2010-04-28 11:25 ` Ohad Ben-Cohen
2010-04-28 11:52 ` Hiroshi DOYU
0 siblings, 1 reply; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-28 11:25 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2
On Wed, Apr 28, 2010 at 2:16 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
> Date: Wed, 28 Apr 2010 13:02:06 +0200
>
>>>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>>>> index 72b17ad..b1324f3 100644
>>>> --- a/arch/arm/plat-omap/mailbox.c
>>>> +++ b/arch/arm/plat-omap/mailbox.c
>>>> @@ -26,6 +26,7 @@
>>>> #include <linux/device.h>
>>>> #include <linux/delay.h>
>>>> #include <linux/slab.h>
>>>> +#include <linux/kfifo.h>
>>>>
>>>> #include <plat/mailbox.h>
>>>>
>>>> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
>>>> /*
>>>> * message sender
>>>> */
>>>> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
>>>> {
>>>> int ret = 0, i = 1000;
>>>>
>>>> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>> return -1;
>>>> udelay(1);
>>>> }
>>>> - mbox_fifo_write(mbox, msg);
>>>> return ret;
>>>> }
>>>>
>>>> -
>>>> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>> {
>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>> + int ret = 0, len;
>>>>
>>>> - struct request *rq;
>>>> - struct request_queue *q = mbox->txq->queue;
>>>> + spin_lock(&mq->lock);
>>>>
>>>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>>>> - if (unlikely(!rq))
>>>> - return -ENOMEM;
>>>> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>>> + ret = -ENOMEM;
>>>> + goto out;
>>>> + }
>>>> +
>>>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>>>> + if (unlikely(len != sizeof(msg))) {
>>>> + pr_err("%s: kfifo_in anomaly\n", __func__);
>>>> + ret = -ENOMEM;
>>>> + }
>>>>
>>>> - blk_insert_request(q, rq, 0, (void *) msg);
>>>> tasklet_schedule(&mbox->txq->tasklet);
>>>>
>>>> - return 0;
>>>> +out:
>>>> + spin_unlock(&mq->lock);
>>>> + return ret;
>>>> }
>>>> EXPORT_SYMBOL(omap_mbox_msg_send);
>>>>
>>>> static void mbox_tx_tasklet(unsigned long tx_data)
>>>> {
>>>> - int ret;
>>>> - struct request *rq;
>>>> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
>>>> - struct request_queue *q = mbox->txq->queue;
>>>> -
>>>> - while (1) {
>>>> -
>>>> - rq = blk_fetch_request(q);
>>>> -
>>>> - if (!rq)
>>>> - break;
>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>> + mbox_msg_t msg;
>>>> + int ret;
>>>>
>>>> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
>>>> - if (ret) {
>>>> + while (kfifo_len(&mq->fifo)) {
>>>> + if (__mbox_poll_for_space(mbox)) {
>>>> omap_mbox_enable_irq(mbox, IRQ_TX);
>>>> - blk_requeue_request(q, rq);
>>>> - return;
>>>> + break;
>>>> }
>>>> - blk_end_request_all(rq, 0);
>>>> +
>>>> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
>>>> + sizeof(msg));
>>>> + if (unlikely(ret != sizeof(msg)))
>>>> + pr_err("%s: kfifo_out anomaly\n", __func__);
>>>
>>> No error recovery? same for other anomalies.
>>
>> I thought about it too, but eventually I think it doesn't really make
>> a lot of sense:
>> The only reason that kfifo_out/kfifo_in can fail here is if there's
>> not enough data/space (respectively).
>> Since we are checking for the availability of data/space before calling
>> kfifo_out/kfifo_in, it cannot really fail.
>> If it does fail, that's a critical bug that we cannot really recover from.
>> Only reasonable explanation can be a bug in the code (either ours or kfifo's),
>> and with such a bug it really feels futile to put some recovery.
>> So maybe we should really make this a WARN_ON.
>> What do you think ?
>
> Makes sense. What about BUG_ON if it shouldn't happen theoretically?
I'm not sure this bug is critical enough to panic and enforce the user
to reboot immediately - he can probably still do a clean shutdown here.
Thanks,
Ohad.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 11:25 ` Ohad Ben-Cohen
@ 2010-04-28 11:52 ` Hiroshi DOYU
2010-04-28 12:03 ` Ohad Ben-Cohen
0 siblings, 1 reply; 20+ messages in thread
From: Hiroshi DOYU @ 2010-04-28 11:52 UTC (permalink / raw)
To: ohad; +Cc: linux-omap, h-kanigeri2
From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
Date: Wed, 28 Apr 2010 13:25:41 +0200
> On Wed, Apr 28, 2010 at 2:16 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> From: ext Ohad Ben-Cohen <ohad@wizery.com>
>> Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
>> Date: Wed, 28 Apr 2010 13:02:06 +0200
>>
>>>>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>>>>> index 72b17ad..b1324f3 100644
>>>>> --- a/arch/arm/plat-omap/mailbox.c
>>>>> +++ b/arch/arm/plat-omap/mailbox.c
>>>>> @@ -26,6 +26,7 @@
>>>>> #include <linux/device.h>
>>>>> #include <linux/delay.h>
>>>>> #include <linux/slab.h>
>>>>> +#include <linux/kfifo.h>
>>>>>
>>>>> #include <plat/mailbox.h>
>>>>>
>>>>> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
>>>>> /*
>>>>> * message sender
>>>>> */
>>>>> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
>>>>> {
>>>>> int ret = 0, i = 1000;
>>>>>
>>>>> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>> return -1;
>>>>> udelay(1);
>>>>> }
>>>>> - mbox_fifo_write(mbox, msg);
>>>>> return ret;
>>>>> }
>>>>>
>>>>> -
>>>>> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>> {
>>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>>> + int ret = 0, len;
>>>>>
>>>>> - struct request *rq;
>>>>> - struct request_queue *q = mbox->txq->queue;
>>>>> + spin_lock(&mq->lock);
>>>>>
>>>>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>>>>> - if (unlikely(!rq))
>>>>> - return -ENOMEM;
>>>>> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>>>> + ret = -ENOMEM;
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>>>>> + if (unlikely(len != sizeof(msg))) {
>>>>> + pr_err("%s: kfifo_in anomaly\n", __func__);
>>>>> + ret = -ENOMEM;
>>>>> + }
>>>>>
>>>>> - blk_insert_request(q, rq, 0, (void *) msg);
>>>>> tasklet_schedule(&mbox->txq->tasklet);
>>>>>
>>>>> - return 0;
>>>>> +out:
>>>>> + spin_unlock(&mq->lock);
>>>>> + return ret;
>>>>> }
>>>>> EXPORT_SYMBOL(omap_mbox_msg_send);
>>>>>
>>>>> static void mbox_tx_tasklet(unsigned long tx_data)
>>>>> {
>>>>> - int ret;
>>>>> - struct request *rq;
>>>>> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
>>>>> - struct request_queue *q = mbox->txq->queue;
>>>>> -
>>>>> - while (1) {
>>>>> -
>>>>> - rq = blk_fetch_request(q);
>>>>> -
>>>>> - if (!rq)
>>>>> - break;
>>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>>> + mbox_msg_t msg;
>>>>> + int ret;
>>>>>
>>>>> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
>>>>> - if (ret) {
>>>>> + while (kfifo_len(&mq->fifo)) {
>>>>> + if (__mbox_poll_for_space(mbox)) {
>>>>> omap_mbox_enable_irq(mbox, IRQ_TX);
>>>>> - blk_requeue_request(q, rq);
>>>>> - return;
>>>>> + break;
>>>>> }
>>>>> - blk_end_request_all(rq, 0);
>>>>> +
>>>>> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
>>>>> + sizeof(msg));
>>>>> + if (unlikely(ret != sizeof(msg)))
>>>>> + pr_err("%s: kfifo_out anomaly\n", __func__);
>>>>
>>>> No error recovery? same for other anomalies.
>>>
>>> I thought about it too, but eventually I think it doesn't really make
>>> a lot of sense:
>>> The only reason that kfifo_out/kfifo_in can fail here is if there's
>>> not enough data/space (respectively).
>>> Since we are checking for the availability of data/space before calling
>>> kfifo_out/kfifo_in, it cannot really fail.
>>> If it does fail, that's a critical bug that we cannot really recover from.
>>> Only reasonable explanation can be a bug in the code (either ours or kfifo's),
>>> and with such a bug it really feels futile to put some recovery.
>>> So maybe we should really make this a WARN_ON.
>>> What do you think ?
>>
>> Makes sense. What about BUG_ON if it shouldn't happen theoretically?
>
> I'm not sure this bug is critical enough to panic and enforce the user
> to reboot immediately - he can probably still do a clean shutdown here.
I agree that WARN_ON would be enough for this case. I just thought of
the rareness of this triggering.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-04-28 11:52 ` Hiroshi DOYU
@ 2010-04-28 12:03 ` Ohad Ben-Cohen
0 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-28 12:03 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, h-kanigeri2
On Wed, Apr 28, 2010 at 2:52 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
> Date: Wed, 28 Apr 2010 13:25:41 +0200
>
>> On Wed, Apr 28, 2010 at 2:16 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>>> From: ext Ohad Ben-Cohen <ohad@wizery.com>
>>> Subject: Re: [PATCH 4/4] omap: mailbox: convert block api to kfifo
>>> Date: Wed, 28 Apr 2010 13:02:06 +0200
>>>
>>>>>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>>>>>> index 72b17ad..b1324f3 100644
>>>>>> --- a/arch/arm/plat-omap/mailbox.c
>>>>>> +++ b/arch/arm/plat-omap/mailbox.c
>>>>>> @@ -26,6 +26,7 @@
>>>>>> #include <linux/device.h>
>>>>>> #include <linux/delay.h>
>>>>>> #include <linux/slab.h>
>>>>>> +#include <linux/kfifo.h>
>>>>>>
>>>>>> #include <plat/mailbox.h>
>>>>>>
>>>>>> @@ -67,7 +68,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
>>>>>> /*
>>>>>> * message sender
>>>>>> */
>>>>>> -static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>>> +static int __mbox_poll_for_space(struct omap_mbox *mbox)
>>>>>> {
>>>>>> int ret = 0, i = 1000;
>>>>>>
>>>>>> @@ -78,49 +79,54 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>>> return -1;
>>>>>> udelay(1);
>>>>>> }
>>>>>> - mbox_fifo_write(mbox, msg);
>>>>>> return ret;
>>>>>> }
>>>>>>
>>>>>> -
>>>>>> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
>>>>>> {
>>>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>>>> + int ret = 0, len;
>>>>>>
>>>>>> - struct request *rq;
>>>>>> - struct request_queue *q = mbox->txq->queue;
>>>>>> + spin_lock(&mq->lock);
>>>>>>
>>>>>> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>>>>>> - if (unlikely(!rq))
>>>>>> - return -ENOMEM;
>>>>>> + if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
>>>>>> + ret = -ENOMEM;
>>>>>> + goto out;
>>>>>> + }
>>>>>> +
>>>>>> + len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
>>>>>> + if (unlikely(len != sizeof(msg))) {
>>>>>> + pr_err("%s: kfifo_in anomaly\n", __func__);
>>>>>> + ret = -ENOMEM;
>>>>>> + }
>>>>>>
>>>>>> - blk_insert_request(q, rq, 0, (void *) msg);
>>>>>> tasklet_schedule(&mbox->txq->tasklet);
>>>>>>
>>>>>> - return 0;
>>>>>> +out:
>>>>>> + spin_unlock(&mq->lock);
>>>>>> + return ret;
>>>>>> }
>>>>>> EXPORT_SYMBOL(omap_mbox_msg_send);
>>>>>>
>>>>>> static void mbox_tx_tasklet(unsigned long tx_data)
>>>>>> {
>>>>>> - int ret;
>>>>>> - struct request *rq;
>>>>>> struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
>>>>>> - struct request_queue *q = mbox->txq->queue;
>>>>>> -
>>>>>> - while (1) {
>>>>>> -
>>>>>> - rq = blk_fetch_request(q);
>>>>>> -
>>>>>> - if (!rq)
>>>>>> - break;
>>>>>> + struct omap_mbox_queue *mq = mbox->txq;
>>>>>> + mbox_msg_t msg;
>>>>>> + int ret;
>>>>>>
>>>>>> - ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
>>>>>> - if (ret) {
>>>>>> + while (kfifo_len(&mq->fifo)) {
>>>>>> + if (__mbox_poll_for_space(mbox)) {
>>>>>> omap_mbox_enable_irq(mbox, IRQ_TX);
>>>>>> - blk_requeue_request(q, rq);
>>>>>> - return;
>>>>>> + break;
>>>>>> }
>>>>>> - blk_end_request_all(rq, 0);
>>>>>> +
>>>>>> + ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
>>>>>> + sizeof(msg));
>>>>>> + if (unlikely(ret != sizeof(msg)))
>>>>>> + pr_err("%s: kfifo_out anomaly\n", __func__);
>>>>>
>>>>> No error recovery? same for other anomalies.
>>>>
>>>> I thought about it too, but eventually I think it doesn't really make
>>>> a lot of sense:
>>>> The only reason that kfifo_out/kfifo_in can fail here is if there's
>>>> not enough data/space (respectively).
>>>> Since we are checking for the availability of data/space before calling
>>>> kfifo_out/kfifo_in, it cannot really fail.
>>>> If it does fail, that's a critical bug that we cannot really recover from.
>>>> Only reasonable explanation can be a bug in the code (either ours or kfifo's),
>>>> and with such a bug it really feels futile to put some recovery.
>>>> So maybe we should really make this a WARN_ON.
>>>> What do you think ?
>>>
>>> Makes sense. What about BUG_ON if it shouldn't happen theoretically?
>>
>> I'm not sure this bug is critical enough to panic and enforce the user
>> to reboot immediately - he can probably still do a clean shutdown here.
>
> I agree that WARN_ON would be enough for this case. I just thought of
> the rareness of this triggering.
Yeah, I was thinking exactly the same, and wanted to put BUG_ON too initially,
but the combination of its calling panic and its header comment
convinced me otherwise:
/*
* Don't use BUG() or BUG_ON() unless there's really no way out; one
* example might be detecting data structure corruption in the middle
* of an operation that can't be backed out of. If the (sub)system
* can somehow continue operating, perhaps with reduced functionality,
* it's probably not BUG-worthy.
*
* If you're tempted to BUG(), think again: is completely giving up
* really the *only* solution? There are usually better options, where
* users don't need to reboot ASAP and can mostly shut down cleanly.
*/
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
2010-04-28 7:50 ` Hiroshi DOYU
@ 2010-04-29 12:44 ` Kanigeri, Hari
2010-04-29 13:14 ` Ohad Ben-Cohen
1 sibling, 1 reply; 20+ messages in thread
From: Kanigeri, Hari @ 2010-04-29 12:44 UTC (permalink / raw)
To: Ohad Ben-Cohen, linux-omap@vger.kernel.org; +Cc: Hiroshi Doyu
Ohad,
Looks like the conversion was missed in few places resulting in compile warnings.
Please see the below fix. Let me know if you agree with the change.
#########
[PATCH] omap: mailbox: rwlocks to spinlock: compilation fix
fixed the missed rwlocks in few places resultion in following
compiler warning.
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
arch/arm/plat-omap/mailbox.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 27a8d98..d6a700d 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
struct omap_mbox_queue *mq;
if (likely(mbox->ops->startup)) {
- write_lock(&mboxes_lock);
+ spin_lock(&mboxes_lock);
if (!mbox_configured)
ret = mbox->ops->startup(mbox);
if (unlikely(ret)) {
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
return ret;
}
mbox_configured++;
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
}
ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
@@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
free_irq(mbox->irq, mbox);
if (likely(mbox->ops->shutdown)) {
- write_lock(&mboxes_lock);
+ spin_lock(&mboxes_lock);
if (mbox_configured > 0)
mbox_configured--;
if (!mbox_configured)
mbox->ops->shutdown(mbox);
- write_unlock(&mboxes_lock);
+ spin_unlock(&mboxes_lock);
}
}
--
################
> -----Original Message-----
> From: Ohad Ben-Cohen [mailto:ohad@wizery.com]
> Sent: Tuesday, April 27, 2010 12:56 PM
> To: linux-omap@vger.kernel.org
> Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
> Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks
> to spinlock
>
> rwlocks are slower and have potential starvation issues so
> spinlocks are generally preferred
>
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> ---
> arch/arm/plat-omap/mailbox.c | 20 ++++++++++----------
> 1 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/plat-omap/mailbox.c
> b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -31,7 +31,7 @@
>
> static struct workqueue_struct *mboxd;
> static struct omap_mbox *mboxes;
> -static DEFINE_RWLOCK(mboxes_lock);
> +static DEFINE_SPINLOCK(mboxes_lock);
>
> static int mbox_configured;
>
> @@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const
> char *name)
> struct omap_mbox *mbox;
> int ret;
>
> - read_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
> mbox = *(find_mboxes(name));
> if (mbox == NULL) {
> - read_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> return ERR_PTR(-ENOENT);
> }
>
> - read_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>
> ret = omap_mbox_startup(mbox);
> if (ret)
> @@ -363,15 +363,15 @@ int omap_mbox_register(struct device
> *parent, struct omap_mbox *mbox)
> if (mbox->next)
> return -EBUSY;
>
> - write_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
> tmp = find_mboxes(mbox->name);
> if (*tmp) {
> ret = -EBUSY;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> goto err_find;
> }
> *tmp = mbox;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>
> return 0;
>
> @@ -384,18 +384,18 @@ int omap_mbox_unregister(struct
> omap_mbox *mbox) {
> struct omap_mbox **tmp;
>
> - write_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
> tmp = &mboxes;
> while (*tmp) {
> if (mbox == *tmp) {
> *tmp = mbox->next;
> mbox->next = NULL;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> return 0;
> }
> tmp = &(*tmp)->next;
> }
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
>
> return -EINVAL;
> }
> --
> 1.6.3.3
>
>
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-29 12:44 ` Kanigeri, Hari
@ 2010-04-29 13:14 ` Ohad Ben-Cohen
2010-04-29 13:35 ` Ohad Ben-Cohen
0 siblings, 1 reply; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-29 13:14 UTC (permalink / raw)
To: Kanigeri, Hari; +Cc: linux-omap@vger.kernel.org, Hiroshi Doyu
Hi Hari,
On Thu, Apr 29, 2010 at 3:44 PM, Kanigeri, Hari <h-kanigeri2@ti.com> wrote:
> Ohad,
>
> Looks like the conversion was missed in few places resulting in compile warnings.
Thanks for pointing that out - that's really strange, it somehow
slipped in the rebase.
The interesting part is that because you compile for OMAP4 (SMP),
you got to see the warnings, while I didn't because I compiled for OMAP3
(in the UP case there's no real locking and both types of locks are
actually the same).
>
> Please see the below fix. Let me know if you agree with the change.
Sure. I'll add that missing part back together with the review comments,
and submit a v2 patchset.
Thanks again,
Ohad.
>
> #########
> [PATCH] omap: mailbox: rwlocks to spinlock: compilation fix
>
> fixed the missed rwlocks in few places resultion in following
> compiler warning.
>
> arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
> arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
> arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
> arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
> arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
> arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
> arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
>
> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
> ---
> arch/arm/plat-omap/mailbox.c | 10 +++++-----
> 1 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 27a8d98..d6a700d 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> struct omap_mbox_queue *mq;
>
> if (likely(mbox->ops->startup)) {
> - write_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
> if (!mbox_configured)
> ret = mbox->ops->startup(mbox);
>
> if (unlikely(ret)) {
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> return ret;
> }
> mbox_configured++;
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> }
>
> ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
> @@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
> free_irq(mbox->irq, mbox);
>
> if (likely(mbox->ops->shutdown)) {
> - write_lock(&mboxes_lock);
> + spin_lock(&mboxes_lock);
> if (mbox_configured > 0)
> mbox_configured--;
> if (!mbox_configured)
> mbox->ops->shutdown(mbox);
> - write_unlock(&mboxes_lock);
> + spin_unlock(&mboxes_lock);
> }
> }
>
> --
>
> ################
>
>> -----Original Message-----
>> From: Ohad Ben-Cohen [mailto:ohad@wizery.com]
>> Sent: Tuesday, April 27, 2010 12:56 PM
>> To: linux-omap@vger.kernel.org
>> Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
>> Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks
>> to spinlock
>>
>> rwlocks are slower and have potential starvation issues so
>> spinlocks are generally preferred
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> ---
>> arch/arm/plat-omap/mailbox.c | 20 ++++++++++----------
>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/mailbox.c
>> b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -31,7 +31,7 @@
>>
>> static struct workqueue_struct *mboxd;
>> static struct omap_mbox *mboxes;
>> -static DEFINE_RWLOCK(mboxes_lock);
>> +static DEFINE_SPINLOCK(mboxes_lock);
>>
>> static int mbox_configured;
>>
>> @@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const
>> char *name)
>> struct omap_mbox *mbox;
>> int ret;
>>
>> - read_lock(&mboxes_lock);
>> + spin_lock(&mboxes_lock);
>> mbox = *(find_mboxes(name));
>> if (mbox == NULL) {
>> - read_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> return ERR_PTR(-ENOENT);
>> }
>>
>> - read_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>>
>> ret = omap_mbox_startup(mbox);
>> if (ret)
>> @@ -363,15 +363,15 @@ int omap_mbox_register(struct device
>> *parent, struct omap_mbox *mbox)
>> if (mbox->next)
>> return -EBUSY;
>>
>> - write_lock(&mboxes_lock);
>> + spin_lock(&mboxes_lock);
>> tmp = find_mboxes(mbox->name);
>> if (*tmp) {
>> ret = -EBUSY;
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> goto err_find;
>> }
>> *tmp = mbox;
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>>
>> return 0;
>>
>> @@ -384,18 +384,18 @@ int omap_mbox_unregister(struct
>> omap_mbox *mbox) {
>> struct omap_mbox **tmp;
>>
>> - write_lock(&mboxes_lock);
>> + spin_lock(&mboxes_lock);
>> tmp = &mboxes;
>> while (*tmp) {
>> if (mbox == *tmp) {
>> *tmp = mbox->next;
>> mbox->next = NULL;
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> return 0;
>> }
>> tmp = &(*tmp)->next;
>> }
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>>
>> return -EINVAL;
>> }
>> --
>> 1.6.3.3
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock
2010-04-29 13:14 ` Ohad Ben-Cohen
@ 2010-04-29 13:35 ` Ohad Ben-Cohen
0 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-04-29 13:35 UTC (permalink / raw)
To: Kanigeri, Hari; +Cc: linux-omap@vger.kernel.org, Hiroshi Doyu
On Thu, Apr 29, 2010 at 4:14 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> Hi Hari,
>
> On Thu, Apr 29, 2010 at 3:44 PM, Kanigeri, Hari <h-kanigeri2@ti.com> wrote:
>> Ohad,
>>
>> Looks like the conversion was missed in few places resulting in compile warnings.
>
> Thanks for pointing that out - that's really strange, it somehow
> slipped in the rebase.
>
> The interesting part is that because you compile for OMAP4 (SMP),
> you got to see the warnings, while I didn't because I compiled for OMAP3
> (in the UP case there's no real locking and both types of locks are
> actually the same).
>
>>
>> Please see the below fix. Let me know if you agree with the change.
>
> Sure. I'll add that missing part back together with the review comments,
> and submit a v2 patchset.
If anyone needs the complete patch now, the original can be found here:
http://dev.omapzoom.org/?p=tisyslink/kernel-syslink.git;a=commitdiff;h=8da0385a57cc470ee0a013b164fd3d2438455e79
Thanks,
Ohad.
>
> Thanks again,
> Ohad.
>
>
>>
>> #########
>> [PATCH] omap: mailbox: rwlocks to spinlock: compilation fix
>>
>> fixed the missed rwlocks in few places resultion in following
>> compiler warning.
>>
>> arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_startup':
>> arch/arm/plat-omap/mailbox.c:246: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
>> arch/arm/plat-omap/mailbox.c:251: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
>> arch/arm/plat-omap/mailbox.c:255: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
>> arch/arm/plat-omap/mailbox.c: In function 'omap_mbox_fini':
>> arch/arm/plat-omap/mailbox.c:301: warning: passing argument 1 of '_raw_write_lock' from incompatible pointer type
>> arch/arm/plat-omap/mailbox.c:306: warning: passing argument 1 of '_raw_write_unlock' from incompatible pointer type
>>
>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>> ---
>> arch/arm/plat-omap/mailbox.c | 10 +++++-----
>> 1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index 27a8d98..d6a700d 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -243,16 +243,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>> struct omap_mbox_queue *mq;
>>
>> if (likely(mbox->ops->startup)) {
>> - write_lock(&mboxes_lock);
>> + spin_lock(&mboxes_lock);
>> if (!mbox_configured)
>> ret = mbox->ops->startup(mbox);
>>
>> if (unlikely(ret)) {
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> return ret;
>> }
>> mbox_configured++;
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> }
>>
>> ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
>> @@ -298,12 +298,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>> free_irq(mbox->irq, mbox);
>>
>> if (likely(mbox->ops->shutdown)) {
>> - write_lock(&mboxes_lock);
>> + spin_lock(&mboxes_lock);
>> if (mbox_configured > 0)
>> mbox_configured--;
>> if (!mbox_configured)
>> mbox->ops->shutdown(mbox);
>> - write_unlock(&mboxes_lock);
>> + spin_unlock(&mboxes_lock);
>> }
>> }
>>
>> --
>>
>> ################
>>
>>> -----Original Message-----
>>> From: Ohad Ben-Cohen [mailto:ohad@wizery.com]
>>> Sent: Tuesday, April 27, 2010 12:56 PM
>>> To: linux-omap@vger.kernel.org
>>> Cc: Kanigeri, Hari; Hiroshi Doyu; Ohad Ben-Cohen
>>> Subject: [PATCH 1/4] omap: mailbox cleanup: convert rwlocks
>>> to spinlock
>>>
>>> rwlocks are slower and have potential starvation issues so
>>> spinlocks are generally preferred
>>>
>>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>>> ---
>>> arch/arm/plat-omap/mailbox.c | 20 ++++++++++----------
>>> 1 files changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/arch/arm/plat-omap/mailbox.c
>>> b/arch/arm/plat-omap/mailbox.c index 08a2df7..d73d51a 100644
>>> --- a/arch/arm/plat-omap/mailbox.c
>>> +++ b/arch/arm/plat-omap/mailbox.c
>>> @@ -31,7 +31,7 @@
>>>
>>> static struct workqueue_struct *mboxd;
>>> static struct omap_mbox *mboxes;
>>> -static DEFINE_RWLOCK(mboxes_lock);
>>> +static DEFINE_SPINLOCK(mboxes_lock);
>>>
>>> static int mbox_configured;
>>>
>>> @@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const
>>> char *name)
>>> struct omap_mbox *mbox;
>>> int ret;
>>>
>>> - read_lock(&mboxes_lock);
>>> + spin_lock(&mboxes_lock);
>>> mbox = *(find_mboxes(name));
>>> if (mbox == NULL) {
>>> - read_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>> return ERR_PTR(-ENOENT);
>>> }
>>>
>>> - read_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>>
>>> ret = omap_mbox_startup(mbox);
>>> if (ret)
>>> @@ -363,15 +363,15 @@ int omap_mbox_register(struct device
>>> *parent, struct omap_mbox *mbox)
>>> if (mbox->next)
>>> return -EBUSY;
>>>
>>> - write_lock(&mboxes_lock);
>>> + spin_lock(&mboxes_lock);
>>> tmp = find_mboxes(mbox->name);
>>> if (*tmp) {
>>> ret = -EBUSY;
>>> - write_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>> goto err_find;
>>> }
>>> *tmp = mbox;
>>> - write_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>>
>>> return 0;
>>>
>>> @@ -384,18 +384,18 @@ int omap_mbox_unregister(struct
>>> omap_mbox *mbox) {
>>> struct omap_mbox **tmp;
>>>
>>> - write_lock(&mboxes_lock);
>>> + spin_lock(&mboxes_lock);
>>> tmp = &mboxes;
>>> while (*tmp) {
>>> if (mbox == *tmp) {
>>> *tmp = mbox->next;
>>> mbox->next = NULL;
>>> - write_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>> return 0;
>>> }
>>> tmp = &(*tmp)->next;
>>> }
>>> - write_unlock(&mboxes_lock);
>>> + spin_unlock(&mboxes_lock);
>>>
>>> return -EINVAL;
>>> }
>>> --
>>> 1.6.3.3
>>>
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-05-03 9:41 [PATCH v2 " Ohad Ben-Cohen
@ 2010-05-03 10:27 ` Ohad Ben-Cohen
0 siblings, 0 replies; 20+ messages in thread
From: Ohad Ben-Cohen @ 2010-05-03 10:27 UTC (permalink / raw)
To: linux-omap; +Cc: Kanigeri Hari, Hiroshi Doyu, Ohad Ben-Cohen
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the mbox_kfifo_size module parameter).
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
if you want, you can also reach me at < ohadb at ti dot com >.
arch/arm/plat-omap/Kconfig | 9 +++
arch/arm/plat-omap/include/plat/mailbox.h | 4 +-
arch/arm/plat-omap/mailbox.c | 112 +++++++++++++---------------
3 files changed, 63 insertions(+), 62 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
Say Y here if you want to use OMAP Mailbox framework support for
DSP, IVA1.0 and IVA2 in OMAP1/2/3.
+config OMAP_MBOX_KFIFO_SIZE
+ int "Mailbox kfifo default buffer size (bytes)"
+ depends on OMAP_MBOX_FWK
+ default 256
+ help
+ Specify the default size of mailbox's kfifo buffers (bytes).
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
config OMAP_IOMMU
tristate
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
-#include <linux/blkdev.h>
#include <linux/interrupt.h>
+#include <linux/kfifo.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue *queue;
+ struct kfifo fifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5309213..8d1581f 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,11 +21,14 @@
*
*/
+#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/err.h>
#include <plat/mailbox.h>
@@ -35,6 +38,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo (bytes)");
+
/* Mailbox FIFO handle functions */
static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
{
@@ -67,7 +74,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +85,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox->txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox->txq->queue;
+ spin_lock(&mq->lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(&mbox->txq->tasklet);
- return 0;
+out:
+ spin_unlock(&mq->lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox->txq->queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox->txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
- if (ret) {
+ while (kfifo_len(&mq->fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
+ sizeof(msg));
+ WARN_ON(ret != sizeof(msg));
+
+ mbox_fifo_write(mbox, msg);
}
}
@@ -131,36 +139,21 @@ static void mbox_rx_work(struct work_struct *work)
{
struct omap_mbox_queue *mq =
container_of(work, struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
- struct request_queue *q = mbox->rxq->queue;
- struct request *rq;
mbox_msg_t msg;
- unsigned long flags;
+ int len;
- while (1) {
- spin_lock_irqsave(q->queue_lock, flags);
- rq = blk_fetch_request(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
- if (!rq)
- break;
+ while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+ len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- msg = (mbox_msg_t)rq->special;
- blk_end_request_all(rq, 0);
- mbox->rxq->callback((void *)msg);
+ if (mq->callback)
+ mq->callback((void *)msg);
}
}
/*
* Mailbox interrupt handler
*/
-static void mbox_txq_fn(struct request_queue *q)
-{
-}
-
-static void mbox_rxq_fn(struct request_queue *q)
-{
-}
-
static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
@@ -170,19 +163,19 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
{
- struct request *rq;
+ struct omap_mbox_queue *mq = mbox->rxq;
mbox_msg_t msg;
- struct request_queue *q = mbox->rxq->queue;
+ int len;
while (!mbox_fifo_empty(mbox)) {
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
+ if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
goto nomem;
msg = mbox_fifo_read(mbox);
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *)msg);
if (mbox->ops->type == OMAP_MBOX_TYPE1)
break;
}
@@ -207,11 +200,9 @@ static irqreturn_t mbox_interrupt(int irq, void *p)
}
static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
- request_fn_proc *proc,
void (*work) (struct work_struct *),
void (*tasklet)(unsigned long))
{
- struct request_queue *q;
struct omap_mbox_queue *mq;
mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
@@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
spin_lock_init(&mq->lock);
- q = blk_init_queue(proc, &mq->lock);
- if (!q)
+ if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
goto error;
- q->queuedata = mbox;
- mq->queue = q;
if (work)
INIT_WORK(&mq->work, work);
@@ -239,7 +227,7 @@ error:
static void mbox_queue_free(struct omap_mbox_queue *q)
{
- blk_cleanup_queue(q->queue);
+ kfifo_free(&q->fifo);
kfree(q);
}
@@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
+ mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_txq;
}
mbox->txq = mq;
- mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
+ mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
@@ -407,6 +395,10 @@ static int __init omap_mbox_init(void)
if (!mboxd)
return -ENOMEM;
+ /* kfifo size sanity check: alignment and minimal size */
+ mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
+ mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t));
+
return 0;
}
module_init(omap_mbox_init);
--
1.6.3.3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH 4/4] omap: mailbox: convert block api to kfifo
2010-05-10 9:16 [PATCH 1/4] omap: mailbox: convert rwlocks to spinlock Hiroshi DOYU
@ 2010-05-10 9:16 ` Hiroshi DOYU
0 siblings, 0 replies; 20+ messages in thread
From: Hiroshi DOYU @ 2010-05-10 9:16 UTC (permalink / raw)
To: linux-arm-kernel; +Cc: linux-omap, Ohad Ben-Cohen, Hari Kanigeri, Hiroshi DOYU
From: Ohad Ben-Cohen <ohad@wizery.com>
The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.
The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the mbox_kfifo_size module parameter).
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
arch/arm/plat-omap/Kconfig | 9 +++
arch/arm/plat-omap/include/plat/mailbox.h | 4 +-
arch/arm/plat-omap/mailbox.c | 112 +++++++++++++---------------
3 files changed, 63 insertions(+), 62 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
Say Y here if you want to use OMAP Mailbox framework support for
DSP, IVA1.0 and IVA2 in OMAP1/2/3.
+config OMAP_MBOX_KFIFO_SIZE
+ int "Mailbox kfifo default buffer size (bytes)"
+ depends on OMAP_MBOX_FWK
+ default 256
+ help
+ Specify the default size of mailbox's kfifo buffers (bytes).
+ This can also be changed at runtime (via the mbox_kfifo_size
+ module parameter).
+
config OMAP_IOMMU
tristate
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
-#include <linux/blkdev.h>
#include <linux/interrupt.h>
+#include <linux/kfifo.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
- struct request_queue *queue;
+ struct kfifo fifo;
struct work_struct work;
struct tasklet_struct tasklet;
int (*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 7c60550..908d9d2 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,11 +21,14 @@
*
*/
+#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/interrupt.h>
#include <linux/device.h>
#include <linux/delay.h>
#include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/err.h>
#include <plat/mailbox.h>
@@ -35,6 +38,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
static int mbox_configured;
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo (bytes)");
+
/* Mailbox FIFO handle functions */
static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
{
@@ -67,7 +74,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
/*
* message sender
*/
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
{
int ret = 0, i = 1000;
@@ -78,49 +85,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return -1;
udelay(1);
}
- mbox_fifo_write(mbox, msg);
return ret;
}
-
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
+ struct omap_mbox_queue *mq = mbox->txq;
+ int ret = 0, len;
- struct request *rq;
- struct request_queue *q = mbox->txq->queue;
+ spin_lock(&mq->lock);
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
- return -ENOMEM;
+ if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *) msg);
tasklet_schedule(&mbox->txq->tasklet);
- return 0;
+out:
+ spin_unlock(&mq->lock);
+ return ret;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_tasklet(unsigned long tx_data)
{
- int ret;
- struct request *rq;
struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
- struct request_queue *q = mbox->txq->queue;
-
- while (1) {
-
- rq = blk_fetch_request(q);
-
- if (!rq)
- break;
+ struct omap_mbox_queue *mq = mbox->txq;
+ mbox_msg_t msg;
+ int ret;
- ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
- if (ret) {
+ while (kfifo_len(&mq->fifo)) {
+ if (__mbox_poll_for_space(mbox)) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- blk_requeue_request(q, rq);
- return;
+ break;
}
- blk_end_request_all(rq, 0);
+
+ ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
+ sizeof(msg));
+ WARN_ON(ret != sizeof(msg));
+
+ mbox_fifo_write(mbox, msg);
}
}
@@ -131,36 +139,21 @@ static void mbox_rx_work(struct work_struct *work)
{
struct omap_mbox_queue *mq =
container_of(work, struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
- struct request_queue *q = mbox->rxq->queue;
- struct request *rq;
mbox_msg_t msg;
- unsigned long flags;
+ int len;
- while (1) {
- spin_lock_irqsave(q->queue_lock, flags);
- rq = blk_fetch_request(q);
- spin_unlock_irqrestore(q->queue_lock, flags);
- if (!rq)
- break;
+ while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+ len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- msg = (mbox_msg_t)rq->special;
- blk_end_request_all(rq, 0);
- mbox->rxq->callback((void *)msg);
+ if (mq->callback)
+ mq->callback((void *)msg);
}
}
/*
* Mailbox interrupt handler
*/
-static void mbox_txq_fn(struct request_queue *q)
-{
-}
-
-static void mbox_rxq_fn(struct request_queue *q)
-{
-}
-
static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
@@ -170,19 +163,19 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
{
- struct request *rq;
+ struct omap_mbox_queue *mq = mbox->rxq;
mbox_msg_t msg;
- struct request_queue *q = mbox->rxq->queue;
+ int len;
while (!mbox_fifo_empty(mbox)) {
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq))
+ if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
goto nomem;
msg = mbox_fifo_read(mbox);
+ len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+ WARN_ON(len != sizeof(msg));
- blk_insert_request(q, rq, 0, (void *)msg);
if (mbox->ops->type == OMAP_MBOX_TYPE1)
break;
}
@@ -207,11 +200,9 @@ static irqreturn_t mbox_interrupt(int irq, void *p)
}
static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
- request_fn_proc *proc,
void (*work) (struct work_struct *),
void (*tasklet)(unsigned long))
{
- struct request_queue *q;
struct omap_mbox_queue *mq;
mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
@@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
spin_lock_init(&mq->lock);
- q = blk_init_queue(proc, &mq->lock);
- if (!q)
+ if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
goto error;
- q->queuedata = mbox;
- mq->queue = q;
if (work)
INIT_WORK(&mq->work, work);
@@ -239,7 +227,7 @@ error:
static void mbox_queue_free(struct omap_mbox_queue *q)
{
- blk_cleanup_queue(q->queue);
+ kfifo_free(&q->fifo);
kfree(q);
}
@@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
+ mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_txq;
}
mbox->txq = mq;
- mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
+ mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
@@ -407,6 +395,10 @@ static int __init omap_mbox_init(void)
if (!mboxd)
return -ENOMEM;
+ /* kfifo size sanity check: alignment and minimal size */
+ mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
+ mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t));
+
return 0;
}
module_init(omap_mbox_init);
--
1.7.1.rc1
^ permalink raw reply related [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-05-10 9:17 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-04-27 17:56 [PATCH 0/4] omap: mailbox: cleanup & simplify Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 1/4] omap: mailbox cleanup: convert rwlocks to spinlock Ohad Ben-Cohen
2010-04-28 7:50 ` Hiroshi DOYU
2010-04-28 10:41 ` Ohad Ben-Cohen
2010-04-29 12:44 ` Kanigeri, Hari
2010-04-29 13:14 ` Ohad Ben-Cohen
2010-04-29 13:35 ` Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 3/4] omap: mailbox: fix reverse likeliness Ohad Ben-Cohen
2010-04-27 17:56 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Ohad Ben-Cohen
2010-04-28 5:52 ` Hiroshi DOYU
2010-04-28 11:02 ` Ohad Ben-Cohen
2010-04-28 11:16 ` Hiroshi DOYU
2010-04-28 11:25 ` Ohad Ben-Cohen
2010-04-28 11:52 ` Hiroshi DOYU
2010-04-28 12:03 ` Ohad Ben-Cohen
2010-04-28 5:56 ` Hiroshi DOYU
2010-04-28 11:02 ` Ohad Ben-Cohen
-- strict thread matches above, loose matches on Subject: below --
2010-05-03 9:41 [PATCH v2 " Ohad Ben-Cohen
2010-05-03 10:27 ` [PATCH " Ohad Ben-Cohen
2010-05-10 9:16 [PATCH 1/4] omap: mailbox: convert rwlocks to spinlock Hiroshi DOYU
2010-05-10 9:16 ` [PATCH 4/4] omap: mailbox: convert block api to kfifo Hiroshi DOYU
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).