* [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
@ 2009-09-04 11:48 C.A, Subramaniam
2009-09-07 8:49 ` Hiroshi DOYU
0 siblings, 1 reply; 12+ messages in thread
From: C.A, Subramaniam @ 2009-09-04 11:48 UTC (permalink / raw)
To: linux-omap; +Cc: tony, Hiroshi.DOYU, rmk, Kanigeri, Hari, Gupta, Ramesh
>From e759173e77f73fabce5177b4f685df20b16c6922 Mon Sep 17 00:00:00 2001
From: C A Subramaniam <subramaniam.ca@ti.com>
Date: Thu, 3 Sep 2009 17:42:35 +0530
Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet
implementation
This patch uses a tasklet implementation for
sending mailbox messages.
Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
---
arch/arm/mach-omap2/mailbox.c | 43 ++++++++++-----
arch/arm/plat-omap/include/mach/mailbox.h | 16 +++++-
arch/arm/plat-omap/mailbox.c | 80 ++++++++++++++---------------
3 files changed, 81 insertions(+), 58 deletions(-)
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 52a1670..45aea98 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -236,6 +237,17 @@ static struct omap_mbox_ops omap2_mbox_ops = {
.restore_ctx = omap2_mbox_restore_ctx,
};
+
+static struct omap_mbox_task omap_mbox_1_tasklet = {
+ .arg = NULL,
+};
+
+static struct omap_mbox_task omap_mbox_2_tasklet = {
+ .arg = NULL,
+};
+
+
+
/*
* MAILBOX 0: ARM -> DSP,
* MAILBOX 1: ARM <- DSP.
@@ -272,6 +284,7 @@ struct omap_mbox mbox_1_info = {
.name = "mailbox-1",
.ops = &omap2_mbox_ops,
.priv = &omap2_mbox_1_priv,
+ .tx_tasklet = &omap_mbox_1_tasklet,
};
EXPORT_SYMBOL(mbox_1_info);
#else
@@ -305,8 +318,10 @@ struct omap_mbox mbox_2_info = {
.name = "mailbox-2",
.ops = &omap2_mbox_ops,
.priv = &omap2_mbox_2_priv,
+ .tx_tasklet = &omap_mbox_2_tasklet,
};
EXPORT_SYMBOL(mbox_2_info);
+
#endif
diff --git a/arch/arm/plat-omap/include/mach/mailbox.h b/arch/arm/plat-omap/include/mach/mailbox.h
index bf06953..5271345 100644
--- a/arch/arm/plat-omap/include/mach/mailbox.h
+++ b/arch/arm/plat-omap/include/mach/mailbox.h
@@ -28,8 +28,10 @@ struct omap_mbox_ops {
int (*fifo_empty)(struct omap_mbox *mbox);
int (*fifo_full)(struct omap_mbox *mbox);
/* irq */
- void (*enable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
- void (*disable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+ void (*enable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
+ void (*disable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
void (*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
int (*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
/* ctx */
@@ -45,12 +47,22 @@ struct omap_mbox_queue {
struct omap_mbox *mbox;
};
+struct omap_mbox_task{
+ spinlock_t lock;
+ struct tasklet_struct *t;
+ mbox_msg_t msg;
+ void *arg;
+};
+
+
struct omap_mbox {
char *name;
unsigned int irq;
struct omap_mbox_queue *txq, *rxq;
+ struct omap_mbox_task *tx_tasklet;
+
struct omap_mbox_ops *ops;
mbox_msg_t seq_snd, seq_rcv;
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 84cf6af..b5e53d4 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -63,60 +63,49 @@ 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 void __mbox_msg_send(unsigned long tx)
{
+ struct omap_mbox_task *tx_data = (struct omap_mbox_task *)tx;
+ struct omap_mbox *mbox = (struct omap_mbox *)tx_data->arg;
+ mbox_msg_t msg = tx_data->msg;
int ret = 0, i = 1000;
while (mbox_fifo_full(mbox)) {
- if (mbox->ops->type == OMAP_MBOX_TYPE2)
- return -1;
- if (--i == 0)
- return -1;
+ if (mbox->ops->type == OMAP_MBOX_TYPE2) {
+ printk(KERN_ERR "Invalid mailbox type\n");
+ return ;
+ }
+ if (--i == 0) {
+ printk(KERN_ERR "Time out writing to mailbox\n");
+ return ;
+ }
udelay(1);
}
mbox_fifo_write(mbox, msg);
- return ret;
-}
-
-struct omap_msg_tx_data {
- mbox_msg_t msg;
- void *arg;
-};
+ tx_data->arg = NULL;
+ spin_unlock(&(mbox->tx_tasklet->lock));
-static void omap_msg_tx_end_io(struct request *rq, int error)
-{
- kfree(rq->special);
- __blk_put_request(rq->q, rq);
+ return;
}
+
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
- struct omap_msg_tx_data *tx_data;
- struct request *rq;
- struct request_queue *q = mbox->txq->queue;
+ struct omap_mbox_task *tx_task = mbox->tx_tasklet;
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (unlikely(!tx_data))
- return -ENOMEM;
-
- rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq)) {
- kfree(tx_data);
- return -ENOMEM;
- }
+ spin_lock(&(mbox->tx_tasklet->lock));
+ tx_task->arg = (void *)mbox;
+ tx_task->msg = msg;
- tx_data->msg = msg;
- rq->end_io = omap_msg_tx_end_io;
- blk_insert_request(q, rq, 0, tx_data);
+ tasklet_schedule(tx_task->t);
- schedule_work(&mbox->txq->work);
return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
static void mbox_tx_work(struct work_struct *work)
{
- int ret;
+ int ret = 0;
struct request *rq;
struct omap_mbox_queue *mq = container_of(work,
struct omap_mbox_queue, work);
@@ -124,8 +113,6 @@ static void mbox_tx_work(struct work_struct *work)
struct request_queue *q = mbox->txq->queue;
while (1) {
- struct omap_msg_tx_data *tx_data;
-
spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
spin_unlock(q->queue_lock);
@@ -133,9 +120,6 @@ static void mbox_tx_work(struct work_struct *work)
if (!rq)
break;
- tx_data = rq->special;
-
- ret = __mbox_msg_send(mbox, tx_data->msg);
if (ret) {
omap_mbox_enable_irq(mbox, IRQ_TX);
spin_lock(q->queue_lock);
@@ -206,7 +190,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
goto nomem;
msg = mbox_fifo_read(mbox);
-
+ rq->special = (void *)msg;
blk_insert_request(q, rq, 0, (void *)msg);
if (mbox->ops->type == OMAP_MBOX_TYPE1)
@@ -298,13 +282,25 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
}
mbox->rxq = mq;
+ mbox->tx_tasklet->t = kzalloc(sizeof(struct tasklet_struct),
+ GFP_KERNEL);
+ if (!mbox->tx_tasklet->t) {
+ ret = -ENOMEM;
+ goto fail_alloc_tasklet;
+ }
+ spin_lock_init(&mbox->tx_tasklet->lock);
+ tasklet_init(mbox->tx_tasklet->t, __mbox_msg_send,
+ (unsigned long)mbox->tx_tasklet);
+
return 0;
- fail_alloc_rxq:
+fail_alloc_tasklet:
+ mbox_queue_free(mbox->rxq);
+fail_alloc_rxq:
mbox_queue_free(mbox->txq);
- fail_alloc_txq:
+fail_alloc_txq:
free_irq(mbox->irq, mbox);
- fail_request_irq:
+fail_request_irq:
if (unlikely(mbox->ops->shutdown))
mbox->ops->shutdown(mbox);
--
1.5.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-04 11:48 [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation C.A, Subramaniam
@ 2009-09-07 8:49 ` Hiroshi DOYU
2009-09-08 17:43 ` C.A, Subramaniam
0 siblings, 1 reply; 12+ messages in thread
From: Hiroshi DOYU @ 2009-09-07 8:49 UTC (permalink / raw)
To: subramaniam.ca; +Cc: linux-omap, tony, rmk, h-kanigeri2, grgupta
From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
Date: Fri, 4 Sep 2009 13:48:45 +0200
> From e759173e77f73fabce5177b4f685df20b16c6922 Mon Sep 17 00:00:00 2001
> From: C A Subramaniam <subramaniam.ca@ti.com>
> Date: Thu, 3 Sep 2009 17:42:35 +0530
> Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet
> implementation
>
> This patch uses a tasklet implementation for
> sending mailbox messages.
>
> Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
> Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
> ---
> arch/arm/mach-omap2/mailbox.c | 43 ++++++++++-----
> arch/arm/plat-omap/include/mach/mailbox.h | 16 +++++-
> arch/arm/plat-omap/mailbox.c | 80 ++++++++++++++---------------
> 3 files changed, 81 insertions(+), 58 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index 52a1670..45aea98 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -236,6 +237,17 @@ static struct omap_mbox_ops omap2_mbox_ops = {
> .restore_ctx = omap2_mbox_restore_ctx,
> };
>
> +
> +static struct omap_mbox_task omap_mbox_1_tasklet = {
> + .arg = NULL,
> +};
> +
> +static struct omap_mbox_task omap_mbox_2_tasklet = {
> + .arg = NULL,
> +};
As I described in the comment on [PATCH 8/10], I think that it would
be better to keep "mach-omap2/mailbox.c" simple and to add some
additional logic on "plat-omap/mailbox.c".
Would it be possbile to move this tasklet implementation to
"plat-omap/mailbox.c"?
> +
> +
> +
> /*
> * MAILBOX 0: ARM -> DSP,
> * MAILBOX 1: ARM <- DSP.
> @@ -272,6 +284,7 @@ struct omap_mbox mbox_1_info = {
> .name = "mailbox-1",
> .ops = &omap2_mbox_ops,
> .priv = &omap2_mbox_1_priv,
> + .tx_tasklet = &omap_mbox_1_tasklet,
> };
> EXPORT_SYMBOL(mbox_1_info);
> #else
> @@ -305,8 +318,10 @@ struct omap_mbox mbox_2_info = {
> .name = "mailbox-2",
> .ops = &omap2_mbox_ops,
> .priv = &omap2_mbox_2_priv,
> + .tx_tasklet = &omap_mbox_2_tasklet,
> };
> EXPORT_SYMBOL(mbox_2_info);
> +
> #endif
>
>
> diff --git a/arch/arm/plat-omap/include/mach/mailbox.h b/arch/arm/plat-omap/include/mach/mailbox.h
> index bf06953..5271345 100644
> --- a/arch/arm/plat-omap/include/mach/mailbox.h
> +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> @@ -28,8 +28,10 @@ struct omap_mbox_ops {
> int (*fifo_empty)(struct omap_mbox *mbox);
> int (*fifo_full)(struct omap_mbox *mbox);
> /* irq */
> - void (*enable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
> - void (*disable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
> + void (*enable_irq)(struct omap_mbox *mbox,
> + omap_mbox_irq_t irq);
> + void (*disable_irq)(struct omap_mbox *mbox,
> + omap_mbox_irq_t irq);
> void (*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
> int (*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
> /* ctx */
> @@ -45,12 +47,22 @@ struct omap_mbox_queue {
> struct omap_mbox *mbox;
> };
>
> +struct omap_mbox_task{
> + spinlock_t lock;
> + struct tasklet_struct *t;
> + mbox_msg_t msg;
> + void *arg;
> +};
> +
> +
> struct omap_mbox {
> char *name;
> unsigned int irq;
>
> struct omap_mbox_queue *txq, *rxq;
>
> + struct omap_mbox_task *tx_tasklet;
> +
> struct omap_mbox_ops *ops;
>
> mbox_msg_t seq_snd, seq_rcv;
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 84cf6af..b5e53d4 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -63,60 +63,49 @@ 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 void __mbox_msg_send(unsigned long tx)
> {
> + struct omap_mbox_task *tx_data = (struct omap_mbox_task *)tx;
> + struct omap_mbox *mbox = (struct omap_mbox *)tx_data->arg;
> + mbox_msg_t msg = tx_data->msg;
> int ret = 0, i = 1000;
>
> while (mbox_fifo_full(mbox)) {
> - if (mbox->ops->type == OMAP_MBOX_TYPE2)
> - return -1;
> - if (--i == 0)
> - return -1;
> + if (mbox->ops->type == OMAP_MBOX_TYPE2) {
> + printk(KERN_ERR "Invalid mailbox type\n");
> + return ;
> + }
> + if (--i == 0) {
> + printk(KERN_ERR "Time out writing to mailbox\n");
> + return ;
> + }
> udelay(1);
> }
> mbox_fifo_write(mbox, msg);
> - return ret;
> -}
> -
> -struct omap_msg_tx_data {
> - mbox_msg_t msg;
> - void *arg;
> -};
> + tx_data->arg = NULL;
> + spin_unlock(&(mbox->tx_tasklet->lock));
>
> -static void omap_msg_tx_end_io(struct request *rq, int error)
> -{
> - kfree(rq->special);
> - __blk_put_request(rq->q, rq);
> + return;
> }
>
> +
> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
> {
> - struct omap_msg_tx_data *tx_data;
> - struct request *rq;
> - struct request_queue *q = mbox->txq->queue;
> + struct omap_mbox_task *tx_task = mbox->tx_tasklet;
>
> - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> - if (unlikely(!tx_data))
> - return -ENOMEM;
> -
> - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> - if (unlikely(!rq)) {
> - kfree(tx_data);
> - return -ENOMEM;
> - }
> + spin_lock(&(mbox->tx_tasklet->lock));
> + tx_task->arg = (void *)mbox;
> + tx_task->msg = msg;
>
> - tx_data->msg = msg;
> - rq->end_io = omap_msg_tx_end_io;
> - blk_insert_request(q, rq, 0, tx_data);
> + tasklet_schedule(tx_task->t);
>
> - schedule_work(&mbox->txq->work);
> return 0;
> }
> EXPORT_SYMBOL(omap_mbox_msg_send);
>
> static void mbox_tx_work(struct work_struct *work)
> {
> - int ret;
> + int ret = 0;
> struct request *rq;
> struct omap_mbox_queue *mq = container_of(work,
> struct omap_mbox_queue, work);
> @@ -124,8 +113,6 @@ static void mbox_tx_work(struct work_struct *work)
> struct request_queue *q = mbox->txq->queue;
>
> while (1) {
> - struct omap_msg_tx_data *tx_data;
> -
> spin_lock(q->queue_lock);
> rq = blk_fetch_request(q);
> spin_unlock(q->queue_lock);
> @@ -133,9 +120,6 @@ static void mbox_tx_work(struct work_struct *work)
> if (!rq)
> break;
>
> - tx_data = rq->special;
> -
> - ret = __mbox_msg_send(mbox, tx_data->msg);
> if (ret) {
> omap_mbox_enable_irq(mbox, IRQ_TX);
> spin_lock(q->queue_lock);
> @@ -206,7 +190,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> goto nomem;
>
> msg = mbox_fifo_read(mbox);
> -
> + rq->special = (void *)msg;
>
> blk_insert_request(q, rq, 0, (void *)msg);
> if (mbox->ops->type == OMAP_MBOX_TYPE1)
> @@ -298,13 +282,25 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> }
> mbox->rxq = mq;
>
> + mbox->tx_tasklet->t = kzalloc(sizeof(struct tasklet_struct),
> + GFP_KERNEL);
> + if (!mbox->tx_tasklet->t) {
> + ret = -ENOMEM;
> + goto fail_alloc_tasklet;
> + }
> + spin_lock_init(&mbox->tx_tasklet->lock);
> + tasklet_init(mbox->tx_tasklet->t, __mbox_msg_send,
> + (unsigned long)mbox->tx_tasklet);
> +
> return 0;
>
> - fail_alloc_rxq:
> +fail_alloc_tasklet:
> + mbox_queue_free(mbox->rxq);
> +fail_alloc_rxq:
> mbox_queue_free(mbox->txq);
> - fail_alloc_txq:
> +fail_alloc_txq:
> free_irq(mbox->irq, mbox);
> - fail_request_irq:
> +fail_request_irq:
> if (unlikely(mbox->ops->shutdown))
> mbox->ops->shutdown(mbox);
>
> --
> 1.5.3.2
> --
> 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] 12+ messages in thread
* RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-07 8:49 ` Hiroshi DOYU
@ 2009-09-08 17:43 ` C.A, Subramaniam
2009-09-09 9:02 ` Hiroshi DOYU
0 siblings, 1 reply; 12+ messages in thread
From: C.A, Subramaniam @ 2009-09-08 17:43 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, tony, rmk, Kanigeri, Hari, Gupta, Ramesh
Hi Hiroshi,
> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> Sent: Monday, September 07, 2009 2:20 PM
> To: C.A, Subramaniam
> Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
>
> From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
> Date: Fri, 4 Sep 2009 13:48:45 +0200
>
> > From e759173e77f73fabce5177b4f685df20b16c6922 Mon Sep 17
> 00:00:00 2001
> > From: C A Subramaniam <subramaniam.ca@ti.com>
> > Date: Thu, 3 Sep 2009 17:42:35 +0530
> > Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet
> > implementation
> >
> > This patch uses a tasklet implementation for sending
> mailbox messages.
> >
> > Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
> > Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
> > ---
> > arch/arm/mach-omap2/mailbox.c | 43 ++++++++++-----
> > arch/arm/plat-omap/include/mach/mailbox.h | 16 +++++-
> > arch/arm/plat-omap/mailbox.c | 80
> ++++++++++++++---------------
> > 3 files changed, 81 insertions(+), 58 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/mailbox.c
> > b/arch/arm/mach-omap2/mailbox.c index 52a1670..45aea98 100644
> > --- a/arch/arm/mach-omap2/mailbox.c
> > +++ b/arch/arm/mach-omap2/mailbox.c
> > @@ -236,6 +237,17 @@ static struct omap_mbox_ops omap2_mbox_ops = {
> > .restore_ctx = omap2_mbox_restore_ctx,
> > };
> >
> > +
> > +static struct omap_mbox_task omap_mbox_1_tasklet = {
> > + .arg = NULL,
> > +};
> > +
> > +static struct omap_mbox_task omap_mbox_2_tasklet = {
> > + .arg = NULL,
> > +};
>
> As I described in the comment on [PATCH 8/10], I think that
> it would be better to keep "mach-omap2/mailbox.c" simple and
> to add some additional logic on "plat-omap/mailbox.c".
>
> Would it be possbile to move this tasklet implementation to
> "plat-omap/mailbox.c"?
The implementation of the tasklet itself is maintained in plat-omap/mailbox.c
Since, I wanted to maintain a separate tasklet for each mailbox
instance used to send messages from MPU, I had to associate the the tasklet
to the mailbox. Hence, the changes were done in mach-omap2/mailbox.c
Please give your comments on this approach.
>
> > +
> > +
> > +
> > /*
> > * MAILBOX 0: ARM -> DSP,
> > * MAILBOX 1: ARM <- DSP.
> > @@ -272,6 +284,7 @@ struct omap_mbox mbox_1_info = {
> > .name = "mailbox-1",
> > .ops = &omap2_mbox_ops,
> > .priv = &omap2_mbox_1_priv,
> > + .tx_tasklet = &omap_mbox_1_tasklet,
> > };
> > EXPORT_SYMBOL(mbox_1_info);
> > #else
> > @@ -305,8 +318,10 @@ struct omap_mbox mbox_2_info = {
> > .name = "mailbox-2",
> > .ops = &omap2_mbox_ops,
> > .priv = &omap2_mbox_2_priv,
> > + .tx_tasklet = &omap_mbox_2_tasklet,
> > };
> > EXPORT_SYMBOL(mbox_2_info);
> > +
> > #endif
> >
> >
> > diff --git a/arch/arm/plat-omap/include/mach/mailbox.h
> > b/arch/arm/plat-omap/include/mach/mailbox.h
> > index bf06953..5271345 100644
> > --- a/arch/arm/plat-omap/include/mach/mailbox.h
> > +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> > @@ -28,8 +28,10 @@ struct omap_mbox_ops {
> > int (*fifo_empty)(struct omap_mbox *mbox);
> > int (*fifo_full)(struct omap_mbox *mbox);
> > /* irq */
> > - void (*enable_irq)(struct omap_mbox *mbox,
> omap_mbox_irq_t irq);
> > - void (*disable_irq)(struct omap_mbox *mbox,
> omap_mbox_irq_t irq);
> > + void (*enable_irq)(struct omap_mbox *mbox,
> > + omap_mbox_irq_t irq);
> > + void (*disable_irq)(struct omap_mbox *mbox,
> > + omap_mbox_irq_t irq);
> > void (*ack_irq)(struct omap_mbox *mbox,
> omap_mbox_irq_t irq);
> > int (*is_irq)(struct omap_mbox *mbox,
> omap_mbox_irq_t irq);
> > /* ctx */
> > @@ -45,12 +47,22 @@ struct omap_mbox_queue {
> > struct omap_mbox *mbox;
> > };
> >
> > +struct omap_mbox_task{
> > + spinlock_t lock;
> > + struct tasklet_struct *t;
> > + mbox_msg_t msg;
> > + void *arg;
> > +};
> > +
> > +
> > struct omap_mbox {
> > char *name;
> > unsigned int irq;
> >
> > struct omap_mbox_queue *txq, *rxq;
> >
> > + struct omap_mbox_task *tx_tasklet;
> > +
> > struct omap_mbox_ops *ops;
> >
> > mbox_msg_t seq_snd, seq_rcv;
> > diff --git a/arch/arm/plat-omap/mailbox.c
> > b/arch/arm/plat-omap/mailbox.c index 84cf6af..b5e53d4 100644
> > --- a/arch/arm/plat-omap/mailbox.c
> > +++ b/arch/arm/plat-omap/mailbox.c
> > @@ -63,60 +63,49 @@ 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 void __mbox_msg_send(unsigned long tx)
> > {
> > + struct omap_mbox_task *tx_data = (struct omap_mbox_task *)tx;
> > + struct omap_mbox *mbox = (struct omap_mbox *)tx_data->arg;
> > + mbox_msg_t msg = tx_data->msg;
> > int ret = 0, i = 1000;
> >
> > while (mbox_fifo_full(mbox)) {
> > - if (mbox->ops->type == OMAP_MBOX_TYPE2)
> > - return -1;
> > - if (--i == 0)
> > - return -1;
> > + if (mbox->ops->type == OMAP_MBOX_TYPE2) {
> > + printk(KERN_ERR "Invalid mailbox type\n");
> > + return ;
> > + }
> > + if (--i == 0) {
> > + printk(KERN_ERR "Time out writing to
> mailbox\n");
> > + return ;
> > + }
> > udelay(1);
> > }
> > mbox_fifo_write(mbox, msg);
> > - return ret;
> > -}
> > -
> > -struct omap_msg_tx_data {
> > - mbox_msg_t msg;
> > - void *arg;
> > -};
> > + tx_data->arg = NULL;
> > + spin_unlock(&(mbox->tx_tasklet->lock));
> >
> > -static void omap_msg_tx_end_io(struct request *rq, int error) -{
> > - kfree(rq->special);
> > - __blk_put_request(rq->q, rq);
> > + return;
> > }
> >
> > +
> > int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg) {
> > - struct omap_msg_tx_data *tx_data;
> > - struct request *rq;
> > - struct request_queue *q = mbox->txq->queue;
> > + struct omap_mbox_task *tx_task = mbox->tx_tasklet;
> >
> > - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> > - if (unlikely(!tx_data))
> > - return -ENOMEM;
> > -
> > - rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> > - if (unlikely(!rq)) {
> > - kfree(tx_data);
> > - return -ENOMEM;
> > - }
> > + spin_lock(&(mbox->tx_tasklet->lock));
> > + tx_task->arg = (void *)mbox;
> > + tx_task->msg = msg;
> >
> > - tx_data->msg = msg;
> > - rq->end_io = omap_msg_tx_end_io;
> > - blk_insert_request(q, rq, 0, tx_data);
> > + tasklet_schedule(tx_task->t);
> >
> > - schedule_work(&mbox->txq->work);
> > return 0;
> > }
> > EXPORT_SYMBOL(omap_mbox_msg_send);
> >
> > static void mbox_tx_work(struct work_struct *work) {
> > - int ret;
> > + int ret = 0;
> > struct request *rq;
> > struct omap_mbox_queue *mq = container_of(work,
> > struct omap_mbox_queue, work);
> > @@ -124,8 +113,6 @@ static void mbox_tx_work(struct
> work_struct *work)
> > struct request_queue *q = mbox->txq->queue;
> >
> > while (1) {
> > - struct omap_msg_tx_data *tx_data;
> > -
> > spin_lock(q->queue_lock);
> > rq = blk_fetch_request(q);
> > spin_unlock(q->queue_lock);
> > @@ -133,9 +120,6 @@ static void mbox_tx_work(struct
> work_struct *work)
> > if (!rq)
> > break;
> >
> > - tx_data = rq->special;
> > -
> > - ret = __mbox_msg_send(mbox, tx_data->msg);
> > if (ret) {
> > omap_mbox_enable_irq(mbox, IRQ_TX);
> > spin_lock(q->queue_lock);
> > @@ -206,7 +190,7 @@ static void __mbox_rx_interrupt(struct
> omap_mbox *mbox)
> > goto nomem;
> >
> > msg = mbox_fifo_read(mbox);
> > -
> > + rq->special = (void *)msg;
> >
> > blk_insert_request(q, rq, 0, (void *)msg);
> > if (mbox->ops->type == OMAP_MBOX_TYPE1) @@
> -298,13 +282,25 @@
> > static int omap_mbox_startup(struct omap_mbox *mbox)
> > }
> > mbox->rxq = mq;
> >
> > + mbox->tx_tasklet->t = kzalloc(sizeof(struct tasklet_struct),
> > + GFP_KERNEL);
> > + if (!mbox->tx_tasklet->t) {
> > + ret = -ENOMEM;
> > + goto fail_alloc_tasklet;
> > + }
> > + spin_lock_init(&mbox->tx_tasklet->lock);
> > + tasklet_init(mbox->tx_tasklet->t, __mbox_msg_send,
> > + (unsigned long)mbox->tx_tasklet);
> > +
> > return 0;
> >
> > - fail_alloc_rxq:
> > +fail_alloc_tasklet:
> > + mbox_queue_free(mbox->rxq);
> > +fail_alloc_rxq:
> > mbox_queue_free(mbox->txq);
> > - fail_alloc_txq:
> > +fail_alloc_txq:
> > free_irq(mbox->irq, mbox);
> > - fail_request_irq:
> > +fail_request_irq:
> > if (unlikely(mbox->ops->shutdown))
> > mbox->ops->shutdown(mbox);
> >
> > --
> > 1.5.3.2
> > --
> > 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] 12+ messages in thread
* Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-08 17:43 ` C.A, Subramaniam
@ 2009-09-09 9:02 ` Hiroshi DOYU
2009-09-09 9:14 ` C.A, Subramaniam
2009-09-10 11:46 ` C.A, Subramaniam
0 siblings, 2 replies; 12+ messages in thread
From: Hiroshi DOYU @ 2009-09-09 9:02 UTC (permalink / raw)
To: subramaniam.ca; +Cc: linux-omap, tony, rmk, h-kanigeri2, grgupta
Hi Subbu,
From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
Date: Tue, 8 Sep 2009 19:43:48 +0200
[...]
> > As I described in the comment on [PATCH 8/10], I think that
> > it would be better to keep "mach-omap2/mailbox.c" simple and
> > to add some additional logic on "plat-omap/mailbox.c".
> >
> > Would it be possbile to move this tasklet implementation to
> > "plat-omap/mailbox.c"?
>
> The implementation of the tasklet itself is maintained in plat-omap/mailbox.c
> Since, I wanted to maintain a separate tasklet for each mailbox
> instance used to send messages from MPU, I had to associate the the tasklet
> to the mailbox. Hence, the changes were done in mach-omap2/mailbox.c
>
> Please give your comments on this approach.
Wouldn't just converting work_queue to tasklet work like below?
(I havne't tested this at all, though...)
diff --git a/arch/arm/plat-omap/include/mach/mailbox.h b/arch/arm/plat-omap/include/mach/mailbox.h
index b7a6991..1f4e53e 100644
--- a/arch/arm/plat-omap/include/mach/mailbox.h
+++ b/arch/arm/plat-omap/include/mach/mailbox.h
@@ -52,6 +52,8 @@ struct omap_mbox {
struct omap_mbox_queue *txq, *rxq;
+ struct tasklet_struct tx_tasklet;
+
struct omap_mbox_ops *ops;
mbox_msg_t seq_snd, seq_rcv;
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 40424ed..37267ca 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg, void* arg)
}
EXPORT_SYMBOL(omap_mbox_msg_send);
-static void mbox_tx_work(struct work_struct *work)
+static void mbox_tx_tasklet(unsigned long data)
{
int ret;
struct request *rq;
- struct omap_mbox_queue *mq = container_of(work,
- struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
+ struct omap_mbox *mbox = (struct omap_mbox *)data;
struct request_queue *q = mbox->txq->queue;
while (1) {
struct omap_msg_tx_data *tx_data;
- spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
- spin_unlock(q->queue_lock);
-
if (!rq)
break;
@@ -208,15 +203,10 @@ static void mbox_tx_work(struct work_struct *work)
ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
if (ret) {
enable_mbox_irq(mbox, IRQ_TX);
- spin_lock(q->queue_lock);
blk_requeue_request(q, rq);
- spin_unlock(q->queue_lock);
return;
}
-
- spin_lock(q->queue_lock);
__blk_end_request_all(rq, 0);
- spin_unlock(q->queue_lock);
}
}
@@ -266,7 +256,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
disable_mbox_irq(mbox, IRQ_TX);
ack_mbox_irq(mbox, IRQ_TX);
- schedule_work(&mbox->txq->work);
+ tasklet_schedule(&mbox->tx_tasklet);
}
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
@@ -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
+ tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet, (unsigned long)mbox);
+
+ mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_txq;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-09 9:02 ` Hiroshi DOYU
@ 2009-09-09 9:14 ` C.A, Subramaniam
2009-09-10 11:46 ` C.A, Subramaniam
1 sibling, 0 replies; 12+ messages in thread
From: C.A, Subramaniam @ 2009-09-09 9:14 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, tony, rmk, Kanigeri, Hari, Gupta, Ramesh
Hi Hiroshi,
> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> Sent: Wednesday, September 09, 2009 2:32 PM
> To: C.A, Subramaniam
> Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
>
> Hi Subbu,
>
> From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
> Date: Tue, 8 Sep 2009 19:43:48 +0200
>
> [...]
>
> > > As I described in the comment on [PATCH 8/10], I think
> that it would
> > > be better to keep "mach-omap2/mailbox.c" simple and to add some
> > > additional logic on "plat-omap/mailbox.c".
> > >
> > > Would it be possbile to move this tasklet implementation to
> > > "plat-omap/mailbox.c"?
> >
> > The implementation of the tasklet itself is maintained in
> > plat-omap/mailbox.c Since, I wanted to maintain a separate
> tasklet for
> > each mailbox instance used to send messages from MPU, I had to
> > associate the the tasklet to the mailbox. Hence, the
> changes were done
> > in mach-omap2/mailbox.c
> >
> > Please give your comments on this approach.
>
> Wouldn't just converting work_queue to tasklet work like below?
>
> (I havne't tested this at all, though...)
I will try that out and let you know. Also your approach seems better
as you are replacing the work queue implementaion as well.
>
> diff --git a/arch/arm/plat-omap/include/mach/mailbox.h
> b/arch/arm/plat-omap/include/mach/mailbox.h
> index b7a6991..1f4e53e 100644
> --- a/arch/arm/plat-omap/include/mach/mailbox.h
> +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> @@ -52,6 +52,8 @@ struct omap_mbox {
>
> struct omap_mbox_queue *txq, *rxq;
>
> + struct tasklet_struct tx_tasklet;
> +
> struct omap_mbox_ops *ops;
>
> mbox_msg_t seq_snd, seq_rcv;
> diff --git a/arch/arm/plat-omap/mailbox.c
> b/arch/arm/plat-omap/mailbox.c index 40424ed..37267ca 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct omap_mbox
> *mbox, mbox_msg_t msg, void* arg) }
> EXPORT_SYMBOL(omap_mbox_msg_send);
>
> -static void mbox_tx_work(struct work_struct *work)
> +static void mbox_tx_tasklet(unsigned long data)
> {
> int ret;
> struct request *rq;
> - struct omap_mbox_queue *mq = container_of(work,
> - struct omap_mbox_queue, work);
> - struct omap_mbox *mbox = mq->queue->queuedata;
> + struct omap_mbox *mbox = (struct omap_mbox *)data;
> struct request_queue *q = mbox->txq->queue;
>
> while (1) {
> struct omap_msg_tx_data *tx_data;
>
> - spin_lock(q->queue_lock);
> rq = blk_fetch_request(q);
> - spin_unlock(q->queue_lock);
> -
> if (!rq)
> break;
>
> @@ -208,15 +203,10 @@ static void mbox_tx_work(struct
> work_struct *work)
> ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
> if (ret) {
> enable_mbox_irq(mbox, IRQ_TX);
> - spin_lock(q->queue_lock);
> blk_requeue_request(q, rq);
> - spin_unlock(q->queue_lock);
> return;
> }
> -
> - spin_lock(q->queue_lock);
> __blk_end_request_all(rq, 0);
> - spin_unlock(q->queue_lock);
> }
> }
>
> @@ -266,7 +256,7 @@ static void __mbox_tx_interrupt(struct
> omap_mbox *mbox) {
> disable_mbox_irq(mbox, IRQ_TX);
> ack_mbox_irq(mbox, IRQ_TX);
> - schedule_work(&mbox->txq->work);
> + tasklet_schedule(&mbox->tx_tasklet);
> }
>
> static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@
> -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox)
> goto fail_request_irq;
> }
>
> - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet,
> (unsigned long)mbox);
> +
> + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_txq;
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-09 9:02 ` Hiroshi DOYU
2009-09-09 9:14 ` C.A, Subramaniam
@ 2009-09-10 11:46 ` C.A, Subramaniam
2009-09-10 12:28 ` Hiroshi DOYU
1 sibling, 1 reply; 12+ messages in thread
From: C.A, Subramaniam @ 2009-09-10 11:46 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, tony, rmk, Kanigeri, Hari, Gupta, Ramesh
Hi Hiroshi,
> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> Sent: Wednesday, September 09, 2009 2:32 PM
> To: C.A, Subramaniam
> Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
>
> Hi Subbu,
>
> From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
> Date: Tue, 8 Sep 2009 19:43:48 +0200
>
> [...]
>
> > > As I described in the comment on [PATCH 8/10], I think
> that it would
> > > be better to keep "mach-omap2/mailbox.c" simple and to add some
> > > additional logic on "plat-omap/mailbox.c".
> > >
> > > Would it be possbile to move this tasklet implementation to
> > > "plat-omap/mailbox.c"?
> >
> > The implementation of the tasklet itself is maintained in
> > plat-omap/mailbox.c Since, I wanted to maintain a separate
> tasklet for
> > each mailbox instance used to send messages from MPU, I had to
> > associate the the tasklet to the mailbox. Hence, the
> changes were done
> > in mach-omap2/mailbox.c
> >
> > Please give your comments on this approach.
>
> Wouldn't just converting work_queue to tasklet work like below?
>
> (I havne't tested this at all, though...)
>
> diff --git a/arch/arm/plat-omap/include/mach/mailbox.h
> b/arch/arm/plat-omap/include/mach/mailbox.h
> index b7a6991..1f4e53e 100644
> --- a/arch/arm/plat-omap/include/mach/mailbox.h
> +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> @@ -52,6 +52,8 @@ struct omap_mbox {
>
> struct omap_mbox_queue *txq, *rxq;
>
> + struct tasklet_struct tx_tasklet;
> +
> struct omap_mbox_ops *ops;
>
> mbox_msg_t seq_snd, seq_rcv;
Moved the tasklet structure to the omap_mbox_queue as follows:
@@ -40,7 +43,8 @@ struct omap_mbox_ops {
struct omap_mbox_queue {
spinlock_t lock;
struct request_queue *queue;
- struct work_struct work;
+ struct work_struct rx_work;
+ struct tasklet_struct tx_tasklet;
int (*callback)(void *);
struct omap_mbox *mbox;
};
Also chagned the omap_mbox_msg_send(). Removed the struct omap_msg_tx_data.
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
- struct omap_msg_tx_data *tx_data;
+
struct request *rq;
struct request_queue *q = mbox->txq->queue;
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (unlikely(!tx_data))
- return -ENOMEM;
-
rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq)) {
- kfree(tx_data);
+ if (unlikely(!rq))
return -ENOMEM;
- }
- tx_data->msg = msg;
- rq->end_io = omap_msg_tx_end_io;
- blk_insert_request(q, rq, 0, tx_data);
+ blk_insert_request(q, rq, 0, (void *) msg);
+ tasklet_schedule(&mbox->txq->tx_tasklet);
- schedule_work(&mbox->txq->work);
return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
> diff --git a/arch/arm/plat-omap/mailbox.c
> b/arch/arm/plat-omap/mailbox.c index 40424ed..37267ca 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct omap_mbox
> *mbox, mbox_msg_t msg, void* arg) }
> EXPORT_SYMBOL(omap_mbox_msg_send);
>
> -static void mbox_tx_work(struct work_struct *work)
> +static void mbox_tx_tasklet(unsigned long data)
> {
> int ret;
> struct request *rq;
> - struct omap_mbox_queue *mq = container_of(work,
> - struct omap_mbox_queue, work);
> - struct omap_mbox *mbox = mq->queue->queuedata;
> + struct omap_mbox *mbox = (struct omap_mbox *)data;
> struct request_queue *q = mbox->txq->queue;
>
> while (1) {
> struct omap_msg_tx_data *tx_data;
>
> - spin_lock(q->queue_lock);
> rq = blk_fetch_request(q);
> - spin_unlock(q->queue_lock);
> -
> if (!rq)
> break;
>
> @@ -208,15 +203,10 @@ static void mbox_tx_work(struct
> work_struct *work)
> ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
> if (ret) {
> enable_mbox_irq(mbox, IRQ_TX);
> - spin_lock(q->queue_lock);
> blk_requeue_request(q, rq);
> - spin_unlock(q->queue_lock);
> return;
> }
> -
> - spin_lock(q->queue_lock);
> __blk_end_request_all(rq, 0);
> - spin_unlock(q->queue_lock);
> }
> }
>
Changed the mbox_tx_tasklet as follows:
-static void mbox_tx_work(struct work_struct *work)
+static void mbox_tx_tasklet(unsigned long tx_data)
{
int ret;
struct request *rq;
- struct omap_mbox_queue *mq = container_of(work,
- struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
+ struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
struct request_queue *q = mbox->txq->queue;
while (1) {
- struct omap_msg_tx_data *tx_data;
- spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
- spin_unlock(q->queue_lock);
if (!rq)
break;
- tx_data = rq->special;
-
- ret = __mbox_msg_send(mbox, tx_data->msg);
+ ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
if (ret) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- spin_lock(q->queue_lock);
blk_requeue_request(q, rq);
- spin_unlock(q->queue_lock);
return;
}
-
- spin_lock(q->queue_lock);
- __blk_end_request_all(rq, 0);
- spin_unlock(q->queue_lock);
+ /* FIXME: We get a WARN_ON() if __blk_end_request_all()
+ * is used. Not sure if we can remove the queue locks
+ * for blk_requeue_request() and blk_fetch_request()
+ * calls as well.*/
+ blk_end_request_all(rq, 0);
}
}
While testing I got a WARN_ON() using the __blk_end_request_all().
Tried using the blk_end_request_all() instead, and that worked fine.
Is it safe to remove the spin lock protection for all the calls
inside the tasklet function as you had suggested?
Please comment.
> @@ -266,7 +256,7 @@ static void __mbox_tx_interrupt(struct
> omap_mbox *mbox) {
> disable_mbox_irq(mbox, IRQ_TX);
> ack_mbox_irq(mbox, IRQ_TX);
> - schedule_work(&mbox->txq->work);
> + tasklet_schedule(&mbox->tx_tasklet);
> }
>
> static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@
> -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox)
> goto fail_request_irq;
> }
>
Changes in the names used for work queue (rx_work) and tasklet as tx_tasklet.
@@ -157,7 +132,7 @@ static void mbox_tx_work(struct work_struct *work)
static void mbox_rx_work(struct work_struct *work)
{
struct omap_mbox_queue *mq =
- container_of(work, struct omap_mbox_queue, work);
+ container_of(work, struct omap_mbox_queue, rx_work);
struct omap_mbox *mbox = mq->queue->queuedata;
struct request_queue *q = mbox->rxq->queue;
struct request *rq;
@@ -192,7 +167,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
ack_mbox_irq(mbox, IRQ_TX);
- schedule_work(&mbox->txq->work);
+ tasklet_schedule(&mbox->txq->tx_tasklet);
}
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
@@ -217,7 +192,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
/* no more messages in the fifo. clear IRQ source. */
ack_mbox_irq(mbox, IRQ_RX);
nomem:
- schedule_work(&mbox->rxq->work);
+ schedule_work(&mbox->rxq->rx_work);
}
> - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet,
> (unsigned long)mbox);
> +
> + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_txq;
>
>
Changed the signature of the mbox_queue_alloc function.
The work queue / tasklet initialization is done in the
omap_mbox_startup() function.
static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
- request_fn_proc *proc,
- void (*work) (struct work_struct *))
+ request_fn_proc *proc)
{
struct request_queue *q;
struct omap_mbox_queue *mq;
@@ -252,8 +226,6 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
q->queuedata = mbox;
mq->queue = q;
- INIT_WORK(&mq->work, work);
-
return mq;
error:
kfree(mq);
@@ -292,18 +264,22 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
+ mq = mbox_queue_alloc(mbox, mbox_txq_fn);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_txq;
}
+
+ tasklet_init(&mq->tx_tasklet, mbox_tx_tasklet, (unsigned long)mbox);
mbox->txq = mq;
- mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work);
+ mq = mbox_queue_alloc(mbox, mbox_rxq_fn);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
}
+
+ INIT_WORK(&mq->rx_work, mbox_rx_work);
mbox->rxq = mq;
return 0;
--
Please give your comments on the changes.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-10 11:46 ` C.A, Subramaniam
@ 2009-09-10 12:28 ` Hiroshi DOYU
2009-09-14 11:35 ` C.A, Subramaniam
0 siblings, 1 reply; 12+ messages in thread
From: Hiroshi DOYU @ 2009-09-10 12:28 UTC (permalink / raw)
To: subramaniam.ca; +Cc: linux-omap, tony, rmk, h-kanigeri2, grgupta
From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
Date: Thu, 10 Sep 2009 13:46:53 +0200
>
>
> Hi Hiroshi,
>
> > -----Original Message-----
> > From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> > Sent: Wednesday, September 09, 2009 2:32 PM
> > To: C.A, Subramaniam
> > Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> > rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> > Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> > Patch to support tasklet implementation
> >
> > Hi Subbu,
> >
> > From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> > Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> > Patch to support tasklet implementation
> > Date: Tue, 8 Sep 2009 19:43:48 +0200
> >
> > [...]
> >
> > > > As I described in the comment on [PATCH 8/10], I think
> > that it would
> > > > be better to keep "mach-omap2/mailbox.c" simple and to add some
> > > > additional logic on "plat-omap/mailbox.c".
> > > >
> > > > Would it be possbile to move this tasklet implementation to
> > > > "plat-omap/mailbox.c"?
> > >
> > > The implementation of the tasklet itself is maintained in
> > > plat-omap/mailbox.c Since, I wanted to maintain a separate
> > tasklet for
> > > each mailbox instance used to send messages from MPU, I had to
> > > associate the the tasklet to the mailbox. Hence, the
> > changes were done
> > > in mach-omap2/mailbox.c
> > >
> > > Please give your comments on this approach.
> >
> > Wouldn't just converting work_queue to tasklet work like below?
> >
> > (I havne't tested this at all, though...)
> >
> > diff --git a/arch/arm/plat-omap/include/mach/mailbox.h
> > b/arch/arm/plat-omap/include/mach/mailbox.h
> > index b7a6991..1f4e53e 100644
> > --- a/arch/arm/plat-omap/include/mach/mailbox.h
> > +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> > @@ -52,6 +52,8 @@ struct omap_mbox {
> >
> > struct omap_mbox_queue *txq, *rxq;
> >
> > + struct tasklet_struct tx_tasklet;
> > +
> > struct omap_mbox_ops *ops;
> >
> > mbox_msg_t seq_snd, seq_rcv;
>
> Moved the tasklet structure to the omap_mbox_queue as follows:
This is better.
>
> @@ -40,7 +43,8 @@ struct omap_mbox_ops {
> struct omap_mbox_queue {
> spinlock_t lock;
> struct request_queue *queue;
> - struct work_struct work;
> + struct work_struct rx_work;
> + struct tasklet_struct tx_tasklet;
> int (*callback)(void *);
> struct omap_mbox *mbox;
> };
I think that "rx_/tx_" prefix may not be necessary if you add tasklet
feature in "omap_mbox_queue" as followed since "omap_mbox_queue" can
be considered as s/w queue which can evoke workqueue/tasklet
accordingly.
+ struct work_struct work;
+ struct tasklet_struct tasklet;
int (*callback)(void *);
struct omap_mbox *mbox;
};
>
> Also chagned the omap_mbox_msg_send(). Removed the struct omap_msg_tx_data.
>
>
> int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
> {
> - struct omap_msg_tx_data *tx_data;
> +
> struct request *rq;
> struct request_queue *q = mbox->txq->queue;
>
> - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> - if (unlikely(!tx_data))
> - return -ENOMEM;
> -
> rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> - if (unlikely(!rq)) {
> - kfree(tx_data);
> + if (unlikely(!rq))
> return -ENOMEM;
> - }
>
> - tx_data->msg = msg;
> - rq->end_io = omap_msg_tx_end_io;
> - blk_insert_request(q, rq, 0, tx_data);
> + blk_insert_request(q, rq, 0, (void *) msg);
> + tasklet_schedule(&mbox->txq->tx_tasklet);
>
> - schedule_work(&mbox->txq->work);
> return 0;
> }
> EXPORT_SYMBOL(omap_mbox_msg_send);
>
> > diff --git a/arch/arm/plat-omap/mailbox.c
> > b/arch/arm/plat-omap/mailbox.c index 40424ed..37267ca 100644
> > --- a/arch/arm/plat-omap/mailbox.c
> > +++ b/arch/arm/plat-omap/mailbox.c
> > @@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct omap_mbox
> > *mbox, mbox_msg_t msg, void* arg) }
> > EXPORT_SYMBOL(omap_mbox_msg_send);
> >
> > -static void mbox_tx_work(struct work_struct *work)
> > +static void mbox_tx_tasklet(unsigned long data)
> > {
> > int ret;
> > struct request *rq;
> > - struct omap_mbox_queue *mq = container_of(work,
> > - struct omap_mbox_queue, work);
> > - struct omap_mbox *mbox = mq->queue->queuedata;
> > + struct omap_mbox *mbox = (struct omap_mbox *)data;
> > struct request_queue *q = mbox->txq->queue;
> >
> > while (1) {
> > struct omap_msg_tx_data *tx_data;
> >
> > - spin_lock(q->queue_lock);
> > rq = blk_fetch_request(q);
> > - spin_unlock(q->queue_lock);
> > -
> > if (!rq)
> > break;
> >
> > @@ -208,15 +203,10 @@ static void mbox_tx_work(struct
> > work_struct *work)
> > ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
> > if (ret) {
> > enable_mbox_irq(mbox, IRQ_TX);
> > - spin_lock(q->queue_lock);
> > blk_requeue_request(q, rq);
> > - spin_unlock(q->queue_lock);
> > return;
> > }
> > -
> > - spin_lock(q->queue_lock);
> > __blk_end_request_all(rq, 0);
> > - spin_unlock(q->queue_lock);
> > }
> > }
> >
>
> Changed the mbox_tx_tasklet as follows:
>
> -static void mbox_tx_work(struct work_struct *work)
> +static void mbox_tx_tasklet(unsigned long tx_data)
> {
> int ret;
> struct request *rq;
> - struct omap_mbox_queue *mq = container_of(work,
> - struct omap_mbox_queue, work);
> - struct omap_mbox *mbox = mq->queue->queuedata;
> + struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
> struct request_queue *q = mbox->txq->queue;
>
> while (1) {
> - struct omap_msg_tx_data *tx_data;
>
> - spin_lock(q->queue_lock);
> rq = blk_fetch_request(q);
> - spin_unlock(q->queue_lock);
>
> if (!rq)
> break;
>
> - tx_data = rq->special;
> -
> - ret = __mbox_msg_send(mbox, tx_data->msg);
> + ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
> if (ret) {
> omap_mbox_enable_irq(mbox, IRQ_TX);
> - spin_lock(q->queue_lock);
> blk_requeue_request(q, rq);
> - spin_unlock(q->queue_lock);
> return;
> }
> -
> - spin_lock(q->queue_lock);
> - __blk_end_request_all(rq, 0);
> - spin_unlock(q->queue_lock);
> + /* FIXME: We get a WARN_ON() if __blk_end_request_all()
> + * is used. Not sure if we can remove the queue locks
> + * for blk_requeue_request() and blk_fetch_request()
> + * calls as well.*/
> + blk_end_request_all(rq, 0);
> }
> }
>
> While testing I got a WARN_ON() using the __blk_end_request_all().
> Tried using the blk_end_request_all() instead, and that worked fine.
> Is it safe to remove the spin lock protection for all the calls
> inside the tasklet function as you had suggested?
> Please comment.
I think that it's safe since it's being executed in tasklet context,
no preemption.
Which WARN_ON() did you get?
>
> > @@ -266,7 +256,7 @@ static void __mbox_tx_interrupt(struct
> > omap_mbox *mbox) {
> > disable_mbox_irq(mbox, IRQ_TX);
> > ack_mbox_irq(mbox, IRQ_TX);
> > - schedule_work(&mbox->txq->work);
> > + tasklet_schedule(&mbox->tx_tasklet);
> > }
> >
> > static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@
> > -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox)
> > goto fail_request_irq;
> > }
> >
>
> Changes in the names used for work queue (rx_work) and tasklet as
> tx_tasklet.
As I explained above, the prefix 'tx_/rx_' isn't necessary.
>
> @@ -157,7 +132,7 @@ static void mbox_tx_work(struct work_struct *work)
> static void mbox_rx_work(struct work_struct *work)
> {
> struct omap_mbox_queue *mq =
> - container_of(work, struct omap_mbox_queue, work);
> + container_of(work, struct omap_mbox_queue, rx_work);
> struct omap_mbox *mbox = mq->queue->queuedata;
> struct request_queue *q = mbox->rxq->queue;
> struct request *rq;
> @@ -192,7 +167,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
> {
> omap_mbox_disable_irq(mbox, IRQ_TX);
> ack_mbox_irq(mbox, IRQ_TX);
> - schedule_work(&mbox->txq->work);
> + tasklet_schedule(&mbox->txq->tx_tasklet);
> }
>
> static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> @@ -217,7 +192,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> /* no more messages in the fifo. clear IRQ source. */
> ack_mbox_irq(mbox, IRQ_RX);
> nomem:
> - schedule_work(&mbox->rxq->work);
> + schedule_work(&mbox->rxq->rx_work);
> }
>
>
> > - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> > + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet,
> > (unsigned long)mbox);
> > +
> > + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
> > if (!mq) {
> > ret = -ENOMEM;
> > goto fail_alloc_txq;
> >
> >
>
> Changed the signature of the mbox_queue_alloc function.
> The work queue / tasklet initialization is done in the
> omap_mbox_startup() function.
why?
>
> static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
> - request_fn_proc *proc,
> - void (*work) (struct work_struct *))
> + request_fn_proc *proc)
> {
> struct request_queue *q;
> struct omap_mbox_queue *mq;
> @@ -252,8 +226,6 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
> q->queuedata = mbox;
> mq->queue = q;
>
> - INIT_WORK(&mq->work, work);
> -
> return mq;
> error:
> kfree(mq);
> @@ -292,18 +264,22 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> goto fail_request_irq;
> }
>
> - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> + mq = mbox_queue_alloc(mbox, mbox_txq_fn);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_txq;
> }
> +
> + tasklet_init(&mq->tx_tasklet, mbox_tx_tasklet, (unsigned long)mbox);
> mbox->txq = mq;
>
> - mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work);
> + mq = mbox_queue_alloc(mbox, mbox_rxq_fn);
> if (!mq) {
> ret = -ENOMEM;
> goto fail_alloc_rxq;
> }
> +
> + INIT_WORK(&mq->rx_work, mbox_rx_work);
> mbox->rxq = mq;
>
> return 0;
> --
>
> Please give your comments on the changes.
> --
> 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] 12+ messages in thread
* RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-10 12:28 ` Hiroshi DOYU
@ 2009-09-14 11:35 ` C.A, Subramaniam
2009-09-14 12:43 ` Hiroshi DOYU
0 siblings, 1 reply; 12+ messages in thread
From: C.A, Subramaniam @ 2009-09-14 11:35 UTC (permalink / raw)
To: Hiroshi DOYU; +Cc: linux-omap, tony, rmk, Kanigeri, Hari, Gupta, Ramesh
Hi Hiroshi,
Sorry for the delayed response.
> -----Original Message-----
> From: Hiroshi DOYU [mailto:Hiroshi.DOYU@Nokia.com]
> Sent: Thursday, September 10, 2009 5:59 PM
> To: C.A, Subramaniam
> Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
>
> From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver
> Patch to support tasklet implementation
> Date: Thu, 10 Sep 2009 13:46:53 +0200
>
> >
> >
> > Hi Hiroshi,
> >
> > > -----Original Message-----
> > > From: Hiroshi DOYU [mailto:Hiroshi.DOYU@nokia.com]
> > > Sent: Wednesday, September 09, 2009 2:32 PM
> > > To: C.A, Subramaniam
> > > Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> > > rmk@arm.linux.org.uk; Kanigeri, Hari; Gupta, Ramesh
> > > Subject: Re: [PATCH 10/10] omap mailbox: OMAP4
> Mailbox-driver Patch
> > > to support tasklet implementation
> > >
> > > Hi Subbu,
> > >
> > > From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
> > > Subject: RE: [PATCH 10/10] omap mailbox: OMAP4
> Mailbox-driver Patch
> > > to support tasklet implementation
> > > Date: Tue, 8 Sep 2009 19:43:48 +0200
> > >
> > > [...]
> > >
> > > > > As I described in the comment on [PATCH 8/10], I think
> > > that it would
> > > > > be better to keep "mach-omap2/mailbox.c" simple and
> to add some
> > > > > additional logic on "plat-omap/mailbox.c".
> > > > >
> > > > > Would it be possbile to move this tasklet implementation to
> > > > > "plat-omap/mailbox.c"?
> > > >
> > > > The implementation of the tasklet itself is maintained in
> > > > plat-omap/mailbox.c Since, I wanted to maintain a separate
> > > tasklet for
> > > > each mailbox instance used to send messages from MPU, I had to
> > > > associate the the tasklet to the mailbox. Hence, the
> > > changes were done
> > > > in mach-omap2/mailbox.c
> > > >
> > > > Please give your comments on this approach.
> > >
> > > Wouldn't just converting work_queue to tasklet work like below?
> > >
> > > (I havne't tested this at all, though...)
> > >
> > > diff --git a/arch/arm/plat-omap/include/mach/mailbox.h
> > > b/arch/arm/plat-omap/include/mach/mailbox.h
> > > index b7a6991..1f4e53e 100644
> > > --- a/arch/arm/plat-omap/include/mach/mailbox.h
> > > +++ b/arch/arm/plat-omap/include/mach/mailbox.h
> > > @@ -52,6 +52,8 @@ struct omap_mbox {
> > >
> > > struct omap_mbox_queue *txq, *rxq;
> > >
> > > + struct tasklet_struct tx_tasklet;
> > > +
> > > struct omap_mbox_ops *ops;
> > >
> > > mbox_msg_t seq_snd, seq_rcv;
> >
> > Moved the tasklet structure to the omap_mbox_queue as follows:
>
> This is better.
>
> >
> > @@ -40,7 +43,8 @@ struct omap_mbox_ops { struct omap_mbox_queue {
> > spinlock_t lock;
> > struct request_queue *queue;
> > - struct work_struct work;
> > + struct work_struct rx_work;
> > + struct tasklet_struct tx_tasklet;
> > int (*callback)(void *);
> > struct omap_mbox *mbox;
> > };
>
>
> I think that "rx_/tx_" prefix may not be necessary if you add
> tasklet feature in "omap_mbox_queue" as followed since
> "omap_mbox_queue" can be considered as s/w queue which can
> evoke workqueue/tasklet accordingly.
>
>
> + struct work_struct work;
> + struct tasklet_struct tasklet;
> int (*callback)(void *);
> struct omap_mbox *mbox;
> };
This is fine. Will remove rx_ and tx_ prefix.
>
>
> >
> > Also chagned the omap_mbox_msg_send(). Removed the struct
> omap_msg_tx_data.
> >
> >
> > int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg) {
> > - struct omap_msg_tx_data *tx_data;
> > +
> > struct request *rq;
> > struct request_queue *q = mbox->txq->queue;
> >
> > - tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
> > - if (unlikely(!tx_data))
> > - return -ENOMEM;
> > -
> > rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> > - if (unlikely(!rq)) {
> > - kfree(tx_data);
> > + if (unlikely(!rq))
> > return -ENOMEM;
> > - }
> >
> > - tx_data->msg = msg;
> > - rq->end_io = omap_msg_tx_end_io;
> > - blk_insert_request(q, rq, 0, tx_data);
> > + blk_insert_request(q, rq, 0, (void *) msg);
> > + tasklet_schedule(&mbox->txq->tx_tasklet);
> >
> > - schedule_work(&mbox->txq->work);
> > return 0;
> > }
> > EXPORT_SYMBOL(omap_mbox_msg_send);
> >
> > > diff --git a/arch/arm/plat-omap/mailbox.c
> > > b/arch/arm/plat-omap/mailbox.c index 40424ed..37267ca 100644
> > > --- a/arch/arm/plat-omap/mailbox.c
> > > +++ b/arch/arm/plat-omap/mailbox.c
> > > @@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct
> omap_mbox *mbox,
> > > mbox_msg_t msg, void* arg) } EXPORT_SYMBOL(omap_mbox_msg_send);
> > >
> > > -static void mbox_tx_work(struct work_struct *work)
> > > +static void mbox_tx_tasklet(unsigned long data)
> > > {
> > > int ret;
> > > struct request *rq;
> > > - struct omap_mbox_queue *mq = container_of(work,
> > > - struct omap_mbox_queue, work);
> > > - struct omap_mbox *mbox = mq->queue->queuedata;
> > > + struct omap_mbox *mbox = (struct omap_mbox *)data;
> > > struct request_queue *q = mbox->txq->queue;
> > >
> > > while (1) {
> > > struct omap_msg_tx_data *tx_data;
> > >
> > > - spin_lock(q->queue_lock);
> > > rq = blk_fetch_request(q);
> > > - spin_unlock(q->queue_lock);
> > > -
> > > if (!rq)
> > > break;
> > >
> > > @@ -208,15 +203,10 @@ static void mbox_tx_work(struct work_struct
> > > *work)
> > > ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg);
> > > if (ret) {
> > > enable_mbox_irq(mbox, IRQ_TX);
> > > - spin_lock(q->queue_lock);
> > > blk_requeue_request(q, rq);
> > > - spin_unlock(q->queue_lock);
> > > return;
> > > }
> > > -
> > > - spin_lock(q->queue_lock);
> > > __blk_end_request_all(rq, 0);
> > > - spin_unlock(q->queue_lock);
> > > }
> > > }
> > >
> >
> > Changed the mbox_tx_tasklet as follows:
> >
> > -static void mbox_tx_work(struct work_struct *work)
> > +static void mbox_tx_tasklet(unsigned long tx_data)
> > {
> > int ret;
> > struct request *rq;
> > - struct omap_mbox_queue *mq = container_of(work,
> > - struct omap_mbox_queue, work);
> > - struct omap_mbox *mbox = mq->queue->queuedata;
> > + struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
> > struct request_queue *q = mbox->txq->queue;
> >
> > while (1) {
> > - struct omap_msg_tx_data *tx_data;
> >
> > - spin_lock(q->queue_lock);
> > rq = blk_fetch_request(q);
> > - spin_unlock(q->queue_lock);
> >
> > if (!rq)
> > break;
> >
> > - tx_data = rq->special;
> > -
> > - ret = __mbox_msg_send(mbox, tx_data->msg);
> > + ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
> > if (ret) {
> > omap_mbox_enable_irq(mbox, IRQ_TX);
> > - spin_lock(q->queue_lock);
> > blk_requeue_request(q, rq);
> > - spin_unlock(q->queue_lock);
> > return;
> > }
> > -
> > - spin_lock(q->queue_lock);
> > - __blk_end_request_all(rq, 0);
> > - spin_unlock(q->queue_lock);
> > + /* FIXME: We get a WARN_ON() if __blk_end_request_all()
> > + * is used. Not sure if we can remove the queue locks
> > + * for blk_requeue_request() and blk_fetch_request()
> > + * calls as well.*/
> > + blk_end_request_all(rq, 0);
> > }
> > }
> >
> > While testing I got a WARN_ON() using the __blk_end_request_all().
> > Tried using the blk_end_request_all() instead, and that
> worked fine.
> > Is it safe to remove the spin lock protection for all the
> calls inside
> > the tasklet function as you had suggested?
> > Please comment.
>
> I think that it's safe since it's being executed in tasklet
> context, no preemption.
>
> Which WARN_ON() did you get?
>
WARNING: at include/linux/blkdev.h:522
WARN_ON_ONCE(!queue_is_locked(q));
The __blk_end_request_all() needs the queue to be locked before making the call.
However, the blk_end_request_all() call does not have this requirement.
> >
> > > @@ -266,7 +256,7 @@ static void
> __mbox_tx_interrupt(struct omap_mbox
> > > *mbox) {
> > > disable_mbox_irq(mbox, IRQ_TX);
> > > ack_mbox_irq(mbox, IRQ_TX);
> > > - schedule_work(&mbox->txq->work);
> > > + tasklet_schedule(&mbox->tx_tasklet);
> > > }
> > >
> > > static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@
> > > -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox)
> > > goto fail_request_irq;
> > > }
> > >
> >
> > Changes in the names used for work queue (rx_work) and tasklet as
> > tx_tasklet.
>
> As I explained above, the prefix 'tx_/rx_' isn't necessary.
>
I will remove the tx_ rx_ as you had suggested.
> >
> > @@ -157,7 +132,7 @@ static void mbox_tx_work(struct
> work_struct *work)
> > static void mbox_rx_work(struct work_struct *work) {
> > struct omap_mbox_queue *mq =
> > - container_of(work, struct
> omap_mbox_queue, work);
> > + container_of(work, struct
> omap_mbox_queue, rx_work);
> > struct omap_mbox *mbox = mq->queue->queuedata;
> > struct request_queue *q = mbox->rxq->queue;
> > struct request *rq;
> > @@ -192,7 +167,7 @@ static void __mbox_tx_interrupt(struct
> omap_mbox
> > *mbox) {
> > omap_mbox_disable_irq(mbox, IRQ_TX);
> > ack_mbox_irq(mbox, IRQ_TX);
> > - schedule_work(&mbox->txq->work);
> > + tasklet_schedule(&mbox->txq->tx_tasklet);
> > }
> >
> > static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@ -217,7
> > +192,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> > /* no more messages in the fifo. clear IRQ source. */
> > ack_mbox_irq(mbox, IRQ_RX);
> > nomem:
> > - schedule_work(&mbox->rxq->work);
> > + schedule_work(&mbox->rxq->rx_work);
> > }
> >
> >
> > > - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> > > + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet,
> > > (unsigned long)mbox);
> > > +
> > > + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
> > > if (!mq) {
> > > ret = -ENOMEM;
> > > goto fail_alloc_txq;
> > >
> > >
> >
> > Changed the signature of the mbox_queue_alloc function.
> > The work queue / tasklet initialization is done in the
> > omap_mbox_startup() function.
>
> why?
The mbox_queue_alloc() currently, takes only the work queue function
as an argument. With the tasklet introduced, I felt it is better to
have work queue/ tasklet initializations,done outside the
mbox_queue_alloc() function.
Doing the tasklet initializtion in startup looks more like a work-around.
Another option would be to pass both the work_queue and tasklet functions
as arguments to the mbox_queue_alloc() function.
Please comment.
>
> >
> > static struct omap_mbox_queue *mbox_queue_alloc(struct
> omap_mbox *mbox,
> > - request_fn_proc *proc,
> > - void (*work) (struct
> work_struct *))
> > + request_fn_proc *proc)
> > {
> > struct request_queue *q;
> > struct omap_mbox_queue *mq;
> > @@ -252,8 +226,6 @@ static struct omap_mbox_queue
> *mbox_queue_alloc(struct omap_mbox *mbox,
> > q->queuedata = mbox;
> > mq->queue = q;
> >
> > - INIT_WORK(&mq->work, work);
> > -
> > return mq;
> > error:
> > kfree(mq);
> > @@ -292,18 +264,22 @@ static int omap_mbox_startup(struct
> omap_mbox *mbox)
> > goto fail_request_irq;
> > }
> >
> > - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> > + mq = mbox_queue_alloc(mbox, mbox_txq_fn);
> > if (!mq) {
> > ret = -ENOMEM;
> > goto fail_alloc_txq;
> > }
> > +
> > + tasklet_init(&mq->tx_tasklet, mbox_tx_tasklet,
> (unsigned long)mbox);
> > mbox->txq = mq;
> >
> > - mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work);
> > + mq = mbox_queue_alloc(mbox, mbox_rxq_fn);
> > if (!mq) {
> > ret = -ENOMEM;
> > goto fail_alloc_rxq;
> > }
> > +
> > + INIT_WORK(&mq->rx_work, mbox_rx_work);
> > mbox->rxq = mq;
> >
> > return 0;
> > --
> >
> > Please give your comments on the changes.
> > --
> > 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] 12+ messages in thread
* Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-09-14 11:35 ` C.A, Subramaniam
@ 2009-09-14 12:43 ` Hiroshi DOYU
0 siblings, 0 replies; 12+ messages in thread
From: Hiroshi DOYU @ 2009-09-14 12:43 UTC (permalink / raw)
To: subramaniam.ca; +Cc: linux-omap, tony, rmk, h-kanigeri2, grgupta
Hi Subbu,
From: "ext C.A, Subramaniam" <subramaniam.ca@ti.com>
Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
Date: Mon, 14 Sep 2009 13:35:23 +0200
[...]
> > > -
> > > - spin_lock(q->queue_lock);
> > > - __blk_end_request_all(rq, 0);
> > > - spin_unlock(q->queue_lock);
> > > + /* FIXME: We get a WARN_ON() if __blk_end_request_all()
> > > + * is used. Not sure if we can remove the queue locks
> > > + * for blk_requeue_request() and blk_fetch_request()
> > > + * calls as well.*/
> > > + blk_end_request_all(rq, 0);
> > > }
> > > }
> > >
> > > While testing I got a WARN_ON() using the __blk_end_request_all().
> > > Tried using the blk_end_request_all() instead, and that
> > worked fine.
> > > Is it safe to remove the spin lock protection for all the
> > calls inside
> > > the tasklet function as you had suggested?
> > > Please comment.
> >
> > I think that it's safe since it's being executed in tasklet
> > context, no preemption.
> >
> > Which WARN_ON() did you get?
> >
>
> WARNING: at include/linux/blkdev.h:522
> WARN_ON_ONCE(!queue_is_locked(q));
> The __blk_end_request_all() needs the queue to be locked before making the call.
> However, the blk_end_request_all() call does not have this requirement.
Although it's safe without locking, __blk_end_request_all() wasn't
supposed to be used in tasklet context.
Let's use "blk_end_request_all()" until we'll come up with a better
idea while I think that request queue may be a little bit too heavy
for this porpose.
[...]
> > > static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@ -217,7
> > > +192,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
> > > /* no more messages in the fifo. clear IRQ source. */
> > > ack_mbox_irq(mbox, IRQ_RX);
> > > nomem:
> > > - schedule_work(&mbox->rxq->work);
> > > + schedule_work(&mbox->rxq->rx_work);
> > > }
> > >
> > >
> > > > - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
> > > > + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet,
> > > > (unsigned long)mbox);
> > > > +
> > > > + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL);
> > > > if (!mq) {
> > > > ret = -ENOMEM;
> > > > goto fail_alloc_txq;
> > > >
> > > >
> > >
> > > Changed the signature of the mbox_queue_alloc function.
> > > The work queue / tasklet initialization is done in the
> > > omap_mbox_startup() function.
> >
> > why?
>
> The mbox_queue_alloc() currently, takes only the work queue function
> as an argument. With the tasklet introduced, I felt it is better to
> have work queue/ tasklet initializations,done outside the
> mbox_queue_alloc() function.
>
> Doing the tasklet initializtion in startup looks more like a work-around.
> Another option would be to pass both the work_queue and tasklet functions
> as arguments to the mbox_queue_alloc() function.
I second passing tasklet func as argument for mbox_queue_alloc because
mbox_queue_alloc() was originally introduced to avoid repeating the
same initialization steps for rx and tx. For me, doing all
initialization per mbox queue in this function seems to be the way.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 10/10] omap: mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
2009-11-18 19:19 [PATCH 00/10] Omap mailbox updates for omap4 for 2.6.33 Tony Lindgren
@ 2009-11-18 19:21 ` Tony Lindgren
0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2009-11-18 19:21 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Ramesh Gupta G, linux-omap, Hiroshi DOYU, C A Subramaniam
From: C A Subramaniam <subramaniam.ca@ti.com>
This patch uses a tasklet implementation for
sending mailbox messages.
Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/plat-omap/include/plat/mailbox.h | 8 +++-
arch/arm/plat-omap/mailbox.c | 59 ++++++++---------------------
2 files changed, 23 insertions(+), 44 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index bf06953..729166b 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -6,6 +6,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/blkdev.h>
+#include <linux/interrupt.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -28,8 +29,10 @@ struct omap_mbox_ops {
int (*fifo_empty)(struct omap_mbox *mbox);
int (*fifo_full)(struct omap_mbox *mbox);
/* irq */
- void (*enable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
- void (*disable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+ void (*enable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
+ void (*disable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
void (*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
int (*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
/* ctx */
@@ -41,6 +44,7 @@ struct omap_mbox_queue {
spinlock_t lock;
struct request_queue *queue;
struct work_struct work;
+ struct tasklet_struct tasklet;
int (*callback)(void *);
struct omap_mbox *mbox;
};
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 4d7947e..8e90633 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -80,74 +80,45 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return ret;
}
-struct omap_msg_tx_data {
- mbox_msg_t msg;
-};
-
-static void omap_msg_tx_end_io(struct request *rq, int error)
-{
- kfree(rq->special);
- __blk_put_request(rq->q, rq);
-}
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
- struct omap_msg_tx_data *tx_data;
+
struct request *rq;
struct request_queue *q = mbox->txq->queue;
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (unlikely(!tx_data))
- return -ENOMEM;
-
rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq)) {
- kfree(tx_data);
+ if (unlikely(!rq))
return -ENOMEM;
- }
- tx_data->msg = msg;
- rq->end_io = omap_msg_tx_end_io;
- blk_insert_request(q, rq, 0, tx_data);
+ blk_insert_request(q, rq, 0, (void *) msg);
+ tasklet_schedule(&mbox->txq->tasklet);
- schedule_work(&mbox->txq->work);
return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
-static void mbox_tx_work(struct work_struct *work)
+static void mbox_tx_tasklet(unsigned long tx_data)
{
int ret;
struct request *rq;
- struct omap_mbox_queue *mq = container_of(work,
- struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
+ struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
struct request_queue *q = mbox->txq->queue;
while (1) {
- struct omap_msg_tx_data *tx_data;
- spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
- spin_unlock(q->queue_lock);
if (!rq)
break;
- tx_data = rq->special;
-
- ret = __mbox_msg_send(mbox, tx_data->msg);
+ ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
if (ret) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- spin_lock(q->queue_lock);
blk_requeue_request(q, rq);
- spin_unlock(q->queue_lock);
return;
}
-
- spin_lock(q->queue_lock);
- __blk_end_request_all(rq, 0);
- spin_unlock(q->queue_lock);
+ blk_end_request_all(rq, 0);
}
}
@@ -192,7 +163,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
ack_mbox_irq(mbox, IRQ_TX);
- schedule_work(&mbox->txq->work);
+ tasklet_schedule(&mbox->txq->tasklet);
}
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
@@ -235,7 +206,8 @@ 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 (*work) (struct work_struct *),
+ void (*tasklet)(unsigned long))
{
struct request_queue *q;
struct omap_mbox_queue *mq;
@@ -252,8 +224,11 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
q->queuedata = mbox;
mq->queue = q;
- INIT_WORK(&mq->work, work);
+ if (work)
+ INIT_WORK(&mq->work, work);
+ if (tasklet)
+ tasklet_init(&mq->tasklet, tasklet, (unsigned long)mbox);
return mq;
error:
kfree(mq);
@@ -292,14 +267,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
+ mq = mbox_queue_alloc(mbox, mbox_txq_fn, 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);
+ mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 10/10] omap: mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
@ 2009-11-18 19:21 ` Tony Lindgren
0 siblings, 0 replies; 12+ messages in thread
From: Tony Lindgren @ 2009-11-18 19:21 UTC (permalink / raw)
To: linux-arm-kernel
From: C A Subramaniam <subramaniam.ca@ti.com>
This patch uses a tasklet implementation for
sending mailbox messages.
Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
arch/arm/plat-omap/include/plat/mailbox.h | 8 +++-
arch/arm/plat-omap/mailbox.c | 59 ++++++++---------------------
2 files changed, 23 insertions(+), 44 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index bf06953..729166b 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -6,6 +6,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/blkdev.h>
+#include <linux/interrupt.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -28,8 +29,10 @@ struct omap_mbox_ops {
int (*fifo_empty)(struct omap_mbox *mbox);
int (*fifo_full)(struct omap_mbox *mbox);
/* irq */
- void (*enable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
- void (*disable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+ void (*enable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
+ void (*disable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
void (*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
int (*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
/* ctx */
@@ -41,6 +44,7 @@ struct omap_mbox_queue {
spinlock_t lock;
struct request_queue *queue;
struct work_struct work;
+ struct tasklet_struct tasklet;
int (*callback)(void *);
struct omap_mbox *mbox;
};
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 4d7947e..8e90633 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -80,74 +80,45 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return ret;
}
-struct omap_msg_tx_data {
- mbox_msg_t msg;
-};
-
-static void omap_msg_tx_end_io(struct request *rq, int error)
-{
- kfree(rq->special);
- __blk_put_request(rq->q, rq);
-}
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
- struct omap_msg_tx_data *tx_data;
+
struct request *rq;
struct request_queue *q = mbox->txq->queue;
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (unlikely(!tx_data))
- return -ENOMEM;
-
rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq)) {
- kfree(tx_data);
+ if (unlikely(!rq))
return -ENOMEM;
- }
- tx_data->msg = msg;
- rq->end_io = omap_msg_tx_end_io;
- blk_insert_request(q, rq, 0, tx_data);
+ blk_insert_request(q, rq, 0, (void *) msg);
+ tasklet_schedule(&mbox->txq->tasklet);
- schedule_work(&mbox->txq->work);
return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
-static void mbox_tx_work(struct work_struct *work)
+static void mbox_tx_tasklet(unsigned long tx_data)
{
int ret;
struct request *rq;
- struct omap_mbox_queue *mq = container_of(work,
- struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
+ struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
struct request_queue *q = mbox->txq->queue;
while (1) {
- struct omap_msg_tx_data *tx_data;
- spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
- spin_unlock(q->queue_lock);
if (!rq)
break;
- tx_data = rq->special;
-
- ret = __mbox_msg_send(mbox, tx_data->msg);
+ ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
if (ret) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- spin_lock(q->queue_lock);
blk_requeue_request(q, rq);
- spin_unlock(q->queue_lock);
return;
}
-
- spin_lock(q->queue_lock);
- __blk_end_request_all(rq, 0);
- spin_unlock(q->queue_lock);
+ blk_end_request_all(rq, 0);
}
}
@@ -192,7 +163,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
ack_mbox_irq(mbox, IRQ_TX);
- schedule_work(&mbox->txq->work);
+ tasklet_schedule(&mbox->txq->tasklet);
}
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
@@ -235,7 +206,8 @@ 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 (*work) (struct work_struct *),
+ void (*tasklet)(unsigned long))
{
struct request_queue *q;
struct omap_mbox_queue *mq;
@@ -252,8 +224,11 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
q->queuedata = mbox;
mq->queue = q;
- INIT_WORK(&mq->work, work);
+ if (work)
+ INIT_WORK(&mq->work, work);
+ if (tasklet)
+ tasklet_init(&mq->tasklet, tasklet, (unsigned long)mbox);
return mq;
error:
kfree(mq);
@@ -292,14 +267,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
+ mq = mbox_queue_alloc(mbox, mbox_txq_fn, 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);
+ mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
@ 2009-11-13 12:34 C.A, Subramaniam
0 siblings, 0 replies; 12+ messages in thread
From: C.A, Subramaniam @ 2009-11-13 12:34 UTC (permalink / raw)
To: Tony Lindgren, linux-omap; +Cc: Gupta, Ramesh, Kanigeri, Hari, Hiroshi DOYU
>From 5fd7c2bfae11879edfcae7db073deb11bea1f584 Mon Sep 17 00:00:00 2001
From: C A Subramaniam <subramaniam.ca@ti.com>
Date: Fri, 13 Nov 2009 15:59:58 +0530
Subject: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation
This patch uses a tasklet implementation for
sending mailbox messages.
Signed-off-by: C A Subramaniam <subramaniam.ca@ti.com>
Signed-off-by: Ramesh Gupta G <grgupta@ti.com>
Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
arch/arm/plat-omap/include/plat/mailbox.h | 8 +++-
arch/arm/plat-omap/mailbox.c | 59 ++++++++--------------------
2 files changed, 23 insertions(+), 44 deletions(-)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index bf06953..729166b 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -6,6 +6,7 @@
#include <linux/wait.h>
#include <linux/workqueue.h>
#include <linux/blkdev.h>
+#include <linux/interrupt.h>
typedef u32 mbox_msg_t;
struct omap_mbox;
@@ -28,8 +29,10 @@ struct omap_mbox_ops {
int (*fifo_empty)(struct omap_mbox *mbox);
int (*fifo_full)(struct omap_mbox *mbox);
/* irq */
- void (*enable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
- void (*disable_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
+ void (*enable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
+ void (*disable_irq)(struct omap_mbox *mbox,
+ omap_mbox_irq_t irq);
void (*ack_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
int (*is_irq)(struct omap_mbox *mbox, omap_mbox_irq_t irq);
/* ctx */
@@ -41,6 +44,7 @@ struct omap_mbox_queue {
spinlock_t lock;
struct request_queue *queue;
struct work_struct work;
+ struct tasklet_struct tasklet;
int (*callback)(void *);
struct omap_mbox *mbox;
};
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 4d7947e..8e90633 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -80,74 +80,45 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
return ret;
}
-struct omap_msg_tx_data {
- mbox_msg_t msg;
-};
-
-static void omap_msg_tx_end_io(struct request *rq, int error)
-{
- kfree(rq->special);
- __blk_put_request(rq->q, rq);
-}
int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
{
- struct omap_msg_tx_data *tx_data;
+
struct request *rq;
struct request_queue *q = mbox->txq->queue;
- tx_data = kmalloc(sizeof(*tx_data), GFP_ATOMIC);
- if (unlikely(!tx_data))
- return -ENOMEM;
-
rq = blk_get_request(q, WRITE, GFP_ATOMIC);
- if (unlikely(!rq)) {
- kfree(tx_data);
+ if (unlikely(!rq))
return -ENOMEM;
- }
- tx_data->msg = msg;
- rq->end_io = omap_msg_tx_end_io;
- blk_insert_request(q, rq, 0, tx_data);
+ blk_insert_request(q, rq, 0, (void *) msg);
+ tasklet_schedule(&mbox->txq->tasklet);
- schedule_work(&mbox->txq->work);
return 0;
}
EXPORT_SYMBOL(omap_mbox_msg_send);
-static void mbox_tx_work(struct work_struct *work)
+static void mbox_tx_tasklet(unsigned long tx_data)
{
int ret;
struct request *rq;
- struct omap_mbox_queue *mq = container_of(work,
- struct omap_mbox_queue, work);
- struct omap_mbox *mbox = mq->queue->queuedata;
+ struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
struct request_queue *q = mbox->txq->queue;
while (1) {
- struct omap_msg_tx_data *tx_data;
- spin_lock(q->queue_lock);
rq = blk_fetch_request(q);
- spin_unlock(q->queue_lock);
if (!rq)
break;
- tx_data = rq->special;
-
- ret = __mbox_msg_send(mbox, tx_data->msg);
+ ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
if (ret) {
omap_mbox_enable_irq(mbox, IRQ_TX);
- spin_lock(q->queue_lock);
blk_requeue_request(q, rq);
- spin_unlock(q->queue_lock);
return;
}
-
- spin_lock(q->queue_lock);
- __blk_end_request_all(rq, 0);
- spin_unlock(q->queue_lock);
+ blk_end_request_all(rq, 0);
}
}
@@ -192,7 +163,7 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
{
omap_mbox_disable_irq(mbox, IRQ_TX);
ack_mbox_irq(mbox, IRQ_TX);
- schedule_work(&mbox->txq->work);
+ tasklet_schedule(&mbox->txq->tasklet);
}
static void __mbox_rx_interrupt(struct omap_mbox *mbox)
@@ -235,7 +206,8 @@ 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 (*work) (struct work_struct *),
+ void (*tasklet)(unsigned long))
{
struct request_queue *q;
struct omap_mbox_queue *mq;
@@ -252,8 +224,11 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
q->queuedata = mbox;
mq->queue = q;
- INIT_WORK(&mq->work, work);
+ if (work)
+ INIT_WORK(&mq->work, work);
+ if (tasklet)
+ tasklet_init(&mq->tasklet, tasklet, (unsigned long)mbox);
return mq;
error:
kfree(mq);
@@ -292,14 +267,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
goto fail_request_irq;
}
- mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work);
+ mq = mbox_queue_alloc(mbox, mbox_txq_fn, 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);
+ mq = mbox_queue_alloc(mbox, mbox_rxq_fn, mbox_rx_work, NULL);
if (!mq) {
ret = -ENOMEM;
goto fail_alloc_rxq;
--
1.5.3.2
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2009-11-18 19:21 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-04 11:48 [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation C.A, Subramaniam
2009-09-07 8:49 ` Hiroshi DOYU
2009-09-08 17:43 ` C.A, Subramaniam
2009-09-09 9:02 ` Hiroshi DOYU
2009-09-09 9:14 ` C.A, Subramaniam
2009-09-10 11:46 ` C.A, Subramaniam
2009-09-10 12:28 ` Hiroshi DOYU
2009-09-14 11:35 ` C.A, Subramaniam
2009-09-14 12:43 ` Hiroshi DOYU
2009-11-13 12:34 C.A, Subramaniam
2009-11-18 19:19 [PATCH 00/10] Omap mailbox updates for omap4 for 2.6.33 Tony Lindgren
2009-11-18 19:21 ` [PATCH 10/10] omap: mailbox: OMAP4 Mailbox-driver Patch to support tasklet implementation Tony Lindgren
2009-11-18 19:21 ` Tony Lindgren
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.