Phil,

Thanks for looking at the patch.


On Wed, Jul 21, 2010 at 4:45 AM,  <ext-phil.2.carm...@nokia.com> wrote:
> Apologies - top posting and not quoting properly due to having to use MS's 
> braindead OWA.
>
> It appears that most of the "changes" are simply indentation changes caused 
> by the inclusion of a new inner block here:

-- I don't know if we can say most of the changes are simply
indentation changes.

> +       if (atomic_inc_return(&mbox->use_count) == 1) {
> rather than just using a goto to skip past the unneeded parts. (Hmmm, is it 
> not simply an immediate return now?)
> The goto/return is both more idiomatic in linux, and I'm sure a simpler patch.

-- Sure, I will make the change.

>
> Phil
> ________________________________________
> From: linux-omap-ow...@vger.kernel.org [linux-omap-ow...@vger.kernel.org] On 
> Behalf Of ext Hari Kanigeri [h-kanige...@ti.com]
> Sent: 21 July 2010 01:41
> To: Linux Omap; Tony Lindgren; Doyu Hiroshi (Nokia-MS/Helsinki)
> Cc: Ohad Ben-Cohen; Hari Kanigeri
> Subject: [PATCH 2/2] omap:mailbox-provide multiple reader support
>
> This patch provides mutiple readers support for a mailbox
> instance.
>
> Signed-off-by: Hari Kanigeri <h-kanige...@ti.com>
> ---
>  arch/arm/plat-omap/include/plat/mailbox.h |    6 ++-
>  arch/arm/plat-omap/mailbox.c              |   63 ++++++++++++++++------------
>  2 files changed, 40 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h 
> b/arch/arm/plat-omap/include/plat/mailbox.h
> index 0486d64..c8e47d8 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -68,13 +68,15 @@ struct omap_mbox {
>        void                    *priv;
>
>        void                    (*err_notify)(void);
> +       atomic_t                use_count;
> +       struct blocking_notifier_head   notifier;
>  };
>
>  int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
>  void omap_mbox_init_seq(struct omap_mbox *);
>
> -struct omap_mbox *omap_mbox_get(const char *);
> -void omap_mbox_put(struct omap_mbox *);
> +struct omap_mbox *omap_mbox_get(const char *, struct notifier_block *nb);
> +void omap_mbox_put(struct omap_mbox *, struct notifier_block *nb);
>
>  int omap_mbox_register(struct device *parent, struct omap_mbox *);
>  int omap_mbox_unregister(struct omap_mbox *);
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index baac315..f9f2af4 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -149,8 +149,8 @@ static void mbox_rx_work(struct work_struct *work)
>                if (unlikely(len != sizeof(msg)))
>                        pr_err("%s: kfifo_out anomaly detected\n", __func__);
>
> -               if (mq->callback)
> -                       mq->callback((void *)msg);
> +               blocking_notifier_call_chain(&mq->mbox->notifier, len,
> +                                                       (void *)msg);
>        }
>  }
>
> @@ -252,28 +252,30 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>                }
>        }
>
> -       ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
> -                               mbox->name, mbox);
> -       if (unlikely(ret)) {
> -               printk(KERN_ERR
> -                       "failed to register mailbox interrupt:%d\n", ret);
> -               goto fail_request_irq;
> -       }
> +       if (atomic_inc_return(&mbox->use_count) == 1) {
> +               ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
> +                                       mbox->name, mbox);
> +               if (unlikely(ret)) {
> +                       printk(KERN_ERR "failed to register mailbox 
> interrupt:"
> +                                                               "%d\n", ret);
> +                       goto fail_request_irq;
> +               }
>
> -       mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
> -       if (!mq) {
> -               ret = -ENOMEM;
> -               goto fail_alloc_txq;
> -       }
> -       mbox->txq = mq;
> +               mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet);
> +               if (!mq) {
> +                       ret = -ENOMEM;
> +                       goto fail_alloc_txq;
> +               }
> +               mbox->txq = mq;
>
> -       mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
> -       if (!mq) {
> -               ret = -ENOMEM;
> -               goto fail_alloc_rxq;
> +               mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
> +               if (!mq) {
> +                       ret = -ENOMEM;
> +                       goto fail_alloc_rxq;
> +               }
> +               mbox->rxq = mq;
> +               mq->mbox = mbox;
>        }
> -       mbox->rxq = mq;
> -
>        return 0;
>
>  fail_alloc_rxq:
> @@ -281,6 +283,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>  fail_alloc_txq:
>        free_irq(mbox->irq, mbox);
>  fail_request_irq:
> +       atomic_dec(&mbox->use_count);
>        if (likely(mbox->ops->shutdown)) {
>                if (atomic_dec_return(&mbox_refcount) == 0)
>                        mbox->ops->shutdown(mbox);
> @@ -291,10 +294,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>
>  static void omap_mbox_fini(struct omap_mbox *mbox)
>  {
> -       mbox_queue_free(mbox->txq);
> -       mbox_queue_free(mbox->rxq);
>
> -       free_irq(mbox->irq, mbox);
> +       if (atomic_dec_return(&mbox->use_count) == 0) {
> +               mbox_queue_free(mbox->txq);
> +               mbox_queue_free(mbox->rxq);
> +               free_irq(mbox->irq, mbox);
> +       }
>
>        if (likely(mbox->ops->shutdown)) {
>                if (atomic_dec_return(&mbox_refcount) == 0)
> @@ -314,7 +319,7 @@ static struct omap_mbox **find_mboxes(const char *name)
>        return p;
>  }
>
> -struct omap_mbox *omap_mbox_get(const char *name)
> +struct omap_mbox *omap_mbox_get(const char *name, struct notifier_block *nb)
>  {
>        struct omap_mbox *mbox;
>        int ret;
> @@ -325,19 +330,21 @@ struct omap_mbox *omap_mbox_get(const char *name)
>                spin_unlock(&mboxes_lock);
>                return ERR_PTR(-ENOENT);
>        }
> -
>        spin_unlock(&mboxes_lock);
>
>        ret = omap_mbox_startup(mbox);
>        if (ret)
>                return ERR_PTR(-ENODEV);
> +       if (nb)
> +               blocking_notifier_chain_register(&mbox->notifier, nb);
>
>        return mbox;
>  }
>  EXPORT_SYMBOL(omap_mbox_get);
>
> -void omap_mbox_put(struct omap_mbox *mbox)
> +void omap_mbox_put(struct omap_mbox *mbox, struct notifier_block *nb)
>  {
> +       blocking_notifier_chain_unregister(&mbox->notifier, nb);
>        omap_mbox_fini(mbox);
>  }
>  EXPORT_SYMBOL(omap_mbox_put);
> @@ -361,6 +368,8 @@ int omap_mbox_register(struct device *parent, struct 
> omap_mbox *mbox)
>        }
>        *tmp = mbox;
>        spin_unlock(&mboxes_lock);
> +       BLOCKING_INIT_NOTIFIER_HEAD(&mbox->notifier);
> +       atomic_set(&mbox->use_count, 0);
>
>        return 0;
>
> --
> 1.7.0
>
> --
> 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
> --
> 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
>
--
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