Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo
Hi Rene, On Wed, Jun 9, 2010 at 8:38 AM, Guzman Lugo, Fernando fernando.l...@ti.com wrote: On Tue, Jun 8, 2010 at 7:16 PM, Sapiens, Rene rene.sapi...@ti.com wrote: In mbox_rx_work() you are removing the lines that enable back the mbox irq for the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in the case that the kfifo for Rx mailbox gets full. So I think that we need to enable it back as soon as there is space in this kfifo. Actually these irq on/off lines are not part of my patch; they are introduced by patch 05/10 on top of which my patches were rebased. Nevertheless I agree with you - the kfifo migration patch should not affect that irq on/off behavior. It's probably just a rebase gotcha. But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Fernando what do you think ? Yes, you are right Ohad. Only should be disable the new message interrupt of the mailbox which kfifo is full. Once Fernando's fix will get thru, we will be able to fix the rebase error that you pointed out. Unfortunately I will not have any email access in the next 3 weeks, and I was hoping maybe you could submit a fix for this once Fernando's fix is accepted ? I would really like us to fix this early in the days of 2.6.36, maybe even during the merge window. Thanks a lot, Ohad. regards, Fernando. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] omap: mailbox: convert block api to kfifo
Hi Ohad, Sure I will do it. Regards, Rene -Original Message- From: Ohad Ben-Cohen [mailto:o...@wizery.com] Sent: Tuesday, August 10, 2010 9:43 AM To: Guzman Lugo, Fernando; Sapiens, Rene Cc: Hiroshi DOYU; linux-omap@vger.kernel.org; linux-arm- ker...@lists.infradead.org; Kanigeri, Hari Subject: Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo Hi Rene, On Wed, Jun 9, 2010 at 8:38 AM, Guzman Lugo, Fernando fernando.l...@ti.com wrote: On Tue, Jun 8, 2010 at 7:16 PM, Sapiens, Rene rene.sapi...@ti.com wrote: In mbox_rx_work() you are removing the lines that enable back the mbox irq for the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in the case that the kfifo for Rx mailbox gets full. So I think that we need to enable it back as soon as there is space in this kfifo. Actually these irq on/off lines are not part of my patch; they are introduced by patch 05/10 on top of which my patches were rebased. Nevertheless I agree with you - the kfifo migration patch should not affect that irq on/off behavior. It's probably just a rebase gotcha. But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Fernando what do you think ? Yes, you are right Ohad. Only should be disable the new message interrupt of the mailbox which kfifo is full. Once Fernando's fix will get thru, we will be able to fix the rebase error that you pointed out. Unfortunately I will not have any email access in the next 3 weeks, and I was hoping maybe you could submit a fix for this once Fernando's fix is accepted ? I would really like us to fix this early in the days of 2.6.36, maybe even during the merge window. Thanks a lot, Ohad. regards, Fernando. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo
On Tue, Aug 10, 2010 at 6:00 PM, Sapiens, Rene rene.sapi...@ti.com wrote: Hi Ohad, Sure I will do it. Thanks a lot, Rene ! Ohad. Regards, Rene -Original Message- From: Ohad Ben-Cohen [mailto:o...@wizery.com] Sent: Tuesday, August 10, 2010 9:43 AM To: Guzman Lugo, Fernando; Sapiens, Rene Cc: Hiroshi DOYU; linux-omap@vger.kernel.org; linux-arm- ker...@lists.infradead.org; Kanigeri, Hari Subject: Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo Hi Rene, On Wed, Jun 9, 2010 at 8:38 AM, Guzman Lugo, Fernando fernando.l...@ti.com wrote: On Tue, Jun 8, 2010 at 7:16 PM, Sapiens, Rene rene.sapi...@ti.com wrote: In mbox_rx_work() you are removing the lines that enable back the mbox irq for the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in the case that the kfifo for Rx mailbox gets full. So I think that we need to enable it back as soon as there is space in this kfifo. Actually these irq on/off lines are not part of my patch; they are introduced by patch 05/10 on top of which my patches were rebased. Nevertheless I agree with you - the kfifo migration patch should not affect that irq on/off behavior. It's probably just a rebase gotcha. But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Fernando what do you think ? Yes, you are right Ohad. Only should be disable the new message interrupt of the mailbox which kfifo is full. Once Fernando's fix will get thru, we will be able to fix the rebase error that you pointed out. Unfortunately I will not have any email access in the next 3 weeks, and I was hoping maybe you could submit a fix for this once Fernando's fix is accepted ? I would really like us to fix this early in the days of 2.6.36, maybe even during the merge window. Thanks a lot, Ohad. regards, Fernando. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo
On Wed, Jun 9, 2010 at 12:38 AM, Guzman Lugo, Fernando fernando.l...@ti.com wrote: From: Ohad Ben-Cohen [o...@wizery.com] But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Yes, you are right Ohad. Only should be disable the new message interrupt of the mailbox which kfifo is full. Great! do you plan to submit a fix to patch 05/10 then ? Thanks, Ohad. regards, Fernando. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] omap: mailbox: convert block api to kfifo
Hi Ohad,
In mbox_rx_work() you are removing the lines that enable back the mbox irq for
the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in
the case that the kfifo for Rx mailbox gets full. So I think that we need to
enable it back as soon as there is space in this kfifo.
Regards,
Rene
-Original Message-
From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
ow...@vger.kernel.org] On Behalf Of Hiroshi DOYU
Sent: Thursday, June 03, 2010 1:34 AM
To: linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org
Cc: Ohad Ben-Cohen; Kanigeri, Hari; Hiroshi DOYU
Subject: [PATCH 09/10] omap: mailbox: convert block api to kfifo
From: Ohad Ben-Cohen o...@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 o...@wizery.com
Signed-off-by: Hari Kanigeri h-kanige...@ti.com
Signed-off-by: Hiroshi DOYU hiroshi.d...@nokia.com
---
arch/arm/plat-omap/Kconfig|9 ++
arch/arm/plat-omap/include/plat/mailbox.h |4 +-
arch/arm/plat-omap/mailbox.c | 119 +---
-
3 files changed, 64 insertions(+), 68 deletions(-)
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 78b49a6..111d39a 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 kfifofifo;
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 81076b5..ec0e159 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
@@ -37,6 +40,10 @@ static bool rq_full;
static int mbox_configured;
static DEFINE_MUTEX(mbox_configured_lock);
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, Size of omap's mailbox kfifo
(bytes));
+
/* Mailbox FIFO handle functions */
static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
{
@@ -69,7 +76,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;
@@ -80,49 +87,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);
+
Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo
Hi Rene, On Tue, Jun 8, 2010 at 7:16 PM, Sapiens, Rene rene.sapi...@ti.com wrote: In mbox_rx_work() you are removing the lines that enable back the mbox irq for the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in the case that the kfifo for Rx mailbox gets full. So I think that we need to enable it back as soon as there is space in this kfifo. Actually these irq on/off lines are not part of my patch; they are introduced by patch 05/10 on top of which my patches were rebased. Nevertheless I agree with you - the kfifo migration patch should not affect that irq on/off behavior. It's probably just a rebase gotcha. But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Fernando what do you think ? Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 09/10] omap: mailbox: convert block api to kfifo
Hi Ohad, From: Ohad Ben-Cohen [o...@wizery.com] Sent: Tuesday, June 08, 2010 9:35 PM To: Sapiens, Rene; Guzman Lugo, Fernando Cc: Hiroshi DOYU; linux-omap@vger.kernel.org; linux-arm-ker...@lists.infradead.org; Kanigeri, Hari Subject: Re: [PATCH 09/10] omap: mailbox: convert block api to kfifo Hi Rene, On Tue, Jun 8, 2010 at 7:16 PM, Sapiens, Rene rene.sapi...@ti.com wrote: In mbox_rx_work() you are removing the lines that enable back the mbox irq for the RX case, but inside __mbox_rx_interrupt() this interrupt is disabled in the case that the kfifo for Rx mailbox gets full. So I think that we need to enable it back as soon as there is space in this kfifo. Actually these irq on/off lines are not part of my patch; they are introduced by patch 05/10 on top of which my patches were rebased. Nevertheless I agree with you - the kfifo migration patch should not affect that irq on/off behavior. It's probably just a rebase gotcha. But now that you point me to this irq on/off thing, it looks a bit broken in terms of multiple concurrent mbox support since it relies on a global rq_full state. I guess it'd be better to hold that rq_full state in the relevant mbox queue state itself. Fernando what do you think ? Yes, you are right Ohad. Only should be disable the new message interrupt of the mailbox which kfifo is full. regards, Fernando. Thanks, Ohad. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
