[PATCHv6 0/7] ARM: OMAP4: core retention support

[PATCHv6 0/7] ARM: OMAP4: core retention support

[Tero Kristo]

Hi,

Changes compared to previous version:

- moved basic omap4 pm errata support from dev-off set to this one
- changed ordering of patches a bit (core ret enabled at last patch)
- dropped DSP reset hack patch from set, as it is no longer needed
- added arch specific hwmod_ops support, needed for proper functioning
  of the context loss support
- context offset register invalidity is now marked with USHRT_MAX
  instead of a separate flag
- patches rebased on top of Jean Pihet's functional power state code
- patches rebased on top of 3.5-rc2
- dropped debug option for enabling OSWR mode for suspend, this is now
  enabled by default

Tested with omap4 panda board + omap3 beagle (just to verify nothing
gets broken.)

Fully functioning branch available here:
tree: git://gitorious.org/~kristo/omap-pm/omap-pm-work.git
branch: mainline-3.5-rc2-omap4-ret-v6

Branch contains also hwmod fixes from Paul Walmsley, functional power
state code from Jean Pihet, IO chain set and a few other needed misc
fixes to make suspend to core retention + wakeup to work properly.

-Tero

[PATCHv6 1/7] ARM: OMAP4: PM: add errata support

[Tero Kristo]

Added similar PM errata flag support as omap3 has. This should be used
in similar manner, set the flags during init time, and check the flag
values during runtime.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm.h     |    7 +++++++
 arch/arm/mach-omap2/pm44xx.c |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 7856489..46ab9d9 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -88,6 +88,13 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
 static inline void enable_omap3630_toggle_l2_on_restore(void) { }
 #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
 
+#if defined(CONFIG_ARCH_OMAP4)
+extern u16 pm44xx_errata;
+#define IS_PM44XX_ERRATUM(id)		(pm44xx_errata & (id))
+#else
+#define IS_PM44XX_ERRATUM(id)		0
+#endif
+
 #ifdef CONFIG_OMAP_SMARTREFLEX
 extern int omap_devinit_smartreflex(void);
 extern void omap_enable_smartreflex_on_init(void);
diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index c234733..3a484b1 100644
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -33,6 +33,7 @@ struct power_state {
 };
 
 static LIST_HEAD(pwrst_list);
+u16 pm44xx_errata;
 
 #ifdef CONFIG_SUSPEND
 static int omap4_pm_suspend(void)
-- 
1.7.4.1

[PATCHv6 2/7] ARM: OMAP4460: Workaround for ROM bug because of CA9 r2pX GIC control register change.

[Tero Kristo]

From: Santosh Shilimkar <santosh.shilimkar@ti.com>

On OMAP4+ devices, GIC register context is lost when MPUSS hits
the OSWR(Open Switch Retention). On the CPU wakeup path, ROM code
gets executed and one of the steps in it is to restore the
saved context of the GIC. The ROM Code GIC distributor restoration
is split in two parts: CPU specific register done by each CPU and
common register done by only one CPU.

Below is the abstract flow.

...............................................................
- MPUSS in OSWR state.
- CPU0 wakes up on the event(interrupt) and start executing ROM code.

[..]

- CPU0 executes "GIC Restoration:"

[...]

- CPU0 swicthes to non-secure mode and jumps to OS resume code.

[...]

- CPU0 is online in OS
- CPU0 enables the GIC distributor. GICD.Enable Non-secure = 1
- CPU0 wakes up CPU1 with clock-domain force wakeup method.
- CPU0 continues it's execution.
[..]

- CPU1 wakes up and start executing ROM code.

[..]

- CPU1 executes "GIC Restoration:"

[..]

- CPU1 swicthes to non-secure mode and jumps to OS resume code.

[...]

- CPU1 is online in OS and start executing.
[...]   -

GIC Restoration: /* Common routine for HS and GP devices */
{
       if (GICD != 1)  { /* This will be true in OSWR state */
               if (GIC_SAR_BACKUP_STATE == SAVED)
                       - CPU restores GIC distributor
               else
                       - reconfigure GIC distributor to boot values.

               GICD.Enable secure = 1
       }

       if (GIC_SAR_BACKUP_STATE == SAVED)
               - CPU restore its GIC CPU interface registers if saved.
       else
               - reconfigure its GIC CPU interface registers to boot
                       values.
}
...............................................................

So as mentioned in the flow, GICD != 1 condition decides how
the GIC registers are handled in ROM code wakeup path from
OSWR. As evident from the flow, ROM code relies on the entire
GICD register value and not specific register bits.

The assumption was valid till CortexA9 r1pX version since there
was only one banked bit to control secure and non-secure GICD.
Secure view which ROM code sees:
       bit 0 == Enable Non-secure
Non-secure view which HLOS sees:
       bit 0 == Enable secure

But GICD register has changed between CortexA9 r1pX and r2pX.
On r2pX GICD register is composed of 2 bits.
Secure view which ROM code sees:
       bit 1 == Enable Non-secure
       bit 0 == Enable secure
Non-secure view which HLOS sees:
       bit 0 == Enable Non-secure

Hence on OMAP4460(r2pX) devices, if you go through the
above flow again during CPU1 wakeup, GICD == 3 and hence
ROM code fails to understand the real wakeup power state
and reconfigures GIC distributor to boot values. This is
nasty since you loose the entire interrupt controller
context in a live system.

The ROM code fix done on next OMAP4 device (OMAP4470 - r2px) is to
check "GICD.Enable secure != 1" for GIC restoration in OSWR wakeup path.

Since ROM code can't be fixed on OMAP4460 devices, a work around
needs to be implemented. As evident from the flow, as long as
CPU1 sees GICD == 1 in it's wakeup path from OSWR, the issue
won't happen. Below is the flow with the work-around.

...............................................................
- MPUSS in OSWR state.
- CPU0 wakes up on the event(interrupt) and start executing ROM code.

[..]

- CPU0 executes "GIC Restoration:"

[..]

- CPU0 swicthes to non-secure mode and jumps to OS resume code.

[..]

- CPU0 is online in OS.
- CPU0 does GICD.Enable Non-secure = 0
- CPU0 wakes up CPU1 with clock domain force wakeup method.
- CPU0 waits for GICD.Enable Non-secure = 1
- CPU0 coninues it's execution.
[..]

- CPU1 wakes up and start executing ROM code.

[..]

- CPU1 executes "GIC Restoration:"

[..]

- CPU1 swicthes to non-secure mode and jumps to OS resume code.

[..]

- CPU1 is online in OS
- CPU1 does GICD.Enable Non-secure = 1
- CPU1 start executing
[...]
...............................................................

With this procedure, the GIC configuration done between the
CPU0 wakeup and CPU1 wakeup will not be lost but during this
short windows, the CPU0 will not receive interrupts.

The BUG is applicable to only OMAP4460(r2pX) devices.
OMAP4470 (also r2pX) is not affected by this bug because
ROM code has been fixed.

Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/common.h              |    2 +
 arch/arm/mach-omap2/omap-headsmp.S        |   38 +++++++++++++++++++++++++++++
 arch/arm/mach-omap2/omap-mpuss-lowpower.c |    9 ++++++-
 arch/arm/mach-omap2/omap-smp.c            |   28 ++++++++++++++++++++-
 arch/arm/mach-omap2/omap4-common.c        |    8 +++++-
 arch/arm/mach-omap2/pm.h                  |    2 +
 6 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index be9dfd1..a793ab3 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -246,6 +246,7 @@ static inline void __iomem *omap4_get_scu_base(void)
 #endif
 
 extern void __init gic_init_irq(void);
+extern void gic_dist_disable(void);
 extern void omap_smc1(u32 fn, u32 arg);
 extern void __iomem *omap4_get_sar_ram_base(void);
 extern void omap_do_wfi(void);
@@ -253,6 +254,7 @@ extern void omap_do_wfi(void);
 #ifdef CONFIG_SMP
 /* Needed for secondary core boot */
 extern void omap_secondary_startup(void);
+extern void omap_secondary_startup_4460(void);
 extern u32 omap_modify_auxcoreboot0(u32 set_mask, u32 clear_mask);
 extern void omap_auxcoreboot_addr(u32 cpu_addr);
 extern u32 omap_read_auxcoreboot0(void);
diff --git a/arch/arm/mach-omap2/omap-headsmp.S b/arch/arm/mach-omap2/omap-headsmp.S
index 503ac77..7bbb66e 100644
--- a/arch/arm/mach-omap2/omap-headsmp.S
+++ b/arch/arm/mach-omap2/omap-headsmp.S
@@ -18,6 +18,8 @@
 #include <linux/linkage.h>
 #include <linux/init.h>
 
+#include <plat/omap44xx.h>
+
 	__CPUINIT
 /*
  * OMAP4 specific entry point for secondary CPU to jump from ROM
@@ -43,3 +45,39 @@ hold:	ldr	r12,=0x103
 	b	secondary_startup
 ENDPROC(omap_secondary_startup)
 
+ENTRY(omap_secondary_startup_4460)
+hold_2:	ldr	r12,=0x103
+	dsb
+	smc	#0			@ read from AuxCoreBoot0
+	mov	r0, r0, lsr #9
+	mrc	p15, 0, r4, c0, c0, 5
+	and	r4, r4, #0x0f
+	cmp	r0, r4
+	bne	hold_2
+
+	/*
+	 * GIC distributor control register has changed between
+	 * CortexA9 r1pX and r2pX. The Control Register secure
+	 * banked version is now composed of 2 bits:
+	 * bit 0 == Secure Enable
+	 * bit 1 == Non-Secure Enable
+	 * The Non-Secure banked register has not changed
+	 * Because the ROM Code is based on the r1pX GIC, the CPU1
+	 * GIC restoration will cause a problem to CPU0 Non-Secure SW.
+	 * The workaround must be:
+	 * 1) Before doing the CPU1 wakeup, CPU0 must disable
+	 * the GIC distributor
+	 * 2) CPU1 must re-enable the GIC distributor on
+	 * it's wakeup path.
+	 */
+	ldr	r1, =OMAP44XX_GIC_DIST_BASE
+	ldr	r0, [r1]
+	orr	r0, #1
+	str	r0, [r1]
+
+	/*
+	 * we've been released from the wait loop,secondary_stack
+	 * should now contain the SVC stack for this core
+	 */
+	b	secondary_startup
+ENDPROC(omap_secondary_startup_4460)
diff --git a/arch/arm/mach-omap2/omap-mpuss-lowpower.c b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
index c79cc0f..0e5f81b 100644
--- a/arch/arm/mach-omap2/omap-mpuss-lowpower.c
+++ b/arch/arm/mach-omap2/omap-mpuss-lowpower.c
@@ -68,6 +68,7 @@ struct omap4_cpu_pm_info {
 	void __iomem *scu_sar_addr;
 	void __iomem *wkup_sar_addr;
 	void __iomem *l2x0_sar_addr;
+	void (*secondary_startup)(void);
 };
 
 static DEFINE_PER_CPU(struct omap4_cpu_pm_info, omap4_pm_info);
@@ -301,6 +302,7 @@ int omap4_enter_lowpower(unsigned int cpu, unsigned int power_state)
 int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state)
 {
 	unsigned int cpu_state = 0;
+	struct omap4_cpu_pm_info *pm_info = &per_cpu(omap4_pm_info, cpu);
 
 	if (omap_rev() == OMAP4430_REV_ES1_0)
 		return -ENXIO;
@@ -310,7 +312,7 @@ int __cpuinit omap4_hotplug_cpu(unsigned int cpu, unsigned int power_state)
 
 	clear_cpu_prev_pwrst(cpu);
 	set_cpu_next_pwrst(cpu, power_state);
-	set_cpu_wakeup_addr(cpu, virt_to_phys(omap_secondary_startup));
+	set_cpu_wakeup_addr(cpu, virt_to_phys(pm_info->secondary_startup));
 	scu_pwrst_prepare(cpu, power_state);
 
 	/*
@@ -361,6 +363,11 @@ int __init omap4_mpuss_init(void)
 	pm_info->scu_sar_addr = sar_base + SCU_OFFSET1;
 	pm_info->wkup_sar_addr = sar_base + CPU1_WAKEUP_NS_PA_ADDR_OFFSET;
 	pm_info->l2x0_sar_addr = sar_base + L2X0_SAVE_OFFSET1;
+	if (cpu_is_omap446x())
+		pm_info->secondary_startup = omap_secondary_startup_4460;
+	else
+		pm_info->secondary_startup = omap_secondary_startup;
+
 	pm_info->pwrdm = pwrdm_lookup("cpu1_pwrdm");
 	if (!pm_info->pwrdm) {
 		pr_err("Lookup failed for CPU1 pwrdm\n");
diff --git a/arch/arm/mach-omap2/omap-smp.c b/arch/arm/mach-omap2/omap-smp.c
index deffbf1..e21d42e 100644
--- a/arch/arm/mach-omap2/omap-smp.c
+++ b/arch/arm/mach-omap2/omap-smp.c
@@ -30,6 +30,7 @@
 #include "iomap.h"
 #include "common.h"
 #include "clockdomain.h"
+#include "pm.h"
 
 /* SCU base address */
 static void __iomem *scu_base;
@@ -104,6 +105,24 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
 	 *	4.3.4.2 Power States of CPU0 and CPU1
 	 */
 	if (booted) {
+		/*
+		 * GIC distributor control register has changed between
+		 * CortexA9 r1pX and r2pX. The Control Register secure
+		 * banked version is now composed of 2 bits:
+		 * bit 0 == Secure Enable
+		 * bit 1 == Non-Secure Enable
+		 * The Non-Secure banked register has not changed
+		 * Because the ROM Code is based on the r1pX GIC, the CPU1
+		 * GIC restoration will cause a problem to CPU0 Non-Secure SW.
+		 * The workaround must be:
+		 * 1) Before doing the CPU1 wakeup, CPU0 must disable
+		 * the GIC distributor
+		 * 2) CPU1 must re-enable the GIC distributor on
+		 * it's wakeup path.
+		 */
+		if (IS_PM44XX_ERRATUM(PM_OMAP4_ROM_SMP_BOOT_ERRATUM_xxx))
+			gic_dist_disable();
+
 		clkdm_wakeup(cpu1_clkdm);
 		clkdm_allow_idle(cpu1_clkdm);
 	} else {
@@ -124,13 +143,20 @@ int __cpuinit boot_secondary(unsigned int cpu, struct task_struct *idle)
 
 static void __init wakeup_secondary(void)
 {
+	void *startup_addr = omap_secondary_startup;
+
 	/*
 	 * Write the address of secondary startup routine into the
 	 * AuxCoreBoot1 where ROM code will jump and start executing
 	 * on secondary core once out of WFE
 	 * A barrier is added to ensure that write buffer is drained
 	 */
-	omap_auxcoreboot_addr(virt_to_phys(omap_secondary_startup));
+	if (cpu_is_omap446x()) {
+		startup_addr = omap_secondary_startup_4460;
+		pm44xx_errata |= PM_OMAP4_ROM_SMP_BOOT_ERRATUM_xxx;
+	}
+
+	omap_auxcoreboot_addr(virt_to_phys(startup_addr));
 	smp_wmb();
 
 	/*
diff --git a/arch/arm/mach-omap2/omap4-common.c b/arch/arm/mach-omap2/omap4-common.c
index a8161e5..07ca05b 100644
--- a/arch/arm/mach-omap2/omap4-common.c
+++ b/arch/arm/mach-omap2/omap4-common.c
@@ -40,6 +40,7 @@ static void __iomem *l2cache_base;
 #endif
 
 static void __iomem *sar_ram_base;
+static void __iomem *gic_dist_base_addr;
 
 #ifdef CONFIG_OMAP4_ERRATA_I688
 /* Used to implement memory barrier on DRAM path */
@@ -94,7 +95,6 @@ void __init omap_barriers_init(void)
 void __init gic_init_irq(void)
 {
 	void __iomem *omap_irq_base;
-	void __iomem *gic_dist_base_addr;
 
 	/* Static mapping, never released */
 	gic_dist_base_addr = ioremap(OMAP44XX_GIC_DIST_BASE, SZ_4K);
@@ -109,6 +109,12 @@ void __init gic_init_irq(void)
 	gic_init(0, 29, gic_dist_base_addr, omap_irq_base);
 }
 
+void gic_dist_disable(void)
+{
+	if (gic_dist_base_addr)
+		__raw_writel(0x0, gic_dist_base_addr + GIC_DIST_CTRL);
+}
+
 #ifdef CONFIG_CACHE_L2X0
 
 void __iomem *omap4_get_l2cache_base(void)
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 46ab9d9..8d7de4f 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -88,6 +88,8 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
 static inline void enable_omap3630_toggle_l2_on_restore(void) { }
 #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
 
+#define PM_OMAP4_ROM_SMP_BOOT_ERRATUM_xxx	(1 << 0)
+
 #if defined(CONFIG_ARCH_OMAP4)
 extern u16 pm44xx_errata;
 #define IS_PM44XX_ERRATUM(id)		(pm44xx_errata & (id))
-- 
1.7.4.1

[PATCHv6 2/7] ARM: OMAP4460: Workaround for ROM bug because of CA9 r2pX GIC control register change.

[Paul Walmsley]

Hi

one trivial comment

On Mon, 11 Jun 2012, Tero Kristo wrote:

> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index 46ab9d9..8d7de4f 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -88,6 +88,8 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
>  static inline void enable_omap3630_toggle_l2_on_restore(void) { }
>  #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
>  
> +#define PM_OMAP4_ROM_SMP_BOOT_ERRATUM_xxx	(1 << 0)

"xxx" isn't unique, nor is it descriptive :-)  

Looking@the mail threads on this one, it sounds like there may not be a 
public erratum number associated with this.  If there is, the 'xxx' should 
be replaced with the number.  If there isn't, then it would be good to 
make up something short, like "GICD" to replace the "xxx".


- Paul

[PATCHv6 2/7] ARM: OMAP4460: Workaround for ROM bug because of CA9 r2pX GIC control register change.

[Tero Kristo]

On Wed, 2012-07-18 at 03:17 -0600, Paul Walmsley wrote:
> Hi
> 
> one trivial comment
> 
> On Mon, 11 Jun 2012, Tero Kristo wrote:
> 
> > diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> > index 46ab9d9..8d7de4f 100644
> > --- a/arch/arm/mach-omap2/pm.h
> > +++ b/arch/arm/mach-omap2/pm.h
> > @@ -88,6 +88,8 @@ extern void enable_omap3630_toggle_l2_on_restore(void);
> >  static inline void enable_omap3630_toggle_l2_on_restore(void) { }
> >  #endif		/* defined(CONFIG_PM) && defined(CONFIG_ARCH_OMAP3) */
> >  
> > +#define PM_OMAP4_ROM_SMP_BOOT_ERRATUM_xxx	(1 << 0)
> 
> "xxx" isn't unique, nor is it descriptive :-)  
> 
> Looking at the mail threads on this one, it sounds like there may not be a 
> public erratum number associated with this.  If there is, the 'xxx' should 
> be replaced with the number.  If there isn't, then it would be good to 
> make up something short, like "GICD" to replace the "xxx".

There isn't public erratum number for this, thats the reason for xxx. I
can change the xxx to GICD though if that is better.

-Tero

[PATCHv6 2/7] ARM: OMAP4460: Workaround for ROM bug because of CA9 r2pX GIC control register change.

[Paul Walmsley]

On Wed, 18 Jul 2012, Tero Kristo wrote:

> There isn't public erratum number for this, thats the reason for xxx. I
> can change the xxx to GICD though if that is better.

Yep please do (or something else more meaningful than GICD if you can 
think of something better).


- Paul

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Tero Kristo]

On OMAP4 most modules/hwmods support module level context status. On
OMAP3 and earlier, we relied on the power domain level context status.
Identify all modules that don't support 'context_offs' by marking the
offset as USHRT_MAX. Rest have a valid 'context_offs' populated in
.prcm structure already.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 86fc513..828e7b8 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -208,6 +208,7 @@ static struct omap_hwmod omap44xx_l4_abe_hwmod = {
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM1_ABE_L4ABE_CLKCTRL_OFFSET,
+			.context_offs = USHRT_MAX,
 		},
 	},
 };
-- 
1.7.4.1

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Menon, Nishanth]

On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> On OMAP4 most modules/hwmods support module level context status. On
> OMAP3 and earlier, we relied on the power domain level context status.
> Identify all modules that don't support 'context_offs' by marking the
> offset as USHRT_MAX. Rest have a valid 'context_offs' populated in
> .prcm structure already.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 86fc513..828e7b8 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -208,6 +208,7 @@ static struct omap_hwmod omap44xx_l4_abe_hwmod = {
>         .prcm = {
>                 .omap4 = {
>                         .clkctrl_offs = OMAP4_CM1_ABE_L4ABE_CLKCTRL_OFFSET,
> +                       .context_offs = USHRT_MAX,

OMAP4430_RM_ABE_AESS_CONTEXT? why not use LOSTMEM_AESSMEM ? ABE will
need to know when it lost context to be able to reload it's firmware,
no?

Regards,
Nishanth Menon

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Tero Kristo]

On Tue, 2012-07-17 at 03:11 -0500, Menon, Nishanth wrote:
> On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > On OMAP4 most modules/hwmods support module level context status. On
> > OMAP3 and earlier, we relied on the power domain level context status.
> > Identify all modules that don't support 'context_offs' by marking the
> > offset as USHRT_MAX. Rest have a valid 'context_offs' populated in
> > .prcm structure already.
> >
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > ---
> >  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > index 86fc513..828e7b8 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > @@ -208,6 +208,7 @@ static struct omap_hwmod omap44xx_l4_abe_hwmod = {
> >         .prcm = {
> >                 .omap4 = {
> >                         .clkctrl_offs = OMAP4_CM1_ABE_L4ABE_CLKCTRL_OFFSET,
> > +                       .context_offs = USHRT_MAX,
> 
> OMAP4430_RM_ABE_AESS_CONTEXT? why not use LOSTMEM_AESSMEM ? ABE will
> need to know when it lost context to be able to reload it's firmware,
> no?

You are right, this should be possible to use. I just searched for
modules which didn't have context_offs defined and added USHRT_MAX
blindly there.

-Tero

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Tero Kristo]

On Tue, 2012-07-17 at 03:11 -0500, Menon, Nishanth wrote:
> On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > On OMAP4 most modules/hwmods support module level context status. On
> > OMAP3 and earlier, we relied on the power domain level context status.
> > Identify all modules that don't support 'context_offs' by marking the
> > offset as USHRT_MAX. Rest have a valid 'context_offs' populated in
> > .prcm structure already.
> >
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > ---
> >  arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > index 86fc513..828e7b8 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > @@ -208,6 +208,7 @@ static struct omap_hwmod omap44xx_l4_abe_hwmod = {
> >         .prcm = {
> >                 .omap4 = {
> >                         .clkctrl_offs = OMAP4_CM1_ABE_L4ABE_CLKCTRL_OFFSET,
> > +                       .context_offs = USHRT_MAX,
> 
> OMAP4430_RM_ABE_AESS_CONTEXT? why not use LOSTMEM_AESSMEM ? ABE will
> need to know when it lost context to be able to reload it's firmware,
> no?

It looks like current hwmod data doesn't support specific bits to be
used for the context declaration, it is only specifying the register
offset. Also, the same register is used by aess hwmod, so this will
cause a conflict if I take the same register into use.

This could be fixed by adding a field for the context bits, but I guess
this should be commented upon by someone (Benoit / Paul) before I craft
some sort of patch for that.

Meanwhile, I will keep the USHRT_MAX here until better solution is
available.

-Tero

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Paul Walmsley]

On Wed, 18 Jul 2012, Tero Kristo wrote:

> On Tue, 2012-07-17 at 03:11 -0500, Menon, Nishanth wrote:
>
> > OMAP4430_RM_ABE_AESS_CONTEXT? why not use LOSTMEM_AESSMEM ? ABE will
> > need to know when it lost context to be able to reload it's firmware,
> > no?
> 
> It looks like current hwmod data doesn't support specific bits to be
> used for the context declaration, it is only specifying the register
> offset. Also, the same register is used by aess hwmod, so this will
> cause a conflict if I take the same register into use.
> 
> This could be fixed by adding a field for the context bits, but I guess
> this should be commented upon by someone (Benoit / Paul) before I craft
> some sort of patch for that.

If you need to add a u8 there to specify the bitshift, go ahead and do it.  
u8 lostmem_bit, perhaps?


- Paul

[PATCHv6 3/7] ARM: OMAP4: hwmod: flag hwmods/modules not supporting module level context status

[Tero Kristo]

On Wed, 2012-07-18 at 11:24 -0600, Paul Walmsley wrote:
> On Wed, 18 Jul 2012, Tero Kristo wrote:
> 
> > On Tue, 2012-07-17 at 03:11 -0500, Menon, Nishanth wrote:
> >
> > > OMAP4430_RM_ABE_AESS_CONTEXT? why not use LOSTMEM_AESSMEM ? ABE will
> > > need to know when it lost context to be able to reload it's firmware,
> > > no?
> > 
> > It looks like current hwmod data doesn't support specific bits to be
> > used for the context declaration, it is only specifying the register
> > offset. Also, the same register is used by aess hwmod, so this will
> > cause a conflict if I take the same register into use.
> > 
> > This could be fixed by adding a field for the context bits, but I guess
> > this should be commented upon by someone (Benoit / Paul) before I craft
> > some sort of patch for that.
> 
> If you need to add a u8 there to specify the bitshift, go ahead and do it.  
> u8 lostmem_bit, perhaps?

Mask might be better, as we have RFF / DFF bits, and also if mask is not
defined, we can assume we want to check the whole register (current
behavior.) I'll add this for next rev.

-Tero

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Tero Kristo]

From: Rajendra Nayak <rnayak@ti.com>

OMAP4 has module specific context lost registers which makes it now
possible to have module level context loss count, instead of relying
on the powerdomain level context count.

Add 2 private hwmod api's to update/clear the hwmod/module specific
context lost counters/register.

Update the module specific context_lost_counter and clear the hardware
bits just after enabling the module.

omap_hwmod_get_context_loss_count() now returns the hwmod context loss
count them on platforms where they exist (OMAP4), else fall back on
the pwrdm level counters for older platforms.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
[paul at pwsan.com: added function kerneldoc, fixed structure kerneldoc,
 rearranged structure to avoid memory waste, marked fns as OMAP4-specific,
 prevent fn entry on non-OMAP4 chips, reduced indentation, merged update
 and clear, merged patches]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
[t-kristo at ti.com: added support for arch specific hwmod ops, and changed
 the no context offset indicator to USHRT_MAX]
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   70 ++++++++++++++++++++++++--
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    8 ++-
 2 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 00007b1..742e507 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -170,6 +170,13 @@
 /* omap_hwmod_list contains all registered struct omap_hwmods */
 static LIST_HEAD(omap_hwmod_list);
 
+struct hwmod_ops {
+	void	(*hwmod_update_context_lost)(struct omap_hwmod *oh);
+	int	(*hwmod_get_context_lost)(struct omap_hwmod *oh);
+};
+
+static struct hwmod_ops *arch_hwmod;
+
 /* mpu_oh: used to add/remove MPU initiator from sleepdep list */
 static struct omap_hwmod *mpu_oh;
 
@@ -1773,6 +1780,51 @@ static void _reconfigure_io_chain(void)
 }
 
 /**
+ * _omap4_update_context_lost - increment hwmod context loss counter if
+ * hwmod context was lost, and clear hardware context loss reg
+ * @oh: hwmod to check for context loss
+ *
+ * If the PRCM indicates that the hwmod @oh lost context, increment
+ * our in-memory context loss counter, and clear the RM_*_CONTEXT
+ * bits. No return value.
+ */
+static void _omap4_update_context_lost(struct omap_hwmod *oh)
+{
+	u32 r;
+
+	if (oh->prcm.omap4.context_offs == USHRT_MAX)
+		return;
+
+	r = omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition,
+					oh->clkdm->pwrdm.ptr->prcm_offs,
+					oh->prcm.omap4.context_offs);
+
+	if (!r)
+		return;
+
+	oh->prcm.omap4.context_lost_counter++;
+
+	omap4_prminst_write_inst_reg(r, oh->clkdm->pwrdm.ptr->prcm_partition,
+				     oh->clkdm->pwrdm.ptr->prcm_offs,
+				     oh->prcm.omap4.context_offs);
+}
+
+/**
+ * _omap4_get_context_lost - get context loss counter for a hwmod
+ *
+ * Returns the in-memory context loss counter for a hwmod.
+ */
+static int _omap4_get_context_lost(struct omap_hwmod *oh)
+{
+	return oh->prcm.omap4.context_lost_counter;
+}
+
+static struct hwmod_ops omap4_hwmod_ops = {
+	.hwmod_update_context_lost	= _omap4_update_context_lost,
+	.hwmod_get_context_lost		= _omap4_get_context_lost,
+};
+
+/**
  * _enable - enable an omap_hwmod
  * @oh: struct omap_hwmod *
  *
@@ -1853,6 +1905,9 @@ static int _enable(struct omap_hwmod *oh)
 	_enable_clocks(oh);
 	_enable_module(oh);
 
+	if (arch_hwmod && arch_hwmod->hwmod_update_context_lost)
+		arch_hwmod->hwmod_update_context_lost(oh);
+
 	r = _wait_target_ready(oh);
 	if (!r) {
 		/*
@@ -2711,6 +2766,9 @@ int __init omap_hwmod_setup_one(const char *oh_name)
  */
 static int __init omap_hwmod_setup_all(void)
 {
+	if (cpu_is_omap44xx())
+		arch_hwmod = &omap4_hwmod_ops;
+
 	_ensure_mpu_hwmod_is_setup(NULL);
 
 	omap_hwmod_for_each(_init, NULL);
@@ -3364,17 +3422,21 @@ ohsps_unlock:
  * omap_hwmod_get_context_loss_count - get lost context count
  * @oh: struct omap_hwmod *
  *
- * Query the powerdomain of of @oh to get the context loss
- * count for this device.
+ * Returns the context loss count of associated @oh
+ * upon success, or zero if no context loss data is available.
  *
- * Returns the context loss count of the powerdomain assocated with @oh
- * upon success, or zero if no powerdomain exists for @oh.
+ * On OMAP4, this queries the per-hwmod context loss register,
+ * assuming one exists.  If not, or on OMAP2/3, this queries the
+ * enclosing powerdomain context loss count.
  */
 int omap_hwmod_get_context_loss_count(struct omap_hwmod *oh)
 {
 	struct powerdomain *pwrdm;
 	int ret = 0;
 
+	if (arch_hwmod && arch_hwmod->hwmod_get_context_lost)
+		return arch_hwmod->hwmod_get_context_lost(oh);
+
 	pwrdm = omap_hwmod_get_pwrdm(oh);
 	if (pwrdm)
 		ret = pwrdm_get_context_loss_count(pwrdm);
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index b5f3efc..34ace8c 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -372,9 +372,12 @@ struct omap_hwmod_omap2_prcm {
 
 /**
  * struct omap_hwmod_omap4_prcm - OMAP4-specific PRCM data
- * @clkctrl_reg: PRCM address of the clock control register
- * @rstctrl_reg: address of the XXX_RSTCTRL register located in the PRM
+ * @clkctrl_offs: offset of the PRCM clock control register
+ * @rstctrl_offs: offset of the XXX_RSTCTRL register located in the PRM
+ * @context_offs: offset of the RM_*_CONTEXT register
  * @submodule_wkdep_bit: bit shift of the WKDEP range
+ * @modulemode: allowable modulemodes
+ * @context_lost_counter: Count of module level context lost
  */
 struct omap_hwmod_omap4_prcm {
 	u16		clkctrl_offs;
@@ -382,6 +385,7 @@ struct omap_hwmod_omap4_prcm {
 	u16		context_offs;
 	u8		submodule_wkdep_bit;
 	u8		modulemode;
+	unsigned	context_lost_counter;
 };
 
 
-- 
1.7.4.1

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Menon, Nishanth]

Couple of minor comments:
On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
[...]
>  /**
> + * _omap4_update_context_lost - increment hwmod context loss counter if
> + * hwmod context was lost, and clear hardware context loss reg
> + * @oh: hwmod to check for context loss
> + *
> + * If the PRCM indicates that the hwmod @oh lost context, increment
> + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> + * bits. No return value.
> + */
> +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> +{
> +       u32 r;
> +
> +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> +               return;
would'nt it be better to return a dummy incremental counter instead of
returning no context loss (count = 0)?

> +
> +       r = omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition,
> +                                       oh->clkdm->pwrdm.ptr->prcm_offs,
> +                                       oh->prcm.omap4.context_offs);
> +
> +       if (!r)
> +               return;
> +
> +       oh->prcm.omap4.context_lost_counter++;
need to be careful about counter overflow.


Regards,
Nishanth Menon

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Tero Kristo]

On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote:
> Couple of minor comments:
> On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> [...]
> >  /**
> > + * _omap4_update_context_lost - increment hwmod context loss counter if
> > + * hwmod context was lost, and clear hardware context loss reg
> > + * @oh: hwmod to check for context loss
> > + *
> > + * If the PRCM indicates that the hwmod @oh lost context, increment
> > + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> > + * bits. No return value.
> > + */
> > +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> > +{
> > +       u32 r;
> > +
> > +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> > +               return;
> would'nt it be better to return a dummy incremental counter instead of
> returning no context loss (count = 0)?

I guess you are right, this way we may have some extra context restores
for modules which don't have context offs defined, rather than not
restoring them at all. Only thing I can think might prevent this is if
there are modules that never lose context but don't have context
register? How about omap5+?

> 
> > +
> > +       r = omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition,
> > +                                       oh->clkdm->pwrdm.ptr->prcm_offs,
> > +                                       oh->prcm.omap4.context_offs);
> > +
> > +       if (!r)
> > +               return;
> > +
> > +       oh->prcm.omap4.context_lost_counter++;
> need to be careful about counter overflow.

Well, this code can't do much for that even if it overflows... the type
for the context_lost_counter is unsigned though, maybe it should be
expanded if you are worried...?

-Tero

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Menon, Nishanth]

On Wed, Jul 18, 2012 at 4:15 AM, Tero Kristo <t-kristo@ti.com> wrote:
>
> On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote:
> > Couple of minor comments:
> > On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > [...]
> > >  /**
> > > + * _omap4_update_context_lost - increment hwmod context loss counter
> > > if
> > > + * hwmod context was lost, and clear hardware context loss reg
> > > + * @oh: hwmod to check for context loss
> > > + *
> > > + * If the PRCM indicates that the hwmod @oh lost context, increment
> > > + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> > > + * bits. No return value.
> > > + */
> > > +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> > > +{
> > > +       u32 r;
> > > +
> > > +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> > > +               return;
> > would'nt it be better to return a dummy incremental counter instead of
> > returning no context loss (count = 0)?
>
> I guess you are right, this way we may have some extra context restores
> for modules which don't have context offs defined, rather than not
> restoring them at all. Only thing I can think might prevent this is if
> there are modules that never lose context but don't have context
> register? How about omap5+?

there has been an interesting debate ongoing with HWAUTO and context
loss count handling -> since we update only on _enable, this might
actually be interesting to consider:
enable
idle
un_idle (lost context)
read counter -> no update

Now to handle modules that never loose context - they have to be in
wakeup domain.. should we consider a flag for those? would'nt matter
o5 or not, context is still the same.. this issue could be resolved if
counter update is done even when a check is done.


> >
> > > +
> > > +       r =
> > > omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition,
> > > +
> > > oh->clkdm->pwrdm.ptr->prcm_offs,
> > > +                                       oh->prcm.omap4.context_offs);
> > > +
> > > +       if (!r)
> > > +               return;
> > > +
> > > +       oh->prcm.omap4.context_lost_counter++;
> > need to be careful about counter overflow.
>
> Well, this code can't do much for that even if it overflows... the type
> for the context_lost_counter is unsigned though, maybe it should be
> expanded if you are worried...?

it can hit 0 with overflow(no context loss). that will not be good, right?

How about something like:

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index eac813a..5fb9572 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1606,6 +1606,18 @@ static void _reconfigure_io_chain(void)
        spin_unlock_irqrestore(&io_chain_lock, flags);
 }

+static inline void _omap4_inc_context_loss(unsigned int *v)
+{
+
+       /*
+        * Context loss count has to be a non-negative value.
+        * Clear the sign bit to get a value range from 1 to
+        * INT_MAX.
+        */
+       *v = (*v + 1) & INT_MAX;
+       *v = *v ? *v : 1;
+}
+
 /**
  * _omap4_update_context_lost - increment hwmod context loss counter if
  * hwmod context was lost, and clear hardware context loss reg
@@ -1629,7 +1641,7 @@ static void _omap4_update_context_lost(struct
omap_hwmod *oh)
        if (!r)
                return;

-       oh->prcm.omap4.context_lost_counter++;
+       _omap4_inc_context_loss(&oh->prcm.omap4.context_lost_counter);

        omap4_prminst_write_inst_reg(r, oh->clkdm->pwrdm.ptr->prcm_partition,
                                     oh->clkdm->pwrdm.ptr->prcm_offs,

Regards,
Nishanth Menon

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Tero Kristo]

On Thu, 2012-07-19 at 00:55 -0500, Menon, Nishanth wrote:
> On Wed, Jul 18, 2012 at 4:15 AM, Tero Kristo <t-kristo@ti.com> wrote:
> >
> > On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote:
> > > Couple of minor comments:
> > > On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > > [...]
> > > >  /**
> > > > + * _omap4_update_context_lost - increment hwmod context loss counter
> > > > if
> > > > + * hwmod context was lost, and clear hardware context loss reg
> > > > + * @oh: hwmod to check for context loss
> > > > + *
> > > > + * If the PRCM indicates that the hwmod @oh lost context, increment
> > > > + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> > > > + * bits. No return value.
> > > > + */
> > > > +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> > > > +{
> > > > +       u32 r;
> > > > +
> > > > +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> > > > +               return;
> > > would'nt it be better to return a dummy incremental counter instead of
> > > returning no context loss (count = 0)?
> >
> > I guess you are right, this way we may have some extra context restores
> > for modules which don't have context offs defined, rather than not
> > restoring them at all. Only thing I can think might prevent this is if
> > there are modules that never lose context but don't have context
> > register? How about omap5+?
> 
> there has been an interesting debate ongoing with HWAUTO and context
> loss count handling -> since we update only on _enable, this might
> actually be interesting to consider:
> enable
> idle
> un_idle (lost context)
> read counter -> no update
> 
> Now to handle modules that never loose context - they have to be in
> wakeup domain.. should we consider a flag for those? would'nt matter
> o5 or not, context is still the same.. this issue could be resolved if
> counter update is done even when a check is done.

Yea, that would be an option. I think I'll add a flag for not losing
context ever.

> 
> 
> > >
> > > > +
> > > > +       r =
> > > > omap4_prminst_read_inst_reg(oh->clkdm->pwrdm.ptr->prcm_partition,
> > > > +
> > > > oh->clkdm->pwrdm.ptr->prcm_offs,
> > > > +                                       oh->prcm.omap4.context_offs);
> > > > +
> > > > +       if (!r)
> > > > +               return;
> > > > +
> > > > +       oh->prcm.omap4.context_lost_counter++;
> > > need to be careful about counter overflow.
> >
> > Well, this code can't do much for that even if it overflows... the type
> > for the context_lost_counter is unsigned though, maybe it should be
> > expanded if you are worried...?
> 
> it can hit 0 with overflow(no context loss). that will not be good, right?

Zero doesn't mean no context loss. If counter was previous MAX_INT, if
it goes to zero it is still a context loss, as the counter value
differs. Drivers do check against diff in the context loss counter, and
if there is one, they do restore which is the right way to handle it. No
need to unnecessarily make this more complicated than it is.

-Tero

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Menon, Nishanth]

On Thu, Jul 19, 2012 at 4:49 AM, Tero Kristo <t-kristo@ti.com> wrote:
>
> Zero doesn't mean no context loss. If counter was previous MAX_INT, if
> it goes to zero it is still a context loss, as the counter value
> differs. Drivers do check against diff in the context loss counter, and
> if there is one, they do restore which is the right way to handle it. No
> need to unnecessarily make this more complicated than it is.

so we flip the responsibility of overflow to drivers. considering a
similar scenario of jiffies
/*
 *      These inlines deal with timer wrapping correctly. You are
 *      strongly encouraged to use them
 *      1. Because people otherwise forget
 *      2. Because if the timer wrap changes in future you won't have to
 *         alter your driver code.
 *
 * time_after(a,b) returns true if the time a is after time b.
...
*/
from past experience, it is highly possible that drivers never get
this right. if the intent is just to let the drivers know context was
lost, why not go back to the alternate possibility of a bool
lost_context which tells the driver if it lost context since it last
called the lost_context api.

Regards,
Nishanth Menon

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Tero Kristo]

On Thu, 2012-07-19 at 05:27 -0500, Menon, Nishanth wrote:
> On Thu, Jul 19, 2012 at 4:49 AM, Tero Kristo <t-kristo@ti.com> wrote:
> >
> > Zero doesn't mean no context loss. If counter was previous MAX_INT, if
> > it goes to zero it is still a context loss, as the counter value
> > differs. Drivers do check against diff in the context loss counter, and
> > if there is one, they do restore which is the right way to handle it. No
> > need to unnecessarily make this more complicated than it is.
> 
> so we flip the responsibility of overflow to drivers. considering a
> similar scenario of jiffies
> /*
>  *      These inlines deal with timer wrapping correctly. You are
>  *      strongly encouraged to use them
>  *      1. Because people otherwise forget
>  *      2. Because if the timer wrap changes in future you won't have to
>  *         alter your driver code.
>  *
>  * time_after(a,b) returns true if the time a is after time b.
> ...
> */
> from past experience, it is highly possible that drivers never get
> this right. if the intent is just to let the drivers know context was
> lost, why not go back to the alternate possibility of a bool
> lost_context which tells the driver if it lost context since it last
> called the lost_context api.

This goes to the discussion whether the API of lost context stuff is
correct or not, and goes out of scope for this set.

I am just attempting to bring omap4 to omap3 level in the first place,
we can discuss about the potential API problems separately, and a change
like that should be relatively easy to implement anyway.... but will
break several drivers.

-Tero

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Shilimkar, Santosh]

On Thu, Jul 19, 2012 at 3:19 PM, Tero Kristo <t-kristo@ti.com> wrote:
>
> On Thu, 2012-07-19 at 00:55 -0500, Menon, Nishanth wrote:
> > On Wed, Jul 18, 2012 at 4:15 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > >
> > > On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote:
> > > > Couple of minor comments:
> > > > On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com>
> > > > wrote:
> > > > [...]
> > > > >  /**
> > > > > + * _omap4_update_context_lost - increment hwmod context loss
> > > > > counter
> > > > > if
> > > > > + * hwmod context was lost, and clear hardware context loss reg
> > > > > + * @oh: hwmod to check for context loss
> > > > > + *
> > > > > + * If the PRCM indicates that the hwmod @oh lost context,
> > > > > increment
> > > > > + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> > > > > + * bits. No return value.
> > > > > + */
> > > > > +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> > > > > +{
> > > > > +       u32 r;
> > > > > +
> > > > > +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> > > > > +               return;
> > > > would'nt it be better to return a dummy incremental counter instead
> > > > of
> > > > returning no context loss (count = 0)?
> > >
> > > I guess you are right, this way we may have some extra context
> > > restores
> > > for modules which don't have context offs defined, rather than not
> > > restoring them at all. Only thing I can think might prevent this is if
> > > there are modules that never lose context but don't have context
> > > register? How about omap5+?
> >
> > there has been an interesting debate ongoing with HWAUTO and context
> > loss count handling -> since we update only on _enable, this might
> > actually be interesting to consider:
> > enable
> > idle
> > un_idle (lost context)
> > read counter -> no update
> >
> > Now to handle modules that never loose context - they have to be in
> > wakeup domain.. should we consider a flag for those? would'nt matter
> > o5 or not, context is still the same.. this issue could be resolved if
> > counter update is done even when a check is done.
>
> Yea, that would be an option. I think I'll add a flag for not losing
> context ever.
>
You just access the module power domain from hwmod and then
you already know whether it is AON or not. The flag idea
was discussed in context of [1]. See if you can re-use that same idea.

regards
Santosh

[1] https://patchwork.kernel.org/patch/1133491/

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Tero Kristo]

On Thu, 2012-07-19 at 16:05 +0530, Shilimkar, Santosh wrote:
> On Thu, Jul 19, 2012 at 3:19 PM, Tero Kristo <t-kristo@ti.com> wrote:
> >
> > On Thu, 2012-07-19 at 00:55 -0500, Menon, Nishanth wrote:
> > > On Wed, Jul 18, 2012 at 4:15 AM, Tero Kristo <t-kristo@ti.com> wrote:
> > > >
> > > > On Tue, 2012-07-17 at 02:59 -0500, Menon, Nishanth wrote:
> > > > > Couple of minor comments:
> > > > > On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com>
> > > > > wrote:
> > > > > [...]
> > > > > >  /**
> > > > > > + * _omap4_update_context_lost - increment hwmod context loss
> > > > > > counter
> > > > > > if
> > > > > > + * hwmod context was lost, and clear hardware context loss reg
> > > > > > + * @oh: hwmod to check for context loss
> > > > > > + *
> > > > > > + * If the PRCM indicates that the hwmod @oh lost context,
> > > > > > increment
> > > > > > + * our in-memory context loss counter, and clear the RM_*_CONTEXT
> > > > > > + * bits. No return value.
> > > > > > + */
> > > > > > +static void _omap4_update_context_lost(struct omap_hwmod *oh)
> > > > > > +{
> > > > > > +       u32 r;
> > > > > > +
> > > > > > +       if (oh->prcm.omap4.context_offs == USHRT_MAX)
> > > > > > +               return;
> > > > > would'nt it be better to return a dummy incremental counter instead
> > > > > of
> > > > > returning no context loss (count = 0)?
> > > >
> > > > I guess you are right, this way we may have some extra context
> > > > restores
> > > > for modules which don't have context offs defined, rather than not
> > > > restoring them at all. Only thing I can think might prevent this is if
> > > > there are modules that never lose context but don't have context
> > > > register? How about omap5+?
> > >
> > > there has been an interesting debate ongoing with HWAUTO and context
> > > loss count handling -> since we update only on _enable, this might
> > > actually be interesting to consider:
> > > enable
> > > idle
> > > un_idle (lost context)
> > > read counter -> no update
> > >
> > > Now to handle modules that never loose context - they have to be in
> > > wakeup domain.. should we consider a flag for those? would'nt matter
> > > o5 or not, context is still the same.. this issue could be resolved if
> > > counter update is done even when a check is done.
> >
> > Yea, that would be an option. I think I'll add a flag for not losing
> > context ever.
> >
> You just access the module power domain from hwmod and then
> you already know whether it is AON or not. The flag idea
> was discussed in context of [1]. See if you can re-use that same idea.

That looks better still, thanks for the tip.

-Tero

[PATCHv6 4/7] ARM: OMAP: hwmod: Add support for per hwmod/module context lost count

[Menon, Nishanth]

Minor nitpick:

On Mon, Jun 11, 2012 at 10:26 AM, Tero Kristo <t-kristo@ti.com> wrote:
> +/**
> + * _omap4_get_context_lost - get context loss counter for a hwmod
Documentation missing for oh
btw, you might be interested in using http://www.omappedia.org/wiki/Kmake
to provide list of kerneldoc errors in addition to other easily
catchable errors..

> + *
> + * Returns the in-memory context loss counter for a hwmod.
> + */
> +static int _omap4_get_context_lost(struct omap_hwmod *oh)
> +{
> +       return oh->prcm.omap4.context_lost_counter;
> +}


Regards,
Nishanth Menon

[PATCHv6 5/7] ARM: OMAP4: pwrdm: add support for reading prev logic and mem states

[Tero Kristo]

On OMAP4, there is no support to read previous logic state
or previous memory state achieved when a power domain transitions
to RET. Instead there are module level context registers.

In order to support the powerdomain level logic/mem_off_counters
on OMAP4, instead use the previous power state achieved (RET) and
the *programmed* logic/mem RET state to derive if a powerdomain lost
logic or did not.

If the powerdomain is programmed to enter RET state and lose logic
in RET state, knowing that the powerdomain entered RET is good enough
to derive that the logic was lost as well, in such cases.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/powerdomain44xx.c |   59 +++++++++++++++++++++++++++++++++
 1 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/powerdomain44xx.c b/arch/arm/mach-omap2/powerdomain44xx.c
index 030d10c..ab93f08 100644
--- a/arch/arm/mach-omap2/powerdomain44xx.c
+++ b/arch/arm/mach-omap2/powerdomain44xx.c
@@ -151,6 +151,34 @@ static int omap4_pwrdm_read_logic_retst(struct powerdomain *pwrdm)
 	return v;
 }
 
+/**
+ * omap4_pwrdm_read_prev_logic_pwrst - read the previous logic powerstate
+ * @pwrdm: struct powerdomain * to read the state for
+ *
+ * Reads the previous logic powerstate for a powerdomain. This function
+ * must determine the previous logic powerstate by first checking the
+ * previous powerstate for the domain. If that was OFF, then logic has
+ * been lost. If previous state was RETENTION, the function reads the
+ * setting for the next retention logic state to see the actual value.
+ * In every other case, the logic is retained. Returns either
+ * PWRDM_LOGIC_MEM_PWRST_OFF or PWRDM_LOGIC_MEM_PWRST_RET depending
+ * whether the logic was retained or not.
+ */
+static int omap4_pwrdm_read_prev_logic_pwrst(struct powerdomain *pwrdm)
+{
+	int state;
+
+	state = omap4_pwrdm_read_prev_pwrst(pwrdm);
+
+	if (state == PWRDM_POWER_OFF)
+		return PWRDM_LOGIC_MEM_PWRST_OFF;
+
+	if (state != PWRDM_POWER_RET)
+		return PWRDM_LOGIC_MEM_PWRST_RET;
+
+	return omap4_pwrdm_read_logic_retst(pwrdm);
+}
+
 static int omap4_pwrdm_read_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
 {
 	u32 m, v;
@@ -179,6 +207,35 @@ static int omap4_pwrdm_read_mem_retst(struct powerdomain *pwrdm, u8 bank)
 	return v;
 }
 
+/**
+ * omap4_pwrdm_read_prev_mem_pwrst - reads the previous memory powerstate
+ * @pwrdm: struct powerdomain * to read mem powerstate for
+ * @bank: memory bank index
+ *
+ * Reads the previous memory powerstate for a powerdomain. This function
+ * must determine the previous memory powerstate by first checking the
+ * previous powerstate for the domain. If that was OFF, then logic has
+ * been lost. If previous state was RETENTION, the function reads the
+ * setting for the next memory retention state to see the actual value.
+ * In every other case, the logic is retained. Returns either
+ * PWRDM_LOGIC_MEM_PWRST_OFF or PWRDM_LOGIC_MEM_PWRST_RET depending
+ * whether logic was retained or not.
+ */
+static int omap4_pwrdm_read_prev_mem_pwrst(struct powerdomain *pwrdm, u8 bank)
+{
+	int state;
+
+	state = omap4_pwrdm_read_prev_pwrst(pwrdm);
+
+	if (state == PWRDM_POWER_OFF)
+		return PWRDM_LOGIC_MEM_PWRST_OFF;
+
+	if (state != PWRDM_POWER_RET)
+		return PWRDM_LOGIC_MEM_PWRST_RET;
+
+	return omap4_pwrdm_read_mem_retst(pwrdm, bank);
+}
+
 static int omap4_pwrdm_wait_transition(struct powerdomain *pwrdm)
 {
 	u32 c = 0;
@@ -220,9 +277,11 @@ struct pwrdm_ops omap4_pwrdm_operations = {
 	.pwrdm_clear_all_prev_pwrst	= omap4_pwrdm_clear_all_prev_pwrst,
 	.pwrdm_set_logic_retst	= omap4_pwrdm_set_logic_retst,
 	.pwrdm_read_logic_pwrst	= omap4_pwrdm_read_logic_pwrst,
+	.pwrdm_read_prev_logic_pwrst	= omap4_pwrdm_read_prev_logic_pwrst,
 	.pwrdm_read_logic_retst	= omap4_pwrdm_read_logic_retst,
 	.pwrdm_read_mem_pwrst	= omap4_pwrdm_read_mem_pwrst,
 	.pwrdm_read_mem_retst	= omap4_pwrdm_read_mem_retst,
+	.pwrdm_read_prev_mem_pwrst	= omap4_pwrdm_read_prev_mem_pwrst,
 	.pwrdm_set_mem_onst	= omap4_pwrdm_set_mem_onst,
 	.pwrdm_set_mem_retst	= omap4_pwrdm_set_mem_retst,
 	.pwrdm_wait_transition	= omap4_pwrdm_wait_transition,
-- 
1.7.4.1

[PATCHv6 6/7] ARM: OMAP4: suspend: Program all domains to retention

[Tero Kristo]

From: Rajendra Nayak <rnayak@ti.com>

Remove the FIXME's in the suspend sequence since
we now intend to support system level RET support.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
[Jean Pihet <j-pihet@ti.com>: ported on top of the functional power
states]
Reviewed-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/pm44xx.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index 3a484b1..1e845e8 100644
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -97,19 +97,15 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
 	if (!strncmp(pwrdm->name, "cpu", 3))
 		return 0;
 
-	/*
-	 * FIXME: Remove this check when core retention is supported
-	 * Only MPUSS power domain is added in the list.
-	 */
-	if (strcmp(pwrdm->name, "mpu_pwrdm"))
-		return 0;
 
 	pwrst = kmalloc(sizeof(struct power_state), GFP_ATOMIC);
 	if (!pwrst)
 		return -ENOMEM;
 
 	pwrst->pwrdm = pwrdm;
-	pwrst->next_state = PWRDM_FUNC_PWRST_CSWR;
+	pwrst->next_state = pwrdm_get_achievable_func_pwrst(
+						pwrdm,
+						PWRDM_FUNC_PWRST_CSWR);
 	list_add(&pwrst->node, &pwrst_list);
 
 	return omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
-- 
1.7.4.1

[PATCHv6 7/7] ARM: OMAP4: PM: put all domains to OSWR during suspend

[Tero Kristo]

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm44xx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/pm44xx.c b/arch/arm/mach-omap2/pm44xx.c
index 1e845e8..eb3e073 100644
--- a/arch/arm/mach-omap2/pm44xx.c
+++ b/arch/arm/mach-omap2/pm44xx.c
@@ -105,7 +105,7 @@ static int __init pwrdms_setup(struct powerdomain *pwrdm, void *unused)
 	pwrst->pwrdm = pwrdm;
 	pwrst->next_state = pwrdm_get_achievable_func_pwrst(
 						pwrdm,
-						PWRDM_FUNC_PWRST_CSWR);
+						PWRDM_FUNC_PWRST_OSWR);
 	list_add(&pwrst->node, &pwrst_list);
 
 	return omap_set_pwrdm_state(pwrst->pwrdm, pwrst->next_state);
-- 
1.7.4.1