Hi Hiroshi,

> -----Original Message-----
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com] 
> Sent: Wednesday, September 09, 2009 2:32 PM
> To: C.A, Subramaniam
> Cc: linux-omap@vger.kernel.org; t...@atomide.com; 
> r...@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...@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;
> 
> --
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

Reply via email to