Skip to contentSkip to conversation

omap: mailbox: hwmod support

PATCH Superseded Omar Ramirez Luna 5 participants 16 messages Dec 16, 2010 – Dec 17, 2010

Subsystems (4)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover omap: mailbox: hwmod support

Tested on 3430, based of pm-core branch. Patches affecting mailbox code may require rebase once the mailbox git pull is made.

http://www.mail-archive.com/linux-omap at vger.kernel.org/msg39719.html

** v4 **
- Merged device latency info with previous patch in series.
- Removed usage of omap_device_enable_clocks and replaced for omap_device_[enable|disable]_hwmods to handle sysc setup and clocks.
- Fixed pm_runtime usage by adding its enable/disable functions, and now handling sysc register with it, because of changes in device latency functions.
- For pm_runtime, instead of adding a global variable to make use of pdev->dev, use 'parent' inside of 'dev' returned by device_create and which is stored inside omap_mbox struct, in future cleanup consider cleaning omap_mbox struct.

** v3 **
- Taken mailbox hwmod as is from Benoit original branch.
- Put back numbers for irq, addresses instead of considering them as magic numbers.
- Follow the declaration layout for omap4 hwmods.
- Using pm_runtime to enable the clocks.

** v2 **
- Added omap4 hwmod support.
- Moved "mailbox_ick" from hwmod to hwmod_if (omap 2/3)
- Declared sysc classes for omap 2/3

** v1 **
1. omap: mailbox: initial hwmod support for omap3 Changes were made to:
- Rebase to latest code.
- Detect the hwmod by filling prcm union for omap2, without this it was unable to build the hwmod at runtime.
- Replace magic number for defines.
- Use ioremap again instead of relying on the one made by hwmod, as noted in http://patchwork.kernel.org/patch/101661/

2. omap: mailbox: initial hwmod support for omap2
Was only compiled tested!! Unfortunately I don't have the HW for it.

Benoit Cousson (1):
OMAP4: hwmod data: add mailbox data

Felipe Contreras (2):
OMAP3: hwmod data: add mailbox data
OMAP: mailbox: build device using omap_device/omap_hwmod

Omar Ramirez Luna (1):
OMAP: mailbox: use runtime pm for clk and sysc handling

omar ramirez (1):
OMAP2: hwmod data: add mailbox data

 arch/arm/mach-omap2/devices.c              |  102 ++++++----------------------
 arch/arm/mach-omap2/mailbox.c              |   27 ++------
 arch/arm/mach-omap2/omap_hwmod_2420_data.c |   73 ++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |   72 +++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   71 +++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 ++++++++++++++++++
 6 files changed, 308 insertions(+), 104 deletions(-)
#1 lore raw

PATCH v4 1/5 OMAP2: hwmod data: add mailbox data

From: omar ramirez <[email protected]>

Mailbox hwmod data for omap2430 and 2420.

---
 arch/arm/mach-omap2/omap_hwmod_2420_data.c |   73 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/omap_hwmod_2430_data.c |   72 +++++++++++++++++++++++++++
 2 files changed, 145 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/omap_hwmod_2420_data.c +73 -0
index ce5d890..64f8f8b 100644
@@ -778,6 +778,76 @@ static struct omap_hwmod omap2420_gpio4_hwmod = {
 	.slaves_cnt	= ARRAY_SIZE(omap2420_gpio4_slaves),
 	.class		= &omap242x_gpio_hwmod_class,
 	.dev_attr	= &gpio_dev_attr,
+	.omap_chip      = OMAP_CHIP_INIT(CHIP_IS_OMAP2420),
+};
+
+/*
+ * 'mailbox' class
+ * mailbox module allowing communication between the on-chip processors
+ * using a queued mailbox-interrupt mechanism.
+ */
+
+static struct omap_hwmod_class_sysconfig omap2420_mailbox_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+				SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap2420_mailbox_hwmod_class = {
+	.name = "mailbox",
+	.sysc = &omap2420_mailbox_sysc,
+};
+
+/* mailbox */
+static struct omap_hwmod omap2420_mailbox_hwmod;
+static struct omap_hwmod_irq_info omap2420_mailbox_irqs[] = {
+	{ .name = "dsp", .irq = 26, },
+	{ .name = "iva", .irq = 34, },
+};
+
+static struct omap_hwmod_addr_space omap2420_mailbox_addrs[] = {
+	{
+		.pa_start	= 0x48094000,
+		.pa_end		= 0x480941ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+};
+
+/* l4_core -> mailbox */
+static struct omap_hwmod_ocp_if omap2420_l4_core__mailbox = {
+	.master		= &omap2420_l4_core_hwmod,
+	.slave		= &omap2420_mailbox_hwmod,
+	.clk		= "mailboxes_ick",
+	.addr		= omap2420_mailbox_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap2420_mailbox_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* mailbox slave ports */
+static struct omap_hwmod_ocp_if *omap2420_mailbox_slaves[] = {
+	&omap2420_l4_core__mailbox,
+};
+
+static struct omap_hwmod omap2420_mailbox_hwmod = {
+	.name		= "mailbox",
+	.class		= &omap2420_mailbox_hwmod_class,
+	.mpu_irqs	= omap2420_mailbox_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap2420_mailbox_irqs),
+	.prcm		= {
+		.omap2 = {
+			.prcm_reg_id = 1,
+			.module_bit = OMAP24XX_EN_MAILBOXES_SHIFT,
+			.module_offs = CORE_MOD,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = OMAP24XX_ST_MAILBOXES_SHIFT,
+		},
+	},
+	.slaves		= omap2420_mailbox_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap2420_mailbox_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP2420),
 };
 
@@ -799,6 +869,9 @@ static __initdata struct omap_hwmod *omap2420_hwmods[] = {
 	&omap2420_gpio2_hwmod,
 	&omap2420_gpio3_hwmod,
 	&omap2420_gpio4_hwmod,
+
+	/* mailbox class */
+	&omap2420_mailbox_hwmod,
 	NULL,
 };
 
arch/arm/mach-omap2/omap_hwmod_2430_data.c +72 -0

84 lines not on this page · raw message

Signature · 1 line
1.7.1
#2 lore raw

PATCH v4 2/5 OMAP3: hwmod data: add mailbox data

In reply to Omar Ramirez Luna

From: Felipe Contreras <[email protected]>

Mailbox hwmod data for omap3.

---
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   71 ++++++++++++++++++++++++++++
 1 files changed, 71 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/omap_hwmod_3xxx_data.c +71 -0
index 8eb81b4..9ce347e 100644
@@ -1089,6 +1089,74 @@ static struct omap_hwmod omap3xxx_gpio6_hwmod = {
 	.slaves_cnt	= ARRAY_SIZE(omap3xxx_gpio6_slaves),
 	.class		= &omap3xxx_gpio_hwmod_class,
 	.dev_attr	= &gpio_dev_attr,
+	.omap_chip      = OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
+};
+
+/*
+ * 'mailbox' class
+ * mailbox module allowing communication between the on-chip processors
+ * using a queued mailbox-interrupt mechanism.
+ */
+
+static struct omap_hwmod_class_sysconfig omap3xxx_mailbox_sysc = {
+	.rev_offs	= 0x000,
+	.sysc_offs	= 0x010,
+	.syss_offs	= 0x014,
+	.sysc_flags	= (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
+				SYSC_HAS_SOFTRESET | SYSC_HAS_AUTOIDLE),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type1,
+};
+
+static struct omap_hwmod_class omap3xxx_mailbox_hwmod_class = {
+	.name = "mailbox",
+	.sysc = &omap3xxx_mailbox_sysc,
+};
+
+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		= 0x480941ff,
+		.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,
+	.clk		= "mailboxes_ick",
+	.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,
+	.mpu_irqs	= omap3xxx_mailbox_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap3xxx_mailbox_irqs),
+	.prcm		= {
+		.omap2 = {
+			.prcm_reg_id = 1,
+			.module_bit = OMAP3430_EN_MAILBOXES_SHIFT,
+			.module_offs = CORE_MOD,
+			.idlest_reg_id = 1,
+			.idlest_idle_bit = OMAP3430_ST_MAILBOXES_SHIFT,
+		},
+	},
+	.slaves		= omap3xxx_mailbox_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap3xxx_mailbox_slaves),
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP3430),
 };
 
@@ -1115,6 +1183,9 @@ static __initdata struct omap_hwmod *omap3xxx_hwmods[] = {
 	&omap3xxx_gpio4_hwmod,
 	&omap3xxx_gpio5_hwmod,
 	&omap3xxx_gpio6_hwmod,
+
+	/* mailbox class */
+	&omap3xxx_mailbox_hwmod,
 	NULL,
 };
 
Signature · 1 line
1.7.1
#3 lore raw

PATCH v4 3/5 OMAP4: hwmod data: add mailbox data

In reply to Omar Ramirez Luna

From: Benoit Cousson <[email protected]>

Mailbox hwmod data for omap4.

---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   67 ++++++++++++++++++++++++++++
 1 files changed, 67 insertions(+), 0 deletions(-)
arch/arm/mach-omap2/omap_hwmod_44xx_data.c +67 -0
index 7367648..f14b01c 100644
@@ -1740,6 +1740,70 @@ static struct omap_hwmod omap44xx_wd_timer3_hwmod = {
 	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
 };
 
+/*
+ * 'mailbox' class
+ * mailbox module allowing communication between the on-chip processors
+ * using a queued mailbox-interrupt mechanism.
+ */
+
+static struct omap_hwmod_class_sysconfig omap44xx_mailbox_sysc = {
+	.rev_offs	= 0x0000,
+	.sysc_offs	= 0x0010,
+	.sysc_flags	= (SYSC_HAS_RESET_STATUS | SYSC_HAS_SIDLEMODE |
+			   SYSC_HAS_SOFTRESET),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
+	.sysc_fields	= &omap_hwmod_sysc_type2,
+};
+
+static struct omap_hwmod_class omap44xx_mailbox_hwmod_class = {
+	.name = "mailbox",
+	.sysc = &omap44xx_mailbox_sysc,
+};
+
+/* mailbox */
+static struct omap_hwmod omap44xx_mailbox_hwmod;
+static struct omap_hwmod_irq_info omap44xx_mailbox_irqs[] = {
+	{ .name = "mbox", .irq = 26 + OMAP44XX_IRQ_GIC_START, },
+};
+
+static struct omap_hwmod_addr_space omap44xx_mailbox_addrs[] = {
+	{
+		.pa_start	= 0x4a0f4000,
+		.pa_end		= 0x4a0f41ff,
+		.flags		= ADDR_TYPE_RT,
+	},
+};
+
+/* l4_cfg -> mailbox */
+static struct omap_hwmod_ocp_if omap44xx_l4_cfg__mailbox = {
+	.master		= &omap44xx_l4_cfg_hwmod,
+	.slave		= &omap44xx_mailbox_hwmod,
+	.clk		= "l4_div_ck",
+	.addr		= omap44xx_mailbox_addrs,
+	.addr_cnt	= ARRAY_SIZE(omap44xx_mailbox_addrs),
+	.user		= OCP_USER_MPU | OCP_USER_SDMA,
+};
+
+/* mailbox slave ports */
+static struct omap_hwmod_ocp_if *omap44xx_mailbox_slaves[] = {
+	&omap44xx_l4_cfg__mailbox,
+};
+
+static struct omap_hwmod omap44xx_mailbox_hwmod = {
+	.name		= "mailbox",
+	.class		= &omap44xx_mailbox_hwmod_class,
+	.mpu_irqs	= omap44xx_mailbox_irqs,
+	.mpu_irqs_cnt	= ARRAY_SIZE(omap44xx_mailbox_irqs),
+	.prcm		= {
+		.omap4 = {
+			.clkctrl_reg = OMAP4430_CM_L4CFG_MAILBOX_CLKCTRL,
+		},
+	},
+	.slaves		= omap44xx_mailbox_slaves,
+	.slaves_cnt	= ARRAY_SIZE(omap44xx_mailbox_slaves),
+	.omap_chip	= OMAP_CHIP_INIT(CHIP_IS_OMAP4430),
+};
+
 static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	/* dmm class */
 	&omap44xx_dmm_hwmod,
@@ -1798,6 +1862,9 @@ static __initdata struct omap_hwmod *omap44xx_hwmods[] = {
 	&omap44xx_wd_timer2_hwmod,
 	&omap44xx_wd_timer3_hwmod,
 
+	/* mailbox class */
+	&omap44xx_mailbox_hwmod,
+
 	NULL,
 };
 
Signature · 1 line
1.7.1
#4 lore raw

PATCH v4 4/5 OMAP: mailbox: build device using omap_device/omap_hwmod

In reply to Omar Ramirez Luna

From: Felipe Contreras <[email protected]>

Remove static platform_device and resource data within omap mailbox driver; use the one defined in the hwmod database along with omap_device framework for device build and registration.

Add device latency functions to be used, so clock can be enabled and sysconfig is configured.

---
 arch/arm/mach-omap2/devices.c |  102 ++++++++---------------------------------
 1 files changed, 20 insertions(+), 82 deletions(-)
arch/arm/mach-omap2/devices.c +20 -82
index b5cafd3..7493c30 100644
@@ -142,95 +142,33 @@ static inline void omap_init_camera(void)
 #endif
 
 #if defined(CONFIG_OMAP_MBOX_FWK) || defined(CONFIG_OMAP_MBOX_FWK_MODULE)
-
-#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 struct omap_device_pm_latency mbox_latencies[] = {
+	[0] = {
+		.activate_func = omap_device_enable_hwmods,
+		.deactivate_func = omap_device_idle_hwmods,
+		.flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
 	},
 };
-static int omap4_mbox_resources_sz = ARRAY_SIZE(omap4_mbox_resources);
-#else
-#define omap4_mbox_resources		NULL
-#define omap4_mbox_resources_sz		0
-#endif
-
-static struct platform_device mbox_device = {
-	.name		= "omap-mailbox",
-	.id		= -1,
-};
 
 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__);
+	struct omap_hwmod *oh;
+	struct omap_device *od;
+
+	oh = omap_hwmod_lookup("mailbox");
+	if (!oh) {
+		pr_err("%s: unable to find hwmod\n", __func__);
+		return;
+	}
+
+	od = omap_device_build("omap-mailbox", -1, oh,
+			NULL, 0,
+			mbox_latencies, ARRAY_SIZE(mbox_latencies),
+			0);
+	if (!od) {
+		pr_err("%s: could not build device\n", __func__);
 		return;
 	}
-	platform_device_register(&mbox_device);
 }
 #else
 static inline void omap_init_mbox(void) { }
Signature · 1 line
1.7.1
#5 lore raw

PATCH v4 4/5 OMAP: mailbox: build device using omap_device/omap_hwmod

couple of comments.

Quoted 21 lines · Omar Ramirez Luna

From: Felipe Contreras <[email protected]>

Remove static platform_device and resource data within omap mailbox driver; use the one defined in the hwmod database along with omap_device framework for device build and registration.

Add device latency functions to be used, so clock can be enabled and sysconfig is configured.

---
?arch/arm/mach-omap2/devices.c | ?102 ++++++++--------------------------------- ?1 files changed, 20 insertions(+), 82 deletions(-)

arch/arm/mach-omap2/devices.c +0 -0
index b5cafd3..7493c30 100644
@@ -142,95 +142,33 @@ static inline void omap_init_camera(void)
Quoted 1 line

?#endif

<<snip>>

arch/arm/mach-omap2/devices.c

Quoted 12 lines
arch/arm/mach-omap2/devices.c +12 -0
+
+ ? ? ? oh = omap_hwmod_lookup("mailbox");
+ ? ? ? if (!oh) {
+ ? ? ? ? ? ? ? pr_err("%s: unable to find hwmod\n", __func__);
+ ? ? ? ? ? ? ? return;
+ ? ? ? }
+
+ ? ? ? od = omap_device_build("omap-mailbox", -1, oh,
+ ? ? ? ? ? ? ? ? ? ? ? NULL, 0,
+ ? ? ? ? ? ? ? ? ? ? ? mbox_latencies, ARRAY_SIZE(mbox_latencies),
+ ? ? ? ? ? ? ? ? ? ? ? 0);
+ ? ? ? if (!od) {

Check for IS_ERR(od).

Quoted 2 lines

+ ? ? ? ? ? ? ? pr_err("%s: could not build device\n", __func__); ? ? ? ? ? ? ? ?return;

This "return" can be removed.

Quoted 9 lines

? ? ? ?}
- ? ? ? platform_device_register(&mbox_device);
?}
?#else
?static inline void omap_init_mbox(void) { }
--
1.7.1

#6 lore raw

PATCH v4 4/5 OMAP: mailbox: build device using omap_device/omap_hwmod

Quoted 12 lines · Charulatha
Quoted 11 lines

+ ? ? ? oh = omap_hwmod_lookup("mailbox");
+ ? ? ? if (!oh) {
+ ? ? ? ? ? ? ? pr_err("%s: unable to find hwmod\n", __func__);
+ ? ? ? ? ? ? ? return;
+ ? ? ? }
+
+ ? ? ? od = omap_device_build("omap-mailbox", -1, oh,
+ ? ? ? ? ? ? ? ? ? ? ? NULL, 0,
+ ? ? ? ? ? ? ? ? ? ? ? mbox_latencies, ARRAY_SIZE(mbox_latencies),
+ ? ? ? ? ? ? ? ? ? ? ? 0);
+ ? ? ? if (!od) {

Quoted 3 lines

Check for IS_ERR(od).

Quoted 1 line

+ ? ? ? ? ? ? ? pr_err("%s: could not build device\n", __func__);

If you have an API which returns errors, and you're bothering to print something when an error occurs, it's often useful to print the returned error code, so that people can find out _why_ the error occurred.

#7 lore raw

PATCH v4 4/5 OMAP: mailbox: build device using omap_device/omap_hwmod

Quoted 13 lines · Russell King - ARM Linux
Quoted 12 lines · Charulatha
Quoted 11 lines

+ ? ? ? oh = omap_hwmod_lookup("mailbox");
+ ? ? ? if (!oh) {
+ ? ? ? ? ? ? ? pr_err("%s: unable to find hwmod\n", __func__);
+ ? ? ? ? ? ? ? return;
+ ? ? ? }
+
+ ? ? ? od = omap_device_build("omap-mailbox", -1, oh,
+ ? ? ? ? ? ? ? ? ? ? ? NULL, 0,
+ ? ? ? ? ? ? ? ? ? ? ? mbox_latencies, ARRAY_SIZE(mbox_latencies),
+ ? ? ? ? ? ? ? ? ? ? ? 0);
+ ? ? ? if (!od) {

Quoted 8 lines
Quoted 3 lines

Check for IS_ERR(od).

Quoted 1 line

+ ? ? ? ? ? ? ? pr_err("%s: could not build device\n", __func__);

If you have an API which returns errors, and you're bothering to print something when an error occurs, it's often useful to print the returned error code, so that people can find out _why_ the error occurred.

Agree.

Thanks,

Omar

#8 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

In reply to Omar Ramirez Luna

Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register.

Based on the patch sent by Felipe Contreras:
https://patchwork.kernel.org/patch/101662/

---
 arch/arm/mach-omap2/mailbox.c |   27 +++++----------------------
 1 files changed, 5 insertions(+), 22 deletions(-)
arch/arm/mach-omap2/mailbox.c +5 -22
index 40ddeca..f5f72ba 100644
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/platform_device.h>
 #include <linux/io.h>
+#include <linux/pm_runtime.h>
 #include <plat/mailbox.h>
 #include <mach/irqs.h>
 
@@ -34,12 +35,8 @@
 #define MAILBOX_IRQ_NOTFULL(m)		(1 << (2 * (m) + 1))
 
 /* SYSCONFIG: register bit definition */
-#define AUTOIDLE	(1 << 0)
 #define SOFTRESET	(1 << 1)
-#define SMARTIDLE	(2 << 3)
 #define OMAP4_SOFTRESET	(1 << 0)
-#define OMAP4_NOIDLE	(1 << 2)
-#define OMAP4_SMARTIDLE	(2 << 2)
 
 /* SYSSTATUS: register bit definition */
 #define RESETDONE	(1 << 0)
@@ -70,8 +67,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,13 +86,8 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 	u32 l;
 	unsigned long timeout;
 
-	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);
+	pm_runtime_enable(mbox->dev->parent);
+	pm_runtime_get_sync(mbox->dev->parent);
 
 	if (cpu_is_omap44xx()) {
 		mbox_write_reg(OMAP4_SOFTRESET, MAILBOX_SYSCONFIG);
@@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 	l = mbox_read_reg(MAILBOX_REVISION);
 	pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
 
-	if (cpu_is_omap44xx())
-		l = OMAP4_SMARTIDLE;
-	else
-		l = SMARTIDLE | AUTOIDLE;
-	mbox_write_reg(l, MAILBOX_SYSCONFIG);
-
 	omap2_mbox_enable_irq(mbox, IRQ_RX);
 
 	return 0;
@@ -143,9 +127,8 @@ 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;
+	pm_runtime_put_sync(mbox->dev->parent);
+	pm_runtime_disable(mbox->dev->parent);
 }
 
 /* Mailbox FIFO handle functions */
Signature · 1 line
1.7.1
#9 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Quoted 34 lines · Omar Ramirez Luna

Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register.

Based on the patch sent by Felipe Contreras:
https://patchwork.kernel.org/patch/101662/

---
?arch/arm/mach-omap2/mailbox.c | ? 27 +++++---------------------- ?1 files changed, 5 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/mailbox.c +0 -0
index 40ddeca..f5f72ba 100644
@@ -14,6 +14,7 @@

?#include <linux/err.h>
?#include <linux/platform_device.h>
?#include <linux/io.h>
+#include <linux/pm_runtime.h>
?#include <plat/mailbox.h>
?#include <mach/irqs.h>

arch/arm/mach-omap2/mailbox.c +0 -0
@@ -34,12 +35,8 @@
 ?#define MAILBOX_IRQ_NOTFULL(m) ? ? ? ? (1 << (2 * (m) + 1))
 

?/* SYSCONFIG: register bit definition */
-#define AUTOIDLE ? ? ? (1 << 0)
?#define SOFTRESET ? ? ?(1 << 1)
-#define SMARTIDLE ? ? ?(2 << 3)
?#define OMAP4_SOFTRESET ? ? ? ?(1 << 0)

arch/arm/mach-omap2/mailbox.c +0 -2
-#define OMAP4_NOIDLE ? (1 << 2)
-#define OMAP4_SMARTIDLE ? ? ? ?(2 << 2)
Quoted 2 lines

?/* SYSSTATUS: register bit definition */
?#define RESETDONE ? ? ?(1 << 0)

Is this still required?

arch/arm/mach-omap2/mailbox.c struct omap_mbox2_priv { · line 72

Quoted 9 lines
arch/arm/mach-omap2/mailbox.c +0 -2
@@ -70,8 +67,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);
 

<<snip>>

#10 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Hi,

Quoted 35 lines · Charulatha
Quoted 34 lines · Omar Ramirez Luna

Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register.

Based on the patch sent by Felipe Contreras:
https://patchwork.kernel.org/patch/101662/

---
?arch/arm/mach-omap2/mailbox.c | ? 27 +++++---------------------- ?1 files changed, 5 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/mailbox.c +0 -0
index 40ddeca..f5f72ba 100644
@@ -14,6 +14,7 @@

?#include <linux/err.h>
?#include <linux/platform_device.h>
?#include <linux/io.h>
+#include <linux/pm_runtime.h>
?#include <plat/mailbox.h>
?#include <mach/irqs.h>

arch/arm/mach-omap2/mailbox.c +0 -0
@@ -34,12 +35,8 @@
 ?#define MAILBOX_IRQ_NOTFULL(m) ? ? ? ? (1 << (2 * (m) + 1))
 

?/* SYSCONFIG: register bit definition */
-#define AUTOIDLE ? ? ? (1 << 0)
?#define SOFTRESET ? ? ?(1 << 1)
-#define SMARTIDLE ? ? ?(2 << 3)
?#define OMAP4_SOFTRESET ? ? ? ?(1 << 0)

arch/arm/mach-omap2/mailbox.c +0 -2
-#define OMAP4_NOIDLE ? (1 << 2)
-#define OMAP4_SMARTIDLE ? ? ? ?(2 << 2)
Quoted 4 lines
Quoted 2 lines

?/* SYSSTATUS: register bit definition */
?#define RESETDONE ? ? ?(1 << 0)

Is this still required?

Yes, mailbox uses the softreset every time it is requested (and that it has no current users at the time), it does it before configuring sysc, as opossed to what this patch does (where clk is enabled, sysc configured, and then softreset), but no harm was seen with this sequence.

AFAIK, enabling/disabling hwmod handles prm reset but not softreset bit, that's why I left it there.

Regards,

Omar

#11 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Hi Omar,

Quoted 43 lines · Omar

Hi,

Quoted 35 lines · Charulatha
Quoted 34 lines · Omar Ramirez Luna

Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register.

Based on the patch sent by Felipe Contreras:
https://patchwork.kernel.org/patch/101662/

---
  arch/arm/mach-omap2/mailbox.c |   27 +++++----------------------
  1 files changed, 5 insertions(+), 22 deletions(-)
arch/arm/mach-omap2/mailbox.c +1 -4
index 40ddeca..f5f72ba 100644
@@ -14,6 +14,7 @@
  #include<linux/err.h>
  #include<linux/platform_device.h>
  #include<linux/io.h>
+#include<linux/pm_runtime.h>
  #include<plat/mailbox.h>
  #include<mach/irqs.h>
 
@@ -34,12 +35,8 @@
  #define MAILBOX_IRQ_NOTFULL(m)         (1<<  (2 * (m) + 1))
 
  /* SYSCONFIG: register bit definition */
-#define AUTOIDLE       (1<<  0)
  #define SOFTRESET      (1<<  1)
-#define SMARTIDLE      (2<<  3)
  #define OMAP4_SOFTRESET        (1<<  0)
-#define OMAP4_NOIDLE   (1<<  2)
-#define OMAP4_SMARTIDLE        (2<<  2)
 
Quoted 4 lines

arch/arm/mach-omap2/mailbox.c line 42

Quoted 2 lines
arch/arm/mach-omap2/mailbox.c +0 -0
  /* SYSSTATUS: register bit definition */
  #define RESETDONE      (1<<  0)

Is this still required?

Quoted 8 lines

Yes, mailbox uses the softreset every time it is requested (and that it has no current users at the time), it does it before configuring sysc, as opossed to what this patch does (where clk is enabled, sysc configured, and then softreset), but no harm was seen with this sequence.

AFAIK, enabling/disabling hwmod handles prm reset but not softreset bit, that's why I left it there.

In fact both are handled now. hardreset is managed for IVA, DSP and IPU, and softreset for most the other IPs.
The only slight difference, is that softreset for the moment is just done once at setup time. The idea being that we do not know the state the bootloader configured IP, so we reset it by default.

I thought I did it for every enable from disable state, but in fact this is not the case, because AFAIK there is not use-case for that yet.

Why do you have to softreset the mailbox? That sounds like some old HW bugs from the past that we solve using the softreset.

Could you elaborate on that use case? If there is a need, I'd rather update the hwmod core than letting you playing directly with the sysconfig. At least we can expose the API we did for that purpose but didn't merge due to the lack of any strong need.

Regards,
Benoit

#12 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Quoted 19 lines · Benoit
Quoted 19 lines
Quoted 8 lines
Quoted 8 lines

?/* SYSCONFIG: register bit definition */
-#define AUTOIDLE ? ? ? (1<< ?0)
?#define SOFTRESET ? ? ?(1<< ?1)
-#define SMARTIDLE ? ? ?(2<< ?3)
?#define OMAP4_SOFTRESET ? ? ? ?(1<< ?0)

(quoted diff) +0 -2
-#define OMAP4_NOIDLE ? (1<< ?2)
-#define OMAP4_SMARTIDLE ? ? ? ?(2<< ?2)
Quoted 4 lines
Quoted 2 lines

?/* SYSSTATUS: register bit definition */
?#define RESETDONE ? ? ?(1<< ?0)

Is this still required?

Yes, mailbox uses the softreset every time it is requested (and that it has no current users at the time), it does it before configuring sysc, as opossed to what this patch does (where clk is enabled, sysc configured, and then softreset), but no harm was seen with this sequence.

Quoted 8 lines
Quoted 2 lines

AFAIK, enabling/disabling hwmod handles prm reset but not softreset bit, that's why I left it there.

In fact both are handled now. hardreset is managed for IVA, DSP and IPU, and softreset for most the other IPs.
The only slight difference, is that softreset for the moment is just done once at setup time. The idea being that we do not know the state the bootloader configured IP, so we reset it by default.

Yes, I noticed that softreset was handled on setup only.

Quoted 5 lines

I thought I did it for every enable from disable state, but in fact this is not the case, because AFAIK there is not use-case for that yet.

Why do you have to softreset the mailbox? That sounds like some old HW bugs from the past that we solve using the softreset.

It is adviced in the TRM that before initialization of the module, you should do a softreset and set sysc register, I found this explanation too (with git blame):

omap: mailbox: Execute softreset at startup

The softreset at startup is introduced as TRM describes and also some register bit definitions are added instead of magic number

But mailbox doesn't execute this on init, but on every first user request, it doesn't sound like there was a bug solved with this.

Also related (these were the quickest I could check):

- The DSI protocol engine can be reset by software. This reset can be done for debug purposes or after a protocol error.
- UART needs it too, but in this case it was executed one time at init, so hwmod was the perfect match to remove it.

Quoted 4 lines

Could you elaborate on that use case? If there is a need, I'd rather update the hwmod core than letting you playing directly with the sysconfig. At least we can expose the API we did for that purpose but didn't merge due to the lack of any strong need.

I also thought of this, but then driver would need:

pm_runtime_get_sync() // to enable the clock
pdata->device_reset() // to set the softreset
pm_runtime_put_sync() // to disable the clock
pm_runtime_get_sync() // to enable again and reconfigure sysc lost
with softreset

If needed I think softreset should be handled in the device enable path rather than a new API.

Regards,

Omar

#13 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

In reply to Omar Ramirez Luna

Omar,

Quoted 62 lines · Omar Ramirez Luna

Use runtime pm APIs to enable/disable mailbox clocks and to configure SYSC register.

Based on the patch sent by Felipe Contreras:
https://patchwork.kernel.org/patch/101662/

---
?arch/arm/mach-omap2/mailbox.c | ? 27 +++++---------------------- ?1 files changed, 5 insertions(+), 22 deletions(-)

arch/arm/mach-omap2/mailbox.c +0 -0
index 40ddeca..f5f72ba 100644
@@ -14,6 +14,7 @@

?#include <linux/err.h>
?#include <linux/platform_device.h>
?#include <linux/io.h>
+#include <linux/pm_runtime.h>
?#include <plat/mailbox.h>
?#include <mach/irqs.h>

arch/arm/mach-omap2/mailbox.c +0 -0
@@ -34,12 +35,8 @@
 ?#define MAILBOX_IRQ_NOTFULL(m) ? ? ? ? (1 << (2 * (m) + 1))
 

?/* SYSCONFIG: register bit definition */
-#define AUTOIDLE ? ? ? (1 << 0)
?#define SOFTRESET ? ? ?(1 << 1)
-#define SMARTIDLE ? ? ?(2 << 3)
?#define OMAP4_SOFTRESET ? ? ? ?(1 << 0)

arch/arm/mach-omap2/mailbox.c +0 -2
-#define OMAP4_NOIDLE ? (1 << 2)
-#define OMAP4_SMARTIDLE ? ? ? ?(2 << 2)

?/* SYSSTATUS: register bit definition */
?#define RESETDONE ? ? ?(1 << 0)

arch/arm/mach-omap2/mailbox.c +2 -9
@@ -70,8 +67,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,13 +86,8 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 ? ? ? ?u32 l;
 ? ? ? ?unsigned long timeout;
 
- ? ? ? 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);
+ ? ? ? pm_runtime_enable(mbox->dev->parent);
+ ? ? ? pm_runtime_get_sync(mbox->dev->parent);
 
 ? ? ? ?if (cpu_is_omap44xx()) {
 ? ? ? ? ? ? ? ?mbox_write_reg(OMAP4_SOFTRESET, MAILBOX_SYSCONFIG);

arch/arm/mach-omap2/mailbox.c static int omap2_mbox_startup(struct omap_mbox *mbox) · line 138

Quoted 9 lines
arch/arm/mach-omap2/mailbox.c +0 -6
@@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 ? ? ? ?l = mbox_read_reg(MAILBOX_REVISION);
 ? ? ? ?pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
 
- ? ? ? if (cpu_is_omap44xx())
- ? ? ? ? ? ? ? l = OMAP4_SMARTIDLE;
- ? ? ? else
- ? ? ? ? ? ? ? l = SMARTIDLE | AUTOIDLE;
- ? ? ? mbox_write_reg(l, MAILBOX_SYSCONFIG);
-

The OMAP4 mailbox sysconfig register bits are laid out differently from previous OMAP mailbox's. Example is smart idle bit location is different from previous OMAPs. Can I know as how are you handling this aspect in hwmod code ?

Thank you,
Best regards,
Hari Kanigeri

#14 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Hi,

Quoted 4 lines · Hari
Quoted 4 lines
(quoted diff) +0 -0
@@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 ? ? ? ?l = mbox_read_reg(MAILBOX_REVISION);
 ? ? ? ?pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
 
Quoted 11 lines

Review static int omap2_mbox_startup(struct omap_mbox *mbox) · line 138

Quoted 6 lines
(quoted diff) +0 -6
- ? ? ? if (cpu_is_omap44xx())
- ? ? ? ? ? ? ? l = OMAP4_SMARTIDLE;
- ? ? ? else
- ? ? ? ? ? ? ? l = SMARTIDLE | AUTOIDLE;
- ? ? ? mbox_write_reg(l, MAILBOX_SYSCONFIG);
-

The OMAP4 mailbox sysconfig register bits are laid out differently from previous OMAP mailbox's. Example is smart idle bit location is different from previous OMAPs. Can I know as how are you handling this aspect in hwmod code ?

hwmod framework provides definition for both IP models (omap_hwmod_sysc_type1for omap2/3 or omap_hwmod_sysc_type2 for omap4), these are located at arch/arm/mach-omap2/omap_hwmod_common_data.c

Regards,

Omar

#15 lore raw

PATCH v4 5/5 OMAP: mailbox: use runtime pm for clk and sysc handling

Quoted 19 lines · Omar

Hi,

Quoted 4 lines · Hari
Quoted 4 lines
(quoted diff) +0 -0
@@ -130,12 +120,6 @@ static int omap2_mbox_startup(struct omap_mbox *mbox)
 ? ? ? ?l = mbox_read_reg(MAILBOX_REVISION);
 ? ? ? ?pr_debug("omap mailbox rev %d.%d\n", (l & 0xf0) >> 4, (l & 0x0f));
 
Quoted 11 lines

Review static int omap2_mbox_startup(struct omap_mbox *mbox) · line 138

Quoted 6 lines
(quoted diff) +0 -6
- ? ? ? if (cpu_is_omap44xx())
- ? ? ? ? ? ? ? l = OMAP4_SMARTIDLE;
- ? ? ? else
- ? ? ? ? ? ? ? l = SMARTIDLE | AUTOIDLE;
- ? ? ? mbox_write_reg(l, MAILBOX_SYSCONFIG);
-

The OMAP4 mailbox sysconfig register bits are laid out differently from previous OMAP mailbox's. Example is smart idle bit location is different from previous OMAPs. Can I know as how are you handling this aspect in hwmod code ?

Quoted 4 lines

hwmod framework provides definition for both IP models (omap_hwmod_sysc_type1for omap2/3 or omap_hwmod_sysc_type2 for omap4), these are located at arch/arm/mach-omap2/omap_hwmod_common_data.c

Thanks, Omar.

Best regards,
Hari Kanigeri