[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Felipe Contreras]

Hi,

These are hopefully non-functional changes. Just shuffling code around, and
removing unecessary stuff.

In v2 I tried to split platform_device and also make mailbox_mach built-in. Not
any more.

Comments from Felipe Balbi, Tony Lindgren, Hiroshi DOYU, and Russell King.

Felipe Contreras (14):
  omap: mailbox: trivial whitespace cleanups
  omap: mailbox: trivial cleanups
  omap: mailbox: reorganize structures
  omap: mailbox: 2420 should be detected at run-time
  omap: mailbox: use correct config for omap1
  omap: mailbox: update omap1 probing
  omap: mailbox: don't export unecessary symbols
  omap: mailbox: remove unecessary fields
  omap: mailbox: add IRQ names
  omap: mailbox: reorganize registering
  omap: mailbox: only compile for configured archs
  omap: mailbox: standarize on 'omap-mailbox'
  omap: mailbox: simplify omap_mbox_register()
  omap: mailbox: reorganize headers

 arch/arm/mach-omap1/devices.c             |   11 +-
 arch/arm/mach-omap1/mailbox.c             |   52 ++++-----
 arch/arm/mach-omap2/devices.c             |    6 +-
 arch/arm/mach-omap2/mailbox.c             |  177 +++++++++++++----------------
 arch/arm/plat-omap/include/plat/mailbox.h |   16 +--
 arch/arm/plat-omap/mailbox.c              |  136 +++++++++-------------
 6 files changed, 168 insertions(+), 230 deletions(-)

[PATCH v3 01/14] omap: mailbox: trivial whitespace cleanups

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

Hiroshi: I think there's something wrong with your editor =/

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

diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 36b3aa2..38a6cb1 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -142,14 +142,14 @@ static void mbox_rx_work(struct work_struct *work)
 	struct omap_mbox_queue *mq =
 			container_of(work, struct omap_mbox_queue, work);
 	mbox_msg_t msg;
- 	int len;
-  
- 	while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
- 		len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
- 		WARN_ON(len != sizeof(msg));
-
- 		if (mq->callback)
- 			mq->callback((void *)msg);
+	int len;
+
+	while (kfifo_len(&mq->fifo) >= sizeof(msg)) {
+		len = kfifo_out(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
+
+		if (mq->callback)
+			mq->callback((void *)msg);
 	}
 }
 
@@ -165,12 +165,12 @@ static void __mbox_tx_interrupt(struct omap_mbox *mbox)
 
 static void __mbox_rx_interrupt(struct omap_mbox *mbox)
 {
- 	struct omap_mbox_queue *mq = mbox->rxq;
+	struct omap_mbox_queue *mq = mbox->rxq;
 	mbox_msg_t msg;
- 	int len;
+	int len;
 
 	while (!mbox_fifo_empty(mbox)) {
- 		if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg))) {
+		if (unlikely(kfifo_avail(&mq->fifo) < sizeof(msg))) {
 			omap_mbox_disable_irq(mbox, IRQ_RX);
 			rq_full = true;
 			goto nomem;
@@ -178,8 +178,8 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox)
 
 		msg = mbox_fifo_read(mbox);
 
- 		len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
- 		WARN_ON(len != sizeof(msg));
+		len = kfifo_in(&mq->fifo, (unsigned char *)&msg, sizeof(msg));
+		WARN_ON(len != sizeof(msg));
 
 		if (mbox->ops->type == OMAP_MBOX_TYPE1)
 			break;
@@ -278,11 +278,11 @@ static int omap_mbox_startup(struct omap_mbox *mbox)
 
 	return 0;
 
- fail_alloc_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 (mbox->ops->shutdown)
 		mbox->ops->shutdown(mbox);
 
@@ -359,10 +359,10 @@ int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
 	if (mbox->next)
 		return -EBUSY;
 
- 	mbox->dev = device_create(&omap_mbox_class,
- 				  parent, 0, mbox, "%s", mbox->name);
- 	if (IS_ERR(mbox->dev))
- 		return PTR_ERR(mbox->dev);
+	mbox->dev = device_create(&omap_mbox_class,
+				  parent, 0, mbox, "%s", mbox->name);
+	if (IS_ERR(mbox->dev))
+		return PTR_ERR(mbox->dev);
 
 	spin_lock(&mboxes_lock);
 	tmp = find_mboxes(mbox->name);
@@ -405,7 +405,7 @@ EXPORT_SYMBOL(omap_mbox_unregister);
 
 static int __init omap_mbox_init(void)
 {
- 	int err;
+	int err;
 
 	err = class_register(&omap_mbox_class);
 	if (err)
@@ -426,7 +426,7 @@ subsys_initcall(omap_mbox_init);
 static void __exit omap_mbox_exit(void)
 {
 	destroy_workqueue(mboxd);
- 	class_unregister(&omap_mbox_class);
+	class_unregister(&omap_mbox_class);
 }
 module_exit(omap_mbox_exit);
 
-- 
1.7.1

[PATCH v3 01/14] omap: mailbox: trivial whitespace cleanups

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 01/14] omap: mailbox: trivial whitespace cleanups
Date: Sat, 22 May 2010 19:14:12 +0200

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
> 
> Hiroshi: I think there's something wrong with your editor =/

Hm...I did something wrong to save them. I'll fix. Thanks.

[PATCH v3 02/14] omap: mailbox: trivial cleanups

[Felipe Contreras]

And fix a few compilation warnings.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/devices.c |    6 ++----
 arch/arm/mach-omap1/mailbox.c |    2 +-
 arch/arm/mach-omap2/mailbox.c |   10 ++++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap1/devices.c b/arch/arm/mach-omap1/devices.c
index 379100c..462b59c 100644
--- a/arch/arm/mach-omap1/devices.c
+++ b/arch/arm/mach-omap1/devices.c
@@ -73,12 +73,10 @@ static inline void omap_init_rtc(void) {}
 #  define INT_DSP_MAILBOX1	INT_1610_DSP_MAILBOX1
 #endif
 
-#define OMAP1_MBOX_BASE		OMAP16XX_MAILBOX_BASE
-
 static struct resource mbox_resources[] = {
 	{
-		.start		= OMAP1_MBOX_BASE,
-		.end		= OMAP1_MBOX_BASE + OMAP1_MBOX_SIZE,
+		.start		= OMAP16XX_MAILBOX_BASE,
+		.end		= OMAP16XX_MAILBOX_BASE + OMAP1_MBOX_SIZE,
 		.flags		= IORESOURCE_MEM,
 	},
 	{
diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 4f5b3da..15bf2a2 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -83,7 +83,7 @@ static int omap1_mbox_fifo_full(struct omap_mbox *mbox)
 	struct omap_mbox1_fifo *fifo =
 		&((struct omap_mbox1_priv *)mbox->priv)->rx_fifo;
 
-	return (mbox_read_reg(fifo->flag));
+	return mbox_read_reg(fifo->flag);
 }
 
 /* irq */
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 763272c..8c1070c 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -131,7 +131,7 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 	}
 
 	l = mbox_read_reg(MAILBOX_REVISION);
-	pr_info("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
+	pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
 
 	if (cpu_is_omap44xx())
 		l = OMAP4_SMARTIDLE;
@@ -300,8 +300,6 @@ static struct omap_mbox2_priv omap2_mbox_dsp_priv = {
 	.irqdisable	= MAILBOX_IRQENABLE(0),
 };
 
-
-
 /* OMAP4 specific data structure. Use the cpu_is_omap4xxx()
 to use this*/
 static struct omap_mbox2_priv omap2_mbox_1_priv = {
@@ -357,7 +355,6 @@ struct omap_mbox mbox_2_info = {
 };
 EXPORT_SYMBOL(mbox_2_info);
 
-
 #if defined(CONFIG_ARCH_OMAP2420) /* IVA */
 static struct omap_mbox2_priv omap2_mbox_iva_priv = {
 	.tx_fifo = {
@@ -443,6 +440,11 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 #endif
 	return 0;
 
+#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
+err_iva1:
+	omap_mbox_unregister(&mbox_dsp_info);
+#endif
+
 err_dsp:
 	iounmap(mbox_base);
 	return ret;
-- 
1.7.1

[PATCH v3 03/14] omap: mailbox: reorganize structures

[Felipe Contreras]

OMAP4 ones messed up the organization.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/mailbox.c |   68 +++++++++++++++++++++--------------------
 1 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 8c1070c..edcfec6 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -283,6 +283,7 @@ static struct omap_mbox_ops omap2_mbox_ops = {
  */
 
 /* FIXME: the following structs should be filled automatically by the user id */
+
 /* DSP */
 static struct omap_mbox2_priv omap2_mbox_dsp_priv = {
 	.tx_fifo = {
@@ -300,8 +301,40 @@ static struct omap_mbox2_priv omap2_mbox_dsp_priv = {
 	.irqdisable	= MAILBOX_IRQENABLE(0),
 };
 
-/* OMAP4 specific data structure. Use the cpu_is_omap4xxx()
-to use this*/
+struct omap_mbox mbox_dsp_info = {
+	.name	= "dsp",
+	.ops	= &omap2_mbox_ops,
+	.priv	= &omap2_mbox_dsp_priv,
+};
+EXPORT_SYMBOL(mbox_dsp_info);
+
+#if defined(CONFIG_ARCH_OMAP2420)
+
+/* IVA */
+static struct omap_mbox2_priv omap2_mbox_iva_priv = {
+	.tx_fifo = {
+		.msg		= MAILBOX_MESSAGE(2),
+		.fifo_stat	= MAILBOX_FIFOSTATUS(2),
+	},
+	.rx_fifo = {
+		.msg		= MAILBOX_MESSAGE(3),
+		.msg_stat	= MAILBOX_MSGSTATUS(3),
+	},
+	.irqenable	= MAILBOX_IRQENABLE(3),
+	.irqstatus	= MAILBOX_IRQSTATUS(3),
+	.notfull_bit	= MAILBOX_IRQ_NOTFULL(2),
+	.newmsg_bit	= MAILBOX_IRQ_NEWMSG(3),
+	.irqdisable	= MAILBOX_IRQENABLE(3),
+};
+
+static struct omap_mbox mbox_iva_info = {
+	.name	= "iva",
+	.ops	= &omap2_mbox_ops,
+	.priv	= &omap2_mbox_iva_priv,
+};
+#endif
+
+/* OMAP4 */
 static struct omap_mbox2_priv omap2_mbox_1_priv = {
 	.tx_fifo = {
 		.msg		= MAILBOX_MESSAGE(0),
@@ -325,13 +358,6 @@ struct omap_mbox mbox_1_info = {
 };
 EXPORT_SYMBOL(mbox_1_info);
 
-struct omap_mbox mbox_dsp_info = {
-	.name	= "dsp",
-	.ops	= &omap2_mbox_ops,
-	.priv	= &omap2_mbox_dsp_priv,
-};
-EXPORT_SYMBOL(mbox_dsp_info);
-
 static struct omap_mbox2_priv omap2_mbox_2_priv = {
 	.tx_fifo = {
 		.msg		= MAILBOX_MESSAGE(3),
@@ -355,30 +381,6 @@ struct omap_mbox mbox_2_info = {
 };
 EXPORT_SYMBOL(mbox_2_info);
 
-#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
-static struct omap_mbox2_priv omap2_mbox_iva_priv = {
-	.tx_fifo = {
-		.msg		= MAILBOX_MESSAGE(2),
-		.fifo_stat	= MAILBOX_FIFOSTATUS(2),
-	},
-	.rx_fifo = {
-		.msg		= MAILBOX_MESSAGE(3),
-		.msg_stat	= MAILBOX_MSGSTATUS(3),
-	},
-	.irqenable	= MAILBOX_IRQENABLE(3),
-	.irqstatus	= MAILBOX_IRQSTATUS(3),
-	.notfull_bit	= MAILBOX_IRQ_NOTFULL(2),
-	.newmsg_bit	= MAILBOX_IRQ_NEWMSG(3),
-	.irqdisable	= MAILBOX_IRQENABLE(3),
-};
-
-static struct omap_mbox mbox_iva_info = {
-	.name	= "iva",
-	.ops	= &omap2_mbox_ops,
-	.priv	= &omap2_mbox_iva_priv,
-};
-#endif
-
 static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
-- 
1.7.1

[PATCH v3 04/14] omap: mailbox: 2420 should be detected at run-time

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/mailbox.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index edcfec6..f67cb2c 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -455,7 +455,8 @@ err_dsp:
 static int __devexit omap2_mbox_remove(struct platform_device *pdev)
 {
 #if defined(CONFIG_ARCH_OMAP2420)
-	omap_mbox_unregister(&mbox_iva_info);
+	if (cpu_is_omap2420())
+		omap_mbox_unregister(&mbox_iva_info);
 #endif
 
 	if (cpu_is_omap44xx()) {
-- 
1.7.1

[PATCH v3 05/14] omap: mailbox: use correct config for omap1

[Felipe Contreras]

CONFIG_OMAP_DSP is not in mainline, CONFIG_OMAP_MBOX_FWK is.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/devices.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap1/devices.c b/arch/arm/mach-omap1/devices.c
index 462b59c..da796f2 100644
--- a/arch/arm/mach-omap1/devices.c
+++ b/arch/arm/mach-omap1/devices.c
@@ -63,7 +63,7 @@ static void omap_init_rtc(void)
 static inline void omap_init_rtc(void) {}
 #endif
 
-#if defined(CONFIG_OMAP_DSP) || defined(CONFIG_OMAP_DSP_MODULE)
+#if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
 
 #if defined(CONFIG_ARCH_OMAP15XX)
 #  define OMAP1_MBOX_SIZE	0x23
-- 
1.7.1

[PATCH v3 06/14] omap: mailbox: update omap1 probing

[Felipe Contreras]

Based on omap2 code.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/mailbox.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 15bf2a2..211b9fc 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -146,12 +146,7 @@ EXPORT_SYMBOL(mbox_dsp_info);
 static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
-
-	if (pdev->num_resources != 2) {
-		dev_err(&pdev->dev, "invalid number of resources: %d\n",
-			pdev->num_resources);
-		return -ENODEV;
-	}
+	int ret;
 
 	/* MBOX base */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -161,27 +156,32 @@ static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 	}
 
 	mbox_base = ioremap(res->start, resource_size(res));
-	if (!mbox_base) {
-		dev_err(&pdev->dev, "ioremap failed\n");
-		return -ENODEV;
-	}
+	if (!mbox_base)
+		return -ENOMEM;
 
 	/* DSP IRQ */
 	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
 	if (unlikely(!res)) {
 		dev_err(&pdev->dev, "invalid irq resource\n");
-		iounmap(mbox_base);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto err_out;
 	}
 	mbox_dsp_info.irq = res->start;
 
-	return omap_mbox_register(&pdev->dev, &mbox_dsp_info);
+	ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
+	if (ret)
+		goto err_out;
+	return 0;
+
+err_out:
+	iounmap(mbox_base);
+	return ret;
 }
 
 static int __devexit omap1_mbox_remove(struct platform_device *pdev)
 {
 	omap_mbox_unregister(&mbox_dsp_info);
-
+	iounmap(mbox_base);
 	return 0;
 }
 
-- 
1.7.1

[PATCH v3 07/14] omap: mailbox: don't export unecessary symbols

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/mailbox.c |    1 -
 arch/arm/mach-omap2/mailbox.c |    3 ---
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 211b9fc..590ac66 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -141,7 +141,6 @@ struct omap_mbox mbox_dsp_info = {
 	.ops	= &omap1_mbox_ops,
 	.priv	= &omap1_mbox_dsp_priv,
 };
-EXPORT_SYMBOL(mbox_dsp_info);
 
 static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 {
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index f67cb2c..3ff1ad5 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -306,7 +306,6 @@ struct omap_mbox mbox_dsp_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_dsp_priv,
 };
-EXPORT_SYMBOL(mbox_dsp_info);
 
 #if defined(CONFIG_ARCH_OMAP2420)
 
@@ -356,7 +355,6 @@ struct omap_mbox mbox_1_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_1_priv,
 };
-EXPORT_SYMBOL(mbox_1_info);
 
 static struct omap_mbox2_priv omap2_mbox_2_priv = {
 	.tx_fifo = {
@@ -379,7 +377,6 @@ struct omap_mbox mbox_2_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_2_priv,
 };
-EXPORT_SYMBOL(mbox_2_info);
 
 static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
-- 
1.7.1

[PATCH v3 08/14] omap: mailbox: remove unecessary fields

[Felipe Contreras]

Nobody is using them.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/plat-omap/include/plat/mailbox.h |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 0c3c4a5..aad8bf8 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -52,19 +52,11 @@ struct omap_mbox_queue {
 struct omap_mbox {
 	char			*name;
 	unsigned int		irq;
-
 	struct omap_mbox_queue	*txq, *rxq;
-
 	struct omap_mbox_ops	*ops;
-
-	mbox_msg_t		seq_snd, seq_rcv;
-
 	struct device		*dev;
-
 	struct omap_mbox	*next;
 	void			*priv;
-
-	void			(*err_notify)(void);
 };
 
 int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
-- 
1.7.1

[PATCH v3 09/14] omap: mailbox: add IRQ names

[Felipe Contreras]

Will be useful to identify them later.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/devices.c |    1 +
 arch/arm/mach-omap2/devices.c |    4 ++++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap1/devices.c b/arch/arm/mach-omap1/devices.c
index da796f2..314fea3 100644
--- a/arch/arm/mach-omap1/devices.c
+++ b/arch/arm/mach-omap1/devices.c
@@ -82,6 +82,7 @@ static struct resource mbox_resources[] = {
 	{
 		.start		= INT_DSP_MAILBOX1,
 		.flags		= IORESOURCE_IRQ,
+		.name		= "dsp",
 	},
 };
 
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 2271b9b..b621476 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -150,10 +150,12 @@ static struct resource omap2_mbox_resources[] = {
 	{
 		.start		= INT_24XX_MAIL_U0_MPU,
 		.flags		= IORESOURCE_IRQ,
+		.name		= "dsp",
 	},
 	{
 		.start		= INT_24XX_MAIL_U3_MPU,
 		.flags		= IORESOURCE_IRQ,
+		.name		= "iva",
 	},
 };
 static int omap2_mbox_resources_sz = ARRAY_SIZE(omap2_mbox_resources);
@@ -172,6 +174,7 @@ static struct resource omap3_mbox_resources[] = {
 	{
 		.start		= INT_24XX_MAIL_U0_MPU,
 		.flags		= IORESOURCE_IRQ,
+		.name		= "dsp",
 	},
 };
 static int omap3_mbox_resources_sz = ARRAY_SIZE(omap3_mbox_resources);
@@ -193,6 +196,7 @@ static struct resource omap4_mbox_resources[] = {
 	{
 		.start          = OMAP44XX_IRQ_MAIL_U0,
 		.flags          = IORESOURCE_IRQ,
+		.name		= "mbox",
 	},
 };
 static int omap4_mbox_resources_sz = ARRAY_SIZE(omap4_mbox_resources);
-- 
1.7.1

[PATCH v3 10/14] omap: mailbox: reorganize registering

[Felipe Contreras]

It's more extensible this way.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/mailbox.c |   41 +++++++++-------
 arch/arm/mach-omap2/mailbox.c |  106 +++++++++++++++++------------------------
 2 files changed, 66 insertions(+), 81 deletions(-)

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 590ac66..38c21a2 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -29,6 +29,8 @@
 
 static void __iomem *mbox_base;
 
+static struct omap_mbox **list;
+
 struct omap_mbox1_fifo {
 	unsigned long cmd;
 	unsigned long data;
@@ -142,44 +144,45 @@ struct omap_mbox mbox_dsp_info = {
 	.priv	= &omap1_mbox_dsp_priv,
 };
 
+struct omap_mbox *omap1_mboxes[] = { &mbox_dsp_info, NULL };
+
 static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int ret;
+	int i;
 
-	/* MBOX base */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid mem resource\n");
-		return -ENODEV;
-	}
+	res = pdev->resource;
+
+	list = omap1_mboxes;
 
-	mbox_base = ioremap(res->start, resource_size(res));
+	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
+
+	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
 	if (!mbox_base)
 		return -ENOMEM;
 
-	/* DSP IRQ */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid irq resource\n");
-		ret = -ENODEV;
-		goto err_out;
+	for (i = 0; list[i]; i++) {
+		ret = omap_mbox_register(&pdev->dev, list[i]);
+		if (ret)
+			goto err_out;
 	}
-	mbox_dsp_info.irq = res->start;
-
-	ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
-	if (ret)
-		goto err_out;
 	return 0;
 
 err_out:
+	while (i--)
+		omap_mbox_unregister(list[i]);
 	iounmap(mbox_base);
 	return ret;
 }
 
 static int __devexit omap1_mbox_remove(struct platform_device *pdev)
 {
-	omap_mbox_unregister(&mbox_dsp_info);
+	int i;
+
+	for (i = 0; list[i]; i++)
+		omap_mbox_unregister(list[i]);
+
 	iounmap(mbox_base);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 3ff1ad5..f55fa84 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -56,6 +56,8 @@
 
 static void __iomem *mbox_base;
 
+static struct omap_mbox **list;
+
 struct omap_mbox2_fifo {
 	unsigned long msg;
 	unsigned long fifo_stat;
@@ -307,6 +309,8 @@ struct omap_mbox mbox_dsp_info = {
 	.priv	= &omap2_mbox_dsp_priv,
 };
 
+struct omap_mbox *omap3_mboxes[] = { &mbox_dsp_info, NULL };
+
 #if defined(CONFIG_ARCH_OMAP2420)
 
 /* IVA */
@@ -331,6 +335,8 @@ static struct omap_mbox mbox_iva_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_iva_priv,
 };
+
+struct omap_mbox *omap2_mboxes[] = { &mbox_iva_info, &mbox_dsp_info, NULL };
 #endif
 
 /* OMAP4 */
@@ -378,89 +384,65 @@ struct omap_mbox mbox_2_info = {
 	.priv	= &omap2_mbox_2_priv,
 };
 
+struct omap_mbox *omap4_mboxes[] = { &mbox_1_info, &mbox_2_info, NULL };
+
 static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int ret;
+	int i;
 
-	/* MBOX base */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid mem resource\n");
-		return -ENODEV;
-	}
-	mbox_base = ioremap(res->start, resource_size(res));
-	if (!mbox_base)
-		return -ENOMEM;
+	res = pdev->resource;
 
-	/* DSP or IVA2 IRQ */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (cpu_is_omap3430()) {
+		list = omap3_mboxes;
 
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid irq resource\n");
-		ret = -ENODEV;
-		goto err_dsp;
+		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
 	}
-	if (cpu_is_omap44xx()) {
-		mbox_1_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_1_info);
-	} else {
-		mbox_dsp_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
+#if defined(CONFIG_ARCH_OMAP2420)
+	else if (cpu_is_omap2420()) {
+		list = omap2_mboxes;
+
+		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
+		list[1]->irq = platform_get_irq_byname(pdev, "iva");
 	}
-	if (ret)
-		goto err_dsp;
+#endif
+	else if (cpu_is_omap44xx()) {
+		list = omap4_mboxes;
 
-	if (cpu_is_omap44xx()) {
-		mbox_2_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_2_info);
-		if (ret) {
-			omap_mbox_unregister(&mbox_1_info);
-			goto err_dsp;
-		}
+		list[0]->irq = list[1]->irq =
+			platform_get_irq_byname(pdev, "mbox");
 	}
-#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
-	if (cpu_is_omap2420()) {
-		/* IVA IRQ */
-		res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
-		if (unlikely(!res)) {
-			dev_err(&pdev->dev, "invalid irq resource\n");
-			ret = -ENODEV;
-			omap_mbox_unregister(&mbox_dsp_info);
-			goto err_dsp;
-		}
-		mbox_iva_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_iva_info);
-		if (ret) {
-			omap_mbox_unregister(&mbox_dsp_info);
-			goto err_dsp;
-		}
+	else {
+		pr_err("%s: platform not supported\n", __func__);
+		return -ENODEV;
 	}
-#endif
-	return 0;
 
-#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
-err_iva1:
-	omap_mbox_unregister(&mbox_dsp_info);
-#endif
+	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
+	if (!mbox_base)
+		return -ENOMEM;
 
-err_dsp:
+	for (i = 0; list[i]; i++) {
+		ret = omap_mbox_register(&pdev->dev, list[i]);
+		if (ret)
+			goto err_out;
+	}
+	return 0;
+
+err_out:
+	while (i--)
+		omap_mbox_unregister(list[i]);
 	iounmap(mbox_base);
 	return ret;
 }
 
 static int __devexit omap2_mbox_remove(struct platform_device *pdev)
 {
-#if defined(CONFIG_ARCH_OMAP2420)
-	if (cpu_is_omap2420())
-		omap_mbox_unregister(&mbox_iva_info);
-#endif
+	int i;
+
+	for (i = 0; list[i]; i++)
+		omap_mbox_unregister(list[i]);
 
-	if (cpu_is_omap44xx()) {
-		omap_mbox_unregister(&mbox_2_info);
-		omap_mbox_unregister(&mbox_1_info);
-	} else
-		omap_mbox_unregister(&mbox_dsp_info);
 	iounmap(mbox_base);
 	return 0;
 }
-- 
1.7.1

[PATCH v3 10/14] omap: mailbox: reorganize registering

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 10/14] omap: mailbox: reorganize registering
Date: Sat, 22 May 2010 19:14:21 +0200

> platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (unlikely(!res)) {
> -		dev_err(&pdev->dev, "invalid mem resource\n");
> -		return -ENODEV;
> -	}
> +	res = pdev->resource;

why not?

	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

> +
> +	list = omap1_mboxes;
>  
> -	mbox_base = ioremap(res->start, resource_size(res));
> +	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
> +
> +	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
>  	if (!mbox_base)
>  		return -ENOMEM;
>  
> -	/* DSP IRQ */
> -	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> -	if (unlikely(!res)) {
> -		dev_err(&pdev->dev, "invalid irq resource\n");
> -		ret = -ENODEV;
> -		goto err_out;
> +	for (i = 0; list[i]; i++) {
> +		ret = omap_mbox_register(&pdev->dev, list[i]);
> +		if (ret)
> +			goto err_out;
>  	}
> -	mbox_dsp_info.irq = res->start;
> -
> -	ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
> -	if (ret)
> -		goto err_out;
>  	return 0;
>  
>  err_out:
> +	while (i--)
> +		omap_mbox_unregister(list[i]);
>  	iounmap(mbox_base);
>  	return ret;

[PATCH v3 10/14] omap: mailbox: reorganize registering

[Russell King - ARM Linux]

On Mon, May 24, 2010 at 06:24:07PM +0300, Hiroshi DOYU wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [PATCH v3 10/14] omap: mailbox: reorganize registering
> Date: Sat, 22 May 2010 19:14:21 +0200
> 
> > platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > -	if (unlikely(!res)) {
> > -		dev_err(&pdev->dev, "invalid mem resource\n");
> > -		return -ENODEV;
> > -	}
> > +	res = pdev->resource;
> 
> why not?
> 
> 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

I agree - always use the accessors provided rather than going beneath
the covers and fiddling directly with stuff inside objects.

[PATCH v3 10/14] omap: mailbox: reorganize registering

[Felipe Contreras]

On Mon, May 24, 2010 at 9:32 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, May 24, 2010 at 06:24:07PM +0300, Hiroshi DOYU wrote:
>> From: ext Felipe Contreras <felipe.contreras@gmail.com>
>> Subject: [PATCH v3 10/14] omap: mailbox: reorganize registering
>> Date: Sat, 22 May 2010 19:14:21 +0200
>>
>> > platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > - ? if (unlikely(!res)) {
>> > - ? ? ? ? ? dev_err(&pdev->dev, "invalid mem resource\n");
>> > - ? ? ? ? ? return -ENODEV;
>> > - ? }
>> > + ? res = pdev->resource;
>>
>> why not?
>>
>> ? ? ? res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>
> I agree - always use the accessors provided rather than going beneath
> the covers and fiddling directly with stuff inside objects.

Yes, this patch slipped from the previous series where resources were
defined in the same file and some tricky assumptions were being made.
Now platform_get_resource() makes more sense.

-- 
Felipe Contreras

[PATCH v3 11/14] omap: mailbox: only compile for configured archs

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/mailbox.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index f55fa84..1f0c5cd 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -286,6 +286,7 @@ static struct omap_mbox_ops omap2_mbox_ops = {
 
 /* FIXME: the following structs should be filled automatically by the user id */
 
+#if defined(CONFIG_ARCH_OMAP3430) || defined(CONFIG_ARCH_OMAP2420)
 /* DSP */
 static struct omap_mbox2_priv omap2_mbox_dsp_priv = {
 	.tx_fifo = {
@@ -308,11 +309,13 @@ struct omap_mbox mbox_dsp_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_dsp_priv,
 };
+#endif
 
+#if defined(CONFIG_ARCH_OMAP3430)
 struct omap_mbox *omap3_mboxes[] = { &mbox_dsp_info, NULL };
+#endif
 
 #if defined(CONFIG_ARCH_OMAP2420)
-
 /* IVA */
 static struct omap_mbox2_priv omap2_mbox_iva_priv = {
 	.tx_fifo = {
@@ -339,6 +342,7 @@ static struct omap_mbox mbox_iva_info = {
 struct omap_mbox *omap2_mboxes[] = { &mbox_iva_info, &mbox_dsp_info, NULL };
 #endif
 
+#if defined(CONFIG_ARCH_OMAP4)
 /* OMAP4 */
 static struct omap_mbox2_priv omap2_mbox_1_priv = {
 	.tx_fifo = {
@@ -385,6 +389,7 @@ struct omap_mbox mbox_2_info = {
 };
 
 struct omap_mbox *omap4_mboxes[] = { &mbox_1_info, &mbox_2_info, NULL };
+#endif
 
 static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
@@ -394,11 +399,14 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 
 	res = pdev->resource;
 
-	if (cpu_is_omap3430()) {
+	if (false);
+#if defined(CONFIG_ARCH_OMAP3430)
+	else if (cpu_is_omap3430()) {
 		list = omap3_mboxes;
 
 		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
 	}
+#endif
 #if defined(CONFIG_ARCH_OMAP2420)
 	else if (cpu_is_omap2420()) {
 		list = omap2_mboxes;
@@ -407,12 +415,14 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 		list[1]->irq = platform_get_irq_byname(pdev, "iva");
 	}
 #endif
+#if defined(CONFIG_ARCH_OMAP4)
 	else if (cpu_is_omap44xx()) {
 		list = omap4_mboxes;
 
 		list[0]->irq = list[1]->irq =
 			platform_get_irq_byname(pdev, "mbox");
 	}
+#endif
 	else {
 		pr_err("%s: platform not supported\n", __func__);
 		return -ENODEV;
-- 
1.7.1

[PATCH v3 11/14] omap: mailbox: only compile for configured archs

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 11/14] omap: mailbox: only compile for configured archs
Date: Sat, 22 May 2010 19:14:22 +0200

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  arch/arm/mach-omap2/mailbox.c |   14 ++++++++++++--
>  1 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index f55fa84..1f0c5cd 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -286,6 +286,7 @@ static struct omap_mbox_ops omap2_mbox_ops = {
>  
>  /* FIXME: the following structs should be filled automatically by the user id */
>  
> +#if defined(CONFIG_ARCH_OMAP3430) || defined(CONFIG_ARCH_OMAP2420)
>  /* DSP */
>  static struct omap_mbox2_priv omap2_mbox_dsp_priv = {
>  	.tx_fifo = {
> @@ -308,11 +309,13 @@ struct omap_mbox mbox_dsp_info = {
>  	.ops	= &omap2_mbox_ops,
>  	.priv	= &omap2_mbox_dsp_priv,
>  };
> +#endif
>  
> +#if defined(CONFIG_ARCH_OMAP3430)
>  struct omap_mbox *omap3_mboxes[] = { &mbox_dsp_info, NULL };
> +#endif
>  
>  #if defined(CONFIG_ARCH_OMAP2420)
> -
>  /* IVA */
>  static struct omap_mbox2_priv omap2_mbox_iva_priv = {
>  	.tx_fifo = {
> @@ -339,6 +342,7 @@ static struct omap_mbox mbox_iva_info = {
>  struct omap_mbox *omap2_mboxes[] = { &mbox_iva_info, &mbox_dsp_info, NULL };
>  #endif
>  
> +#if defined(CONFIG_ARCH_OMAP4)
>  /* OMAP4 */
>  static struct omap_mbox2_priv omap2_mbox_1_priv = {
>  	.tx_fifo = {
> @@ -385,6 +389,7 @@ struct omap_mbox mbox_2_info = {
>  };
>  
>  struct omap_mbox *omap4_mboxes[] = { &mbox_1_info, &mbox_2_info, NULL };
> +#endif
>  
>  static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  {
> @@ -394,11 +399,14 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  
>  	res = pdev->resource;
>  
> -	if (cpu_is_omap3430()) {
> +	if (false);

nitpcik:
The above may look better as below:

	if (false)
		;

"checkpatch.pl" also doesn't complain.

> +#if defined(CONFIG_ARCH_OMAP3430)
> +	else if (cpu_is_omap3430()) {
>  		list = omap3_mboxes;
>  
>  		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
>  	}
> +#endif
>  #if defined(CONFIG_ARCH_OMAP2420)
>  	else if (cpu_is_omap2420()) {
>  		list = omap2_mboxes;
> @@ -407,12 +415,14 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  		list[1]->irq = platform_get_irq_byname(pdev, "iva");
>  	}
>  #endif
> +#if defined(CONFIG_ARCH_OMAP4)
>  	else if (cpu_is_omap44xx()) {
>  		list = omap4_mboxes;
>  
>  		list[0]->irq = list[1]->irq =
>  			platform_get_irq_byname(pdev, "mbox");

For checkpatch.pl,

  		list[0]->irq =
  			platform_get_irq_byname(pdev, "mbox");
  		list[1]->irq = list[0]->irq;

>  	}
> +#endif
>  	else {
>  		pr_err("%s: platform not supported\n", __func__);
>  		return -ENODEV;
> -- 
> 1.7.1
> 

[PATCH v3 11/14] omap: mailbox: only compile for configured archs

[Felipe Contreras]

On Mon, May 24, 2010 at 5:42 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>

>> +     if (false);
>
> nitpcik:
> The above may look better as below:
>
> ? ? ? ?if (false)
> ? ? ? ? ? ? ? ?;
>
> "checkpatch.pl" also doesn't complain.

Personally I think it looks weird and it's a checkpatch bug, but fine by me.

>> +#if defined(CONFIG_ARCH_OMAP3430)
>> + ? ? else if (cpu_is_omap3430()) {
>> ? ? ? ? ? ? ? list = omap3_mboxes;
>>
>> ? ? ? ? ? ? ? list[0]->irq = platform_get_irq_byname(pdev, "dsp");
>> ? ? ? }
>> +#endif
>> ?#if defined(CONFIG_ARCH_OMAP2420)
>> ? ? ? else if (cpu_is_omap2420()) {
>> ? ? ? ? ? ? ? list = omap2_mboxes;
>> @@ -407,12 +415,14 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>> ? ? ? ? ? ? ? list[1]->irq = platform_get_irq_byname(pdev, "iva");
>> ? ? ? }
>> ?#endif
>> +#if defined(CONFIG_ARCH_OMAP4)
>> ? ? ? else if (cpu_is_omap44xx()) {
>> ? ? ? ? ? ? ? list = omap4_mboxes;
>>
>> ? ? ? ? ? ? ? list[0]->irq = list[1]->irq =
>> ? ? ? ? ? ? ? ? ? ? ? platform_get_irq_byname(pdev, "mbox");
>
> For checkpatch.pl,
>
> ? ? ? ? ? ? ? ?list[0]->irq =
> ? ? ? ? ? ? ? ? ? ? ? ?platform_get_irq_byname(pdev, "mbox");
> ? ? ? ? ? ? ? ?list[1]->irq = list[0]->irq;

I don't see checkpatch (0.30) complaining about that.

Cheers.

-- 
Felipe Contreras

[PATCH v3 11/14] omap: mailbox: only compile for configured archs

[Russell King - ARM Linux]

On Mon, May 24, 2010 at 10:29:01PM +0300, Felipe Contreras wrote:
> On Mon, May 24, 2010 at 5:42 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > From: ext Felipe Contreras <felipe.contreras@gmail.com>
> 
> >> +     if (false);
> >
> > nitpcik:
> > The above may look better as below:
> >
> > ? ? ? ?if (false)
> > ? ? ? ? ? ? ? ?;
> >
> > "checkpatch.pl" also doesn't complain.
> 
> Personally I think it looks weird and it's a checkpatch bug, but fine by me.

No, I'd say it's intentional.

	if (false);

looks too much like an error - think about:

	if (false);
		do_something();

which would be very easy to miss while reviewing.

[PATCH v3 12/14] omap: mailbox: standarize on 'omap-mailbox'

[Felipe Contreras]

omap{1,2}-mailbox are modules that provide the 'omap-mailbox' driver.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/devices.c |    2 +-
 arch/arm/mach-omap1/mailbox.c |    4 ++--
 arch/arm/mach-omap2/devices.c |    2 +-
 arch/arm/mach-omap2/mailbox.c |    6 ++----
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap1/devices.c b/arch/arm/mach-omap1/devices.c
index 314fea3..16e43e5 100644
--- a/arch/arm/mach-omap1/devices.c
+++ b/arch/arm/mach-omap1/devices.c
@@ -87,7 +87,7 @@ static struct resource mbox_resources[] = {
 };
 
 static struct platform_device mbox_device = {
-	.name		= "omap1-mailbox",
+	.name		= "omap-mailbox",
 	.id		= -1,
 	.num_resources	= ARRAY_SIZE(mbox_resources),
 	.resource	= mbox_resources,
diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 38c21a2..4210896 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -1,5 +1,5 @@
 /*
- * Mailbox reservation modules for DSP
+ * Mailbox reservation modules for OMAP1
  *
  * Copyright (C) 2006-2009 Nokia Corporation
  * Written by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
@@ -191,7 +191,7 @@ static struct platform_driver omap1_mbox_driver = {
 	.probe	= omap1_mbox_probe,
 	.remove	= __devexit_p(omap1_mbox_remove),
 	.driver	= {
-		.name	= "omap1-mailbox",
+		.name	= "omap-mailbox",
 	},
 };
 
diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index b621476..d9ac853 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -206,7 +206,7 @@ static int omap4_mbox_resources_sz = ARRAY_SIZE(omap4_mbox_resources);
 #endif
 
 static struct platform_device mbox_device = {
-	.name		= "omap2-mailbox",
+	.name		= "omap-mailbox",
 	.id		= -1,
 };
 
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 1f0c5cd..a433ca7 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -18,8 +18,6 @@
 #include <plat/mailbox.h>
 #include <mach/irqs.h>
 
-#define DRV_NAME "omap2-mailbox"
-
 #define MAILBOX_REVISION		0x000
 #define MAILBOX_SYSCONFIG		0x010
 #define MAILBOX_SYSSTATUS		0x014
@@ -461,7 +459,7 @@ static struct platform_driver omap2_mbox_driver = {
 	.probe = omap2_mbox_probe,
 	.remove = __devexit_p(omap2_mbox_remove),
 	.driver = {
-		.name = DRV_NAME,
+		.name = "omap-mailbox",
 	},
 };
 
@@ -482,4 +480,4 @@ MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("omap mailbox: omap2/3/4 architecture specific functions");
 MODULE_AUTHOR("Hiroshi DOYU <Hiroshi.DOYU@nokia.com>");
 MODULE_AUTHOR("Paul Mundt");
-MODULE_ALIAS("platform:"DRV_NAME);
+MODULE_ALIAS("platform:omap2-mailbox");
-- 
1.7.1

[PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()

[Felipe Contreras]

No need to dynamically register mailboxes one by one.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/mailbox.c             |   25 ++------
 arch/arm/mach-omap2/mailbox.c             |   22 ++-----
 arch/arm/plat-omap/include/plat/mailbox.h |    5 +-
 arch/arm/plat-omap/mailbox.c              |   95 +++++++++++------------------
 4 files changed, 50 insertions(+), 97 deletions(-)

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 4210896..e50b3c2 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -29,8 +29,6 @@
 
 static void __iomem *mbox_base;
 
-static struct omap_mbox **list;
-
 struct omap_mbox1_fifo {
 	unsigned long cmd;
 	unsigned long data;
@@ -151,38 +149,29 @@ static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 	struct resource *res;
 	int ret;
 	int i;
+	struct omap_mbox **list;
 
 	res = pdev->resource;
 
 	list = omap1_mboxes;
-
 	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
 
 	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
 	if (!mbox_base)
 		return -ENOMEM;
 
-	for (i = 0; list[i]; i++) {
-		ret = omap_mbox_register(&pdev->dev, list[i]);
-		if (ret)
-			goto err_out;
+	ret = omap_mbox_register(&pdev->dev, list);
+	if (ret) {
+		iounmap(mbox_base);
+		return ret;
 	}
-	return 0;
 
-err_out:
-	while (i--)
-		omap_mbox_unregister(list[i]);
-	iounmap(mbox_base);
-	return ret;
+	return 0;
 }
 
 static int __devexit omap1_mbox_remove(struct platform_device *pdev)
 {
-	int i;
-
-	for (i = 0; list[i]; i++)
-		omap_mbox_unregister(list[i]);
-
+	omap_mbox_unregister();
 	iounmap(mbox_base);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index a433ca7..66d366d 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -54,8 +54,6 @@
 
 static void __iomem *mbox_base;
 
-static struct omap_mbox **list;
-
 struct omap_mbox2_fifo {
 	unsigned long msg;
 	unsigned long fifo_stat;
@@ -393,7 +391,7 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int ret;
-	int i;
+	struct omap_mbox **list;
 
 	res = pdev->resource;
 
@@ -430,27 +428,19 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 	if (!mbox_base)
 		return -ENOMEM;
 
-	for (i = 0; list[i]; i++) {
-		ret = omap_mbox_register(&pdev->dev, list[i]);
-		if (ret)
-			goto err_out;
+	ret = omap_mbox_register(&pdev->dev, list);
+	if (ret) {
+		iounmap(mbox_base);
+		return ret;
 	}
 	return 0;
 
-err_out:
-	while (i--)
-		omap_mbox_unregister(list[i]);
-	iounmap(mbox_base);
 	return ret;
 }
 
 static int __devexit omap2_mbox_remove(struct platform_device *pdev)
 {
-	int i;
-
-	for (i = 0; list[i]; i++)
-		omap_mbox_unregister(list[i]);
-
+	omap_mbox_unregister();
 	iounmap(mbox_base);
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index aad8bf8..c44fde3 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -55,7 +55,6 @@ struct omap_mbox {
 	struct omap_mbox_queue	*txq, *rxq;
 	struct omap_mbox_ops	*ops;
 	struct device		*dev;
-	struct omap_mbox	*next;
 	void			*priv;
 };
 
@@ -65,8 +64,8 @@ void omap_mbox_init_seq(struct omap_mbox *);
 struct omap_mbox *omap_mbox_get(const char *);
 void omap_mbox_put(struct omap_mbox *);
 
-int omap_mbox_register(struct device *parent, struct omap_mbox *);
-int omap_mbox_unregister(struct omap_mbox *);
+int omap_mbox_register(struct device *parent, struct omap_mbox **);
+int omap_mbox_unregister(void);
 
 static inline void omap_mbox_save_ctx(struct omap_mbox *mbox)
 {
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 38a6cb1..a8e22e1 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -33,8 +33,7 @@
 #include <plat/mailbox.h>
 
 static struct workqueue_struct *mboxd;
-static struct omap_mbox *mboxes;
-static DEFINE_SPINLOCK(mboxes_lock);
+static struct omap_mbox **mboxes;
 static bool rq_full;
 
 static int mbox_configured;
@@ -307,31 +306,20 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
 	}
 }
 
-static struct omap_mbox **find_mboxes(const char *name)
-{
-	struct omap_mbox **p;
-
-	for (p = &mboxes; *p; p = &(*p)->next) {
-		if (strcmp((*p)->name, name) == 0)
-			break;
-	}
-
-	return p;
-}
-
 struct omap_mbox *omap_mbox_get(const char *name)
 {
 	struct omap_mbox *mbox;
 	int ret;
 
-	spin_lock(&mboxes_lock);
-	mbox = *(find_mboxes(name));
-	if (mbox == NULL) {
-		spin_unlock(&mboxes_lock);
-		return ERR_PTR(-ENOENT);
-	}
+	if (!mboxes)
+		return ERR_PTR(-EINVAL);
 
-	spin_unlock(&mboxes_lock);
+	for (mbox = *mboxes; mbox; mbox++)
+		if (!strcmp(mbox->name, name))
+			break;
+
+	if (!mbox)
+		return ERR_PTR(-ENOENT);
 
 	ret = omap_mbox_startup(mbox);
 	if (ret)
@@ -349,57 +337,44 @@ EXPORT_SYMBOL(omap_mbox_put);
 
 static struct class omap_mbox_class = { .name = "mbox", };
 
-int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
+int omap_mbox_register(struct device *parent, struct omap_mbox **list)
 {
-	int ret = 0;
-	struct omap_mbox **tmp;
+	int ret;
+	int i;
 
-	if (!mbox)
+	mboxes = list;
+	if (!mboxes)
 		return -EINVAL;
-	if (mbox->next)
-		return -EBUSY;
-
-	mbox->dev = device_create(&omap_mbox_class,
-				  parent, 0, mbox, "%s", mbox->name);
-	if (IS_ERR(mbox->dev))
-		return PTR_ERR(mbox->dev);
-
-	spin_lock(&mboxes_lock);
-	tmp = find_mboxes(mbox->name);
-	if (*tmp) {
-		ret = -EBUSY;
-		spin_unlock(&mboxes_lock);
-		goto err_find;
-	}
-	*tmp = mbox;
-	spin_unlock(&mboxes_lock);
 
+	for (i = 0; mboxes[i]; i++) {
+		struct omap_mbox *mbox = mboxes[i];
+		mbox->dev = device_create(&omap_mbox_class,
+				parent, 0, mbox, "%s", mbox->name);
+		if (IS_ERR(mbox->dev)) {
+			ret = PTR_ERR(mbox->dev);
+			goto err_out;
+		}
+	}
 	return 0;
 
-err_find:
+err_out:
+	while (i--)
+		device_unregister(mboxes[i]->dev);
 	return ret;
 }
 EXPORT_SYMBOL(omap_mbox_register);
 
-int omap_mbox_unregister(struct omap_mbox *mbox)
+int omap_mbox_unregister(void)
 {
-	struct omap_mbox **tmp;
-
-	spin_lock(&mboxes_lock);
-	tmp = &mboxes;
-	while (*tmp) {
-		if (mbox == *tmp) {
-			*tmp = mbox->next;
-			mbox->next = NULL;
-			spin_unlock(&mboxes_lock);
-			device_unregister(mbox->dev);
-			return 0;
-		}
-		tmp = &(*tmp)->next;
-	}
-	spin_unlock(&mboxes_lock);
+	int i;
 
-	return -EINVAL;
+	if (!mboxes)
+		return -EINVAL;
+
+	for (i = 0; mboxes[i]; i++)
+		device_unregister(mboxes[i]->dev);
+	mboxes = NULL;
+	return 0;
 }
 EXPORT_SYMBOL(omap_mbox_unregister);
 
-- 
1.7.1

[PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()
Date: Sat, 22 May 2010 19:14:24 +0200

> No need to dynamically register mailboxes one by one.

Can you squash this into [PATCH 10/14]?

I think that the "struct omap_mbox **list" would make more sense
without those reworking of modifications in this patch.

> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  arch/arm/mach-omap1/mailbox.c             |   25 ++------
>  arch/arm/mach-omap2/mailbox.c             |   22 ++-----
>  arch/arm/plat-omap/include/plat/mailbox.h |    5 +-
>  arch/arm/plat-omap/mailbox.c              |   95 +++++++++++------------------
>  4 files changed, 50 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
> index 4210896..e50b3c2 100644
> --- a/arch/arm/mach-omap1/mailbox.c
> +++ b/arch/arm/mach-omap1/mailbox.c
> @@ -29,8 +29,6 @@
>  
>  static void __iomem *mbox_base;
>  
> -static struct omap_mbox **list;
> -
>  struct omap_mbox1_fifo {
>  	unsigned long cmd;
>  	unsigned long data;
> @@ -151,38 +149,29 @@ static int __devinit omap1_mbox_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret;
>  	int i;
> +	struct omap_mbox **list;
>  
>  	res = pdev->resource;
>  
>  	list = omap1_mboxes;
> -
>  	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
>  
>  	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
>  	if (!mbox_base)
>  		return -ENOMEM;
>  
> -	for (i = 0; list[i]; i++) {
> -		ret = omap_mbox_register(&pdev->dev, list[i]);
> -		if (ret)
> -			goto err_out;
> +	ret = omap_mbox_register(&pdev->dev, list);
> +	if (ret) {
> +		iounmap(mbox_base);
> +		return ret;
>  	}
> -	return 0;
>  
> -err_out:
> -	while (i--)
> -		omap_mbox_unregister(list[i]);
> -	iounmap(mbox_base);
> -	return ret;
> +	return 0;
>  }
>  
>  static int __devexit omap1_mbox_remove(struct platform_device *pdev)
>  {
> -	int i;
> -
> -	for (i = 0; list[i]; i++)
> -		omap_mbox_unregister(list[i]);
> -
> +	omap_mbox_unregister();
>  	iounmap(mbox_base);
>  	return 0;
>  }
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index a433ca7..66d366d 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -54,8 +54,6 @@
>  
>  static void __iomem *mbox_base;
>  
> -static struct omap_mbox **list;
> -
>  struct omap_mbox2_fifo {
>  	unsigned long msg;
>  	unsigned long fifo_stat;
> @@ -393,7 +391,7 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	int ret;
> -	int i;
> +	struct omap_mbox **list;
>  
>  	res = pdev->resource;
>  
> @@ -430,27 +428,19 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  	if (!mbox_base)
>  		return -ENOMEM;
>  
> -	for (i = 0; list[i]; i++) {
> -		ret = omap_mbox_register(&pdev->dev, list[i]);
> -		if (ret)
> -			goto err_out;
> +	ret = omap_mbox_register(&pdev->dev, list);
> +	if (ret) {
> +		iounmap(mbox_base);
> +		return ret;
>  	}
>  	return 0;
>  
> -err_out:
> -	while (i--)
> -		omap_mbox_unregister(list[i]);
> -	iounmap(mbox_base);
>  	return ret;
>  }
>  
>  static int __devexit omap2_mbox_remove(struct platform_device *pdev)
>  {
> -	int i;
> -
> -	for (i = 0; list[i]; i++)
> -		omap_mbox_unregister(list[i]);
> -
> +	omap_mbox_unregister();
>  	iounmap(mbox_base);
>  	return 0;
>  }
> diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
> index aad8bf8..c44fde3 100644
> --- a/arch/arm/plat-omap/include/plat/mailbox.h
> +++ b/arch/arm/plat-omap/include/plat/mailbox.h
> @@ -55,7 +55,6 @@ struct omap_mbox {
>  	struct omap_mbox_queue	*txq, *rxq;
>  	struct omap_mbox_ops	*ops;
>  	struct device		*dev;
> -	struct omap_mbox	*next;
>  	void			*priv;
>  };
>  
> @@ -65,8 +64,8 @@ void omap_mbox_init_seq(struct omap_mbox *);
>  struct omap_mbox *omap_mbox_get(const char *);
>  void omap_mbox_put(struct omap_mbox *);
>  
> -int omap_mbox_register(struct device *parent, struct omap_mbox *);
> -int omap_mbox_unregister(struct omap_mbox *);
> +int omap_mbox_register(struct device *parent, struct omap_mbox **);
> +int omap_mbox_unregister(void);
>  
>  static inline void omap_mbox_save_ctx(struct omap_mbox *mbox)
>  {
> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
> index 38a6cb1..a8e22e1 100644
> --- a/arch/arm/plat-omap/mailbox.c
> +++ b/arch/arm/plat-omap/mailbox.c
> @@ -33,8 +33,7 @@
>  #include <plat/mailbox.h>
>  
>  static struct workqueue_struct *mboxd;
> -static struct omap_mbox *mboxes;
> -static DEFINE_SPINLOCK(mboxes_lock);
> +static struct omap_mbox **mboxes;
>  static bool rq_full;
>  
>  static int mbox_configured;
> @@ -307,31 +306,20 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
>  	}
>  }
>  
> -static struct omap_mbox **find_mboxes(const char *name)
> -{
> -	struct omap_mbox **p;
> -
> -	for (p = &mboxes; *p; p = &(*p)->next) {
> -		if (strcmp((*p)->name, name) == 0)
> -			break;
> -	}
> -
> -	return p;
> -}
> -
>  struct omap_mbox *omap_mbox_get(const char *name)
>  {
>  	struct omap_mbox *mbox;
>  	int ret;
>  
> -	spin_lock(&mboxes_lock);
> -	mbox = *(find_mboxes(name));
> -	if (mbox == NULL) {
> -		spin_unlock(&mboxes_lock);
> -		return ERR_PTR(-ENOENT);
> -	}
> +	if (!mboxes)
> +		return ERR_PTR(-EINVAL);
>  
> -	spin_unlock(&mboxes_lock);
> +	for (mbox = *mboxes; mbox; mbox++)
> +		if (!strcmp(mbox->name, name))
> +			break;
> +
> +	if (!mbox)
> +		return ERR_PTR(-ENOENT);
>  
>  	ret = omap_mbox_startup(mbox);
>  	if (ret)
> @@ -349,57 +337,44 @@ EXPORT_SYMBOL(omap_mbox_put);
>  
>  static struct class omap_mbox_class = { .name = "mbox", };
>  
> -int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
> +int omap_mbox_register(struct device *parent, struct omap_mbox **list)
>  {
> -	int ret = 0;
> -	struct omap_mbox **tmp;
> +	int ret;
> +	int i;
>  
> -	if (!mbox)
> +	mboxes = list;
> +	if (!mboxes)
>  		return -EINVAL;
> -	if (mbox->next)
> -		return -EBUSY;
> -
> -	mbox->dev = device_create(&omap_mbox_class,
> -				  parent, 0, mbox, "%s", mbox->name);
> -	if (IS_ERR(mbox->dev))
> -		return PTR_ERR(mbox->dev);
> -
> -	spin_lock(&mboxes_lock);
> -	tmp = find_mboxes(mbox->name);
> -	if (*tmp) {
> -		ret = -EBUSY;
> -		spin_unlock(&mboxes_lock);
> -		goto err_find;
> -	}
> -	*tmp = mbox;
> -	spin_unlock(&mboxes_lock);
>  
> +	for (i = 0; mboxes[i]; i++) {
> +		struct omap_mbox *mbox = mboxes[i];
> +		mbox->dev = device_create(&omap_mbox_class,
> +				parent, 0, mbox, "%s", mbox->name);
> +		if (IS_ERR(mbox->dev)) {
> +			ret = PTR_ERR(mbox->dev);
> +			goto err_out;
> +		}
> +	}
>  	return 0;
>  
> -err_find:
> +err_out:
> +	while (i--)
> +		device_unregister(mboxes[i]->dev);
>  	return ret;
>  }
>  EXPORT_SYMBOL(omap_mbox_register);
>  
> -int omap_mbox_unregister(struct omap_mbox *mbox)
> +int omap_mbox_unregister(void)
>  {
> -	struct omap_mbox **tmp;
> -
> -	spin_lock(&mboxes_lock);
> -	tmp = &mboxes;
> -	while (*tmp) {
> -		if (mbox == *tmp) {
> -			*tmp = mbox->next;
> -			mbox->next = NULL;
> -			spin_unlock(&mboxes_lock);
> -			device_unregister(mbox->dev);
> -			return 0;
> -		}
> -		tmp = &(*tmp)->next;
> -	}
> -	spin_unlock(&mboxes_lock);
> +	int i;
>  
> -	return -EINVAL;
> +	if (!mboxes)
> +		return -EINVAL;
> +
> +	for (i = 0; mboxes[i]; i++)
> +		device_unregister(mboxes[i]->dev);
> +	mboxes = NULL;
> +	return 0;
>  }
>  EXPORT_SYMBOL(omap_mbox_unregister);
>  
> -- 
> 1.7.1
> 

[PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()

[Felipe Contreras]

On Mon, May 24, 2010 at 5:42 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> From: ext Felipe Contreras <felipe.contreras@gmail.com>
> Subject: [PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()
> Date: Sat, 22 May 2010 19:14:24 +0200
>
>> No need to dynamically register mailboxes one by one.
>
> Can you squash this into [PATCH 10/14]?
>
> I think that the "struct omap_mbox **list" would make more sense
> without those reworking of modifications in this patch.

I don't know, I'm looking at the patch and I think it's much more
difficult to figure out what's happening.

See:

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index 590ac66..3df1c28 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -142,44 +142,35 @@ struct omap_mbox mbox_dsp_info = {
 	.priv	= &omap1_mbox_dsp_priv,
 };

+struct omap_mbox *omap1_mboxes[] = { &mbox_dsp_info, NULL };
+
 static int __devinit omap1_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int ret;
+	int i;
+	struct omap_mbox **list;

-	/* MBOX base */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid mem resource\n");
-		return -ENODEV;
-	}
+	list = omap1_mboxes;
+	list[0]->irq = platform_get_irq_byname(pdev, "dsp");

-	mbox_base = ioremap(res->start, resource_size(res));
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
 	if (!mbox_base)
 		return -ENOMEM;

-	/* DSP IRQ */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid irq resource\n");
-		ret = -ENODEV;
-		goto err_out;
+	ret = omap_mbox_register(&pdev->dev, list);
+	if (ret) {
+		iounmap(mbox_base);
+		return ret;
 	}
-	mbox_dsp_info.irq = res->start;

-	ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
-	if (ret)
-		goto err_out;
 	return 0;
-
-err_out:
-	iounmap(mbox_base);
-	return ret;
 }

 static int __devexit omap1_mbox_remove(struct platform_device *pdev)
 {
-	omap_mbox_unregister(&mbox_dsp_info);
+	omap_mbox_unregister();
 	iounmap(mbox_base);
 	return 0;
 }
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 3ff1ad5..f336515 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -307,6 +307,8 @@ struct omap_mbox mbox_dsp_info = {
 	.priv	= &omap2_mbox_dsp_priv,
 };

+struct omap_mbox *omap3_mboxes[] = { &mbox_dsp_info, NULL };
+
 #if defined(CONFIG_ARCH_OMAP2420)

 /* IVA */
@@ -331,6 +333,8 @@ static struct omap_mbox mbox_iva_info = {
 	.ops	= &omap2_mbox_ops,
 	.priv	= &omap2_mbox_iva_priv,
 };
+
+struct omap_mbox *omap2_mboxes[] = { &mbox_iva_info, &mbox_dsp_info, NULL };
 #endif

 /* OMAP4 */
@@ -378,89 +382,56 @@ struct omap_mbox mbox_2_info = {
 	.priv	= &omap2_mbox_2_priv,
 };

+struct omap_mbox *omap4_mboxes[] = { &mbox_1_info, &mbox_2_info, NULL };
+
 static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	int ret;
+	struct omap_mbox **list;

-	/* MBOX base */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid mem resource\n");
-		return -ENODEV;
+	if (cpu_is_omap3430()) {
+		list = omap3_mboxes;
+
+		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
 	}
-	mbox_base = ioremap(res->start, resource_size(res));
-	if (!mbox_base)
-		return -ENOMEM;
+#if defined(CONFIG_ARCH_OMAP2420)
+	else if (cpu_is_omap2420()) {
+		list = omap2_mboxes;

-	/* DSP or IVA2 IRQ */
-	res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		list[0]->irq = platform_get_irq_byname(pdev, "dsp");
+		list[1]->irq = platform_get_irq_byname(pdev, "iva");
+	}
+#endif
+	else if (cpu_is_omap44xx()) {
+		list = omap4_mboxes;

-	if (unlikely(!res)) {
-		dev_err(&pdev->dev, "invalid irq resource\n");
-		ret = -ENODEV;
-		goto err_dsp;
+		list[0]->irq = list[1]->irq =
+			platform_get_irq_byname(pdev, "mbox");
 	}
-	if (cpu_is_omap44xx()) {
-		mbox_1_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_1_info);
-	} else {
-		mbox_dsp_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_dsp_info);
+	else {
+		pr_err("%s: platform not supported\n", __func__);
+		return -ENODEV;
 	}
-	if (ret)
-		goto err_dsp;

-	if (cpu_is_omap44xx()) {
-		mbox_2_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_2_info);
-		if (ret) {
-			omap_mbox_unregister(&mbox_1_info);
-			goto err_dsp;
-		}
-	}
-#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
-	if (cpu_is_omap2420()) {
-		/* IVA IRQ */
-		res = platform_get_resource(pdev, IORESOURCE_IRQ, 1);
-		if (unlikely(!res)) {
-			dev_err(&pdev->dev, "invalid irq resource\n");
-			ret = -ENODEV;
-			omap_mbox_unregister(&mbox_dsp_info);
-			goto err_dsp;
-		}
-		mbox_iva_info.irq = res->start;
-		ret = omap_mbox_register(&pdev->dev, &mbox_iva_info);
-		if (ret) {
-			omap_mbox_unregister(&mbox_dsp_info);
-			goto err_dsp;
-		}
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
+	if (!mbox_base)
+		return -ENOMEM;
+
+	ret = omap_mbox_register(&pdev->dev, list);
+	if (ret) {
+		iounmap(mbox_base);
+		return ret;
 	}
-#endif
 	return 0;

-#if defined(CONFIG_ARCH_OMAP2420) /* IVA */
-err_iva1:
-	omap_mbox_unregister(&mbox_dsp_info);
-#endif
-
-err_dsp:
-	iounmap(mbox_base);
 	return ret;
 }

 static int __devexit omap2_mbox_remove(struct platform_device *pdev)
 {
-#if defined(CONFIG_ARCH_OMAP2420)
-	if (cpu_is_omap2420())
-		omap_mbox_unregister(&mbox_iva_info);
-#endif
-
-	if (cpu_is_omap44xx()) {
-		omap_mbox_unregister(&mbox_2_info);
-		omap_mbox_unregister(&mbox_1_info);
-	} else
-		omap_mbox_unregister(&mbox_dsp_info);
+	omap_mbox_unregister();
 	iounmap(mbox_base);
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h
b/arch/arm/plat-omap/include/plat/mailbox.h
index aad8bf8..c44fde3 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -55,7 +55,6 @@ struct omap_mbox {
 	struct omap_mbox_queue	*txq, *rxq;
 	struct omap_mbox_ops	*ops;
 	struct device		*dev;
-	struct omap_mbox	*next;
 	void			*priv;
 };

@@ -65,8 +64,8 @@ void omap_mbox_init_seq(struct omap_mbox *);
 struct omap_mbox *omap_mbox_get(const char *);
 void omap_mbox_put(struct omap_mbox *);

-int omap_mbox_register(struct device *parent, struct omap_mbox *);
-int omap_mbox_unregister(struct omap_mbox *);
+int omap_mbox_register(struct device *parent, struct omap_mbox **);
+int omap_mbox_unregister(void);

 static inline void omap_mbox_save_ctx(struct omap_mbox *mbox)
 {
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index 38a6cb1..a8e22e1 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -33,8 +33,7 @@
 #include <plat/mailbox.h>

 static struct workqueue_struct *mboxd;
-static struct omap_mbox *mboxes;
-static DEFINE_SPINLOCK(mboxes_lock);
+static struct omap_mbox **mboxes;
 static bool rq_full;

 static int mbox_configured;
@@ -307,31 +306,20 @@ static void omap_mbox_fini(struct omap_mbox *mbox)
 	}
 }

-static struct omap_mbox **find_mboxes(const char *name)
-{
-	struct omap_mbox **p;
-
-	for (p = &mboxes; *p; p = &(*p)->next) {
-		if (strcmp((*p)->name, name) == 0)
-			break;
-	}
-
-	return p;
-}
-
 struct omap_mbox *omap_mbox_get(const char *name)
 {
 	struct omap_mbox *mbox;
 	int ret;

-	spin_lock(&mboxes_lock);
-	mbox = *(find_mboxes(name));
-	if (mbox == NULL) {
-		spin_unlock(&mboxes_lock);
-		return ERR_PTR(-ENOENT);
-	}
+	if (!mboxes)
+		return ERR_PTR(-EINVAL);

-	spin_unlock(&mboxes_lock);
+	for (mbox = *mboxes; mbox; mbox++)
+		if (!strcmp(mbox->name, name))
+			break;
+
+	if (!mbox)
+		return ERR_PTR(-ENOENT);

 	ret = omap_mbox_startup(mbox);
 	if (ret)
@@ -349,57 +337,44 @@ EXPORT_SYMBOL(omap_mbox_put);

 static struct class omap_mbox_class = { .name = "mbox", };

-int omap_mbox_register(struct device *parent, struct omap_mbox *mbox)
+int omap_mbox_register(struct device *parent, struct omap_mbox **list)
 {
-	int ret = 0;
-	struct omap_mbox **tmp;
+	int ret;
+	int i;

-	if (!mbox)
+	mboxes = list;
+	if (!mboxes)
 		return -EINVAL;
-	if (mbox->next)
-		return -EBUSY;
-
-	mbox->dev = device_create(&omap_mbox_class,
-				  parent, 0, mbox, "%s", mbox->name);
-	if (IS_ERR(mbox->dev))
-		return PTR_ERR(mbox->dev);
-
-	spin_lock(&mboxes_lock);
-	tmp = find_mboxes(mbox->name);
-	if (*tmp) {
-		ret = -EBUSY;
-		spin_unlock(&mboxes_lock);
-		goto err_find;
-	}
-	*tmp = mbox;
-	spin_unlock(&mboxes_lock);

+	for (i = 0; mboxes[i]; i++) {
+		struct omap_mbox *mbox = mboxes[i];
+		mbox->dev = device_create(&omap_mbox_class,
+				parent, 0, mbox, "%s", mbox->name);
+		if (IS_ERR(mbox->dev)) {
+			ret = PTR_ERR(mbox->dev);
+			goto err_out;
+		}
+	}
 	return 0;

-err_find:
+err_out:
+	while (i--)
+		device_unregister(mboxes[i]->dev);
 	return ret;
 }
 EXPORT_SYMBOL(omap_mbox_register);

-int omap_mbox_unregister(struct omap_mbox *mbox)
+int omap_mbox_unregister(void)
 {
-	struct omap_mbox **tmp;
-
-	spin_lock(&mboxes_lock);
-	tmp = &mboxes;
-	while (*tmp) {
-		if (mbox == *tmp) {
-			*tmp = mbox->next;
-			mbox->next = NULL;
-			spin_unlock(&mboxes_lock);
-			device_unregister(mbox->dev);
-			return 0;
-		}
-		tmp = &(*tmp)->next;
-	}
-	spin_unlock(&mboxes_lock);
+	int i;

-	return -EINVAL;
+	if (!mboxes)
+		return -EINVAL;
+
+	for (i = 0; mboxes[i]; i++)
+		device_unregister(mboxes[i]->dev);
+	mboxes = NULL;
+	return 0;
 }
 EXPORT_SYMBOL(omap_mbox_unregister);

-- 
Felipe Contreras

[PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()

[Felipe Contreras]

On Fri, Jun 11, 2010 at 6:17 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, May 24, 2010 at 5:42 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> From: ext Felipe Contreras <felipe.contreras@gmail.com>
>> Subject: [PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()
>> Date: Sat, 22 May 2010 19:14:24 +0200
>>
>>> No need to dynamically register mailboxes one by one.
>>
>> Can you squash this into [PATCH 10/14]?
>>
>> I think that the "struct omap_mbox **list" would make more sense
>> without those reworking of modifications in this patch.
>
> I don't know, I'm looking at the patch and I think it's much more
> difficult to figure out what's happening.

I forgot to mention: what I will do is put one patch after the other.

-- 
Felipe Contreras

[PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 13/14] omap: mailbox: simplify omap_mbox_register()
Date: Sat, 22 May 2010 19:14:24 +0200

> No need to dynamically register mailboxes one by one.
> 
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  arch/arm/mach-omap1/mailbox.c             |   25 ++------
>  arch/arm/mach-omap2/mailbox.c             |   22 ++-----
>  arch/arm/plat-omap/include/plat/mailbox.h |    5 +-
>  arch/arm/plat-omap/mailbox.c              |   95 +++++++++++------------------
>  4 files changed, 50 insertions(+), 97 deletions(-)
> 
> diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
> index 4210896..e50b3c2 100644
> --- a/arch/arm/mach-omap1/mailbox.c
> +++ b/arch/arm/mach-omap1/mailbox.c
> @@ -29,8 +29,6 @@
>  
>  static void __iomem *mbox_base;
>  
> -static struct omap_mbox **list;
> -
>  struct omap_mbox1_fifo {
>  	unsigned long cmd;
>  	unsigned long data;
> @@ -151,38 +149,29 @@ static int __devinit omap1_mbox_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	int ret;
>  	int i;
> +	struct omap_mbox **list;
>  
>  	res = pdev->resource;
>  
>  	list = omap1_mboxes;
> -
>  	list[0]->irq = platform_get_irq_byname(pdev, "dsp");
>  
>  	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
>  	if (!mbox_base)
>  		return -ENOMEM;
>  
> -	for (i = 0; list[i]; i++) {
> -		ret = omap_mbox_register(&pdev->dev, list[i]);
> -		if (ret)
> -			goto err_out;
> +	ret = omap_mbox_register(&pdev->dev, list);
> +	if (ret) {
> +		iounmap(mbox_base);
> +		return ret;
>  	}
> -	return 0;
>  
> -err_out:
> -	while (i--)
> -		omap_mbox_unregister(list[i]);
> -	iounmap(mbox_base);
> -	return ret;
> +	return 0;
>  }
>  
>  static int __devexit omap1_mbox_remove(struct platform_device *pdev)
>  {
> -	int i;
> -
> -	for (i = 0; list[i]; i++)
> -		omap_mbox_unregister(list[i]);
> -
> +	omap_mbox_unregister();
>  	iounmap(mbox_base);
>  	return 0;
>  }
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index a433ca7..66d366d 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -54,8 +54,6 @@
>  
>  static void __iomem *mbox_base;
>  
> -static struct omap_mbox **list;
> -
>  struct omap_mbox2_fifo {
>  	unsigned long msg;
>  	unsigned long fifo_stat;
> @@ -393,7 +391,7 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
>  	int ret;
> -	int i;
> +	struct omap_mbox **list;
>  
>  	res = pdev->resource;
>  
> @@ -430,27 +428,19 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  	if (!mbox_base)
>  		return -ENOMEM;
>  
> -	for (i = 0; list[i]; i++) {
> -		ret = omap_mbox_register(&pdev->dev, list[i]);
> -		if (ret)
> -			goto err_out;
> +	ret = omap_mbox_register(&pdev->dev, list);
> +	if (ret) {
> +		iounmap(mbox_base);
> +		return ret;
>  	}
>  	return 0;
>  
> -err_out:
> -	while (i--)
> -		omap_mbox_unregister(list[i]);
> -	iounmap(mbox_base);
>  	return ret;
>  }

The above result in:

    ....
	ret = omap_mbox_register(&pdev->dev, list);
	if (ret) {
		iounmap(mbox_base);
		return ret;
	}
	return 0;

	return ret;
}

[PATCH v3 14/14] omap: mailbox: reorganize headers

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap1/mailbox.c             |    3 ---
 arch/arm/mach-omap2/mailbox.c             |    1 -
 arch/arm/plat-omap/include/plat/mailbox.h |    3 ++-
 arch/arm/plat-omap/mailbox.c              |    5 ++---
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
index e50b3c2..fdd6cc9 100644
--- a/arch/arm/mach-omap1/mailbox.c
+++ b/arch/arm/mach-omap1/mailbox.c
@@ -9,13 +9,10 @@
  * for more details.
  */
 
-#include <linux/kernel.h>
-#include <linux/resource.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <plat/mailbox.h>
-#include <mach/irqs.h>
 
 #define MAILBOX_ARM2DSP1		0x00
 #define MAILBOX_ARM2DSP1b		0x04
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index 66d366d..d46e439 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -10,7 +10,6 @@
  * for more details.
  */
 
-#include <linux/kernel.h>
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index c44fde3..9976565 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -3,9 +3,10 @@
 #ifndef MAILBOX_H
 #define MAILBOX_H
 
-#include <linux/wait.h>
+#include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <linux/interrupt.h>
+#include <linux/device.h>
 #include <linux/kfifo.h>
 
 typedef u32 mbox_msg_t;
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
index a8e22e1..8d4c7be 100644
--- a/arch/arm/plat-omap/mailbox.c
+++ b/arch/arm/plat-omap/mailbox.c
@@ -21,10 +21,9 @@
  *
  */
 
-#include <linux/kernel.h>
-#include <linux/module.h>
 #include <linux/interrupt.h>
-#include <linux/device.h>
+#include <linux/spinlock.h>
+#include <linux/mutex.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <linux/kfifo.h>
-- 
1.7.1

[PATCH v3 14/14] omap: mailbox: reorganize headers

[Felipe Balbi]

Hi,

On Sat, May 22, 2010 at 07:14:25PM +0200, ext Felipe Contreras wrote:
>Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>---
> arch/arm/mach-omap1/mailbox.c             |    3 ---
> arch/arm/mach-omap2/mailbox.c             |    1 -
> arch/arm/plat-omap/include/plat/mailbox.h |    3 ++-
> arch/arm/plat-omap/mailbox.c              |    5 ++---
> 4 files changed, 4 insertions(+), 8 deletions(-)
>
>diff --git a/arch/arm/mach-omap1/mailbox.c b/arch/arm/mach-omap1/mailbox.c
>index e50b3c2..fdd6cc9 100644
>--- a/arch/arm/mach-omap1/mailbox.c
>+++ b/arch/arm/mach-omap1/mailbox.c
>@@ -9,13 +9,10 @@
>  * for more details.
>  */
>
>-#include <linux/kernel.h>
>-#include <linux/resource.h>
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <plat/mailbox.h>
>-#include <mach/irqs.h>
>
> #define MAILBOX_ARM2DSP1		0x00
> #define MAILBOX_ARM2DSP1b		0x04
>diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
>index 66d366d..d46e439 100644
>--- a/arch/arm/mach-omap2/mailbox.c
>+++ b/arch/arm/mach-omap2/mailbox.c
>@@ -10,7 +10,6 @@
>  * for more details.
>  */
>
>-#include <linux/kernel.h>
> #include <linux/clk.h>
> #include <linux/err.h>
> #include <linux/platform_device.h>
>diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
>index c44fde3..9976565 100644
>--- a/arch/arm/plat-omap/include/plat/mailbox.h
>+++ b/arch/arm/plat-omap/include/plat/mailbox.h
>@@ -3,9 +3,10 @@
> #ifndef MAILBOX_H
> #define MAILBOX_H
>
>-#include <linux/wait.h>
>+#include <linux/spinlock.h>
> #include <linux/workqueue.h>
> #include <linux/interrupt.h>
>+#include <linux/device.h>
> #include <linux/kfifo.h>
>
> typedef u32 mbox_msg_t;
>diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c
>index a8e22e1..8d4c7be 100644
>--- a/arch/arm/plat-omap/mailbox.c
>+++ b/arch/arm/plat-omap/mailbox.c
>@@ -21,10 +21,9 @@
>  *
>  */
>
>-#include <linux/kernel.h>
>-#include <linux/module.h>
> #include <linux/interrupt.h>
>-#include <linux/device.h>
>+#include <linux/spinlock.h>
>+#include <linux/mutex.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> #include <linux/kfifo.h>

personally I don't like this patch since it makes the C-sources rely on 
indirect header inclusion by another header. That's really error prone 
and will cause build failures if someone decides to remove the include 
<linux/device.h> from plat/mailbox.h.

Those headers are safe against multiple inclusions and I think they 
should be included directly in all source files (header or not) that 
need any symbols from it.

-- 
balbi

DefectiveByDesign.org

[PATCH v3 14/14] omap: mailbox: reorganize headers

[Felipe Contreras]

On Mon, May 24, 2010 at 8:16 PM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> personally I don't like this patch since it makes the C-sources rely on
> indirect header inclusion by another header. That's really error prone and
> will cause build failures if someone decides to remove the include
> <linux/device.h> from plat/mailbox.h.

First of all, you cannot expect each and every source file to include
each and every header it's using directly. It would be nice, but it's
not happening now, and probably will not happen ever.

Now, with a few exceptions, I didn't assume somebody else would be
loading anything, in fact, the patch is mostly _adding_ headers that
were missing.

If you take a look at the modified files regarding kernel.h and module.h:

* arch/arm/mach-omap1/mailbox.c:
 Not really using kernel.h, missing module.h

* arch/arm/mach-omap2/mailbox.c:
 Missing module.h

* arch/arm/plat-omap/mailbox.c
 Missing kernel.h

So it was clear to me that these were not carefully added, and
probably were redundant, so I choose to remove them completely. If you
have something against that, I can add them where it makes sense but
the current patch is not really making the situation much worst.

Now, regarding device.h:

* arch/arm/mach-omap2/mailbox.c
 Using it, but missing

* arch/arm/plat-omap/include/plat/mailbox.h
 Was using it, but a 'struct device' would suffice

 * arch/arm/plat-omap/mailbox.c
 Using it correctly

Now, I am pretty sure that platform_device.h will always depend on
device.h that's why I removed them, but I can add them again if
somebody wants to, but again, the situation is already bad.

-- 
Felipe Contreras

[PATCH v3 14/14] omap: mailbox: reorganize headers

[Felipe Balbi]

Hi,

On Mon, May 24, 2010 at 09:14:24PM +0200, ext Felipe Contreras wrote:
>Now, regarding device.h:

which was my only concern

>Now, I am pretty sure that platform_device.h will always depend on
>device.h that's why I removed them, but I can add them again if
>somebody wants to, but again, the situation is already bad.

so there's platform_device.h on that file, a little comment on the 
changelog would suffice to avoid this kind of mis-understanding.

-- 
balbi

DefectiveByDesign.org

[PATCH v3 14/14] omap: mailbox: reorganize headers

[Felipe Contreras]

Hello,

On Tue, May 25, 2010 at 8:49 AM, Felipe Balbi <felipe.balbi@nokia.com> wrote:
> On Mon, May 24, 2010 at 09:14:24PM +0200, ext Felipe Contreras wrote:
>> Now, I am pretty sure that platform_device.h will always depend on
>> device.h that's why I removed them, but I can add them again if
>> somebody wants to, but again, the situation is already bad.
>
> so there's platform_device.h on that file, a little comment on the changelog
> would suffice to avoid this kind of mis-understanding.

Sure, will do.

-- 
Felipe Contreras

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Hiroshi DOYU]

Hi Felipe,

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: [PATCH v3 00/14] omap: mailbox: bunch of cleanups
Date: Sat, 22 May 2010 19:14:11 +0200

> Hi,
> 
> These are hopefully non-functional changes. Just shuffling code around, and
> removing unecessary stuff.
> 
> In v2 I tried to split platform_device and also make mailbox_mach built-in. Not
> any more.
> 
> Comments from Felipe Balbi, Tony Lindgren, Hiroshi DOYU, and Russell King.
> 

Have you had a chance to update them?

> Felipe Contreras (14):
>   omap: mailbox: trivial whitespace cleanups
>   omap: mailbox: trivial cleanups
>   omap: mailbox: reorganize structures
>   omap: mailbox: 2420 should be detected at run-time
>   omap: mailbox: use correct config for omap1
>   omap: mailbox: update omap1 probing
>   omap: mailbox: don't export unecessary symbols
>   omap: mailbox: remove unecessary fields
>   omap: mailbox: add IRQ names
>   omap: mailbox: reorganize registering
>   omap: mailbox: only compile for configured archs
>   omap: mailbox: standarize on 'omap-mailbox'
>   omap: mailbox: simplify omap_mbox_register()
>   omap: mailbox: reorganize headers
> 
>  arch/arm/mach-omap1/devices.c             |   11 +-
>  arch/arm/mach-omap1/mailbox.c             |   52 ++++-----
>  arch/arm/mach-omap2/devices.c             |    6 +-
>  arch/arm/mach-omap2/mailbox.c             |  177 +++++++++++++----------------
>  arch/arm/plat-omap/include/plat/mailbox.h |   16 +--
>  arch/arm/plat-omap/mailbox.c              |  136 +++++++++-------------
>  6 files changed, 168 insertions(+), 230 deletions(-)
> 

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Felipe Contreras]

On Thu, Jun 3, 2010 at 9:40 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> Have you had a chance to update them?

Sorry, no. I've been busy with user-space stuff. Probably this weekend.

There's no merge-window rush or anything, right?

-- 
Felipe Contreras

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Hiroshi DOYU]

From: ext Felipe Contreras <felipe.contreras@gmail.com>
Subject: Re: [PATCH v3 00/14] omap: mailbox: bunch of cleanups
Date: Thu, 3 Jun 2010 12:23:43 +0200

> On Thu, Jun 3, 2010 at 9:40 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> Have you had a chance to update them?
> 
> Sorry, no. I've been busy with user-space stuff. Probably this weekend.
> 
> There's no merge-window rush or anything, right?

No rush. Sorry for just ping.

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Russell King - ARM Linux]

On Thu, Jun 03, 2010 at 09:40:22AM +0300, Hiroshi DOYU wrote:
> > Felipe Contreras (14):
> >   omap: mailbox: trivial whitespace cleanups
> >   omap: mailbox: trivial cleanups

I think folk need to read this:

	http://lkml.org/lkml/2010/6/2/472

and consider what it means for the future, and what can be done to reduce
the amount of "churn".

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Felipe Balbi]

Hi,

On Thu, Jun 03, 2010 at 01:47:26PM +0200, ext Russell King - ARM Linux wrote:
>I think folk need to read this:
>
>	http://lkml.org/lkml/2010/6/2/472
>
>and consider what it means for the future, and what can be done to reduce
>the amount of "churn".

I believe Linus is considering more the amount of files not the amount 
of patches. Personally, I consider trivial cleanup patches the best way 
to start off with a new code (not saying that Felipe C. is new to mbox 
driver) and those patches are as needed as a brand new feature.

One thing to decrease the amount of lines of code in arch/arm is to move 
the drivers to they're proper locations on the tree. Only on omap we 
have 6 platform_drivers that could (should) be moved to proper 
locations after a 'trivial cleanups'.

gpio.c mcbsp.c timer drivers (for clocksource) etc, which alone would 
mean a decrease of 6237 lines out of arch/arm/*omap* only. And I'm not 
even counting the drivers which aren't platform_drivers.

The thing is that ARM is really different than x86 and there's no 
standardization among different SoCs, so other than moving drivers out 
of arch/arm, there isn't much we can do. Or do you have any ideas ?

-- 
balbi

DefectiveByDesign.org

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Baruch Siach]

Hi Russell,

On Thu, Jun 03, 2010 at 12:47:26PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jun 03, 2010 at 09:40:22AM +0300, Hiroshi DOYU wrote:
> > > Felipe Contreras (14):
> > >   omap: mailbox: trivial whitespace cleanups
> > >   omap: mailbox: trivial cleanups
> 
> I think folk need to read this:
> 
> 	http://lkml.org/lkml/2010/6/2/472
> 
> and consider what it means for the future, and what can be done to reduce
> the amount of "churn".

Can you elaborate a bit? I guess you generally agree with Linus on this. What 
should be done in your opinion for the ARM arch to produce less "churn".

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Felipe Contreras]

On Thu, Jun 3, 2010 at 2:47 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Jun 03, 2010 at 09:40:22AM +0300, Hiroshi DOYU wrote:
>> > Felipe Contreras (14):
>> > ? omap: mailbox: trivial whitespace cleanups
>> > ? omap: mailbox: trivial cleanups
>
> I think folk need to read this:
>
> ? ? ? ?http://lkml.org/lkml/2010/6/2/472
>
> and consider what it means for the future, and what can be done to reduce
> the amount of "churn".

If I have to decipher that rant, my best guess is that he is
complaining about the amount of code. If that's the case, this patch
series is only doing good:
6 files changed, 168 insertions(+), 230 deletions(-)

Another part of the rant is the fact that Linus cannot follow the huge
amount of changes. I think "trivial whitespace cleanups" pretty much
says: don't look at me if you want to be productive, while "trivial
cleanups" says, look at me if you feel like looking at easy changes.
If I were to squash these two, I think the patch series would become
harder to follow.

Something that helps in other projects, like git, is that merges are
clearly identified.

Merge branch 'linux-arm'
 Merge branch 'linux-omap'
  Merge branch 'linux-omap-mbox'
   Merge branch 'fc-cleanups'
   Merge branch 'ob-kfifo'

Then it's a bit clearer what each patch series is doing in the mailbox
tree without looking at individual patches.

Cheers.

-- 
Felipe Contreras

[PATCH v3 00/14] omap: mailbox: bunch of cleanups

[Tony Lindgren]

* Felipe Contreras <felipe.contreras@gmail.com> [100603 17:53]:
> On Thu, Jun 3, 2010 at 2:47 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Jun 03, 2010 at 09:40:22AM +0300, Hiroshi DOYU wrote:
> >> > Felipe Contreras (14):
> >> > ? omap: mailbox: trivial whitespace cleanups
> >> > ? omap: mailbox: trivial cleanups
> >
> > I think folk need to read this:
> >
> > ? ? ? ?http://lkml.org/lkml/2010/6/2/472
> >
> > and consider what it means for the future, and what can be done to reduce
> > the amount of "churn".
> 
> If I have to decipher that rant, my best guess is that he is
> complaining about the amount of code. If that's the case, this patch
> series is only doing good:
> 6 files changed, 168 insertions(+), 230 deletions(-)
> 
> Another part of the rant is the fact that Linus cannot follow the huge
> amount of changes. I think "trivial whitespace cleanups" pretty much
> says: don't look at me if you want to be productive, while "trivial
> cleanups" says, look at me if you feel like looking at easy changes.
> If I were to squash these two, I think the patch series would become
> harder to follow.
> 
> Something that helps in other projects, like git, is that merges are
> clearly identified.
> 
> Merge branch 'linux-arm'
>  Merge branch 'linux-omap'
>   Merge branch 'linux-omap-mbox'
>    Merge branch 'fc-cleanups'
>    Merge branch 'ob-kfifo'
> 
> Then it's a bit clearer what each patch series is doing in the mailbox
> tree without looking at individual patches.

Also we need to make sure all the code we merge is really tested
well before the merge window. Too much of the code needs to be
fixed after the merge window. For the upcoming merge window, we
will only merge things that have been sitting in the for-next
for days before the merge window opens.

Tony