[PATCH v2 0/4] omap: mailbox: cleanup & simplify

[PATCH v2 0/4] omap: mailbox: cleanup & simplify

[Ohad Ben-Cohen]

This series include comments from Hiroshi and Hari (thanks!).

Changes since v1:
- add mbox_kfifo_size module parameter
- WARN_ON kfifo anomalies
- remove redundant blkdev.h file
- fix rwlocks->spinlock conversion

Thanks,
Ohad.

---
If you want, you can also reach me at < ohadb at ti dot com >.

Ohad Ben-Cohen (4):
  omap: mailbox: convert rwlocks to spinlock
  omap: mailbox cleanup: split MODULE_AUTHOR line
  omap: mailbox: fix reverse likeliness
  omap: mailbox: convert block api to kfifo

 arch/arm/mach-omap2/mailbox.c             |    3 +-
 arch/arm/plat-omap/Kconfig                |    9 ++
 arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
 arch/arm/plat-omap/mailbox.c              |  144 +++++++++++++----------------
 4 files changed, 79 insertions(+), 81 deletions(-)

[PATCH v2 1/4] omap: mailbox: convert rwlocks to spinlock

[Ohad Ben-Cohen]

rwlocks are slower and have potential starvation issues
therefore spinlocks are generally preferred.

see also: http://lwn.net/Articles/364583/

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Kanigeri Hari <h-kanigeri2@ti.com>
---
If you want, you can also reach me at < ohadb at ti dot com >.

 arch/arm/plat-omap/mailbox.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 08a2df7..af3a6ac 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -31,7 +31,7 @@
 
 static struct workqueue_struct *mboxd;
 static struct omap_mbox *mboxes;
-static DEFINE_RWLOCK(mboxes_lock);
+static DEFINE_SPINLOCK(mboxes_lock);
 
 static int mbox_configured;
 
@@ -249,16 +249,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
 	struct omap_mbox_queue *mq;
 
 	if (likely(mbox->ops->startup)) {
-		write_lock(&mboxes_lock);
+		spin_lock(&mboxes_lock);
 		if (!mbox_configured)
 			ret = mbox->ops->startup(mbox);
 
 		if (unlikely(ret)) {
-			write_unlock(&mboxes_lock);
+			spin_unlock(&mboxes_lock);
 			return ret;
 		}
 		mbox_configured++;
-		write_unlock(&mboxes_lock);
+		spin_unlock(&mboxes_lock);
 	}
 
 	ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
@@ -304,12 +304,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
 	free_irq(mbox->irq, mbox);
 
 	if (unlikely(mbox->ops->shutdown)) {
-		write_lock(&mboxes_lock);
+		spin_lock(&mboxes_lock);
 		if (mbox_configured > 0)
 			mbox_configured--;
 		if (!mbox_configured)
 			mbox->ops->shutdown(mbox);
-		write_unlock(&mboxes_lock);
+		spin_unlock(&mboxes_lock);
 	}
 }
 
@@ -330,14 +330,14 @@ struct omap_mbox *omap_mbox_get(const char *name)
 	struct omap_mbox *mbox;
 	int ret;
 
-	read_lock(&mboxes_lock);
+	spin_lock(&mboxes_lock);
 	mbox = *(find_mboxes(name));
 	if (mbox == NULL) {
-		read_unlock(&mboxes_lock);
+		spin_unlock(&mboxes_lock);
 		return ERR_PTR(-ENOENT);
 	}
 
-	read_unlock(&mboxes_lock);
+	spin_unlock(&mboxes_lock);
 
 	ret = omap_mbox_startup(mbox);
 	if (ret)
@@ -363,15 +363,15 @@ int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
 	if (mbox->next)
 		return -EBUSY;
 
-	write_lock(&mboxes_lock);
+	spin_lock(&mboxes_lock);
 	tmp = find_mboxes(mbox->name);
 	if (*tmp) {
 		ret = -EBUSY;
-		write_unlock(&mboxes_lock);
+		spin_unlock(&mboxes_lock);
 		goto err_find;
 	}
 	*tmp = mbox;
-	write_unlock(&mboxes_lock);
+	spin_unlock(&mboxes_lock);
 
 	return 0;
 
@@ -384,18 +384,18 @@ int omap_mbox_unregister(struct omap_mbox *mbox)
 {
 	struct omap_mbox **tmp;
 
-	write_lock(&mboxes_lock);
+	spin_lock(&mboxes_lock);
 	tmp = &mboxes;
 	while (*tmp) {
 		if (mbox == *tmp) {
 			*tmp = mbox->next;
 			mbox->next = NULL;
-			write_unlock(&mboxes_lock);
+			spin_unlock(&mboxes_lock);
 			return 0;
 		}
 		tmp = &(*tmp)->next;
 	}
-	write_unlock(&mboxes_lock);
+	spin_unlock(&mboxes_lock);
 
 	return -EINVAL;
 }
-- 
1.6.3.3

[PATCH v2 2/4] omap: mailbox cleanup: split MODULE_AUTHOR line

[Ohad Ben-Cohen]

use multiple MODULE_AUTHOR lines for multiple authors

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
If you want, you can also reach me at < ohadb at ti dot com >.

 arch/arm/mach-omap2/mailbox.c |    3 ++-
 arch/arm/plat-omap/mailbox.c  |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 318f363..763272c 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -486,5 +486,6 @@ module_exit(omap2_mbox_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("omap mailbox: omap2/3/4 architecture specific functions");
-MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>, Paul Mundt");
+MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>");
+MODULE_AUTHOR("Paul Mundt");
 MODULE_ALIAS("platform:"DRV_NAME);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index af3a6ac..5140efc 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -419,4 +419,5 @@ module_exit(omap_mbox_exit);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("omap mailbox: interrupt driven messaging");
-MODULE_AUTHOR("Toshihiro Kobayashi and Hiroshi DOYU");
+MODULE_AUTHOR("Toshihiro Kobayashi");
+MODULE_AUTHOR("Hiroshi DOYU");
-- 
1.6.3.3

[PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Ohad Ben-Cohen]

Fix reverse likeliness

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
If you want, you can also reach me at < ohadb at ti dot com >.

 arch/arm/plat-omap/mailbox.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..5309213 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
  fail_alloc_txq:
 	free_irq(mbox->irq, mbox);
  fail_request_irq:
-	if (unlikely(mbox->ops->shutdown))
+	if (likely(mbox->ops->shutdown))
 		mbox->ops->shutdown(mbox);
 
 	return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
 
 	free_irq(mbox->irq, mbox);
 
-	if (unlikely(mbox->ops->shutdown)) {
+	if (likely(mbox->ops->shutdown)) {
 		spin_lock(&mboxes_lock);
 		if (mbox_configured > 0)
 			mbox_configured--;
-- 
1.6.3.3

Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
> Fix reverse likeliness
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> ---
> If you want, you can also reach me at < ohadb at ti dot com >.
> 
>  arch/arm/plat-omap/mailbox.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 5140efc..5309213 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>   fail_alloc_txq:
>  	free_irq(mbox->irq, mbox);
>   fail_request_irq:
> -	if (unlikely(mbox->ops->shutdown))
> +	if (likely(mbox->ops->shutdown))
>  		mbox->ops->shutdown(mbox);
>  
>  	return ret;
> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>  
>  	free_irq(mbox->irq, mbox);
>  
> -	if (unlikely(mbox->ops->shutdown)) {
> +	if (likely(mbox->ops->shutdown)) {
>  		spin_lock(&mboxes_lock);
>  		if (mbox_configured > 0)
>  			mbox_configured--;

Does this code path need to be optimized? :)

How about just get rid of the (un)likely here?

Regards,

Tony

Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Ohad Ben-Cohen]

On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
>> Fix reverse likeliness
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> ---
>> If you want, you can also reach me at < ohadb at ti dot com >.
>>
>>  arch/arm/plat-omap/mailbox.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index 5140efc..5309213 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>>   fail_alloc_txq:
>>       free_irq(mbox->irq, mbox);
>>   fail_request_irq:
>> -     if (unlikely(mbox->ops->shutdown))
>> +     if (likely(mbox->ops->shutdown))
>>               mbox->ops->shutdown(mbox);
>>
>>       return ret;
>> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>>
>>       free_irq(mbox->irq, mbox);
>>
>> -     if (unlikely(mbox->ops->shutdown)) {
>> +     if (likely(mbox->ops->shutdown)) {
>>               spin_lock(&mboxes_lock);
>>               if (mbox_configured > 0)
>>                       mbox_configured--;
>
> Does this code path need to be optimized? :)
>
> How about just get rid of the (un)likely here?

I like this :)

If we're at it, there are additional cold-path (un)likely macros I
want to target:

From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen <ohad@wizery.com>
Date: Sat, 17 Apr 2010 01:57:43 +0300
Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
 arch/arm/plat-omap/mailbox.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5140efc..7c60550 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
        int ret = 0;
        struct omap_mbox_queue *mq;

-       if (likely(mbox->ops->startup)) {
+       if (mbox->ops->startup) {
                spin_lock(&mboxes_lock);
                if (!mbox_configured)
                        ret = mbox->ops->startup(mbox);

-               if (unlikely(ret)) {
+               if (ret) {
                        spin_unlock(&mboxes_lock);
                        return ret;
                }
@@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)

        ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
                                mbox->name, mbox);
-       if (unlikely(ret)) {
+       if (ret) {
                printk(KERN_ERR
                        "failed to register mailbox interrupt:%d\n", ret);
                goto fail_request_irq;
@@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
  fail_alloc_txq:
        free_irq(mbox->irq, mbox);
  fail_request_irq:
-       if (unlikely(mbox->ops->shutdown))
+       if (mbox->ops->shutdown)
                mbox->ops->shutdown(mbox);

        return ret;
@@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)

        free_irq(mbox->irq, mbox);

-       if (unlikely(mbox->ops->shutdown)) {
+       if (mbox->ops->shutdown) {
                spin_lock(&mboxes_lock);
                if (mbox_configured > 0)
                        mbox_configured--;
-- 
1.6.3.3

I'll wait a day or two for more comments, and send a v3 series.

Thanks,
Ohad.

>
> Regards,
>
> Tony
>
--
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

Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Tony Lindgren]

* Ohad Ben-Cohen <ohad@wizery.com> [100504 04:42]:
> On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@atomide.com> wrote:
> > * Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
> >> Fix reverse likeliness
> >>
> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> >> ---
> >> If you want, you can also reach me at < ohadb at ti dot com >.
> >>
> >>  arch/arm/plat-omap/mailbox.c |    4 ++--
> >>  1 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> >> index 5140efc..5309213 100644
> >> --- a/arch/arm/plat-omap/mailbox.c
> >> +++ b/arch/arm/plat-omap/mailbox.c
> >> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> >>   fail_alloc_txq:
> >>       free_irq(mbox->irq, mbox);
> >>   fail_request_irq:
> >> -     if (unlikely(mbox->ops->shutdown))
> >> +     if (likely(mbox->ops->shutdown))
> >>               mbox->ops->shutdown(mbox);
> >>
> >>       return ret;
> >> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
> >>
> >>       free_irq(mbox->irq, mbox);
> >>
> >> -     if (unlikely(mbox->ops->shutdown)) {
> >> +     if (likely(mbox->ops->shutdown)) {
> >>               spin_lock(&mboxes_lock);
> >>               if (mbox_configured > 0)
> >>                       mbox_configured--;
> >
> > Does this code path need to be optimized? :)
> >
> > How about just get rid of the (un)likely here?
> 
> I like this :)
> 
> If we're at it, there are additional cold-path (un)likely macros I
> want to target:

Looks good to me.

Hiroshi, care to ack/nak all the mailbox and iommu patches you want
me to merge? Or if you have them in some git branch against mainline
-rc6 that would be cool too.

We need to get the ones we want to merge into linux-arm-kernel for
review soon, I don't think they been posted there yet.

Regards,

Tony


 
> From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
> From: Ohad Ben-Cohen <ohad@wizery.com>
> Date: Sat, 17 Apr 2010 01:57:43 +0300
> Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths
> 
> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> ---
>  arch/arm/plat-omap/mailbox.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 5140efc..7c60550 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>         int ret = 0;
>         struct omap_mbox_queue *mq;
> 
> -       if (likely(mbox->ops->startup)) {
> +       if (mbox->ops->startup) {
>                 spin_lock(&mboxes_lock);
>                 if (!mbox_configured)
>                         ret = mbox->ops->startup(mbox);
> 
> -               if (unlikely(ret)) {
> +               if (ret) {
>                         spin_unlock(&mboxes_lock);
>                         return ret;
>                 }
> @@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
> 
>         ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
>                                 mbox->name, mbox);
> -       if (unlikely(ret)) {
> +       if (ret) {
>                 printk(KERN_ERR
>                         "failed to register mailbox interrupt:%d\n", ret);
>                 goto fail_request_irq;
> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>   fail_alloc_txq:
>         free_irq(mbox->irq, mbox);
>   fail_request_irq:
> -       if (unlikely(mbox->ops->shutdown))
> +       if (mbox->ops->shutdown)
>                 mbox->ops->shutdown(mbox);
> 
>         return ret;
> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
> 
>         free_irq(mbox->irq, mbox);
> 
> -       if (unlikely(mbox->ops->shutdown)) {
> +       if (mbox->ops->shutdown) {
>                 spin_lock(&mboxes_lock);
>                 if (mbox_configured > 0)
>                         mbox_configured--;
> -- 
> 1.6.3.3
> 
> I'll wait a day or two for more comments, and send a v3 series.
> 
> Thanks,
> Ohad.
> 
> >
> > Regards,
> >
> > Tony
> >
> --
> 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
--
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

Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Ohad Ben-Cohen]

On Wed, May 5, 2010 at 6:21 PM, Tony Lindgren <tony@atomide.com> wrote:
> * Ohad Ben-Cohen <ohad@wizery.com> [100504 04:42]:
>> On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
>> >> Fix reverse likeliness
>> >>
>> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> >> ---
>> >> If you want, you can also reach me at < ohadb at ti dot com >.
>> >>
>> >>  arch/arm/plat-omap/mailbox.c |    4 ++--
>> >>  1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> >> index 5140efc..5309213 100644
>> >> --- a/arch/arm/plat-omap/mailbox.c
>> >> +++ b/arch/arm/plat-omap/mailbox.c
>> >> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>> >>   fail_alloc_txq:
>> >>       free_irq(mbox->irq, mbox);
>> >>   fail_request_irq:
>> >> -     if (unlikely(mbox->ops->shutdown))
>> >> +     if (likely(mbox->ops->shutdown))
>> >>               mbox->ops->shutdown(mbox);
>> >>
>> >>       return ret;
>> >> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>> >>
>> >>       free_irq(mbox->irq, mbox);
>> >>
>> >> -     if (unlikely(mbox->ops->shutdown)) {
>> >> +     if (likely(mbox->ops->shutdown)) {
>> >>               spin_lock(&mboxes_lock);
>> >>               if (mbox_configured > 0)
>> >>                       mbox_configured--;
>> >
>> > Does this code path need to be optimized? :)
>> >
>> > How about just get rid of the (un)likely here?
>>
>> I like this :)
>>
>> If we're at it, there are additional cold-path (un)likely macros I
>> want to target:
>
> Looks good to me.
>
> Hiroshi, care to ack/nak all the mailbox and iommu patches you want
> me to merge? Or if you have them in some git branch against mainline
> -rc6 that would be cool too.

I'll post a v3 with all recent updated in a few minutes. I hope it helps.

Thanks,
Ohad.

>
> We need to get the ones we want to merge into linux-arm-kernel for
> review soon, I don't think they been posted there yet.
>
> Regards,
>
> Tony
>
>
>
>> From a921f13dadc02106a2cabb15e3813411d3fcb3a8 Mon Sep 17 00:00:00 2001
>> From: Ohad Ben-Cohen <ohad@wizery.com>
>> Date: Sat, 17 Apr 2010 01:57:43 +0300
>> Subject: [PATCH 3/4] omap: mailbox: remove (un)likely macros from cold paths
>>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> ---
>>  arch/arm/plat-omap/mailbox.c |   10 +++++-----
>>  1 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> index 5140efc..7c60550 100644
>> --- a/arch/arm/plat-omap/mailbox.c
>> +++ b/arch/arm/plat-omap/mailbox.c
>> @@ -248,12 +248,12 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>>         int ret = 0;
>>         struct omap_mbox_queue *mq;
>>
>> -       if (likely(mbox->ops->startup)) {
>> +       if (mbox->ops->startup) {
>>                 spin_lock(&mboxes_lock);
>>                 if (!mbox_configured)
>>                         ret = mbox->ops->startup(mbox);
>>
>> -               if (unlikely(ret)) {
>> +               if (ret) {
>>                         spin_unlock(&mboxes_lock);
>>                         return ret;
>>                 }
>> @@ -263,7 +263,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>>
>>         ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED,
>>                                 mbox->name, mbox);
>> -       if (unlikely(ret)) {
>> +       if (ret) {
>>                 printk(KERN_ERR
>>                         "failed to register mailbox interrupt:%d\n", ret);
>>                 goto fail_request_irq;
>> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>>   fail_alloc_txq:
>>         free_irq(mbox->irq, mbox);
>>   fail_request_irq:
>> -       if (unlikely(mbox->ops->shutdown))
>> +       if (mbox->ops->shutdown)
>>                 mbox->ops->shutdown(mbox);
>>
>>         return ret;
>> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>>
>>         free_irq(mbox->irq, mbox);
>>
>> -       if (unlikely(mbox->ops->shutdown)) {
>> +       if (mbox->ops->shutdown) {
>>                 spin_lock(&mboxes_lock);
>>                 if (mbox_configured > 0)
>>                         mbox_configured--;
>> --
>> 1.6.3.3
>>
>> I'll wait a day or two for more comments, and send a v3 series.
>>
>> Thanks,
>> Ohad.
>>
>> >
>> > Regards,
>> >
>> > Tony
>> >
>> --
>> 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
>
--
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

Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness

[Hiroshi DOYU]

Hi Tony,

From: ext Tony Lindgren <tony@atomide.com>
Subject: Re: [PATCH v2 3/4] omap: mailbox: fix reverse likeliness
Date: Wed, 5 May 2010 17:21:35 +0200

> * Ohad Ben-Cohen <ohad@wizery.com> [100504 04:42]:
>> On Mon, May 3, 2010 at 9:02 PM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Ohad Ben-Cohen <ohad@wizery.com> [100502 08:40]:
>> >> Fix reverse likeliness
>> >>
>> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> >> ---
>> >> If you want, you can also reach me at < ohadb at ti dot com >.
>> >>
>> >>  arch/arm/plat-omap/mailbox.c |    4 ++--
>> >>  1 files changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>> >> index 5140efc..5309213 100644
>> >> --- a/arch/arm/plat-omap/mailbox.c
>> >> +++ b/arch/arm/plat-omap/mailbox.c
>> >> @@ -290,7 +290,7 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
>> >>   fail_alloc_txq:
>> >>       free_irq(mbox->irq, mbox);
>> >>   fail_request_irq:
>> >> -     if (unlikely(mbox->ops->shutdown))
>> >> +     if (likely(mbox->ops->shutdown))
>> >>               mbox->ops->shutdown(mbox);
>> >>
>> >>       return ret;
>> >> @@ -303,7 +303,7 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>> >>
>> >>       free_irq(mbox->irq, mbox);
>> >>
>> >> -     if (unlikely(mbox->ops->shutdown)) {
>> >> +     if (likely(mbox->ops->shutdown)) {
>> >>               spin_lock(&mboxes_lock);
>> >>               if (mbox_configured > 0)
>> >>                       mbox_configured--;
>> >
>> > Does this code path need to be optimized? :)
>> >
>> > How about just get rid of the (un)likely here?
>> 
>> I like this :)
>> 
>> If we're at it, there are additional cold-path (un)likely macros I
>> want to target:
> 
> Looks good to me.
> 
> Hiroshi, care to ack/nak all the mailbox and iommu patches you want
> me to merge? Or if you have them in some git branch against mainline
> -rc6 that would be cool too.

Feel free to add my ACK on them. Looks ok to me too.

If there's more patches coming, I can consider to collect patches and
to provide git branches for mailbox and iommu for next merge.

--
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

[PATCH v2 4/4] omap: mailbox: convert block api to kfifo

[Ohad Ben-Cohen]

The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.

The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the omap_kfifo_size module parameter).

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
If you want, you can also reach me at < ohadb at ti dot com >.

 arch/arm/plat-omap/Kconfig                |    9 +++
 arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
 arch/arm/plat-omap/mailbox.c              |  107 +++++++++++++----------------
 3 files changed, 58 insertions(+), 62 deletions(-)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
 	  Say Y here if you want to use OMAP Mailbox framework support for
 	  DSP, IVA1.0 and IVA2 in OMAP1/2/3.
 
+config OMAP_MBOX_KFIFO_SIZE
+	int "Mailbox kfifo default buffer size (bytes)"
+	depends on OMAP_MBOX_FWK
+	default 256
+	help
+	  Specify the default size of mailbox's kfifo buffers (bytes).
+	  This can also be changed at runtime (via the mbox_kfifo_size
+	  module parameter).
+
 config OMAP_IOMMU
 	tristate
 
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
 
 #include <linux/wait.h>
 #include <linux/workqueue.h>
-#include <linux/blkdev.h>
 #include <linux/interrupt.h>
+#include <linux/kfifo.h>
 
 typedef u32 mbox_msg_t;
 struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
 
 struct omap_mbox_queue {
 	spinlock_t		lock;
-	struct request_queue	*queue;
+	struct kfifo		fifo;
 	struct work_struct	work;
 	struct tasklet_struct	tasklet;
 	int	(*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5309213..a500ac4 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -26,6 +26,8 @@
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/err.h>
 
 #include <plat/mailbox.h>
 
@@ -35,6 +37,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
 
 static int mbox_configured;
 
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo buffers (bytes)");
+
 /* Mailbox FIFO handle functions */
 static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
 {
@@ -67,7 +73,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
 /*
  * message sender
  */
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
 {
 	int ret = 0, i = 1000;
 
@@ -78,49 +84,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
 			return -1;
 		udelay(1);
 	}
-	mbox_fifo_write(mbox, msg);
 	return ret;
 }
 
-
 int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
 {
+	struct omap_mbox_queue *mq = mbox->txq;
+	int ret = 0, len;
 
-	struct request *rq;
-	struct request_queue *q = mbox->txq->queue;
+	spin_lock(&mq->lock);
 
-	rq = blk_get_request(q, WRITE, GFP_ATOMIC);
-	if (unlikely(!rq))
-		return -ENOMEM;
+	if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+	WARN_ON(len != sizeof(msg));
 
-	blk_insert_request(q, rq, 0, (void *) msg);
 	tasklet_schedule(&mbox->txq->tasklet);
 
-	return 0;
+out:
+	spin_unlock(&mq->lock);
+	return ret;
 }
 EXPORT_SYMBOL(omap_mbox_msg_send);
 
 static void mbox_tx_tasklet(unsigned long tx_data)
 {
-	int ret;
-	struct request *rq;
 	struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
-	struct request_queue *q = mbox->txq->queue;
-
-	while (1) {
-
-		rq = blk_fetch_request(q);
-
-		if (!rq)
-			break;
+	struct omap_mbox_queue *mq = mbox->txq;
+	mbox_msg_t msg;
+	int ret;
 
-		ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
-		if (ret) {
+	while (kfifo_len(&mq->fifo)) {
+		if (__mbox_poll_for_space(mbox)) {
 			omap_mbox_enable_irq(mbox, IRQ_TX);
-			blk_requeue_request(q, rq);
-			return;
+			break;
 		}
-		blk_end_request_all(rq, 0);
+
+		ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
+								sizeof(msg));
+		WARN_ON(ret != sizeof(msg));
+
+		mbox_fifo_write(mbox, msg);
 	}
 }
 
@@ -131,36 +138,21 @@ static void mbox_rx_work(struct work_struct *work)
 {
 	struct omap_mbox_queue *mq =
 			container_of(work, struct omap_mbox_queue, work);
-	struct omap_mbox *mbox = mq->queue->queuedata;
-	struct request_queue *q = mbox->rxq->queue;
-	struct request *rq;
 	mbox_msg_t msg;
-	unsigned long flags;
+	int len;
 
-	while (1) {
-		spin_lock_irqsave(q->queue_lock, flags);
-		rq = blk_fetch_request(q);
-		spin_unlock_irqrestore(q->queue_lock, flags);
-		if (!rq)
-			break;
+	while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+		len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
 
-		msg = (mbox_msg_t)rq->special;
-		blk_end_request_all(rq, 0);
-		mbox->rxq->callback((void *)msg);
+		if (mq->callback)
+			mq->callback((void *)msg);
 	}
 }
 
 /*
  * Mailbox interrupt handler
  */
-static void mbox_txq_fn(struct request_queue *q)
-{
-}
-
-static void mbox_rxq_fn(struct request_queue *q)
-{
-}
-
 static void __mbox_tx_interrupt(struct omap_mbox *mbox)
 {
 	omap_mbox_disable_irq(mbox, IRQ_TX);
@@ -170,19 +162,19 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
 
 static void __mbox_rx_interrupt(struct omap_mbox *mbox)
 {
-	struct request *rq;
+	struct omap_mbox_queue *mq = mbox->rxq;
 	mbox_msg_t msg;
-	struct request_queue *q = mbox->rxq->queue;
+	int len;
 
 	while (!mbox_fifo_empty(mbox)) {
-		rq = blk_get_request(q, WRITE, GFP_ATOMIC);
-		if (unlikely(!rq))
+		if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
 			goto nomem;
 
 		msg = mbox_fifo_read(mbox);
 
+		len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
 
-		blk_insert_request(q, rq, 0, (void *)msg);
 		if (mbox->ops->type == OMAP_MBOX_TYPE1)
 			break;
 	}
@@ -207,11 +199,9 @@ 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 (*tasklet)(unsigned long))
 {
-	struct request_queue *q;
 	struct omap_mbox_queue *mq;
 
 	mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
@@ -220,11 +210,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
 
 	spin_lock_init(&mq->lock);
 
-	q = blk_init_queue(proc, &mq->lock);
-	if (!q)
+	if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
 		goto error;
-	q->queuedata = mbox;
-	mq->queue = q;
 
 	if (work)
 		INIT_WORK(&mq->work, work);
@@ -239,7 +226,7 @@ error:
 
 static void mbox_queue_free(struct omap_mbox_queue *q)
 {
-	blk_cleanup_queue(q->queue);
+	kfifo_free(&q->fifo);
 	kfree(q);
 }
 
@@ -269,14 +256,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
 		goto fail_request_irq;
 	}
 
-	mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
+	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_rxq_fn, mbox_rx_work, NULL);
+	mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
 	if (!mq) {
 		ret = -ENOMEM;
 		goto fail_alloc_rxq;
-- 
1.6.3.3

Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo

[Hiroshi DOYU]

Hi Ohad,

From: ext Ohad Ben-Cohen <ohad@wizery.com>
Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
Date: Sun, 2 May 2010 17:44:31 +0200

> The underlying buffering implementation of mailbox
> is converted from block API to kfifo due to the simplicity
> and speed of kfifo.
> 
> The default size of the kfifo buffer is set to 256 bytes.
> This value is configurable at compile time (via
> CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
> runtime (via the omap_kfifo_size module parameter).

mbox_kfifo_size?

> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
> ---
> If you want, you can also reach me at < ohadb at ti dot com >.
> 
>  arch/arm/plat-omap/Kconfig                |    9 +++
>  arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
>  arch/arm/plat-omap/mailbox.c              |  107 +++++++++++++----------------
>  3 files changed, 58 insertions(+), 62 deletions(-)

Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo

[Hiroshi DOYU]

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Subject: Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
Date: Mon, 03 May 2010 08:30:36 +0300 (EEST)

> Hi Ohad,
> 
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
> Date: Sun, 2 May 2010 17:44:31 +0200
> 
>> The underlying buffering implementation of mailbox
>> is converted from block API to kfifo due to the simplicity
>> and speed of kfifo.
>> 
>> The default size of the kfifo buffer is set to 256 bytes.
>> This value is configurable at compile time (via
>> CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
>> runtime (via the omap_kfifo_size module parameter).
> 
> mbox_kfifo_size?

It may be also nice if there's some checking of 'mbox_kfifo_size % 4 == 0' along with 'CONFIG_OMAP_MBOX_KFIFO_SIZE % 4 == 0'?

> 
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>> ---
>> If you want, you can also reach me at < ohadb at ti dot com >.
>> 
>>  arch/arm/plat-omap/Kconfig                |    9 +++
>>  arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
>>  arch/arm/plat-omap/mailbox.c              |  107 +++++++++++++----------------
>>  3 files changed, 58 insertions(+), 62 deletions(-)

Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo

[Ohad Ben-Cohen]

On Mon, May 3, 2010 at 9:07 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> Subject: Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
> Date: Mon, 03 May 2010 08:30:36 +0300 (EEST)
>
>> Hi Ohad,
>>
>> From: ext Ohad Ben-Cohen <ohad@wizery.com>
>> Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
>> Date: Sun, 2 May 2010 17:44:31 +0200
>>
>>> The underlying buffering implementation of mailbox
>>> is converted from block API to kfifo due to the simplicity
>>> and speed of kfifo.
>>>
>>> The default size of the kfifo buffer is set to 256 bytes.
>>> This value is configurable at compile time (via
>>> CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
>>> runtime (via the omap_kfifo_size module parameter).
>>
>> mbox_kfifo_size?
>
> It may be also nice if there's some checking of 'mbox_kfifo_size % 4 == 0' along with 'CONFIG_OMAP_MBOX_KFIFO_SIZE % 4 == 0'?

Good point. I'll send a new patch together with this:

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index a500ac4..fb3d452 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,6 +21,7 @@
  *
  */

+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
@@ -394,6 +395,10 @@ static int __init omap_mbox_init(void)
        if (!mboxd)
                return -ENOMEM;

+       /* kfifo size sanity check: alignment and minimal size */
+       mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
+       mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size,
sizeof(mbox_msg_t));
+
        return 0;
 }
 module_init(omap_mbox_init);


>
>>
>>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>>> ---
>>> If you want, you can also reach me at < ohadb at ti dot com >.
>>>
>>>  arch/arm/plat-omap/Kconfig                |    9 +++
>>>  arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
>>>  arch/arm/plat-omap/mailbox.c              |  107 +++++++++++++----------------
>>>  3 files changed, 58 insertions(+), 62 deletions(-)
>
--
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

[PATCH 4/4] omap: mailbox: convert block api to kfifo

[Ohad Ben-Cohen]

The underlying buffering implementation of mailbox
is converted from block API to kfifo due to the simplicity
and speed of kfifo.

The default size of the kfifo buffer is set to 256 bytes.
This value is configurable at compile time (via
CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
runtime (via the mbox_kfifo_size module parameter).

Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
---
if you want, you can also reach me at < ohadb at ti dot com >.

 arch/arm/plat-omap/Kconfig                |    9 +++
 arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
 arch/arm/plat-omap/mailbox.c              |  112 +++++++++++++---------------
 3 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 6da796e..f4b0029 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -106,6 +106,15 @@ config OMAP_MBOX_FWK
 	  Say Y here if you want to use OMAP Mailbox framework support for
 	  DSP, IVA1.0 and IVA2 in OMAP1/2/3.
 
+config OMAP_MBOX_KFIFO_SIZE
+	int "Mailbox kfifo default buffer size (bytes)"
+	depends on OMAP_MBOX_FWK
+	default 256
+	help
+	  Specify the default size of mailbox's kfifo buffers (bytes).
+	  This can also be changed at runtime (via the mbox_kfifo_size
+	  module parameter).
+
 config OMAP_IOMMU
 	tristate
 
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 729166b..0c3c4a5 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -5,8 +5,8 @@
 
 #include <linux/wait.h>
 #include <linux/workqueue.h>
-#include <linux/blkdev.h>
 #include <linux/interrupt.h>
+#include <linux/kfifo.h>
 
 typedef u32 mbox_msg_t;
 struct omap_mbox;
@@ -42,7 +42,7 @@ struct omap_mbox_ops {
 
 struct omap_mbox_queue {
 	spinlock_t		lock;
-	struct request_queue	*queue;
+	struct kfifo		fifo;
 	struct work_struct	work;
 	struct tasklet_struct	tasklet;
 	int	(*callback)(void *);
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 5309213..8d1581f 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,11 +21,14 @@
  *
  */
 
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/kfifo.h>
+#include <linux/err.h>
 
 #include <plat/mailbox.h>
 
@@ -35,6 +38,10 @@ static DEFINE_SPINLOCK(mboxes_lock);
 
 static int mbox_configured;
 
+static unsigned int mbox_kfifo_size = CONFIG_OMAP_MBOX_KFIFO_SIZE;
+module_param(mbox_kfifo_size, uint, S_IRUGO);
+MODULE_PARM_DESC(mbox_kfifo_size, "Size of omap's mailbox kfifo (bytes)");
+
 /* Mailbox FIFO handle functions */
 static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox)
 {
@@ -67,7 +74,7 @@ static inline int is_mbox_irq(struct omap_mbox *mbox, omap_mbox_irq_t irq)
 /*
  * message sender
  */
-static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
+static int __mbox_poll_for_space(struct omap_mbox *mbox)
 {
 	int ret = 0, i = 1000;
 
@@ -78,49 +85,50 @@ static int __mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
 			return -1;
 		udelay(1);
 	}
-	mbox_fifo_write(mbox, msg);
 	return ret;
 }
 
-
 int omap_mbox_msg_send(struct omap_mbox *mbox, mbox_msg_t msg)
 {
+	struct omap_mbox_queue *mq = mbox->txq;
+	int ret = 0, len;
 
-	struct request *rq;
-	struct request_queue *q = mbox->txq->queue;
+	spin_lock(&mq->lock);
 
-	rq = blk_get_request(q, WRITE, GFP_ATOMIC);
-	if (unlikely(!rq))
-		return -ENOMEM;
+	if (kfifo_avail(&mq->fifo) < sizeof(msg)) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+	WARN_ON(len != sizeof(msg));
 
-	blk_insert_request(q, rq, 0, (void *) msg);
 	tasklet_schedule(&mbox->txq->tasklet);
 
-	return 0;
+out:
+	spin_unlock(&mq->lock);
+	return ret;
 }
 EXPORT_SYMBOL(omap_mbox_msg_send);
 
 static void mbox_tx_tasklet(unsigned long tx_data)
 {
-	int ret;
-	struct request *rq;
 	struct omap_mbox *mbox = (struct omap_mbox *)tx_data;
-	struct request_queue *q = mbox->txq->queue;
-
-	while (1) {
-
-		rq = blk_fetch_request(q);
-
-		if (!rq)
-			break;
+	struct omap_mbox_queue *mq = mbox->txq;
+	mbox_msg_t msg;
+	int ret;
 
-		ret = __mbox_msg_send(mbox, (mbox_msg_t)rq->special);
-		if (ret) {
+	while (kfifo_len(&mq->fifo)) {
+		if (__mbox_poll_for_space(mbox)) {
 			omap_mbox_enable_irq(mbox, IRQ_TX);
-			blk_requeue_request(q, rq);
-			return;
+			break;
 		}
-		blk_end_request_all(rq, 0);
+
+		ret = kfifo_out(&mq->fifo, (unsigned char *)&msg,
+								sizeof(msg));
+		WARN_ON(ret != sizeof(msg));
+
+		mbox_fifo_write(mbox, msg);
 	}
 }
 
@@ -131,36 +139,21 @@ static void mbox_rx_work(struct work_struct *work)
 {
 	struct omap_mbox_queue *mq =
 			container_of(work, struct omap_mbox_queue, work);
-	struct omap_mbox *mbox = mq->queue->queuedata;
-	struct request_queue *q = mbox->rxq->queue;
-	struct request *rq;
 	mbox_msg_t msg;
-	unsigned long flags;
+	int len;
 
-	while (1) {
-		spin_lock_irqsave(q->queue_lock, flags);
-		rq = blk_fetch_request(q);
-		spin_unlock_irqrestore(q->queue_lock, flags);
-		if (!rq)
-			break;
+	while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+		len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
 
-		msg = (mbox_msg_t)rq->special;
-		blk_end_request_all(rq, 0);
-		mbox->rxq->callback((void *)msg);
+		if (mq->callback)
+			mq->callback((void *)msg);
 	}
 }
 
 /*
  * Mailbox interrupt handler
  */
-static void mbox_txq_fn(struct request_queue *q)
-{
-}
-
-static void mbox_rxq_fn(struct request_queue *q)
-{
-}
-
 static void __mbox_tx_interrupt(struct omap_mbox *mbox)
 {
 	omap_mbox_disable_irq(mbox, IRQ_TX);
@@ -170,19 +163,19 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
 
 static void __mbox_rx_interrupt(struct omap_mbox *mbox)
 {
-	struct request *rq;
+	struct omap_mbox_queue *mq = mbox->rxq;
 	mbox_msg_t msg;
-	struct request_queue *q = mbox->rxq->queue;
+	int len;
 
 	while (!mbox_fifo_empty(mbox)) {
-		rq = blk_get_request(q, WRITE, GFP_ATOMIC);
-		if (unlikely(!rq))
+		if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg)))
 			goto nomem;
 
 		msg = mbox_fifo_read(mbox);
 
+		len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
 
-		blk_insert_request(q, rq, 0, (void *)msg);
 		if (mbox->ops->type == OMAP_MBOX_TYPE1)
 			break;
 	}
@@ -207,11 +200,9 @@ 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 (*tasklet)(unsigned long))
 {
-	struct request_queue *q;
 	struct omap_mbox_queue *mq;
 
 	mq = kzalloc(sizeof(struct omap_mbox_queue), GFP_KERNEL);
@@ -220,11 +211,8 @@ static struct omap_mbox_queue *mbox_queue_alloc(struct omap_mbox *mbox,
 
 	spin_lock_init(&mq->lock);
 
-	q = blk_init_queue(proc, &mq->lock);
-	if (!q)
+	if (kfifo_alloc(&mq->fifo, mbox_kfifo_size, GFP_KERNEL))
 		goto error;
-	q->queuedata = mbox;
-	mq->queue = q;
 
 	if (work)
 		INIT_WORK(&mq->work, work);
@@ -239,7 +227,7 @@ error:
 
 static void mbox_queue_free(struct omap_mbox_queue *q)
 {
-	blk_cleanup_queue(q->queue);
+	kfifo_free(&q->fifo);
 	kfree(q);
 }
 
@@ -269,14 +257,14 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
 		goto fail_request_irq;
 	}
 
-	mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL, mbox_tx_tasklet);
+	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_rxq_fn, mbox_rx_work, NULL);
+	mq = mbox_queue_alloc(mbox, mbox_rx_work, NULL);
 	if (!mq) {
 		ret = -ENOMEM;
 		goto fail_alloc_rxq;
@@ -407,6 +395,10 @@ static int __init omap_mbox_init(void)
 	if (!mboxd)
 		return -ENOMEM;
 
+	/* kfifo size sanity check: alignment and minimal size */
+	mbox_kfifo_size = ALIGN(mbox_kfifo_size, sizeof(mbox_msg_t));
+	mbox_kfifo_size = max_t(unsigned int, mbox_kfifo_size, sizeof(mbox_msg_t));
+
 	return 0;
 }
 module_init(omap_mbox_init);
-- 
1.6.3.3

Re: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo

[Ohad Ben-Cohen]

On Mon, May 3, 2010 at 8:30 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Hi Ohad,
>
> From: ext Ohad Ben-Cohen <ohad@wizery.com>
> Subject: [PATCH v2 4/4] omap: mailbox: convert block api to kfifo
> Date: Sun, 2 May 2010 17:44:31 +0200
>
>> The underlying buffering implementation of mailbox
>> is converted from block API to kfifo due to the simplicity
>> and speed of kfifo.
>>
>> The default size of the kfifo buffer is set to 256 bytes.
>> This value is configurable at compile time (via
>> CONFIG_OMAP_MBOX_KFIFO_SIZE), and can be changed at
>> runtime (via the omap_kfifo_size module parameter).
>
> mbox_kfifo_size?

sure. thanks for noticing.

>
>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
>> Signed-off-by: Hari Kanigeri <h-kanigeri2@ti.com>
>> ---
>> If you want, you can also reach me at < ohadb at ti dot com >.
>>
>>  arch/arm/plat-omap/Kconfig                |    9 +++
>>  arch/arm/plat-omap/include/plat/mailbox.h |    4 +-
>>  arch/arm/plat-omap/mailbox.c              |  107 +++++++++++++----------------
>>  3 files changed, 58 insertions(+), 62 deletions(-)
>
--
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