[PATCH] OMAP: hwmod: softreset fixes with opt clocks

[PATCH] OMAP: hwmod: softreset fixes with opt clocks

[Benoit Cousson]

Hi Paul,

Here are some fixes for the softreset issues that were highlighted
on some IPs like GPIO or DSS.

For the moment they are still based on your previous series:
git://git.pwsan.com/linux-2.6 hwmod_hardreset_dev

If you are OK with these, I'll rebase that on top of your latest submission to l-o / linux-arm-kernel.

Thanks,
Benoit


Benoit Cousson (2):
  OMAP: hwmod: Fix softreset for modules with optional clocks
  OMAP: hwmod: Fix softreset status check for some new OMAP4 IPs

 arch/arm/mach-omap2/omap_hwmod.c             |   73 ++++++++++++++++++++++----
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    8 +++-
 2 files changed, 70 insertions(+), 11 deletions(-)

[PATCH] OMAP: hwmod: Fix softreset for modules with optional clocks

[Benoit Cousson]

Some modules (like GPIO, DSS...) require optionals clock to be enabled
in order to complete the sofreset properly.
Add a HWMOD_CONTROL_OPT_CLKS_IN_RESET flag to force all optional clocks
to be enabled before reset. Disabled them once the reset is done.

TODO:
For the moment it is very hard to understand from the HW spec, which
optional clock is needed and which one is not. So the current approach
will enable all the optional clocks.
Paul proposed a much finer approach that will allow to tag only the needed
clock in the optional clock table. This might be doable as soon as we have
a clear understanding of these dependencies.

Reported-by: Partha Basak <p-basak2@ti.com>
Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   51 +++++++++++++++++++++++---
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    5 +++
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 8c27923..4309107 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -546,6 +546,36 @@ static int _disable_clocks(struct omap_hwmod *oh)
 	return 0;
 }
 
+static void _enable_optional_clocks(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_opt_clk *oc;
+	int i;
+
+	pr_debug("omap_hwmod: %s: enabling optional clocks\n", oh->name);
+
+	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
+		if (oc->_clk) {
+			pr_warning("omap_hwmod: enable %s:%s\n", oc->role,
+				   oc->_clk->name);
+			clk_enable(oc->_clk);
+		}
+}
+
+static void _disable_optional_clocks(struct omap_hwmod *oh)
+{
+	struct omap_hwmod_opt_clk *oc;
+	int i;
+
+	pr_debug("omap_hwmod: %s: disabling optional clocks\n", oh->name);
+
+	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
+		if (oc->_clk) {
+			pr_warning("omap_hwmod: disable %s:%s\n", oc->role,
+				   oc->_clk->name);
+			clk_disable(oc->_clk);
+		}
+}
+
 /**
  * _find_mpu_port_index - find hwmod OCP slave port ID intended for MPU use
  * @oh: struct omap_hwmod *
@@ -970,8 +1000,9 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
  */
 static int _reset(struct omap_hwmod *oh)
 {
-	u32 r, v;
+	u32 v;
 	int c = 0;
+	int ret = 0;
 
 	if (!oh->class->sysc ||
 	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET) ||
@@ -985,12 +1016,16 @@ static int _reset(struct omap_hwmod *oh)
 		return -EINVAL;
 	}
 
+	/* For some modules, all optionnal clocks need to be enabled as well */
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_enable_optional_clocks(oh);
+
 	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
 
 	v = oh->_sysc_cache;
-	r = _set_softreset(oh, &v);
-	if (r)
-		return r;
+	ret = _set_softreset(oh, &v);
+	if (ret)
+		goto dis_opt_clks;
 	_write_sysconfig(v, oh);
 
 	omap_test_timeout((omap_hwmod_readl(oh, oh->class->sysc->syss_offs) &
@@ -1008,7 +1043,13 @@ static int _reset(struct omap_hwmod *oh)
 	 * _wait_target_ready() or _reset()
 	 */
 
-	return (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
+	ret = (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
+
+dis_opt_clks:
+	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
+		_disable_optional_clocks(oh);
+
+	return ret;
 }
 
 /**
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 7fde44d..878f919 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -368,6 +368,10 @@ struct omap_hwmod_omap4_prcm {
  * HWMOD_SET_DEFAULT_CLOCKACT: program CLOCKACTIVITY bits at startup
  * HWMOD_NO_IDLEST : this module does not have idle status - this is the case
  *     only for few initiator modules on OMAP2 & 3.
+ * HWMOD_CONTROL_OPT_CLKS_IN_RESET: Enable all optional clocks during reset.
+ *     This is needed for devices like DSS that require optional clocks enabled
+ *     in order to complete the reset. Optional clocks will be disabled
+ *     again after the reset.
  */
 #define HWMOD_SWSUP_SIDLE			(1 << 0)
 #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
@@ -376,6 +380,7 @@ struct omap_hwmod_omap4_prcm {
 #define HWMOD_NO_OCP_AUTOIDLE			(1 << 4)
 #define HWMOD_SET_DEFAULT_CLOCKACT		(1 << 5)
 #define HWMOD_NO_IDLEST				(1 << 6)
+#define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1 << 7)
 
 /*
  * omap_hwmod._int_flags definitions
-- 
1.6.0.4

[PATCH] OMAP: hwmod: Fix softreset status check for some new OMAP4 IPs

[Benoit Cousson]

In OMAP3 a specific SYSSTATUS register was used to get the softreset status.
Starting in OMAP4, some IPs does not have SYSSTATUS register and instead
use the SYSC softreset bit to provide the status.

Other cases might exist:
- Some IPs like McBSP does have a softreset control but no reset status.
- Some IPs that represent subsystem, like the DSS, can contains
a reset status without softreset control. The status is the aggregation
of all the sub modules reset status.

- Add a new flag (SYSC_HAS_RESET_STATUS) to identify the new programming model
and replace the previous SYSS_MISSING, that was used to flag IP with
softreset control but without the SYSSTATUS register, with a specific
SYSS_HAS_RESET_STATUS flag.

- MCSPI and MMC contains both programming models, so the legacy one
will be prevented by removing the syss offset field that become useless.

Signed-off-by: Benoit Cousson <b-cousson@ti.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Cc: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   22 +++++++++++++++++-----
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    3 ++-
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 4309107..4819a49 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -997,6 +997,12 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
  * enabled for this to work.  Returns -EINVAL if the hwmod cannot be
  * reset this way or if the hwmod is in the wrong state, -ETIMEDOUT if
  * the module did not reset in time, or 0 upon success.
+ *
+ * In OMAP3 a specific SYSSTATUS register is used to get the reset status.
+ * Starting in OMAP4, some IPs does not have SYSSTATUS register and instead
+ * use the SYSCONFIG softreset bit to provide the status.
+ *
+ * Note that some IP like McBSP does have a reset control but no reset status.
  */
 static int _reset(struct omap_hwmod *oh)
 {
@@ -1005,8 +1011,7 @@ static int _reset(struct omap_hwmod *oh)
 	int ret = 0;
 
 	if (!oh->class->sysc ||
-	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET) ||
-	    (oh->class->sysc->sysc_flags & SYSS_MISSING))
+	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET))
 		return -EINVAL;
 
 	/* clocks must be on for this operation */
@@ -1028,9 +1033,16 @@ static int _reset(struct omap_hwmod *oh)
 		goto dis_opt_clks;
 	_write_sysconfig(v, oh);
 
-	omap_test_timeout((omap_hwmod_readl(oh, oh->class->sysc->syss_offs) &
-			   SYSS_RESETDONE_MASK),
-			  MAX_MODULE_SOFTRESET_WAIT, c);
+	if (oh->class->sysc->sysc_flags & SYSS_HAS_RESET_STATUS)
+		omap_test_timeout((omap_hwmod_readl(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)
+		omap_test_timeout(!(omap_hwmod_readl(oh,
+						     oh->class->sysc->sysc_offs)
+				   & SYSC_TYPE2_SOFTRESET_MASK),
+				  MAX_MODULE_SOFTRESET_WAIT, c);
 
 	if (c == MAX_MODULE_SOFTRESET_WAIT)
 		pr_warning("omap_hwmod: %s: softreset failed (waited %d usec)\n",
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index 878f919..ee53758 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -251,8 +251,9 @@ struct omap_hwmod_ocp_if {
 #define SYSC_HAS_CLOCKACTIVITY	(1 << 4)
 #define SYSC_HAS_SIDLEMODE	(1 << 5)
 #define SYSC_HAS_MIDLEMODE	(1 << 6)
-#define SYSS_MISSING		(1 << 7)
+#define SYSS_HAS_RESET_STATUS	(1 << 7)
 #define SYSC_NO_CACHE		(1 << 8)  /* XXX SW flag, belongs elsewhere */
+#define SYSC_HAS_RESET_STATUS	(1 << 9)
 
 /* omap_hwmod_sysconfig.clockact flags */
 #define CLOCKACT_TEST_BOTH	0x0
-- 
1.6.0.4

Re: [PATCH] OMAP: hwmod: softreset fixes with opt clocks

[Cousson, Benoit]

On 9/21/2010 6:57 PM, Cousson, Benoit wrote:
> Hi Paul,
>
> Here are some fixes for the softreset issues that were highlighted
> on some IPs like GPIO or DSS.
>
> For the moment they are still based on your previous series:
> git://git.pwsan.com/linux-2.6 hwmod_hardreset_dev
>
> If you are OK with these, I'll rebase that on top of your latest submission to l-o / linux-arm-kernel.
>
> Thanks,
> Benoit
>
>
> Benoit Cousson (2):
>    OMAP: hwmod: Fix softreset for modules with optional clocks
>    OMAP: hwmod: Fix softreset status check for some new OMAP4 IPs
>
>   arch/arm/mach-omap2/omap_hwmod.c             |   73 ++++++++++++++++++++++----
>   arch/arm/plat-omap/include/plat/omap_hwmod.h |    8 +++-
>   2 files changed, 70 insertions(+), 11 deletions(-)
>

Oops, I forgot... The patches are available here:

git://gitorious.org/omap-pm/linux.git pm-wip/hwmods-reset-fixes

Regards,
Benoit

Re: [PATCH] OMAP: hwmod: softreset fixes with opt clocks

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 823 bytes --]

Hi Benoît,

On Tue, 21 Sep 2010, Benoit Cousson wrote:

> Here are some fixes for the softreset issues that were highlighted
> on some IPs like GPIO or DSS.
> 
> For the moment they are still based on your previous series:
> git://git.pwsan.com/linux-2.6 hwmod_hardreset_dev
> 
> If you are OK with these, I'll rebase that on top of your latest submission to l-o / linux-arm-kernel.

Just took a quick look at them; they both look fine to me.  I do have one 
minor comment on the optional clock patch; I will post it as a reply to 
that message.

That hwmod_hardreset_dev branch is almost identical to the hwmod_2.6.37 
branch.  Maybe I should just add your patches to the hwmod_2.6.37 branch, 
along with Rajendra's ENAWAKEUP patch, and post those three to lakml, if 
you are okay with that?


- Paul

Re: [PATCH] OMAP: hwmod: Fix softreset for modules with optional clocks

[Paul Walmsley]

Hi Benoit,

one minor comment here -

On Tue, 21 Sep 2010, Benoit Cousson wrote:

> Some modules (like GPIO, DSS...) require optionals clock to be enabled
> in order to complete the sofreset properly.
> Add a HWMOD_CONTROL_OPT_CLKS_IN_RESET flag to force all optional clocks
> to be enabled before reset. Disabled them once the reset is done.
> 
> TODO:
> For the moment it is very hard to understand from the HW spec, which
> optional clock is needed and which one is not. So the current approach
> will enable all the optional clocks.
> Paul proposed a much finer approach that will allow to tag only the needed
> clock in the optional clock table. This might be doable as soon as we have
> a clear understanding of these dependencies.
> 
> Reported-by: Partha Basak <p-basak2@ti.com>
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c             |   51 +++++++++++++++++++++++---
>  arch/arm/plat-omap/include/plat/omap_hwmod.h |    5 +++
>  2 files changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 8c27923..4309107 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -546,6 +546,36 @@ static int _disable_clocks(struct omap_hwmod *oh)
>  	return 0;
>  }
>  
> +static void _enable_optional_clocks(struct omap_hwmod *oh)
> +{
> +	struct omap_hwmod_opt_clk *oc;
> +	int i;
> +
> +	pr_debug("omap_hwmod: %s: enabling optional clocks\n", oh->name);
> +
> +	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
> +		if (oc->_clk) {
> +			pr_warning("omap_hwmod: enable %s:%s\n", oc->role,
> +				   oc->_clk->name);

What do you think about maybe converting this to a pr_debug() (and the 
same in the disable code below)?  If you are happy with that, I can make 
the change here when I pull the patch in.

> +			clk_enable(oc->_clk);
> +		}
> +}
> +
> +static void _disable_optional_clocks(struct omap_hwmod *oh)
> +{
> +	struct omap_hwmod_opt_clk *oc;
> +	int i;
> +
> +	pr_debug("omap_hwmod: %s: disabling optional clocks\n", oh->name);
> +
> +	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
> +		if (oc->_clk) {
> +			pr_warning("omap_hwmod: disable %s:%s\n", oc->role,
> +				   oc->_clk->name);
> +			clk_disable(oc->_clk);
> +		}
> +}
> +
>  /**
>   * _find_mpu_port_index - find hwmod OCP slave port ID intended for MPU use
>   * @oh: struct omap_hwmod *
> @@ -970,8 +1000,9 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
>   */
>  static int _reset(struct omap_hwmod *oh)
>  {
> -	u32 r, v;
> +	u32 v;
>  	int c = 0;
> +	int ret = 0;
>  
>  	if (!oh->class->sysc ||
>  	    !(oh->class->sysc->sysc_flags & SYSC_HAS_SOFTRESET) ||
> @@ -985,12 +1016,16 @@ static int _reset(struct omap_hwmod *oh)
>  		return -EINVAL;
>  	}
>  
> +	/* For some modules, all optionnal clocks need to be enabled as well */
> +	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
> +		_enable_optional_clocks(oh);
> +
>  	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>  
>  	v = oh->_sysc_cache;
> -	r = _set_softreset(oh, &v);
> -	if (r)
> -		return r;
> +	ret = _set_softreset(oh, &v);
> +	if (ret)
> +		goto dis_opt_clks;
>  	_write_sysconfig(v, oh);
>  
>  	omap_test_timeout((omap_hwmod_readl(oh, oh->class->sysc->syss_offs) &
> @@ -1008,7 +1043,13 @@ static int _reset(struct omap_hwmod *oh)
>  	 * _wait_target_ready() or _reset()
>  	 */
>  
> -	return (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
> +	ret = (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
> +
> +dis_opt_clks:
> +	if (oh->flags & HWMOD_CONTROL_OPT_CLKS_IN_RESET)
> +		_disable_optional_clocks(oh);
> +
> +	return ret;
>  }
>  
>  /**
> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> index 7fde44d..878f919 100644
> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
> @@ -368,6 +368,10 @@ struct omap_hwmod_omap4_prcm {
>   * HWMOD_SET_DEFAULT_CLOCKACT: program CLOCKACTIVITY bits at startup
>   * HWMOD_NO_IDLEST : this module does not have idle status - this is the case
>   *     only for few initiator modules on OMAP2 & 3.
> + * HWMOD_CONTROL_OPT_CLKS_IN_RESET: Enable all optional clocks during reset.
> + *     This is needed for devices like DSS that require optional clocks enabled
> + *     in order to complete the reset. Optional clocks will be disabled
> + *     again after the reset.
>   */
>  #define HWMOD_SWSUP_SIDLE			(1 << 0)
>  #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
> @@ -376,6 +380,7 @@ struct omap_hwmod_omap4_prcm {
>  #define HWMOD_NO_OCP_AUTOIDLE			(1 << 4)
>  #define HWMOD_SET_DEFAULT_CLOCKACT		(1 << 5)
>  #define HWMOD_NO_IDLEST				(1 << 6)
> +#define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1 << 7)
>  
>  /*
>   * omap_hwmod._int_flags definitions
> -- 
> 1.6.0.4
> 


- Paul

Re: [PATCH] OMAP: hwmod: Fix softreset for modules with optional clocks

[Cousson, Benoit]

On 9/21/2010 7:07 PM, Paul Walmsley wrote:
> Hi Benoit,
>
> one minor comment here -
>
> On Tue, 21 Sep 2010, Benoit Cousson wrote:
>
>> Some modules (like GPIO, DSS...) require optionals clock to be enabled
>> in order to complete the sofreset properly.
>> Add a HWMOD_CONTROL_OPT_CLKS_IN_RESET flag to force all optional clocks
>> to be enabled before reset. Disabled them once the reset is done.
>>
>> TODO:
>> For the moment it is very hard to understand from the HW spec, which
>> optional clock is needed and which one is not. So the current approach
>> will enable all the optional clocks.
>> Paul proposed a much finer approach that will allow to tag only the needed
>> clock in the optional clock table. This might be doable as soon as we have
>> a clear understanding of these dependencies.
>>
>> Reported-by: Partha Basak<p-basak2@ti.com>
>> Signed-off-by: Benoit Cousson<b-cousson@ti.com>
>> Cc: Paul Walmsley<paul@pwsan.com>
>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod.c             |   51 +++++++++++++++++++++++---
>>   arch/arm/plat-omap/include/plat/omap_hwmod.h |    5 +++
>>   2 files changed, 51 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 8c27923..4309107 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -546,6 +546,36 @@ static int _disable_clocks(struct omap_hwmod *oh)
>>   	return 0;
>>   }
>>
>> +static void _enable_optional_clocks(struct omap_hwmod *oh)
>> +{
>> +	struct omap_hwmod_opt_clk *oc;
>> +	int i;
>> +
>> +	pr_debug("omap_hwmod: %s: enabling optional clocks\n", oh->name);
>> +
>> +	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i>  0; i--, oc++)
>> +		if (oc->_clk) {
>> +			pr_warning("omap_hwmod: enable %s:%s\n", oc->role,
>> +				   oc->_clk->name);
>
> What do you think about maybe converting this to a pr_debug() (and the
> same in the disable code below)?  If you are happy with that, I can make
> the change here when I pull the patch in.

Oops... yes, sure, that was the intent... I've just cleaned the other 
ones, but missed these two.
Thanks for catching that.

Regards,
Benoit

>
>> +			clk_enable(oc->_clk);
>> +		}
>> +}
>> +
>> +static void _disable_optional_clocks(struct omap_hwmod *oh)
>> +{
>> +	struct omap_hwmod_opt_clk *oc;
>> +	int i;
>> +
>> +	pr_debug("omap_hwmod: %s: disabling optional clocks\n", oh->name);
>> +
>> +	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i>  0; i--, oc++)
>> +		if (oc->_clk) {
>> +			pr_warning("omap_hwmod: disable %s:%s\n", oc->role,
>> +				   oc->_clk->name);
>> +			clk_disable(oc->_clk);
>> +		}
>> +}
>> +
>>   /**
>>    * _find_mpu_port_index - find hwmod OCP slave port ID intended for MPU use
>>    * @oh: struct omap_hwmod *
>> @@ -970,8 +1000,9 @@ static int _read_hardreset(struct omap_hwmod *oh, const char *name)
>>    */
>>   static int _reset(struct omap_hwmod *oh)
>>   {
>> -	u32 r, v;
>> +	u32 v;
>>   	int c = 0;
>> +	int ret = 0;
>>
>>   	if (!oh->class->sysc ||
>>   	    !(oh->class->sysc->sysc_flags&  SYSC_HAS_SOFTRESET) ||
>> @@ -985,12 +1016,16 @@ static int _reset(struct omap_hwmod *oh)
>>   		return -EINVAL;
>>   	}
>>
>> +	/* For some modules, all optionnal clocks need to be enabled as well */
>> +	if (oh->flags&  HWMOD_CONTROL_OPT_CLKS_IN_RESET)
>> +		_enable_optional_clocks(oh);
>> +
>>   	pr_debug("omap_hwmod: %s: resetting\n", oh->name);
>>
>>   	v = oh->_sysc_cache;
>> -	r = _set_softreset(oh,&v);
>> -	if (r)
>> -		return r;
>> +	ret = _set_softreset(oh,&v);
>> +	if (ret)
>> +		goto dis_opt_clks;
>>   	_write_sysconfig(v, oh);
>>
>>   	omap_test_timeout((omap_hwmod_readl(oh, oh->class->sysc->syss_offs)&
>> @@ -1008,7 +1043,13 @@ static int _reset(struct omap_hwmod *oh)
>>   	 * _wait_target_ready() or _reset()
>>   	 */
>>
>> -	return (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
>> +	ret = (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
>> +
>> +dis_opt_clks:
>> +	if (oh->flags&  HWMOD_CONTROL_OPT_CLKS_IN_RESET)
>> +		_disable_optional_clocks(oh);
>> +
>> +	return ret;
>>   }
>>
>>   /**
>> diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> index 7fde44d..878f919 100644
>> --- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> +++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
>> @@ -368,6 +368,10 @@ struct omap_hwmod_omap4_prcm {
>>    * HWMOD_SET_DEFAULT_CLOCKACT: program CLOCKACTIVITY bits at startup
>>    * HWMOD_NO_IDLEST : this module does not have idle status - this is the case
>>    *     only for few initiator modules on OMAP2&  3.
>> + * HWMOD_CONTROL_OPT_CLKS_IN_RESET: Enable all optional clocks during reset.
>> + *     This is needed for devices like DSS that require optional clocks enabled
>> + *     in order to complete the reset. Optional clocks will be disabled
>> + *     again after the reset.
>>    */
>>   #define HWMOD_SWSUP_SIDLE			(1<<  0)
>>   #define HWMOD_SWSUP_MSTANDBY			(1<<  1)
>> @@ -376,6 +380,7 @@ struct omap_hwmod_omap4_prcm {
>>   #define HWMOD_NO_OCP_AUTOIDLE			(1<<  4)
>>   #define HWMOD_SET_DEFAULT_CLOCKACT		(1<<  5)
>>   #define HWMOD_NO_IDLEST				(1<<  6)
>> +#define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1<<  7)
>>
>>   /*
>>    * omap_hwmod._int_flags definitions
>> --
>> 1.6.0.4
>>
>
>
> - Paul

Re: [PATCH] OMAP: hwmod: softreset fixes with opt clocks

[Cousson, Benoit]

On 9/21/2010 7:05 PM, Paul Walmsley wrote:
> Hi Benoît,
>
> On Tue, 21 Sep 2010, Benoit Cousson wrote:
>
>> Here are some fixes for the softreset issues that were highlighted
>> on some IPs like GPIO or DSS.
>>
>> For the moment they are still based on your previous series:
>> git://git.pwsan.com/linux-2.6 hwmod_hardreset_dev
>>
>> If you are OK with these, I'll rebase that on top of your latest submission to l-o / linux-arm-kernel.
>
> Just took a quick look at them; they both look fine to me.  I do have one
> minor comment on the optional clock patch; I will post it as a reply to
> that message.
>
> That hwmod_hardreset_dev branch is almost identical to the hwmod_2.6.37
> branch.  Maybe I should just add your patches to the hwmod_2.6.37 branch,
> along with Rajendra's ENAWAKEUP patch, and post those three to lakml, if
> you are okay with that?

OK, I'm fine with that.

I think I should as well re-send the one from Partha:  OMAP: hwmod: 
Handle opt clocks node using clk_add_alias.
That one is fine for me, but I didn't see much comment on it.
The GPIO series has a dependency on it as well.

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

Re: [PATCH] OMAP: hwmod: Fix softreset status check for some new OMAP4 IPs

[Paul Walmsley]

On Tue, 21 Sep 2010, Benoit Cousson wrote:

> In OMAP3 a specific SYSSTATUS register was used to get the softreset status.
> Starting in OMAP4, some IPs does not have SYSSTATUS register and instead
> use the SYSC softreset bit to provide the status.
> 
> Other cases might exist:
> - Some IPs like McBSP does have a softreset control but no reset status.
> - Some IPs that represent subsystem, like the DSS, can contains
> a reset status without softreset control. The status is the aggregation
> of all the sub modules reset status.
> 
> - Add a new flag (SYSC_HAS_RESET_STATUS) to identify the new programming model
> and replace the previous SYSS_MISSING, that was used to flag IP with
> softreset control but without the SYSSTATUS register, with a specific
> SYSS_HAS_RESET_STATUS flag.
> 
> - MCSPI and MMC contains both programming models, so the legacy one
> will be prevented by removing the syss offset field that become useless.

Thanks, queued this for 2.6.37.


- Paul

Re: [PATCH] OMAP: hwmod: Fix softreset for modules with optional clocks

[Paul Walmsley]

On Tue, 21 Sep 2010, Benoit Cousson wrote:

> Some modules (like GPIO, DSS...) require optionals clock to be enabled
> in order to complete the sofreset properly.
> Add a HWMOD_CONTROL_OPT_CLKS_IN_RESET flag to force all optional clocks
> to be enabled before reset. Disabled them once the reset is done.
> 
> TODO:
> For the moment it is very hard to understand from the HW spec, which
> optional clock is needed and which one is not. So the current approach
> will enable all the optional clocks.
> Paul proposed a much finer approach that will allow to tag only the needed
> clock in the optional clock table. This might be doable as soon as we have
> a clear understanding of these dependencies.
> 
> Reported-by: Partha Basak <p-basak2@ti.com>
> Signed-off-by: Benoit Cousson <b-cousson@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@deeprootsystems.com>

Thanks, queued for 2.6.37 with the pr_debug() change.


- Paul