[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Tero Kristo]

When waking up from off-mode, some IP blocks are reset automatically by
hardware. For this reason, software must wait until the reset has
completed before attempting to access the IP block.

This patch fixes for example the bug introduced by commit
6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
to SYSCONFIG register"), in which the MMC IP block is reset during
off-mode entry, but the code expects the module to be already available
during the execution of context restore.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index b969ab1..523729b 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1278,6 +1278,7 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	u8 idlemode, sf;
 	u32 v;
 	bool clkdm_act;
+	int c;
 
 	if (!oh->class->sysc)
 		return;
@@ -1338,6 +1339,27 @@ static void _enable_sysc(struct omap_hwmod *oh)
 		_set_module_autoidle(oh, idlemode, &v);
 		_write_sysconfig(v, oh);
 	}
+
+	/*
+	 * Wait until reset has completed, this is needed as the IP
+	 * block is reset automatically by hardware in some cases
+	 * (off-mode for example), and the drivers require the
+	 * IP to be ready when they access it
+	 */
+	if (sf & SYSS_HAS_RESET_STATUS)
+		omap_test_timeout((omap_hwmod_read(oh,
+						   oh->class->sysc->syss_offs)
+				   & SYSS_RESETDONE_MASK),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (sf & SYSC_HAS_RESET_STATUS) {
+		u32 softrst_mask =
+			(0x1 << oh->class->sysc->sysc_fields->srst_shift);
+		omap_test_timeout(!(omap_hwmod_read(oh,
+						    oh->class->sysc->sysc_offs)
+				   & softrst_mask),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	}
 }
 
 /**
-- 
1.7.4.1

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Tero Kristo]

When waking up from off-mode, some IP blocks are reset automatically by
hardware. For this reason, software must wait until the reset has
completed before attempting to access the IP block.

This patch fixes for example the bug introduced by commit
6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
to SYSCONFIG register"), in which the MMC IP block is reset during
off-mode entry, but the code expects the module to be already available
during the execution of context restore.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index b969ab1..523729b 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1278,6 +1278,7 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	u8 idlemode, sf;
 	u32 v;
 	bool clkdm_act;
+	int c;
 
 	if (!oh->class->sysc)
 		return;
@@ -1338,6 +1339,27 @@ static void _enable_sysc(struct omap_hwmod *oh)
 		_set_module_autoidle(oh, idlemode, &v);
 		_write_sysconfig(v, oh);
 	}
+
+	/*
+	 * Wait until reset has completed, this is needed as the IP
+	 * block is reset automatically by hardware in some cases
+	 * (off-mode for example), and the drivers require the
+	 * IP to be ready when they access it
+	 */
+	if (sf & SYSS_HAS_RESET_STATUS)
+		omap_test_timeout((omap_hwmod_read(oh,
+						   oh->class->sysc->syss_offs)
+				   & SYSS_RESETDONE_MASK),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (sf & SYSC_HAS_RESET_STATUS) {
+		u32 softrst_mask =
+			(0x1 << oh->class->sysc->sysc_fields->srst_shift);
+		omap_test_timeout(!(omap_hwmod_read(oh,
+						    oh->class->sysc->sysc_offs)
+				   & softrst_mask),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	}
 }
 
 /**
-- 
1.7.4.1

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
>
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>

I can confirm that this patch the regression in my OMAP3 PM tests where
suspend test (to retention or off) failed if ran after the off-idle
test.

Tested-by: Kevin Hilman <khilman@ti.com>

on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM

Thanks Tero for the fix,

Kevin

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Tero Kristo <t-kristo@ti.com> writes:

> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
>
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>

I can confirm that this patch the regression in my OMAP3 PM tests where
suspend test (to retention or off) failed if ran after the off-idle
test.

Tested-by: Kevin Hilman <khilman@ti.com>

on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM

Thanks Tero for the fix,

Kevin

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Paul Walmsley]

Hi Tero,

On Mon, 22 Oct 2012, Tero Kristo wrote:

> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
> 
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>

What do you think about these modifications?  The code is quite similar to 
what was in the _ocp_softreset() function, so just moved it into a 
function.  Also moved the callsite from the end of _enable_sysc() to the 
beginning, which makes more sense to me, but would like to get your 
opinion.

- Paul

From: Tero Kristo <t-kristo@ti.com>
Date: Mon, 22 Oct 2012 19:15:32 +0300
Subject: [PATCH] [RFC] ARM: OMAP: hwmod: wait for sysreset complete after
 enabling hwmod

When waking up from off-mode, some IP blocks are reset automatically by
hardware. For this reason, software must wait until the reset has
completed before attempting to access the IP block.

This patch fixes for example the bug introduced by commit
6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
to SYSCONFIG register"), in which the MMC IP block is reset during
off-mode entry, but the code expects the module to be already available
during the execution of context restore.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
[paul@pwsan.com: moved softreset wait code into separate function; call
 from top of _enable_sysc() rather than the bottom]

---
 arch/arm/mach-omap2/omap_hwmod.c |   56 ++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 504e0e0..0356a09 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -423,6 +423,38 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
 }
 
 /**
+ * _wait_softreset_complete - wait for an OCP softreset to complete
+ * @oh: struct omap_hwmod * to wait on
+ *
+ * Wait until the IP block represented by @oh reports that its OCP
+ * softreset is complete.  This can be triggered by software (see
+ * _ocp_softreset()) or by hardware upon returning from off-mode (one
+ * example is HSMMC).  Waits for up to MAX_MODULE_SOFTRESET_WAIT
+ * microseconds.  Returns the number of microseconds waited.
+ */
+static int _wait_softreset_complete(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_class_sysconfig *sysc;
+	u32 softrst_mask;
+	int c = 0;
+
+	sysc = oh->class->sysc;
+
+	if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
+		omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
+				   & SYSS_RESETDONE_MASK),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	else if (sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
+		softrst_mask = (0x1 << sysc->sysc_fields->srst_shift);
+		omap_test_timeout(!(omap_hwmod_read(oh, sysc->sysc_offs)
+				    & softrst_mask),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	}
+
+	return c;
+}
+
+/**
  * _set_dmadisable: set OCP_SYSCONFIG.DMADISABLE bit in @v
  * @oh: struct omap_hwmod *
  *
@@ -1283,6 +1315,14 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	if (!oh->class->sysc)
 		return;
 
+	/*
+	 * Wait until reset has completed, this is needed as the IP
+	 * block is reset automatically by hardware in some cases
+	 * (off-mode for example), and the drivers require the
+	 * IP to be ready when they access it
+	 */
+	_wait_softreset_complete(oh);
+
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;
 
@@ -1805,7 +1845,7 @@ static int _am33xx_disable_module(struct omap_hwmod *oh)
  */
 static int _ocp_softreset(struct omap_hwmod *oh)
 {
-	u32 v, softrst_mask;
+	u32 v;
 	int c = 0;
 	int ret = 0;
 
@@ -1835,19 +1875,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
 	if (oh->class->sysc->srst_udelay)
 		udelay(oh->class->sysc->srst_udelay);
 
-	if (oh->class->sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
-		omap_test_timeout((omap_hwmod_read(oh,
-						    oh->class->sysc->syss_offs)
-				   & SYSS_RESETDONE_MASK),
-				  MAX_MODULE_SOFTRESET_WAIT, c);
-	else if (oh->class->sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
-		softrst_mask = (0x1 << oh->class->sysc->sysc_fields->srst_shift);
-		omap_test_timeout(!(omap_hwmod_read(oh,
-						     oh->class->sysc->sysc_offs)
-				   & softrst_mask),
-				  MAX_MODULE_SOFTRESET_WAIT, c);
-	}
-
+	c = _wait_softreset_complete(oh);
 	if (c == MAX_MODULE_SOFTRESET_WAIT)
 		pr_warning("omap_hwmod: %s: softreset failed (waited %d usec)\n",
 			   oh->name, MAX_MODULE_SOFTRESET_WAIT);
-- 
1.7.10.4

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Paul Walmsley]

Hi Tero,

On Mon, 22 Oct 2012, Tero Kristo wrote:

> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
> 
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>

What do you think about these modifications?  The code is quite similar to 
what was in the _ocp_softreset() function, so just moved it into a 
function.  Also moved the callsite from the end of _enable_sysc() to the 
beginning, which makes more sense to me, but would like to get your 
opinion.

- Paul

From: Tero Kristo <t-kristo@ti.com>
Date: Mon, 22 Oct 2012 19:15:32 +0300
Subject: [PATCH] [RFC] ARM: OMAP: hwmod: wait for sysreset complete after
 enabling hwmod

When waking up from off-mode, some IP blocks are reset automatically by
hardware. For this reason, software must wait until the reset has
completed before attempting to access the IP block.

This patch fixes for example the bug introduced by commit
6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
to SYSCONFIG register"), in which the MMC IP block is reset during
off-mode entry, but the code expects the module to be already available
during the execution of context restore.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Venkatraman S <svenkatr@ti.com>
[paul at pwsan.com: moved softreset wait code into separate function; call
 from top of _enable_sysc() rather than the bottom]

---
 arch/arm/mach-omap2/omap_hwmod.c |   56 ++++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 504e0e0..0356a09 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -423,6 +423,38 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
 }
 
 /**
+ * _wait_softreset_complete - wait for an OCP softreset to complete
+ * @oh: struct omap_hwmod * to wait on
+ *
+ * Wait until the IP block represented by @oh reports that its OCP
+ * softreset is complete.  This can be triggered by software (see
+ * _ocp_softreset()) or by hardware upon returning from off-mode (one
+ * example is HSMMC).  Waits for up to MAX_MODULE_SOFTRESET_WAIT
+ * microseconds.  Returns the number of microseconds waited.
+ */
+static int _wait_softreset_complete(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_class_sysconfig *sysc;
+	u32 softrst_mask;
+	int c = 0;
+
+	sysc = oh->class->sysc;
+
+	if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
+		omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
+				   & SYSS_RESETDONE_MASK),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	else if (sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
+		softrst_mask = (0x1 << sysc->sysc_fields->srst_shift);
+		omap_test_timeout(!(omap_hwmod_read(oh, sysc->sysc_offs)
+				    & softrst_mask),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
+	}
+
+	return c;
+}
+
+/**
  * _set_dmadisable: set OCP_SYSCONFIG.DMADISABLE bit in @v
  * @oh: struct omap_hwmod *
  *
@@ -1283,6 +1315,14 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	if (!oh->class->sysc)
 		return;
 
+	/*
+	 * Wait until reset has completed, this is needed as the IP
+	 * block is reset automatically by hardware in some cases
+	 * (off-mode for example), and the drivers require the
+	 * IP to be ready when they access it
+	 */
+	_wait_softreset_complete(oh);
+
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;
 
@@ -1805,7 +1845,7 @@ static int _am33xx_disable_module(struct omap_hwmod *oh)
  */
 static int _ocp_softreset(struct omap_hwmod *oh)
 {
-	u32 v, softrst_mask;
+	u32 v;
 	int c = 0;
 	int ret = 0;
 
@@ -1835,19 +1875,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
 	if (oh->class->sysc->srst_udelay)
 		udelay(oh->class->sysc->srst_udelay);
 
-	if (oh->class->sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
-		omap_test_timeout((omap_hwmod_read(oh,
-						    oh->class->sysc->syss_offs)
-				   & SYSS_RESETDONE_MASK),
-				  MAX_MODULE_SOFTRESET_WAIT, c);
-	else if (oh->class->sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
-		softrst_mask = (0x1 << oh->class->sysc->sysc_fields->srst_shift);
-		omap_test_timeout(!(omap_hwmod_read(oh,
-						     oh->class->sysc->sysc_offs)
-				   & softrst_mask),
-				  MAX_MODULE_SOFTRESET_WAIT, c);
-	}
-
+	c = _wait_softreset_complete(oh);
 	if (c == MAX_MODULE_SOFTRESET_WAIT)
 		pr_warning("omap_hwmod: %s: softreset failed (waited %d usec)\n",
 			   oh->name, MAX_MODULE_SOFTRESET_WAIT);
-- 
1.7.10.4

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Tero Kristo]

Hi Paul,

On Tue, 2012-10-23 at 07:47 +0000, Paul Walmsley wrote:
> Hi Tero,
> 
> On Mon, 22 Oct 2012, Tero Kristo wrote:
> 
> > When waking up from off-mode, some IP blocks are reset automatically by
> > hardware. For this reason, software must wait until the reset has
> > completed before attempting to access the IP block.
> > 
> > This patch fixes for example the bug introduced by commit
> > 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> > to SYSCONFIG register"), in which the MMC IP block is reset during
> > off-mode entry, but the code expects the module to be already available
> > during the execution of context restore.
> > 
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Venkatraman S <svenkatr@ti.com>
> 
> What do you think about these modifications?  The code is quite similar to 
> what was in the _ocp_softreset() function, so just moved it into a 
> function.  Also moved the callsite from the end of _enable_sysc() to the 
> beginning, which makes more sense to me, but would like to get your 
> opinion.

Looks good to me. Only reason I pushed the reset wait to the end of the
function was a minimal potential timing optimization. As the beginning
of the function will write a register, it will consume a bit of time
(especially if in 32k domain), so it can maybe save a single register
read in the delay loop if ordered that way.

Your way is probably safer, it might cause problems in some cases if the
config is written before reset has completed (can it?)

-Tero

> 
> - Paul
> 
> From: Tero Kristo <t-kristo@ti.com>
> Date: Mon, 22 Oct 2012 19:15:32 +0300
> Subject: [PATCH] [RFC] ARM: OMAP: hwmod: wait for sysreset complete after
>  enabling hwmod
> 
> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
> 
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> [paul@pwsan.com: moved softreset wait code into separate function; call
>  from top of _enable_sysc() rather than the bottom]
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |   56 ++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 504e0e0..0356a09 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -423,6 +423,38 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
>  }
>  
>  /**
> + * _wait_softreset_complete - wait for an OCP softreset to complete
> + * @oh: struct omap_hwmod * to wait on
> + *
> + * Wait until the IP block represented by @oh reports that its OCP
> + * softreset is complete.  This can be triggered by software (see
> + * _ocp_softreset()) or by hardware upon returning from off-mode (one
> + * example is HSMMC).  Waits for up to MAX_MODULE_SOFTRESET_WAIT
> + * microseconds.  Returns the number of microseconds waited.
> + */
> +static int _wait_softreset_complete(struct omap_hwmod *oh)
> +{
> +	struct omap_hwmod_class_sysconfig *sysc;
> +	u32 softrst_mask;
> +	int c = 0;
> +
> +	sysc = oh->class->sysc;
> +
> +	if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
> +		omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
> +				   & SYSS_RESETDONE_MASK),
> +				  MAX_MODULE_SOFTRESET_WAIT, c);
> +	else if (sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
> +		softrst_mask = (0x1 << sysc->sysc_fields->srst_shift);
> +		omap_test_timeout(!(omap_hwmod_read(oh, sysc->sysc_offs)
> +				    & softrst_mask),
> +				  MAX_MODULE_SOFTRESET_WAIT, c);
> +	}
> +
> +	return c;
> +}
> +
> +/**
>   * _set_dmadisable: set OCP_SYSCONFIG.DMADISABLE bit in @v
>   * @oh: struct omap_hwmod *
>   *
> @@ -1283,6 +1315,14 @@ static void _enable_sysc(struct omap_hwmod *oh)
>  	if (!oh->class->sysc)
>  		return;
>  
> +	/*
> +	 * Wait until reset has completed, this is needed as the IP
> +	 * block is reset automatically by hardware in some cases
> +	 * (off-mode for example), and the drivers require the
> +	 * IP to be ready when they access it
> +	 */
> +	_wait_softreset_complete(oh);
> +
>  	v = oh->_sysc_cache;
>  	sf = oh->class->sysc->sysc_flags;
>  
> @@ -1805,7 +1845,7 @@ static int _am33xx_disable_module(struct omap_hwmod *oh)
>   */
>  static int _ocp_softreset(struct omap_hwmod *oh)
>  {
> -	u32 v, softrst_mask;
> +	u32 v;
>  	int c = 0;
>  	int ret = 0;
>  
> @@ -1835,19 +1875,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
>  	if (oh->class->sysc->srst_udelay)
>  		udelay(oh->class->sysc->srst_udelay);
>  
> -	if (oh->class->sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
> -		omap_test_timeout((omap_hwmod_read(oh,
> -						    oh->class->sysc->syss_offs)
> -				   & SYSS_RESETDONE_MASK),
> -				  MAX_MODULE_SOFTRESET_WAIT, c);
> -	else if (oh->class->sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
> -		softrst_mask = (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> -		omap_test_timeout(!(omap_hwmod_read(oh,
> -						     oh->class->sysc->sysc_offs)
> -				   & softrst_mask),
> -				  MAX_MODULE_SOFTRESET_WAIT, c);
> -	}
> -
> +	c = _wait_softreset_complete(oh);
>  	if (c == MAX_MODULE_SOFTRESET_WAIT)
>  		pr_warning("omap_hwmod: %s: softreset failed (waited %d usec)\n",
>  			   oh->name, MAX_MODULE_SOFTRESET_WAIT);

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Tero Kristo]

Hi Paul,

On Tue, 2012-10-23 at 07:47 +0000, Paul Walmsley wrote:
> Hi Tero,
> 
> On Mon, 22 Oct 2012, Tero Kristo wrote:
> 
> > When waking up from off-mode, some IP blocks are reset automatically by
> > hardware. For this reason, software must wait until the reset has
> > completed before attempting to access the IP block.
> > 
> > This patch fixes for example the bug introduced by commit
> > 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> > to SYSCONFIG register"), in which the MMC IP block is reset during
> > off-mode entry, but the code expects the module to be already available
> > during the execution of context restore.
> > 
> > Signed-off-by: Tero Kristo <t-kristo@ti.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Venkatraman S <svenkatr@ti.com>
> 
> What do you think about these modifications?  The code is quite similar to 
> what was in the _ocp_softreset() function, so just moved it into a 
> function.  Also moved the callsite from the end of _enable_sysc() to the 
> beginning, which makes more sense to me, but would like to get your 
> opinion.

Looks good to me. Only reason I pushed the reset wait to the end of the
function was a minimal potential timing optimization. As the beginning
of the function will write a register, it will consume a bit of time
(especially if in 32k domain), so it can maybe save a single register
read in the delay loop if ordered that way.

Your way is probably safer, it might cause problems in some cases if the
config is written before reset has completed (can it?)

-Tero

> 
> - Paul
> 
> From: Tero Kristo <t-kristo@ti.com>
> Date: Mon, 22 Oct 2012 19:15:32 +0300
> Subject: [PATCH] [RFC] ARM: OMAP: hwmod: wait for sysreset complete after
>  enabling hwmod
> 
> When waking up from off-mode, some IP blocks are reset automatically by
> hardware. For this reason, software must wait until the reset has
> completed before attempting to access the IP block.
> 
> This patch fixes for example the bug introduced by commit
> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
> to SYSCONFIG register"), in which the MMC IP block is reset during
> off-mode entry, but the code expects the module to be already available
> during the execution of context restore.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Venkatraman S <svenkatr@ti.com>
> [paul at pwsan.com: moved softreset wait code into separate function; call
>  from top of _enable_sysc() rather than the bottom]
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |   56 ++++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 504e0e0..0356a09 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -423,6 +423,38 @@ static int _set_softreset(struct omap_hwmod *oh, u32 *v)
>  }
>  
>  /**
> + * _wait_softreset_complete - wait for an OCP softreset to complete
> + * @oh: struct omap_hwmod * to wait on
> + *
> + * Wait until the IP block represented by @oh reports that its OCP
> + * softreset is complete.  This can be triggered by software (see
> + * _ocp_softreset()) or by hardware upon returning from off-mode (one
> + * example is HSMMC).  Waits for up to MAX_MODULE_SOFTRESET_WAIT
> + * microseconds.  Returns the number of microseconds waited.
> + */
> +static int _wait_softreset_complete(struct omap_hwmod *oh)
> +{
> +	struct omap_hwmod_class_sysconfig *sysc;
> +	u32 softrst_mask;
> +	int c = 0;
> +
> +	sysc = oh->class->sysc;
> +
> +	if (sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
> +		omap_test_timeout((omap_hwmod_read(oh, sysc->syss_offs)
> +				   & SYSS_RESETDONE_MASK),
> +				  MAX_MODULE_SOFTRESET_WAIT, c);
> +	else if (sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
> +		softrst_mask = (0x1 << sysc->sysc_fields->srst_shift);
> +		omap_test_timeout(!(omap_hwmod_read(oh, sysc->sysc_offs)
> +				    & softrst_mask),
> +				  MAX_MODULE_SOFTRESET_WAIT, c);
> +	}
> +
> +	return c;
> +}
> +
> +/**
>   * _set_dmadisable: set OCP_SYSCONFIG.DMADISABLE bit in @v
>   * @oh: struct omap_hwmod *
>   *
> @@ -1283,6 +1315,14 @@ static void _enable_sysc(struct omap_hwmod *oh)
>  	if (!oh->class->sysc)
>  		return;
>  
> +	/*
> +	 * Wait until reset has completed, this is needed as the IP
> +	 * block is reset automatically by hardware in some cases
> +	 * (off-mode for example), and the drivers require the
> +	 * IP to be ready when they access it
> +	 */
> +	_wait_softreset_complete(oh);
> +
>  	v = oh->_sysc_cache;
>  	sf = oh->class->sysc->sysc_flags;
>  
> @@ -1805,7 +1845,7 @@ static int _am33xx_disable_module(struct omap_hwmod *oh)
>   */
>  static int _ocp_softreset(struct omap_hwmod *oh)
>  {
> -	u32 v, softrst_mask;
> +	u32 v;
>  	int c = 0;
>  	int ret = 0;
>  
> @@ -1835,19 +1875,7 @@ static int _ocp_softreset(struct omap_hwmod *oh)
>  	if (oh->class->sysc->srst_udelay)
>  		udelay(oh->class->sysc->srst_udelay);
>  
> -	if (oh->class->sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
> -		omap_test_timeout((omap_hwmod_read(oh,
> -						    oh->class->sysc->syss_offs)
> -				   & SYSS_RESETDONE_MASK),
> -				  MAX_MODULE_SOFTRESET_WAIT, c);
> -	else if (oh->class->sysc->sysc_flags & SYSC_HAS_RESET_STATUS) {
> -		softrst_mask = (0x1 << oh->class->sysc->sysc_fields->srst_shift);
> -		omap_test_timeout(!(omap_hwmod_read(oh,
> -						     oh->class->sysc->sysc_offs)
> -				   & softrst_mask),
> -				  MAX_MODULE_SOFTRESET_WAIT, c);
> -	}
> -
> +	c = _wait_softreset_complete(oh);
>  	if (c == MAX_MODULE_SOFTRESET_WAIT)
>  		pr_warning("omap_hwmod: %s: softreset failed (waited %d usec)\n",
>  			   oh->name, MAX_MODULE_SOFTRESET_WAIT);

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Tero,
>
> On Mon, 22 Oct 2012, Tero Kristo wrote:
>
>> When waking up from off-mode, some IP blocks are reset automatically by
>> hardware. For this reason, software must wait until the reset has
>> completed before attempting to access the IP block.
>> 
>> This patch fixes for example the bug introduced by commit
>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>> to SYSCONFIG register"), in which the MMC IP block is reset during
>> off-mode entry, but the code expects the module to be already available
>> during the execution of context restore.
>> 
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>
> What do you think about these modifications?  The code is quite similar to 
> what was in the _ocp_softreset() function, so just moved it into a 
> function.  Also moved the callsite from the end of _enable_sysc() to the 
> beginning, which makes more sense to me, but would like to get your 
> opinion.

FYI, after some more testing with this patch, I noticed that this patch
(and the original from Tero) cause some sluggishness on UART1 console my
37xx/EVM platform as soon as off-mode is enabled (even without the UART
autosuspend timeouts enabled.)  I don't see this on any other OMAP3
platform but all the others I have have UART3 console (in PER), the EVM
is the only one with UART1 console (in CORE.)

I haven't debugged this any furhter, but thought it should be reported
before this gets merged.

Kevin

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Paul Walmsley <paul@pwsan.com> writes:

> Hi Tero,
>
> On Mon, 22 Oct 2012, Tero Kristo wrote:
>
>> When waking up from off-mode, some IP blocks are reset automatically by
>> hardware. For this reason, software must wait until the reset has
>> completed before attempting to access the IP block.
>> 
>> This patch fixes for example the bug introduced by commit
>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>> to SYSCONFIG register"), in which the MMC IP block is reset during
>> off-mode entry, but the code expects the module to be already available
>> during the execution of context restore.
>> 
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>
> What do you think about these modifications?  The code is quite similar to 
> what was in the _ocp_softreset() function, so just moved it into a 
> function.  Also moved the callsite from the end of _enable_sysc() to the 
> beginning, which makes more sense to me, but would like to get your 
> opinion.

FYI, after some more testing with this patch, I noticed that this patch
(and the original from Tero) cause some sluggishness on UART1 console my
37xx/EVM platform as soon as off-mode is enabled (even without the UART
autosuspend timeouts enabled.)  I don't see this on any other OMAP3
platform but all the others I have have UART3 console (in PER), the EVM
is the only one with UART1 console (in CORE.)

I haven't debugged this any furhter, but thought it should be reported
before this gets merged.

Kevin

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Santosh Shilimkar]

On Tuesday 23 October 2012 07:58 PM, Kevin Hilman wrote:
> Paul Walmsley <paul@pwsan.com> writes:
>
>> Hi Tero,
>>
>> On Mon, 22 Oct 2012, Tero Kristo wrote:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>>
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> What do you think about these modifications?  The code is quite similar to
>> what was in the _ocp_softreset() function, so just moved it into a
>> function.  Also moved the callsite from the end of _enable_sysc() to the
>> beginning, which makes more sense to me, but would like to get your
>> opinion.
>
> FYI, after some more testing with this patch, I noticed that this patch
> (and the original from Tero) cause some sluggishness on UART1 console my
> 37xx/EVM platform as soon as off-mode is enabled (even without the UART
> autosuspend timeouts enabled.)  I don't see this on any other OMAP3
> platform but all the others I have have UART3 console (in PER), the EVM
> is the only one with UART1 console (in CORE.)
>
> I haven't debugged this any furhter, but thought it should be reported
> before this gets merged.
>
Trying to shoot in the dark but the UART sluggishness I observed in the
past on OMAP4 was due to improper setting of sysconfig which is
what $subject + Tero's patch is dealing with. Good to check the
value of UART sysconfig before and after the offmode entry to
see if the smart-idle/smart-idle wakeup setting getting disturbed
for some reason.

Below is the OMAP4 commit am referring to.

commit 5ae256dcd91bf308826a4ac19598b27ebb86a536
Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date:   Fri Apr 13 23:25:04 2012 +0530

     ARM: OMAP: serial: Fix the ocp smart idlemode handling bug

Regards,
Santosh

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Santosh Shilimkar]

On Tuesday 23 October 2012 07:58 PM, Kevin Hilman wrote:
> Paul Walmsley <paul@pwsan.com> writes:
>
>> Hi Tero,
>>
>> On Mon, 22 Oct 2012, Tero Kristo wrote:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>>
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> What do you think about these modifications?  The code is quite similar to
>> what was in the _ocp_softreset() function, so just moved it into a
>> function.  Also moved the callsite from the end of _enable_sysc() to the
>> beginning, which makes more sense to me, but would like to get your
>> opinion.
>
> FYI, after some more testing with this patch, I noticed that this patch
> (and the original from Tero) cause some sluggishness on UART1 console my
> 37xx/EVM platform as soon as off-mode is enabled (even without the UART
> autosuspend timeouts enabled.)  I don't see this on any other OMAP3
> platform but all the others I have have UART3 console (in PER), the EVM
> is the only one with UART1 console (in CORE.)
>
> I haven't debugged this any furhter, but thought it should be reported
> before this gets merged.
>
Trying to shoot in the dark but the UART sluggishness I observed in the
past on OMAP4 was due to improper setting of sysconfig which is
what $subject + Tero's patch is dealing with. Good to check the
value of UART sysconfig before and after the offmode entry to
see if the smart-idle/smart-idle wakeup setting getting disturbed
for some reason.

Below is the OMAP4 commit am referring to.

commit 5ae256dcd91bf308826a4ac19598b27ebb86a536
Author: Santosh Shilimkar <santosh.shilimkar@ti.com>
Date:   Fri Apr 13 23:25:04 2012 +0530

     ARM: OMAP: serial: Fix the ocp smart idlemode handling bug

Regards,
Santosh

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Shubhrajyoti Datta]

On Tue, Oct 23, 2012 at 3:16 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>
>> When waking up from off-mode, some IP blocks are reset automatically by
>> hardware. For this reason, software must wait until the reset has
>> completed before attempting to access the IP block.
>>
>> This patch fixes for example the bug introduced by commit
>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>> to SYSCONFIG register"), in which the MMC IP block is reset during
>> off-mode entry, but the code expects the module to be already available
>> during the execution of context restore.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>
> I can confirm that this patch the regression in my OMAP3 PM tests where
> suspend test (to retention or off) failed if ran after the off-idle
> test.
>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM

on 3630/Beagle works.
However on 3430 on idle path didnot work.
let me know if i am missing some thing.

>
> Thanks Tero for the fix,
>
> 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] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Shubhrajyoti Datta]

On Tue, Oct 23, 2012 at 3:16 AM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Tero Kristo <t-kristo@ti.com> writes:
>
>> When waking up from off-mode, some IP blocks are reset automatically by
>> hardware. For this reason, software must wait until the reset has
>> completed before attempting to access the IP block.
>>
>> This patch fixes for example the bug introduced by commit
>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>> to SYSCONFIG register"), in which the MMC IP block is reset during
>> off-mode entry, but the code expects the module to be already available
>> during the execution of context restore.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> Cc: Paul Walmsley <paul@pwsan.com>
>> Cc: Benoit Cousson <b-cousson@ti.com>
>> Cc: Venkatraman S <svenkatr@ti.com>
>
> I can confirm that this patch the regression in my OMAP3 PM tests where
> suspend test (to retention or off) failed if ran after the off-idle
> test.
>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM

on 3630/Beagle works.
However on 3430 on idle path didnot work.
let me know if i am missing some thing.

>
> Thanks Tero for the fix,
>
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Shubhrajyoti Datta <omaplinuxkernel@gmail.com> writes:

> On Tue, Oct 23, 2012 at 3:16 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>>
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> I can confirm that this patch the regression in my OMAP3 PM tests where
>> suspend test (to retention or off) failed if ran after the off-idle
>> test.
>>
>> Tested-by: Kevin Hilman <khilman@ti.com>
>>
>> on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM
>
> on 3630/Beagle works.
> However on 3430 on idle path didnot work.
> let me know if i am missing some thing.

Could you please explain what worked and didn't work on each platforms?

Also, which 3430 did you test on?  If it's SDP, then it has a UART1
console, correct?

Kevin

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Shubhrajyoti Datta <omaplinuxkernel@gmail.com> writes:

> On Tue, Oct 23, 2012 at 3:16 AM, Kevin Hilman
> <khilman@deeprootsystems.com> wrote:
>> Tero Kristo <t-kristo@ti.com> writes:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>>
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>>
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> I can confirm that this patch the regression in my OMAP3 PM tests where
>> suspend test (to retention or off) failed if ran after the off-idle
>> test.
>>
>> Tested-by: Kevin Hilman <khilman@ti.com>
>>
>> on 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM
>
> on 3630/Beagle works.
> However on 3430 on idle path didnot work.
> let me know if i am missing some thing.

Could you please explain what worked and didn't work on each platforms?

Also, which 3430 did you test on?  If it's SDP, then it has a UART1
console, correct?

Kevin

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Kevin Hilman <khilman@deeprootsystems.com> writes:

> Paul Walmsley <paul@pwsan.com> writes:
>
>> Hi Tero,
>>
>> On Mon, 22 Oct 2012, Tero Kristo wrote:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>> 
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>> 
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> What do you think about these modifications?  The code is quite similar to 
>> what was in the _ocp_softreset() function, so just moved it into a 
>> function.  Also moved the callsite from the end of _enable_sysc() to the 
>> beginning, which makes more sense to me, but would like to get your 
>> opinion.
>
> FYI, after some more testing with this patch, I noticed that this patch
> (and the original from Tero) cause some sluggishness on UART1 console my
> 37xx/EVM platform as soon as off-mode is enabled (even without the UART
> autosuspend timeouts enabled.)  I don't see this on any other OMAP3
> platform but all the others I have have UART3 console (in PER), the EVM
> is the only one with UART1 console (in CORE.)

OK, found it.

The reason for the sluggishness is that the GPIO blocks are timing
out in the omap_test_timout() calls added in this patch, suggesting that
they never detect reset done.

Looking at the other user of _wait_softreset_complete made me remember
that there modules that need their optional clocks enabled in order for
softreset to work.  It appears that the optional clocks are needed not
only to initiate a softrest, but also to check SYSS.RESETDONE.

The patch below on top of Paul's version makes the sluggishness disappear.

Kevin




diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index f9d8b2a..70267d2 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1320,7 +1320,11 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	 * (off-mode for example), and the drivers require the
 	 * IP to be ready when they access it
 	 */
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_enable_optional_clocks(oh);
 	_wait_softreset_complete(oh);
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_disable_optional_clocks(oh);
 
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Kevin Hilman]

Kevin Hilman <khilman@deeprootsystems.com> writes:

> Paul Walmsley <paul@pwsan.com> writes:
>
>> Hi Tero,
>>
>> On Mon, 22 Oct 2012, Tero Kristo wrote:
>>
>>> When waking up from off-mode, some IP blocks are reset automatically by
>>> hardware. For this reason, software must wait until the reset has
>>> completed before attempting to access the IP block.
>>> 
>>> This patch fixes for example the bug introduced by commit
>>> 6c31b2150ff96755d24e0ab6d6fea08a7bf5c44c ("mmc: omap_hsmmc: remove access
>>> to SYSCONFIG register"), in which the MMC IP block is reset during
>>> off-mode entry, but the code expects the module to be already available
>>> during the execution of context restore.
>>> 
>>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Venkatraman S <svenkatr@ti.com>
>>
>> What do you think about these modifications?  The code is quite similar to 
>> what was in the _ocp_softreset() function, so just moved it into a 
>> function.  Also moved the callsite from the end of _enable_sysc() to the 
>> beginning, which makes more sense to me, but would like to get your 
>> opinion.
>
> FYI, after some more testing with this patch, I noticed that this patch
> (and the original from Tero) cause some sluggishness on UART1 console my
> 37xx/EVM platform as soon as off-mode is enabled (even without the UART
> autosuspend timeouts enabled.)  I don't see this on any other OMAP3
> platform but all the others I have have UART3 console (in PER), the EVM
> is the only one with UART1 console (in CORE.)

OK, found it.

The reason for the sluggishness is that the GPIO blocks are timing
out in the omap_test_timout() calls added in this patch, suggesting that
they never detect reset done.

Looking at the other user of _wait_softreset_complete made me remember
that there modules that need their optional clocks enabled in order for
softreset to work.  It appears that the optional clocks are needed not
only to initiate a softrest, but also to check SYSS.RESETDONE.

The patch below on top of Paul's version makes the sluggishness disappear.

Kevin




diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index f9d8b2a..70267d2 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1320,7 +1320,11 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	 * (off-mode for example), and the drivers require the
 	 * IP to be ready when they access it
 	 */
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_enable_optional_clocks(oh);
 	_wait_softreset_complete(oh);
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_disable_optional_clocks(oh);
 
 	v = oh->_sysc_cache;
 	sf = oh->class->sysc->sysc_flags;

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Shubhrajyoti Datta]

On Tue, Oct 23, 2012 at 9:39 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Shubhrajyoti Datta <omaplinuxkernel@gmail.com> writes:
[...]
>
> Could you please explain what worked and didn't work on each platforms?
>
> Also, which 3430 did you test on?  If it's SDP, then it has a UART1
> console, correct?
idle mode

set autosuspend for uart
sleep

off count didnt increment on 3430sdp.

hope not hitting any setup at my end.

>
> Kevin

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Shubhrajyoti Datta]

On Tue, Oct 23, 2012 at 9:39 PM, Kevin Hilman
<khilman@deeprootsystems.com> wrote:
> Shubhrajyoti Datta <omaplinuxkernel@gmail.com> writes:
[...]
>
> Could you please explain what worked and didn't work on each platforms?
>
> Also, which 3430 did you test on?  If it's SDP, then it has a UART1
> console, correct?
idle mode

set autosuspend for uart
sleep

off count didnt increment on 3430sdp.

hope not hitting any setup at my end.

>
> Kevin

Re: [RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Paul Walmsley]

Hi Tero,

On Tue, 23 Oct 2012, Tero Kristo wrote:

> Looks good to me. Only reason I pushed the reset wait to the end of the
> function was a minimal potential timing optimization. As the beginning
> of the function will write a register, it will consume a bit of time
> (especially if in 32k domain), so it can maybe save a single register
> read in the delay loop if ordered that way.

Ah, OK.

> Your way is probably safer, it might cause problems in some cases if the
> config is written before reset has completed (can it?)

Not sure, but seems to make sense to wait until the reset is done before 
doing anything to the IP block's registers.  Queuing for 3.7-rc fixes,


- Paul

[RFC] ARM: OMAP: hwmod: wait for sysreset complete after enabling hwmod

[Paul Walmsley]

Hi Tero,

On Tue, 23 Oct 2012, Tero Kristo wrote:

> Looks good to me. Only reason I pushed the reset wait to the end of the
> function was a minimal potential timing optimization. As the beginning
> of the function will write a register, it will consume a bit of time
> (especially if in 32k domain), so it can maybe save a single register
> read in the delay loop if ordered that way.

Ah, OK.

> Your way is probably safer, it might cause problems in some cases if the
> config is written before reset has completed (can it?)

Not sure, but seems to make sense to wait until the reset is done before 
doing anything to the IP block's registers.  Queuing for 3.7-rc fixes,


- Paul