[RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[Felipe Contreras]

Hi,

I played a bit with omap_hwmod/omap_device for the mailbox, and this is the
result.

I only have an OMAP3 (bb), and seems to work fine :)

Felipe Contreras (2):
  omap: mailbox: initial hwmod support
  omap: mailbox: add omap_device latency information

 arch/arm/mach-omap2/devices.c              |  107 +++++++---------------------
 arch/arm/mach-omap2/mailbox.c              |   34 +++------
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
 arch/arm/plat-omap/include/plat/mailbox.h  |    7 ++
 4 files changed, 90 insertions(+), 103 deletions(-)

[RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Felipe Contreras]

Only OMAP3 would work.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/devices.c              |  103 +++++----------------------
 arch/arm/mach-omap2/mailbox.c              |   14 +---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
 arch/arm/plat-omap/include/plat/mailbox.h  |    4 +
 4 files changed, 72 insertions(+), 94 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index d9ac853..89f6cbc 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -20,12 +20,14 @@
 #include <asm/mach-types.h>
 #include <asm/mach/map.h>
 
+#include <plat/omap_device.h>
 #include <plat/control.h>
 #include <plat/tc.h>
 #include <plat/board.h>
 #include <plat/mux.h>
 #include <mach/gpio.h>
 #include <plat/mmc.h>
+#include <plat/mailbox.h>
 
 #include "mux.h"
 
@@ -137,95 +139,28 @@ static inline void omap_init_camera(void)
 #endif
 
 #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
+static inline void omap_init_mbox(void)
+{
+	struct omap_hwmod *hwmod;
+	struct omap_device *odev;
+	struct omap_mbox_platform_data pdata;
 
-#define MBOX_REG_SIZE   0x120
-
-#ifdef CONFIG_ARCH_OMAP2
-static struct resource omap2_mbox_resources[] = {
-	{
-		.start		= OMAP24XX_MAILBOX_BASE,
-		.end		= OMAP24XX_MAILBOX_BASE + MBOX_REG_SIZE - 1,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.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);
-#else
-#define omap2_mbox_resources		NULL
-#define omap2_mbox_resources_sz		0
-#endif
-
-#ifdef CONFIG_ARCH_OMAP3
-static struct resource omap3_mbox_resources[] = {
-	{
-		.start		= OMAP34XX_MAILBOX_BASE,
-		.end		= OMAP34XX_MAILBOX_BASE + MBOX_REG_SIZE - 1,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= INT_24XX_MAIL_U0_MPU,
-		.flags		= IORESOURCE_IRQ,
-		.name		= "dsp",
-	},
-};
-static int omap3_mbox_resources_sz = ARRAY_SIZE(omap3_mbox_resources);
-#else
-#define omap3_mbox_resources		NULL
-#define omap3_mbox_resources_sz		0
-#endif
-
-#ifdef CONFIG_ARCH_OMAP4
-
-#define OMAP4_MBOX_REG_SIZE	0x130
-static struct resource omap4_mbox_resources[] = {
-	{
-		.start          = OMAP44XX_MAILBOX_BASE,
-		.end            = OMAP44XX_MAILBOX_BASE +
-					OMAP4_MBOX_REG_SIZE - 1,
-		.flags          = IORESOURCE_MEM,
-	},
-	{
-		.start          = OMAP44XX_IRQ_MAIL_U0,
-		.flags          = IORESOURCE_IRQ,
-		.name		= "mbox",
-	},
-};
-static int omap4_mbox_resources_sz = ARRAY_SIZE(omap4_mbox_resources);
-#else
-#define omap4_mbox_resources		NULL
-#define omap4_mbox_resources_sz		0
-#endif
+	hwmod = omap_hwmod_lookup("mailbox");
+	if (!hwmod) {
+		pr_err("%s: unable to find hwmod\n", __func__);
+		return;
+	}
 
-static struct platform_device mbox_device = {
-	.name		= "omap-mailbox",
-	.id		= -1,
-};
+	pdata.base_addr = hwmod->_rt_va;
 
-static inline void omap_init_mbox(void)
-{
-	if (cpu_is_omap24xx()) {
-		mbox_device.resource = omap2_mbox_resources;
-		mbox_device.num_resources = omap2_mbox_resources_sz;
-	} else if (cpu_is_omap34xx()) {
-		mbox_device.resource = omap3_mbox_resources;
-		mbox_device.num_resources = omap3_mbox_resources_sz;
-	} else if (cpu_is_omap44xx()) {
-		mbox_device.resource = omap4_mbox_resources;
-		mbox_device.num_resources = omap4_mbox_resources_sz;
-	} else {
-		pr_err("%s: platform not supported\n", __func__);
+	odev = omap_device_build("omap-mailbox", -1, hwmod,
+			&pdata, sizeof(pdata),
+			NULL, 0,
+			0);
+	if (!odev) {
+		pr_err("%s: could not build device\n", __func__);
 		return;
 	}
-	platform_device_register(&mbox_device);
 }
 #else
 static inline void omap_init_mbox(void) { }
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index d46e439..baaaa5a 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -15,6 +15,7 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 #include <plat/mailbox.h>
+#include <plat/omap_hwmod.h>
 #include <mach/irqs.h>
 
 #define MAILBOX_REVISION		0x000
@@ -388,11 +389,9 @@ 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;
-
-	res = pdev->resource;
+	struct omap_mbox_platform_data *pdata = pdev->dev.platform_data;
 
 	if (false);
 #if defined(CONFIG_ARCH_OMAP3430)
@@ -423,15 +422,11 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
-	if (!mbox_base)
-		return -ENOMEM;
+	mbox_base = pdata->base_addr;
 
 	ret = omap_mbox_register(&pdev->dev, list);
-	if (ret) {
-		iounmap(mbox_base);
+	if (ret)
 		return ret;
-	}
 	return 0;
 
 	return ret;
@@ -440,7 +435,6 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 static int __devexit omap2_mbox_remove(struct platform_device *pdev)
 {
 	omap_mbox_unregister();
-	iounmap(mbox_base);
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index ed60840..62b8fa8 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -164,12 +164,57 @@ static struct omap_hwmod omap3xxx_mpu_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 };
 
+/* mailbox */
+
+static struct omap_hwmod_class omap3xxx_mailbox_hwmod_class = {
+	.name = "mailbox",
+};
+
+static struct omap_hwmod omap3xxx_mailbox_hwmod;
+static struct omap_hwmod_irq_info omap3xxx_mailbox_irqs[] = {
+	{ .name = "dsp", .irq = 26 },
+};
+
+static struct omap_hwmod_addr_space omap3xxx_mailbox_addrs[] = {
+	{
+		.pa_start	= 0x48094000,
+		.pa_end		= 0x48094120,
+		.flags		= ADDR_TYPE_RT
+	},
+};
+
+/* L4_CORE -> mailbox */
+static struct omap_hwmod_ocp_if omap3xxx_l4_core__mailbox = {
+	.master		= &omap3xxx_l4_core_hwmod,
+	.slave		= &omap3xxx_mailbox_hwmod,
+	.addr		= omap3xxx_mailbox_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap3xxx_mailbox_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* mailbox slave ports */
+static struct omap_hwmod_ocp_if *omap3xxx_mailbox_slaves[] = {
+	&omap3xxx_l4_core__mailbox,
+};
+
+static struct omap_hwmod omap3xxx_mailbox_hwmod = {
+	.name		= "mailbox",
+	.class		= &omap3xxx_mailbox_hwmod_class,
+	.main_clk	= "mailboxes_ick",
+	.mpu_irqs	= omap3xxx_mailbox_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3xxx_mailbox_irqs),
+	.slaves		= omap3xxx_mailbox_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_mailbox_slaves),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
 static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_l3_hwmod,
 	&omap3xxx_l4_core_hwmod,
 	&omap3xxx_l4_per_hwmod,
 	&omap3xxx_l4_wkup_hwmod,
 	&omap3xxx_mpu_hwmod,
+	&omap3xxx_mailbox_hwmod,
 	NULL,
 };
 
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 9976565..78060bd 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -59,6 +59,10 @@ struct omap_mbox {
 	void			*priv;
 };
 
+struct omap_mbox_platform_data {
+	void __iomem *base_addr;
+};
+
 int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
 void omap_mbox_init_seq(struct omap_mbox *);
 
-- 
1.7.1

RE: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Kanigeri, Hari]

Felipe,


> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Felipe Contreras
> Sent: Saturday, May 22, 2010 12:25 PM
> To: linux-omap
> Cc: Hiroshi Doyu; Kevin Hilman; Paul Walmsley; Ohad Ben-Cohen; Felipe
> Contreras
> Subject: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support
> 
> Only OMAP3 would work.

-- This is good start. 
I will look into adding/verifying OMAP4 support. If any one already started this effort please inform.
 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-
> omap2/omap_hwmod_3xxx_data.c
> index ed60840..62b8fa8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -164,12 +164,57 @@ static struct omap_hwmod omap3xxx_mpu_hwmod = {
>  	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
>  };
> 

-- Probably this should form a new patch.


Thank you,
Best regards,
Hari

Re: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Kevin Hilman]

"Kanigeri, Hari" <h-kanigeri2@ti.com> writes:

> Felipe,
>
>
>> -----Original Message-----
>> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
>> owner@vger.kernel.org] On Behalf Of Felipe Contreras
>> Sent: Saturday, May 22, 2010 12:25 PM
>> To: linux-omap
>> Cc: Hiroshi Doyu; Kevin Hilman; Paul Walmsley; Ohad Ben-Cohen; Felipe
>> Contreras
>> Subject: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support
>> 
>> Only OMAP3 would work.
>
> -- This is good start. 

Agreed.  Nice!

> I will look into adding/verifying OMAP4 support. If any one already started this effort please inform.


OMAP2 hwmod data also needs to be added for this to go upstream, otherwise
this will break on OMAP2.

Kevin

Re: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Kevin Hilman]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Only OMAP3 would work.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  arch/arm/mach-omap2/devices.c              |  103 +++++----------------------
>  arch/arm/mach-omap2/mailbox.c              |   14 +---
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
>  arch/arm/plat-omap/include/plat/mailbox.h  |    4 +
>  4 files changed, 72 insertions(+), 94 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
> index d9ac853..89f6cbc 100644
> --- a/arch/arm/mach-omap2/devices.c
> +++ b/arch/arm/mach-omap2/devices.c
> @@ -20,12 +20,14 @@
>  #include <asm/mach-types.h>
>  #include <asm/mach/map.h>
>  
> +#include <plat/omap_device.h>
>  #include <plat/control.h>
>  #include <plat/tc.h>
>  #include <plat/board.h>
>  #include <plat/mux.h>
>  #include <mach/gpio.h>
>  #include <plat/mmc.h>
> +#include <plat/mailbox.h>
>  
>  #include "mux.h"
>  
> @@ -137,95 +139,28 @@ static inline void omap_init_camera(void)
>  #endif
>  
>  #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
> +static inline void omap_init_mbox(void)
> +{
> +	struct omap_hwmod *hwmod;
> +	struct omap_device *odev;
> +	struct omap_mbox_platform_data pdata;
>  
> -#define MBOX_REG_SIZE   0x120
> -
> -#ifdef CONFIG_ARCH_OMAP2
> -static struct resource omap2_mbox_resources[] = {
> -	{
> -		.start		= OMAP24XX_MAILBOX_BASE,
> -		.end		= OMAP24XX_MAILBOX_BASE + MBOX_REG_SIZE - 1,
> -		.flags		= IORESOURCE_MEM,
> -	},
> -	{
> -		.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);
> -#else
> -#define omap2_mbox_resources		NULL
> -#define omap2_mbox_resources_sz		0
> -#endif
> -
> -#ifdef CONFIG_ARCH_OMAP3
> -static struct resource omap3_mbox_resources[] = {
> -	{
> -		.start		= OMAP34XX_MAILBOX_BASE,
> -		.end		= OMAP34XX_MAILBOX_BASE + MBOX_REG_SIZE - 1,
> -		.flags		= IORESOURCE_MEM,
> -	},
> -	{
> -		.start		= INT_24XX_MAIL_U0_MPU,
> -		.flags		= IORESOURCE_IRQ,
> -		.name		= "dsp",
> -	},
> -};
> -static int omap3_mbox_resources_sz = ARRAY_SIZE(omap3_mbox_resources);
> -#else
> -#define omap3_mbox_resources		NULL
> -#define omap3_mbox_resources_sz		0
> -#endif
> -
> -#ifdef CONFIG_ARCH_OMAP4
> -
> -#define OMAP4_MBOX_REG_SIZE	0x130
> -static struct resource omap4_mbox_resources[] = {
> -	{
> -		.start          = OMAP44XX_MAILBOX_BASE,
> -		.end            = OMAP44XX_MAILBOX_BASE +
> -					OMAP4_MBOX_REG_SIZE - 1,
> -		.flags          = IORESOURCE_MEM,
> -	},
> -	{
> -		.start          = OMAP44XX_IRQ_MAIL_U0,
> -		.flags          = IORESOURCE_IRQ,
> -		.name		= "mbox",
> -	},
> -};
> -static int omap4_mbox_resources_sz = ARRAY_SIZE(omap4_mbox_resources);
> -#else
> -#define omap4_mbox_resources		NULL
> -#define omap4_mbox_resources_sz		0
> -#endif
> +	hwmod = omap_hwmod_lookup("mailbox");
> +	if (!hwmod) {
> +		pr_err("%s: unable to find hwmod\n", __func__);
> +		return;
> +	}
>  
> -static struct platform_device mbox_device = {
> -	.name		= "omap-mailbox",
> -	.id		= -1,
> -};
> +	pdata.base_addr = hwmod->_rt_va;

Shouldn't need the base address in pdata.  The driver should just do a
platform_get_resource() to get the physical address and then ioremap.

> -static inline void omap_init_mbox(void)
> -{
> -	if (cpu_is_omap24xx()) {
> -		mbox_device.resource = omap2_mbox_resources;
> -		mbox_device.num_resources = omap2_mbox_resources_sz;
> -	} else if (cpu_is_omap34xx()) {
> -		mbox_device.resource = omap3_mbox_resources;
> -		mbox_device.num_resources = omap3_mbox_resources_sz;
> -	} else if (cpu_is_omap44xx()) {
> -		mbox_device.resource = omap4_mbox_resources;
> -		mbox_device.num_resources = omap4_mbox_resources_sz;
> -	} else {
> -		pr_err("%s: platform not supported\n", __func__);
> +	odev = omap_device_build("omap-mailbox", -1, hwmod,
> +			&pdata, sizeof(pdata),
> +			NULL, 0,
> +			0);
> +	if (!odev) {
> +		pr_err("%s: could not build device\n", __func__);
>  		return;
>  	}
> -	platform_device_register(&mbox_device);
>  }
>  #else
>  static inline void omap_init_mbox(void) { }
> diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
> index d46e439..baaaa5a 100644
> --- a/arch/arm/mach-omap2/mailbox.c
> +++ b/arch/arm/mach-omap2/mailbox.c
> @@ -15,6 +15,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/io.h>
>  #include <plat/mailbox.h>
> +#include <plat/omap_hwmod.h>
>  #include <mach/irqs.h>
>  
>  #define MAILBOX_REVISION		0x000
> @@ -388,11 +389,9 @@ 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;
> -
> -	res = pdev->resource;
> +	struct omap_mbox_platform_data *pdata = pdev->dev.platform_data;
>  
>  	if (false);
>  #if defined(CONFIG_ARCH_OMAP3430)
> @@ -423,15 +422,11 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>  
> -	mbox_base = ioremap(res[0].start, resource_size(&res[0]));
> -	if (!mbox_base)
> -		return -ENOMEM;
> +	mbox_base = pdata->base_addr;

Here is where the platform_get_resource() and ioremap should be.

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

[...]

Kevin

Re: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Felipe Contreras]

On Tue, May 25, 2010 at 3:00 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Only OMAP3 would work.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  arch/arm/mach-omap2/devices.c              |  103 +++++----------------------
>>  arch/arm/mach-omap2/mailbox.c              |   14 +---
>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
>>  arch/arm/plat-omap/include/plat/mailbox.h  |    4 +
>>  4 files changed, 72 insertions(+), 94 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>> index d9ac853..89f6cbc 100644
>> --- a/arch/arm/mach-omap2/devices.c
>> +++ b/arch/arm/mach-omap2/devices.c
>> @@ -20,12 +20,14 @@
>>  #include <asm/mach-types.h>
>>  #include <asm/mach/map.h>
>>
>> +#include <plat/omap_device.h>
>>  #include <plat/control.h>
>>  #include <plat/tc.h>
>>  #include <plat/board.h>
>>  #include <plat/mux.h>
>>  #include <mach/gpio.h>
>>  #include <plat/mmc.h>
>> +#include <plat/mailbox.h>
>>
>>  #include "mux.h"
>>
>> @@ -137,95 +139,28 @@ static inline void omap_init_camera(void)
>>  #endif
>>
>>  #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
>> +static inline void omap_init_mbox(void)
>> +{
>> +     struct omap_hwmod *hwmod;
>> +     struct omap_device *odev;
>> +     struct omap_mbox_platform_data pdata;
...

>> +     hwmod = omap_hwmod_lookup("mailbox");
>> +     if (!hwmod) {
>> +             pr_err("%s: unable to find hwmod\n", __func__);
>> +             return;
>> +     }
>>
>> -static struct platform_device mbox_device = {
>> -     .name           = "omap-mailbox",
>> -     .id             = -1,
>> -};
>> +     pdata.base_addr = hwmod->_rt_va;
>
> Shouldn't need the base address in pdata.  The driver should just do a
> platform_get_resource() to get the physical address and then ioremap.

But the hwmod code already did ioremap in _find_mpu_rt_base().

>> -     mbox_base = ioremap(res[0].start, resource_size(&res[0]));
>> -     if (!mbox_base)
>> -             return -ENOMEM;
>> +     mbox_base = pdata->base_addr;
>
> Here is where the platform_get_resource() and ioremap should be.

Yeah, that's how the code was (sorta).

-- 
Felipe Contreras
--
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: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Kevin Hilman]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, May 25, 2010 at 3:00 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> Only OMAP3 would work.
>>>
>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>> ---
>>>  arch/arm/mach-omap2/devices.c              |  103 +++++----------------------
>>>  arch/arm/mach-omap2/mailbox.c              |   14 +---
>>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
>>>  arch/arm/plat-omap/include/plat/mailbox.h  |    4 +
>>>  4 files changed, 72 insertions(+), 94 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>>> index d9ac853..89f6cbc 100644
>>> --- a/arch/arm/mach-omap2/devices.c
>>> +++ b/arch/arm/mach-omap2/devices.c
>>> @@ -20,12 +20,14 @@
>>>  #include <asm/mach-types.h>
>>>  #include <asm/mach/map.h>
>>>
>>> +#include <plat/omap_device.h>
>>>  #include <plat/control.h>
>>>  #include <plat/tc.h>
>>>  #include <plat/board.h>
>>>  #include <plat/mux.h>
>>>  #include <mach/gpio.h>
>>>  #include <plat/mmc.h>
>>> +#include <plat/mailbox.h>
>>>
>>>  #include "mux.h"
>>>
>>> @@ -137,95 +139,28 @@ static inline void omap_init_camera(void)
>>>  #endif
>>>
>>>  #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
>>> +static inline void omap_init_mbox(void)
>>> +{
>>> +     struct omap_hwmod *hwmod;
>>> +     struct omap_device *odev;
>>> +     struct omap_mbox_platform_data pdata;
> ...
>
>>> +     hwmod = omap_hwmod_lookup("mailbox");
>>> +     if (!hwmod) {
>>> +             pr_err("%s: unable to find hwmod\n", __func__);
>>> +             return;
>>> +     }
>>>
>>> -static struct platform_device mbox_device = {
>>> -     .name           = "omap-mailbox",
>>> -     .id             = -1,
>>> -};
>>> +     pdata.base_addr = hwmod->_rt_va;
>>
>> Shouldn't need the base address in pdata.  The driver should just do a
>> platform_get_resource() to get the physical address and then ioremap.
>
> But the hwmod code already did ioremap in _find_mpu_rt_base().

Yes, but that's a hwmod-internal implementation detail that may
change, or may not be needed for some hwmods, or may be iounmapped
after hwmod init etc.

Kevin
--
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: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Felipe Contreras]

On Tue, May 25, 2010 at 5:49 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, May 25, 2010 at 3:00 AM, Kevin Hilman
>> <khilman@deeprootsystems.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> Only OMAP3 would work.
>>>>
>>>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>>>> ---
>>>>  arch/arm/mach-omap2/devices.c              |  103 +++++----------------------
>>>>  arch/arm/mach-omap2/mailbox.c              |   14 +---
>>>>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
>>>>  arch/arm/plat-omap/include/plat/mailbox.h  |    4 +
>>>>  4 files changed, 72 insertions(+), 94 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
>>>> index d9ac853..89f6cbc 100644
>>>> --- a/arch/arm/mach-omap2/devices.c
>>>> +++ b/arch/arm/mach-omap2/devices.c
>>>> @@ -20,12 +20,14 @@
>>>>  #include <asm/mach-types.h>
>>>>  #include <asm/mach/map.h>
>>>>
>>>> +#include <plat/omap_device.h>
>>>>  #include <plat/control.h>
>>>>  #include <plat/tc.h>
>>>>  #include <plat/board.h>
>>>>  #include <plat/mux.h>
>>>>  #include <mach/gpio.h>
>>>>  #include <plat/mmc.h>
>>>> +#include <plat/mailbox.h>
>>>>
>>>>  #include "mux.h"
>>>>
>>>> @@ -137,95 +139,28 @@ static inline void omap_init_camera(void)
>>>>  #endif
>>>>
>>>>  #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
>>>> +static inline void omap_init_mbox(void)
>>>> +{
>>>> +     struct omap_hwmod *hwmod;
>>>> +     struct omap_device *odev;
>>>> +     struct omap_mbox_platform_data pdata;
>> ...
>>
>>>> +     hwmod = omap_hwmod_lookup("mailbox");
>>>> +     if (!hwmod) {
>>>> +             pr_err("%s: unable to find hwmod\n", __func__);
>>>> +             return;
>>>> +     }
>>>>
>>>> -static struct platform_device mbox_device = {
>>>> -     .name           = "omap-mailbox",
>>>> -     .id             = -1,
>>>> -};
>>>> +     pdata.base_addr = hwmod->_rt_va;
>>>
>>> Shouldn't need the base address in pdata.  The driver should just do a
>>> platform_get_resource() to get the physical address and then ioremap.
>>
>> But the hwmod code already did ioremap in _find_mpu_rt_base().
>
> Yes, but that's a hwmod-internal implementation detail that may
> change, or may not be needed for some hwmods, or may be iounmapped
> after hwmod init etc.

So I have to do another ioremap()?

-- 
Felipe Contreras
--
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: [RFC/PATCH 1/2] omap: mailbox: initial hwmod support

[Kevin Hilman]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, May 25, 2010 at 5:49 PM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> On Tue, May 25, 2010 at 3:00 AM, Kevin Hilman
>>> <khilman@deeprootsystems.com> wrote:
>>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>>

[...]

>>>>> -static struct platform_device mbox_device = {
>>>>> -     .name           = "omap-mailbox",
>>>>> -     .id             = -1,
>>>>> -};
>>>>> +     pdata.base_addr = hwmod->_rt_va;
>>>>
>>>> Shouldn't need the base address in pdata.  The driver should just do a
>>>> platform_get_resource() to get the physical address and then ioremap.
>>>
>>> But the hwmod code already did ioremap in _find_mpu_rt_base().
>>
>> Yes, but that's a hwmod-internal implementation detail that may
>> change, or may not be needed for some hwmods, or may be iounmapped
>> after hwmod init etc.
>
> So I have to do another ioremap()?

Yes.

But keep in mind that these L4 devices have static mappings, so there
really not an additional mapping created.

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

[RFC/PATCH 2/2] omap: mailbox: add omap_device latency information

[Felipe Contreras]

So that we can enable the main clock.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 arch/arm/mach-omap2/devices.c             |   12 +++++++++++-
 arch/arm/mach-omap2/mailbox.c             |   20 ++++++++------------
 arch/arm/plat-omap/include/plat/mailbox.h |    3 +++
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 89f6cbc..e6cb1d3 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -139,6 +139,14 @@ static inline void omap_init_camera(void)
 #endif
 
 #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
+static struct omap_device_pm_latency mbox_latencies[] = {
+	[0] = { .activate_func = omap_device_enable_clocks,
+		.activate_lat = 50000, /* FIXME random value */
+		.deactivate_func = omap_device_enable_clocks,
+		.deactivate_lat = 50000, /* FIXME random value */
+	},
+};
+
 static inline void omap_init_mbox(void)
 {
 	struct omap_hwmod *hwmod;
@@ -152,10 +160,12 @@ static inline void omap_init_mbox(void)
 	}
 
 	pdata.base_addr = hwmod->_rt_va;
+	pdata.device_enable = omap_device_enable;
+	pdata.device_disable = omap_device_idle;
 
 	odev = omap_device_build("omap-mailbox", -1, hwmod,
 			&pdata, sizeof(pdata),
-			NULL, 0,
+			mbox_latencies, ARRAY_SIZE(mbox_latencies),
 			0);
 	if (!odev) {
 		pr_err("%s: could not build device\n", __func__);
diff --git a/arch/arm/mach-omap2/mailbox.c b/arch/arm/mach-omap2/mailbox.c
index baaaa5a..157a764 100644
--- a/arch/arm/mach-omap2/mailbox.c
+++ b/arch/arm/mach-omap2/mailbox.c
@@ -53,6 +53,7 @@
 #define OMAP4_MBOX_NR_REGS		(OMAP4_MBOX_REG_SIZE / sizeof(u32))
 
 static void __iomem *mbox_base;
+static struct platform_device *mbox_pdev;
 
 struct omap_mbox2_fifo {
 	unsigned long msg;
@@ -71,8 +72,6 @@ struct omap_mbox2_priv {
 	unsigned long irqdisable;
 };
 
-static struct clk *mbox_ick_handle;
-
 static void omap2_mbox_enable_irq(struct omap_mbox *mbox,
 				  omap_mbox_type_t irq);
 
@@ -91,14 +90,10 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 {
 	u32 l;
 	unsigned long timeout;
+	struct omap_mbox_platform_data *pdata = mbox_pdev->dev.platform_data;
 
-	mbox_ick_handle = clk_get(NULL, "mailboxes_ick");
-	if (IS_ERR(mbox_ick_handle)) {
-		printk(KERN_ERR "Could not get mailboxes_ick: %ld\n",
-			PTR_ERR(mbox_ick_handle));
-		return PTR_ERR(mbox_ick_handle);
-	}
-	clk_enable(mbox_ick_handle);
+	if (pdata->device_enable)
+		pdata->device_enable(mbox_pdev);
 
 	if (cpu_is_omap44xx()) {
 		mbox_write_reg(OMAP4_SOFTRESET, MAILBOX_SYSCONFIG);
@@ -144,9 +139,9 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 
 static void omap2_mbox_shutdown(struct omap_mbox *mbox)
 {
-	clk_disable(mbox_ick_handle);
-	clk_put(mbox_ick_handle);
-	mbox_ick_handle = NULL;
+	struct omap_mbox_platform_data *pdata = mbox_pdev->dev.platform_data;
+	if (pdata->device_disable)
+		pdata->device_disable(mbox_pdev);
 }
 
 /* Mailbox FIFO handle functions */
@@ -423,6 +418,7 @@ static int __devinit omap2_mbox_probe(struct platform_device *pdev)
 	}
 
 	mbox_base = pdata->base_addr;
+	mbox_pdev = pdev;
 
 	ret = omap_mbox_register(&pdev->dev, list);
 	if (ret)
diff --git a/arch/arm/plat-omap/include/plat/mailbox.h b/arch/arm/plat-omap/include/plat/mailbox.h
index 78060bd..a68fdda 100644
--- a/arch/arm/plat-omap/include/plat/mailbox.h
+++ b/arch/arm/plat-omap/include/plat/mailbox.h
@@ -11,6 +11,7 @@
 
 typedef u32 mbox_msg_t;
 struct omap_mbox;
+struct platform_device;
 
 typedef int __bitwise omap_mbox_irq_t;
 #define IRQ_TX ((__force omap_mbox_irq_t) 1)
@@ -61,6 +62,8 @@ struct omap_mbox {
 
 struct omap_mbox_platform_data {
 	void __iomem *base_addr;
+	int (*device_enable)(struct platform_device *pdev);
+	int (*device_disable)(struct platform_device *pdev);
 };
 
 int omap_mbox_msg_send(struct omap_mbox *, mbox_msg_t msg);
-- 
1.7.1

Re: [RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[Ohad Ben-Cohen]

On Sat, May 22, 2010 at 8:25 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Hi,
>
> I played a bit with omap_hwmod/omap_device for the mailbox, and this is the
> result.

Nice move Felipe :)

Can you convert this to use the runtime PM layer, instead of directly
calling omap_device functions via pdata ?

Thanks,
Ohad.

>
> I only have an OMAP3 (bb), and seems to work fine :)
>
> Felipe Contreras (2):
>  omap: mailbox: initial hwmod support
>  omap: mailbox: add omap_device latency information
>
>  arch/arm/mach-omap2/devices.c              |  107 +++++++---------------------
>  arch/arm/mach-omap2/mailbox.c              |   34 +++------
>  arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   45 ++++++++++++
>  arch/arm/plat-omap/include/plat/mailbox.h  |    7 ++
>  4 files changed, 90 insertions(+), 103 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

Re: [RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[Felipe Contreras]

On Mon, May 24, 2010 at 8:29 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> On Sat, May 22, 2010 at 8:25 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> I played a bit with omap_hwmod/omap_device for the mailbox, and this is the
>> result.
>
> Nice move Felipe :)

Thanks :)

> Can you convert this to use the runtime PM layer, instead of directly
> calling omap_device functions via pdata ?

Well, I didn't see anybody using omap_device, so I just assumed from
reading the code.

Recently I saw some patches using this pm_runtime stuff, but it seems
to be for some pm branch... not sure which one.

-- 
Felipe Contreras

Re: [RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[Ohad Ben-Cohen]

On Mon, May 24, 2010 at 10:33 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, May 24, 2010 at 8:29 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>> On Sat, May 22, 2010 at 8:25 PM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> I played a bit with omap_hwmod/omap_device for the mailbox, and this is the
>>> result.
>>
>> Nice move Felipe :)
>
> Thanks :)
>
>> Can you convert this to use the runtime PM layer, instead of directly
>> calling omap_device functions via pdata ?
>
> Well, I didn't see anybody using omap_device, so I just assumed from
> reading the code.
>
> Recently I saw some patches using this pm_runtime stuff, but it seems
> to be for some pm branch... not sure which one.

check out pm-wip/mmc on
git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
:)

>
> --
> Felipe Contreras
>

Re: [RFC/PATCH 0/2] omap: mailbox: hwmod experiment

[Kevin Hilman]

Ohad Ben-Cohen <ohad@wizery.com> writes:

> On Mon, May 24, 2010 at 10:33 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Mon, May 24, 2010 at 8:29 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
>>> On Sat, May 22, 2010 at 8:25 PM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> I played a bit with omap_hwmod/omap_device for the mailbox, and this is the
>>>> result.
>>>
>>> Nice move Felipe :)
>>
>> Thanks :)
>>
>>> Can you convert this to use the runtime PM layer, instead of directly
>>> calling omap_device functions via pdata ?
>>
>> Well, I didn't see anybody using omap_device, so I just assumed from
>> reading the code.
>>
>> Recently I saw some patches using this pm_runtime stuff, but it seems
>> to be for some pm branch... not sure which one.
>
> check out pm-wip/mmc on
> git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git
> :)

Agreed, using runtime PM, you no longer need to call omap_device
functions directly.

As pointed out by Ohad, the pm-wip/mmc branch has an example
conversion of the MMC driver with this approach.

Kevin