[PATCH 7/8] : Hwmod api changes

[PATCH 7/8] : Hwmod api changes

[Hema HK]

From: Hema HK  <hemahk@ti.com>

Omap USBOTG modules has a requirement to set the auto idle bit only after
setting smart idle bit. Modified the _sys_enable api to set the smart idle 
first and then the autoidle bit. Setting this will not have any impact on the 
other modules.

Added 2 wrapper APIs in the omap device layer for wakeup enable/disable 
and sidle/mstandby settings. 

Signed-off-by: Hema HK <hemahk@ti.com>
Signed-off-by: Basak, Partha <p-basak2@ti.com>

Cc: Felipe Balbi <felipe.balbi@nokia.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Kevin Hilman <khilman@deeprootsystems.com>

---
 arch/arm/mach-omap2/omap_hwmod.c              |   18 +++++++----
 arch/arm/plat-omap/include/plat/omap_device.h |    2 +
 arch/arm/plat-omap/omap_device.c              |   42 ++++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 6 deletions(-)

Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
===================================================================
--- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c	2010-08-06 08:59:03.641863815 -0400
+++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c	2010-08-06 09:02:00.021864999 -0400
@@ -653,12 +653,6 @@
 		_set_master_standbymode(oh, idlemode, &v);
 	}
 
-	if (sf & SYSC_HAS_AUTOIDLE) {
-		idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
-			0 : 1;
-		_set_module_autoidle(oh, idlemode, &v);
-	}
-
 	/* XXX OCP ENAWAKEUP bit? */
 
 	/*
@@ -671,6 +665,18 @@
 		_set_clockactivity(oh, oh->class->sysc->clockact, &v);
 
 	_write_sysconfig(v, oh);
+
+	/* Set the auto idle bit only after setting the smartidle bit
+	 * as this is requirement for some modules like USBOTG
+	 * setting this will not have any impact on the other modues.
+	 */
+
+	if (sf & SYSC_HAS_AUTOIDLE) {
+		idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
+			0 : 1;
+		_set_module_autoidle(oh, idlemode, &v);
+	}
+	_write_sysconfig(v, oh);
 }
 
 /**
Index: linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/include/plat/omap_device.h	2010-08-06 08:59:03.661863725 -0400
+++ linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h	2010-08-06 09:02:00.021864999 -0400
@@ -116,6 +116,8 @@
 int omap_device_disable_clocks(struct omap_device *od);
 int omap_device_enable_clocks(struct omap_device *od);
 
+int omap_device_enable_wakeup(struct omap_device *od);
+int omap_device_disable_wakeup(struct omap_device *od);
 
 /*
  * Entries should be kept in latency order ascending
Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
===================================================================
--- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c	2010-08-06 08:59:03.661863725 -0400
+++ linux-omap-pm/arch/arm/plat-omap/omap_device.c	2010-08-06 09:02:00.021864999 -0400
@@ -757,3 +757,45 @@
 	/* XXX pass along return value here? */
 	return 0;
 }
+
+/**
+ * omap_device_enable_wakeup - Enable the wakeup bit
+ * @od: struct omap_device *od
+ *
+ * Enable the wakup bit for omap_hwmods associated
+ * with the omap_device.  Returns 0.
+ */
+
+int omap_device_enable_wakeup(struct omap_device *od)
+{
+	struct omap_hwmod *oh;
+	int i;
+
+	for (i = 0, oh = *od->hwmods; i < od->hwmods_cnt; i++, oh++)
+		omap_hwmod_enable_wakeup(oh);
+
+	/* XXX pass along return value here? */
+	return 0;
+}
+
+/**
+ * omap_device_disable_wakeup -Disable the wakeup bit
+ * @od: struct omap_device *od
+ *
+ * Disable the wakup bit for omap_hwmods associated
+ * with the omap_device.  Returns 0.
+ */
+
+
+int omap_device_disable_wakeup(struct omap_device *od)
+{
+	struct omap_hwmod *oh;
+	int i;
+
+	for (i = 0, oh = *od->hwmods; i < od->hwmods_cnt; i++, oh++)
+		omap_hwmod_disable_wakeup(oh);
+
+	/* XXX pass along return value here? */
+	return 0;
+}
+

RE: [PATCH 7/8] : Hwmod api changes

[Nayak, Rajendra]

 

> -----Original Message-----
> From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> [mailto:linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of 
> Kalliguddi, Hema
> Sent: Friday, August 06, 2010 10:58 PM
> To: linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Kalliguddi, Hema; Basak, Partha; Felipe Balbi; Tony 
> Lindgren; Kevin Hilman
> Subject: [PATCH 7/8] : Hwmod api changes
> 
> From: Hema HK  <hemahk-l0cyMroinI0@public.gmane.org>
> 
> Omap USBOTG modules has a requirement to set the auto idle 
> bit only after
> setting smart idle bit. Modified the _sys_enable api to set 
> the smart idle 
> first and then the autoidle bit. Setting this will not have 
> any impact on the 
> other modules.
> 
> Added 2 wrapper APIs in the omap device layer for wakeup 
> enable/disable 
> and sidle/mstandby settings. 
> 
> Signed-off-by: Hema HK <hemahk-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Basak, Partha <p-basak2-l0cyMroinI0@public.gmane.org>
> 
> Cc: Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Cc: Kevin Hilman <khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
> 
> ---
>  arch/arm/mach-omap2/omap_hwmod.c              |   18 +++++++----
>  arch/arm/plat-omap/include/plat/omap_device.h |    2 +
>  arch/arm/plat-omap/omap_device.c              |   42 
> ++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+), 6 deletions(-)
> 
> Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
> ===================================================================
> --- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c	
> 2010-08-06 08:59:03.641863815 -0400
> +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c	
> 2010-08-06 09:02:00.021864999 -0400
> @@ -653,12 +653,6 @@
>  		_set_master_standbymode(oh, idlemode, &v);
>  	}
>  
> -	if (sf & SYSC_HAS_AUTOIDLE) {
> -		idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
> -			0 : 1;
> -		_set_module_autoidle(oh, idlemode, &v);
> -	}
> -
>  	/* XXX OCP ENAWAKEUP bit? */
>  
>  	/*
> @@ -671,6 +665,18 @@
>  		_set_clockactivity(oh, oh->class->sysc->clockact, &v);
>  
>  	_write_sysconfig(v, oh);
> +
> +	/* Set the auto idle bit only after setting the smartidle bit
> +	 * as this is requirement for some modules like USBOTG
> +	 * setting this will not have any impact on the other modues.
> +	 */
> +
> +	if (sf & SYSC_HAS_AUTOIDLE) {
> +		idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
> +			0 : 1;
> +		_set_module_autoidle(oh, idlemode, &v);
> +	}
> +	_write_sysconfig(v, oh);
>  }
>  
>  /**
> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
> ===================================================================
> --- 
> linux-omap-pm.orig/arch/arm/plat-omap/include/plat/omap_d
> evice.h	2010-08-06 08:59:03.661863725 -0400
> +++ 
> linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h	
> 2010-08-06 09:02:00.021864999 -0400
> @@ -116,6 +116,8 @@
>  int omap_device_disable_clocks(struct omap_device *od);
>  int omap_device_enable_clocks(struct omap_device *od);
>  
> +int omap_device_enable_wakeup(struct omap_device *od);
> +int omap_device_disable_wakeup(struct omap_device *od);

Kevin,Paul,

Does it make sense for the framework itself to enable wakeup
for all devices when the slave port is programmed to be in
Smartidle, instead of exposing 2 more omap device level api;s
to the drivers?
I have a patch for this and can post it for review in case you
feel it makes sense.

regards,
Rajendra

>  
>  /*
>   * Entries should be kept in latency order ascending
> Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
> ===================================================================
> --- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c	
> 2010-08-06 08:59:03.661863725 -0400
> +++ linux-omap-pm/arch/arm/plat-omap/omap_device.c	
> 2010-08-06 09:02:00.021864999 -0400
> @@ -757,3 +757,45 @@
>  	/* XXX pass along return value here? */
>  	return 0;
>  }
> +
> +/**
> + * omap_device_enable_wakeup - Enable the wakeup bit
> + * @od: struct omap_device *od
> + *
> + * Enable the wakup bit for omap_hwmods associated
> + * with the omap_device.  Returns 0.
> + */
> +
> +int omap_device_enable_wakeup(struct omap_device *od)
> +{
> +	struct omap_hwmod *oh;
> +	int i;
> +
> +	for (i = 0, oh = *od->hwmods; i < od->hwmods_cnt; i++, oh++)
> +		omap_hwmod_enable_wakeup(oh);
> +
> +	/* XXX pass along return value here? */
> +	return 0;
> +}
> +
> +/**
> + * omap_device_disable_wakeup -Disable the wakeup bit
> + * @od: struct omap_device *od
> + *
> + * Disable the wakup bit for omap_hwmods associated
> + * with the omap_device.  Returns 0.
> + */
> +
> +
> +int omap_device_disable_wakeup(struct omap_device *od)
> +{
> +	struct omap_hwmod *oh;
> +	int i;
> +
> +	for (i = 0, oh = *od->hwmods; i < od->hwmods_cnt; i++, oh++)
> +		omap_hwmod_disable_wakeup(oh);
> +
> +	/* XXX pass along return value here? */
> +	return 0;
> +}
> +
> --
> To unsubscribe from this list: send the line "unsubscribe 
> linux-omap" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 7/8] : Hwmod api changes

[Paul Walmsley]

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

(cc Benoît)

On Mon, 9 Aug 2010, Nayak, Rajendra wrote:

> Does it make sense for the framework itself to enable wakeup
> for all devices when the slave port is programmed to be in
> Smartidle

It seems to me that they are separate mechanisms?  If a module is 
programmed for slave smart-idle, then the module prevents the PRCM from 
shutting off the module clock(s) until the module is not busy.  This seems 
distinct from ENAWAKEUP, which I thought simply controlled whether the 
module would assert the SWakeup signal to the PRCM when an external wakeup 
condition occurred for that module.  Is that an accurate summary?  

> instead of exposing 2 more omap device level api;s to the drivers?

Something like this probably needs to be exposed to core code that would 
also set/clear PM_WKEN_* for the appropriate processor module.  Right now 
we just set a bunch of these bits directly in pmXXXX.c, and that needs to 
change.

The other issue is that I suspct the module needs to be enabled in order 
for SYSCONFIG writes to succeed; right now the underlying hwmod code does 
not appear to enforce this :-(

But I don't see why drivers would need to call these functions directly.  
Hema, was that your intention?  If so, you could you please explain the 
use case?

> I have a patch for this and can post it for review in case you
> feel it makes sense.


- Paul

RE: [PATCH 7/8] : Hwmod api changes

[Nayak, Rajendra]

 

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com] 
> Sent: Monday, August 09, 2010 1:51 PM
> To: Nayak, Rajendra
> Cc: Kalliguddi, Hema; linux-usb@vger.kernel.org; 
> linux-omap@vger.kernel.org; Basak, Partha; Felipe Balbi; Tony 
> Lindgren; Cousson, Benoit; Kevin Hilman
> Subject: RE: [PATCH 7/8] : Hwmod api changes
> 
> (cc Benoît)
> 
> On Mon, 9 Aug 2010, Nayak, Rajendra wrote:
> 
> > Does it make sense for the framework itself to enable wakeup
> > for all devices when the slave port is programmed to be in
> > Smartidle
> 
> It seems to me that they are separate mechanisms?  If a module is 
> programmed for slave smart-idle, then the module prevents the 
> PRCM from 
> shutting off the module clock(s) until the module is not 
> busy.  This seems 
> distinct from ENAWAKEUP, which I thought simply controlled 
> whether the 
> module would assert the SWakeup signal to the PRCM when an 
> external wakeup 
> condition occurred for that module.  Is that an accurate summary? 

hmm.. the reason I thought the two were related was because the need to
assert a SWakeup will arise only when the module is programmed in smart-idle.
If the module is either in no-idle (in which case no idle-ack is asserted
by the module) or force-idle (when the module is forced to idle and
expected to stay idle) there might not be a need for the module to be
capable of asserting a SWakeup.
 
The reason I proposed ENAWAKEUP to be always enabled with smart-idle was 
with as understanding that we may never have a case wherein the module
is programmed in smart-idle but not expected to assert SWakeup (if it
supports one). I will check up on this if there ever could be such a
case.

> 
> > instead of exposing 2 more omap device level api;s to the drivers?
> 
> Something like this probably needs to be exposed to core code 
> that would 
> also set/clear PM_WKEN_* for the appropriate processor 
> module.  Right now 
> we just set a bunch of these bits directly in pmXXXX.c, and 
> that needs to 
> change.
> 
> The other issue is that I suspct the module needs to be 
> enabled in order 
> for SYSCONFIG writes to succeed; right now the underlying 
> hwmod code does 
> not appear to enforce this :-(
> 
> But I don't see why drivers would need to call these 
> functions directly.  
> Hema, was that your intention?  If so, you could you please 
> explain the 
> use case?
> 
> > I have a patch for this and can post it for review in case you
> > feel it makes sense.
> 
> 
> - Paul--
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 7/8] : Hwmod api changes

[Cousson, Benoit]

On 8/6/2010 7:28 PM, Kalliguddi, Hema wrote:
> From: Hema HK<hemahk-l0cyMroinI0@public.gmane.org>
>
> Omap USBOTG modules has a requirement to set the auto idle bit only after
> setting smart idle bit.

Is it a requirement or an errata? Could you provide more information 
(i.e. TRM page or errata number / description)?

> Modified the _sys_enable api to set the smart idle
> first and then the autoidle bit. Setting this will not have any impact on the
> other modules.
>
> Added 2 wrapper APIs in the omap device layer for wakeup enable/disable
> and sidle/mstandby settings.
>
> Signed-off-by: Hema HK<hemahk-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Basak, Partha<p-basak2-l0cyMroinI0@public.gmane.org>
>
> Cc: Felipe Balbi<felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>
> Cc: Tony Lindgren<tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Cc: Kevin Hilman<khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org>
>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c              |   18 +++++++----
>   arch/arm/plat-omap/include/plat/omap_device.h |    2 +
>   arch/arm/plat-omap/omap_device.c              |   42 ++++++++++++++++++++++++++
>   3 files changed, 56 insertions(+), 6 deletions(-)
>
> Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
> ===================================================================
> --- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c	2010-08-06 08:59:03.641863815 -0400
> +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c	2010-08-06 09:02:00.021864999 -0400
> @@ -653,12 +653,6 @@
>   		_set_master_standbymode(oh, idlemode,&v);
>   	}
>
> -	if (sf&  SYSC_HAS_AUTOIDLE) {
> -		idlemode = (oh->flags&  HWMOD_NO_OCP_AUTOIDLE) ?
> -			0 : 1;
> -		_set_module_autoidle(oh, idlemode,&v);
> -	}
> -
>   	/* XXX OCP ENAWAKEUP bit? */
>
>   	/*
> @@ -671,6 +665,18 @@
>   		_set_clockactivity(oh, oh->class->sysc->clockact,&v);
>
>   	_write_sysconfig(v, oh);
> +
> +	/* Set the auto idle bit only after setting the smartidle bit
> +	 * as this is requirement for some modules like USBOTG
> +	 * setting this will not have any impact on the other modues.
> +	 */

Except that you are adding an extra access to a quite slow L4 slave 
interface. I'm not sure if write posted will help in that case.

> +
> +	if (sf&  SYSC_HAS_AUTOIDLE) {
> +		idlemode = (oh->flags&  HWMOD_NO_OCP_AUTOIDLE) ?
> +			0 : 1;
> +		_set_module_autoidle(oh, idlemode,&v);
> +	}
> +	_write_sysconfig(v, oh);
>   }
>
>   /**
> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
> ===================================================================
> --- linux-omap-pm.orig/arch/arm/plat-omap/include/plat/omap_device.h	2010-08-06 08:59:03.661863725 -0400
> +++ linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h	2010-08-06 09:02:00.021864999 -0400
> @@ -116,6 +116,8 @@
>   int omap_device_disable_clocks(struct omap_device *od);
>   int omap_device_enable_clocks(struct omap_device *od);
>
> +int omap_device_enable_wakeup(struct omap_device *od);
> +int omap_device_disable_wakeup(struct omap_device *od);
>
>   /*
>    * Entries should be kept in latency order ascending
> Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
> ===================================================================
> --- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c	2010-08-06 08:59:03.661863725 -0400
> +++ linux-omap-pm/arch/arm/plat-omap/omap_device.c	2010-08-06 09:02:00.021864999 -0400
> @@ -757,3 +757,45 @@
>   	/* XXX pass along return value here? */
>   	return 0;
>   }
> +
> +/**
> + * omap_device_enable_wakeup - Enable the wakeup bit
> + * @od: struct omap_device *od
> + *
> + * Enable the wakup bit for omap_hwmods associated
> + * with the omap_device.  Returns 0.
> + */
> +
> +int omap_device_enable_wakeup(struct omap_device *od)

Why do you need that? Could you elaborate?

Benoit

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/8] : Hwmod api changes

[Cousson, Benoit]

On 8/9/2010 11:37 AM, Nayak, Rajendra wrote:
>
>
>> From: Paul Walmsley [mailto:paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org]
>> Sent: Monday, August 09, 2010 1:51 PM
>>
>> (cc Benoît)
>>
>> On Mon, 9 Aug 2010, Nayak, Rajendra wrote:
>>
>>> Does it make sense for the framework itself to enable wakeup
>>> for all devices when the slave port is programmed to be in
>>> Smartidle
>>
>> It seems to me that they are separate mechanisms?  If a module is
>> programmed for slave smart-idle, then the module prevents the
>> PRCM from
>> shutting off the module clock(s) until the module is not
>> busy.  This seems
>> distinct from ENAWAKEUP, which I thought simply controlled
>> whether the
>> module would assert the SWakeup signal to the PRCM when an
>> external wakeup
>> condition occurred for that module.  Is that an accurate summary?
>
> hmm.. the reason I thought the two were related was because the need to
> assert a SWakeup will arise only when the module is programmed in smart-idle.
> If the module is either in no-idle (in which case no idle-ack is asserted
> by the module) or force-idle (when the module is forced to idle and
> expected to stay idle) there might not be a need for the module to be
> capable of asserting a SWakeup.

In fact, the SWakeup is asserted as soon as the module is in idle 
(idle_ack = 1) and cannot use the OCP interface to communicate a IRQ_REQ 
or DMA_REQ.
But the wakeup capability is disabled by setting the SidleMode to 
Force-Idle, regardless of the value of Wakeup enable.
Bottom-line is that the SWakeup can be asserted only if the module is in 
smart-idle.

> The reason I proposed ENAWAKEUP to be always enabled with smart-idle was
> with as understanding that we may never have a case wherein the module
> is programmed in smart-idle but not expected to assert SWakeup (if it
> supports one). I will check up on this if there ever could be such a
> case.

This is something that can make sense on OMAP4, because the wakeup 
status is de-asserted automatically as soon as the interrupt status is 
cleared.
On previous platform, as Paul said, we will have to handle that manually 
somewhere, but preferably not in driver directly.
It will not be that straightforward.

>>
>>> instead of exposing 2 more omap device level api;s to the drivers?
>>
>> Something like this probably needs to be exposed to core code
>> that would
>> also set/clear PM_WKEN_* for the appropriate processor
>> module.  Right now
>> we just set a bunch of these bits directly in pmXXXX.c, and
>> that needs to
>> change.
>>
>> The other issue is that I suspct the module needs to be
>> enabled in order
>> for SYSCONFIG writes to succeed; right now the underlying
>> hwmod code does
>> not appear to enforce this :-(

At least we have a comment saying that we should do it :-)
We probably just need to ensure that a module is enabled before 
accessing the sysconfig.

Regards,
Benoit

>>
>> But I don't see why drivers would need to call these
>> functions directly.
>> Hema, was that your intention?  If so, you could you please
>> explain the
>> use case?
>>
>>> I have a patch for this and can post it for review in case you
>>> feel it makes sense.
>>
>>
>> - Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 7/8] : Hwmod api changes

[Kalliguddi, Hema]

Hi,
 
>-----Original Message-----
>From: Cousson, Benoit 
>Sent: Monday, August 09, 2010 5:42 PM
>To: Kalliguddi, Hema
>Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; 
>Basak, Partha; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH 7/8] : Hwmod api changes
>
>On 8/6/2010 7:28 PM, Kalliguddi, Hema wrote:
>> From: Hema HK<hemahk@ti.com>
>>
>> Omap USBOTG modules has a requirement to set the auto idle 
>bit only after
>> setting smart idle bit.
>
>Is it a requirement or an errata? Could you provide more information 
>(i.e. TRM page or errata number / description)?
>

This is a requirement as per the TRM section 24.1.5.4.4
http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf 

There is note on this.

>> Modified the _sys_enable api to set the smart idle
>> first and then the autoidle bit. Setting this will not have 
>any impact on the
>> other modules.
>>
>> Added 2 wrapper APIs in the omap device layer for wakeup 
>enable/disable
>> and sidle/mstandby settings.
>>
>> Signed-off-by: Hema HK<hemahk@ti.com>
>> Signed-off-by: Basak, Partha<p-basak2@ti.com>
>>
>> Cc: Felipe Balbi<felipe.balbi@nokia.com>
>> Cc: Tony Lindgren<tony@atomide.com>
>> Cc: Kevin Hilman<khilman@deeprootsystems.com>
>>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod.c              |   18 +++++++----
>>   arch/arm/plat-omap/include/plat/omap_device.h |    2 +
>>   arch/arm/plat-omap/omap_device.c              |   42 
>++++++++++++++++++++++++++
>>   3 files changed, 56 insertions(+), 6 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
>> ===================================================================
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c	
>2010-08-06 08:59:03.641863815 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c	
>2010-08-06 09:02:00.021864999 -0400
>> @@ -653,12 +653,6 @@
>>   		_set_master_standbymode(oh, idlemode,&v);
>>   	}
>>
>> -	if (sf&  SYSC_HAS_AUTOIDLE) {
>> -		idlemode = (oh->flags&  HWMOD_NO_OCP_AUTOIDLE) ?
>> -			0 : 1;
>> -		_set_module_autoidle(oh, idlemode,&v);
>> -	}
>> -
>>   	/* XXX OCP ENAWAKEUP bit? */
>>
>>   	/*
>> @@ -671,6 +665,18 @@
>>   		_set_clockactivity(oh, oh->class->sysc->clockact,&v);
>>
>>   	_write_sysconfig(v, oh);
>> +
>> +	/* Set the auto idle bit only after setting the smartidle bit
>> +	 * as this is requirement for some modules like USBOTG
>> +	 * setting this will not have any impact on the other modues.
>> +	 */
>
>Except that you are adding an extra access to a quite slow L4 slave 
>interface. I'm not sure if write posted will help in that case.
>
I agree that it will add an extra access on L4 but this required for USBOTG.
I did not quite understood your Next Comment can elaborate please?

>> +
>> +	if (sf&  SYSC_HAS_AUTOIDLE) {
>> +		idlemode = (oh->flags&  HWMOD_NO_OCP_AUTOIDLE) ?
>> +			0 : 1;
>> +		_set_module_autoidle(oh, idlemode,&v);
>> +	}
>> +	_write_sysconfig(v, oh);
>>   }
>>
>>   /**
>> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
>> ===================================================================
>> --- 
>linux-omap-pm.orig/arch/arm/plat-omap/include/plat/omap_d
>evice.h	2010-08-06 08:59:03.661863725 -0400
>> +++ 
>linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h	
>2010-08-06 09:02:00.021864999 -0400
>> @@ -116,6 +116,8 @@
>>   int omap_device_disable_clocks(struct omap_device *od);
>>   int omap_device_enable_clocks(struct omap_device *od);
>>
>> +int omap_device_enable_wakeup(struct omap_device *od);
>> +int omap_device_disable_wakeup(struct omap_device *od);
>>
>>   /*
>>    * Entries should be kept in latency order ascending
>> Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
>> ===================================================================
>> --- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c	
>2010-08-06 08:59:03.661863725 -0400
>> +++ linux-omap-pm/arch/arm/plat-omap/omap_device.c	
>2010-08-06 09:02:00.021864999 -0400
>> @@ -757,3 +757,45 @@
>>   	/* XXX pass along return value here? */
>>   	return 0;
>>   }
>> +
>> +/**
>> + * omap_device_enable_wakeup - Enable the wakeup bit
>> + * @od: struct omap_device *od
>> + *
>> + * Enable the wakup bit for omap_hwmods associated
>> + * with the omap_device.  Returns 0.
>> + */
>> +
>> +int omap_device_enable_wakeup(struct omap_device *od)
>
>Why do you need that? Could you elaborate?
>
This required o set the wakeup enable bit in the sysconfig register for swakeup generation
To enable the OCP clock When module is in smart-idle-mode and there is a 
asyncronous events like resume signalling.

>Benoit
>
>

Re: [PATCH 7/8] : Hwmod api changes

[Paul Walmsley]

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

Hello Benoît, Rajendra, Hema,

On Mon, 9 Aug 2010, Cousson, Benoit wrote:

> On 8/9/2010 11:37 AM, Nayak, Rajendra wrote:
> > 
> > > From: Paul Walmsley [mailto:paul@pwsan.com]
> > > Sent: Monday, August 09, 2010 1:51 PM
> > > 
> > > On Mon, 9 Aug 2010, Nayak, Rajendra wrote:
> > > 
> > > > Does it make sense for the framework itself to enable wakeup
> > > > for all devices when the slave port is programmed to be in
> > > > Smartidle
> > > 
> > > It seems to me that they are separate mechanisms?  If a module is
> > > programmed for slave smart-idle, then the module prevents the
> > > PRCM from
> > > shutting off the module clock(s) until the module is not
> > > busy.  This seems
> > > distinct from ENAWAKEUP, which I thought simply controlled
> > > whether the
> > > module would assert the SWakeup signal to the PRCM when an
> > > external wakeup
> > > condition occurred for that module.  Is that an accurate summary?
> > 
> > hmm.. the reason I thought the two were related was because the need to
> > assert a SWakeup will arise only when the module is programmed in
> > smart-idle.
> > If the module is either in no-idle (in which case no idle-ack is asserted
> > by the module) or force-idle (when the module is forced to idle and
> > expected to stay idle) there might not be a need for the module to be
> > capable of asserting a SWakeup.
> 
> In fact, the SWakeup is asserted as soon as the module is in idle (idle_ack =
> 1) and cannot use the OCP interface to communicate a IRQ_REQ or DMA_REQ.
> But the wakeup capability is disabled by setting the SidleMode to Force-Idle,
> regardless of the value of Wakeup enable.
> Bottom-line is that the SWakeup can be asserted only if the module is in
> smart-idle.

In that case, it sounds like Rajendra's patch would be useful (to 
automatically enable ENAWAKEUP during target smart-idle, and disable it 
during target no-idle or force-idle, would be worthwhile)?

One other situation that we should consider would be if it ever makes 
sense to program a module to target smart-idle, but disable ENAWAKEUP.  I 
suppose it should only be necessary if there is a hardware bug where an IP 
block generates spurious SWakeups.  -- But if we have no current need for 
this type of workaround, maybe we can just ignore this case and have the 
ENAWAKEUP bit follow target smart-idle status.  Then if such a hardware 
bug appears, we can separate ENAWAKEUP programming later.

Thoughts?


- Paul