RE: [PATCH] mailbox: change full flag per mailbox queue instead of global

2010-08-10 Thread Guzman Lugo, Fernando


> -Original Message-
> From: Hiroshi DOYU [mailto:hiroshi.d...@nokia.com]
> Sent: Tuesday, August 10, 2010 10:58 AM
> To: Guzman Lugo, Fernando; o...@wizery.com
> Cc: linux-arm-ker...@lists.infradead.org; linux-omap@vger.kernel.org
> Subject: Re: [PATCH] mailbox: change full flag per mailbox queue instead
> of global
> 
> From: ext Ohad Ben-Cohen 
> Subject: Re: [PATCH] mailbox: change full flag per mailbox queue instead
> of global
> Date: Tue, 10 Aug 2010 16:36:42 +0200
> 
> > Hi Hiroshi,
> >
> > On Mon, Jun 14, 2010 at 7:51 PM, Fernando Guzman Lugo 
> wrote:
> >> As pointed by Ben Ohand, the variable rq_full flag is a global
> >> variable, so if there are multiple mailbox users there will be
> >> conflics. Now there is a full flag per mailbox queue.
> >>
> >> Reported-by: Ohad Ben-Cohen 
> >> Signed-off-by: Fernando Guzman Lugo 
> >
> > It looks like this fix didn't get thru - can you please review and
> ack/nack ?
> 
> Sure.
> 
> Fernando, could you rebase this patch against the latest omap/master?

No problem, I will send it again.

Regards,
Fernando.

--
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] mailbox: change full flag per mailbox queue instead of global

2010-08-10 Thread Hiroshi DOYU
From: ext Ohad Ben-Cohen 
Subject: Re: [PATCH] mailbox: change full flag per mailbox queue instead of 
global
Date: Tue, 10 Aug 2010 16:36:42 +0200

> Hi Hiroshi,
> 
> On Mon, Jun 14, 2010 at 7:51 PM, Fernando Guzman Lugo  wrote:
>> As pointed by Ben Ohand, the variable rq_full flag is a global
>> variable, so if there are multiple mailbox users there will be
>> conflics. Now there is a full flag per mailbox queue.
>>
>> Reported-by: Ohad Ben-Cohen 
>> Signed-off-by: Fernando Guzman Lugo 
> 
> It looks like this fix didn't get thru - can you please review and ack/nack ?

Sure.

Fernando, could you rebase this patch against the latest omap/master?
--
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] mailbox: change full flag per mailbox queue instead of global

2010-08-10 Thread Ohad Ben-Cohen
Hi Hiroshi,

On Mon, Jun 14, 2010 at 7:51 PM, Fernando Guzman Lugo  wrote:
> As pointed by Ben Ohand, the variable rq_full flag is a global
> variable, so if there are multiple mailbox users there will be
> conflics. Now there is a full flag per mailbox queue.
>
> Reported-by: Ohad Ben-Cohen 
> Signed-off-by: Fernando Guzman Lugo 

It looks like this fix didn't get thru - can you please review and ack/nack ?

Thanks,
Ohad.

> ---
>  arch/arm/plat-omap/include/plat/mailbox.h |    1 +
>  arch/arm/plat-omap/mailbox.c              |    7 +++
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h 
> b/arch/arm/plat-omap/include/plat/mailbox.h
> index 729166b..a6144b8 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -47,6 +47,7 @@ struct omap_mbox_queue {
>        struct tasklet_struct   tasklet;
>        int     (*callback)(void *);
>        struct omap_mbox        *mbox;
> +       bool    full;
>  };
>
>  struct omap_mbox {
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 8d86b0b..a1e274e 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -30,7 +30,6 @@
>
>  static struct omap_mbox *mboxes;
>  static DEFINE_RWLOCK(mboxes_lock);
> -static bool rq_full;
>
>  static int mbox_configured;
>  static DEFINE_MUTEX(mbox_configured_lock);
> @@ -140,9 +139,9 @@ static void mbox_rx_work(struct work_struct *work)
>        while (1) {
>                spin_lock_irqsave(q->queue_lock, flags);
>                rq = blk_fetch_request(q);
> -               if (rq_full) {
> +               if (mbox->rxq->full) {
>                        omap_mbox_enable_irq(mbox, IRQ_RX);
> -                       rq_full = false;
> +                       mbox->rxq->full = false;
>                }
>                spin_unlock_irqrestore(q->queue_lock, flags);
>                if (!rq)
> @@ -183,7 +182,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
>                rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>                if (unlikely(!rq)) {
>                        omap_mbox_disable_irq(mbox, IRQ_RX);
> -                       rq_full = true;
> +                       mbox->rxq->full = true;
>                        goto nomem;
>                }
>
> --
> 1.6.3.3
>
>
--
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] mailbox: change full flag per mailbox queue instead of global

2010-06-17 Thread Guzman Lugo, Fernando
Hi Ohad,

> -Original Message-
> From: Ohad Ben-Cohen [mailto:o...@wizery.com]
> Sent: Wednesday, June 16, 2010 4:18 PM
> To: Guzman Lugo, Fernando
> Cc: linux-arm-ker...@lists.infradead.org; linux-omap@vger.kernel.org;
> hiroshi.d...@nokia.com
> Subject: Re: [PATCH] mailbox: change full flag per mailbox queue instead
> of global
> 
> On Mon, Jun 14, 2010 at 11:51 AM, Fernando Guzman Lugo 
> wrote:
> > As pointed by Ben Ohand, the variable rq_full flag is a global
> > variable, so if there are multiple mailbox users there will be
> > conflics. Now there is a full flag per mailbox queue.
> >
> > Reported-by: Ohad Ben-Cohen 
> > Signed-off-by: Fernando Guzman Lugo 
> > ---
> >  arch/arm/plat-omap/include/plat/mailbox.h |    1 +
> >  arch/arm/plat-omap/mailbox.c              |    7 +++
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> 
> Thanks, Fernando, it looks good !
> 
> Since the original patch wasn't merged yet, it might make more sense
> to squash it with this fix into a single patch, instead of introducing
> this fix as a new patch. what do you think ?

Sound good. I will send that patch again then. Please discard this pathc.

Regards,
Fernando.

> 
> >
> > diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-
> omap/include/plat/mailbox.h
> > index 729166b..a6144b8 100644
> > --- a/arch/arm/plat-omap/include/plat/mailbox.h
> > +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> > @@ -47,6 +47,7 @@ struct omap_mbox_queue {
> >        struct tasklet_struct   tasklet;
> >        int     (*callback)(void *);
> >        struct omap_mbox        *mbox;
> > +       bool    full;
> >  };
> >
> >  struct omap_mbox {
> > diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> > index 8d86b0b..a1e274e 100644
> > --- a/arch/arm/plat-omap/mailbox.c
> > +++ b/arch/arm/plat-omap/mailbox.c
> > @@ -30,7 +30,6 @@
> >
> >  static struct omap_mbox *mboxes;
> >  static DEFINE_RWLOCK(mboxes_lock);
> > -static bool rq_full;
> >
> >  static int mbox_configured;
> >  static DEFINE_MUTEX(mbox_configured_lock);
> > @@ -140,9 +139,9 @@ static void mbox_rx_work(struct work_struct *work)
> >        while (1) {
> >                spin_lock_irqsave(q->queue_lock, flags);
> >                rq = blk_fetch_request(q);
> > -               if (rq_full) {
> > +               if (mbox->rxq->full) {
> >                        omap_mbox_enable_irq(mbox, IRQ_RX);
> > -                       rq_full = false;
> > +                       mbox->rxq->full = false;
> >                }
> >                spin_unlock_irqrestore(q->queue_lock, flags);
> >                if (!rq)
> > @@ -183,7 +182,7 @@ static void __mbox_rx_interrupt(struct omap_mbox
> *mbox)
> >                rq = blk_get_request(q, WRITE, GFP_ATOMIC);
> >                if (unlikely(!rq)) {
> >                        omap_mbox_disable_irq(mbox, IRQ_RX);
> > -                       rq_full = true;
> > +                       mbox->rxq->full = true;
> >                        goto nomem;
> >                }
> >
> > --
> > 1.6.3.3
> >
> >
--
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] mailbox: change full flag per mailbox queue instead of global

2010-06-16 Thread Ohad Ben-Cohen
On Mon, Jun 14, 2010 at 11:51 AM, Fernando Guzman Lugo  wrote:
> As pointed by Ben Ohand, the variable rq_full flag is a global
> variable, so if there are multiple mailbox users there will be
> conflics. Now there is a full flag per mailbox queue.
>
> Reported-by: Ohad Ben-Cohen 
> Signed-off-by: Fernando Guzman Lugo 
> ---
>  arch/arm/plat-omap/include/plat/mailbox.h |    1 +
>  arch/arm/plat-omap/mailbox.c              |    7 +++
>  2 files changed, 4 insertions(+), 4 deletions(-)


Thanks, Fernando, it looks good !

Since the original patch wasn't merged yet, it might make more sense
to squash it with this fix into a single patch, instead of introducing
this fix as a new patch. what do you think ?

>
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h 
> b/arch/arm/plat-omap/include/plat/mailbox.h
> index 729166b..a6144b8 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -47,6 +47,7 @@ struct omap_mbox_queue {
>        struct tasklet_struct   tasklet;
>        int     (*callback)(void *);
>        struct omap_mbox        *mbox;
> +       bool    full;
>  };
>
>  struct omap_mbox {
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 8d86b0b..a1e274e 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -30,7 +30,6 @@
>
>  static struct omap_mbox *mboxes;
>  static DEFINE_RWLOCK(mboxes_lock);
> -static bool rq_full;
>
>  static int mbox_configured;
>  static DEFINE_MUTEX(mbox_configured_lock);
> @@ -140,9 +139,9 @@ static void mbox_rx_work(struct work_struct *work)
>        while (1) {
>                spin_lock_irqsave(q->queue_lock, flags);
>                rq = blk_fetch_request(q);
> -               if (rq_full) {
> +               if (mbox->rxq->full) {
>                        omap_mbox_enable_irq(mbox, IRQ_RX);
> -                       rq_full = false;
> +                       mbox->rxq->full = false;
>                }
>                spin_unlock_irqrestore(q->queue_lock, flags);
>                if (!rq)
> @@ -183,7 +182,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
>                rq = blk_get_request(q, WRITE, GFP_ATOMIC);
>                if (unlikely(!rq)) {
>                        omap_mbox_disable_irq(mbox, IRQ_RX);
> -                       rq_full = true;
> +                       mbox->rxq->full = true;
>                        goto nomem;
>                }
>
> --
> 1.6.3.3
>
>
--
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