[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Santosh Shilimkar]

OMAP UART IP needs manual idle modes based on functional state of the
IP. Currently this is handled by the driver with function pointers
implemented in platform code.

This however breaks in case of device tree because of missing
idle handling. 

The series tries to address the issue and tries to remove complete
sysc handling from serial driver.

Patches has been tested on OMAP4430 SDP and OMAP5 EVM where the console
sluggishness was observed without idle mode handling. CPUIDLE and
suspend tested ok on these devices.

Thanks to Vaibhav Bedia for testing on Beaglebone(AM33XX) with suspend and
also the UART async wakeup from suspend.

Thanks to Sourav Poddar for Beagle XM(OMAP3630) devices testing with
retention and offmode in Idle and suspend for non-DT builds.

Serial driver platform code still has one more function pointer
left which is used to trigger io_ring(). This one needs some
discussion with pincontrol driver folks on how to implement
such a feature with generic pincontrol driver.

Rajendra Nayak (4):
  ARM: OMAP2+: hwmod: Remove unused _HWMOD_WAKEUP_ENABLED flag
  ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming
  ARM: OMAP2+: hwmod: Always have OCP_SYSCONFIG.ENAWAKEUP enabled
  ARM: OMAP2+: hwmod: Add a new flag to handle SIDLE in SWSUP only in
    active

Santosh Shilimkar (4):
  ARM: OMAP2+: hwmod-data: UART IP needs software control to manage
    sidle modes
  SERIAL: OMAP: Remove the slave idle handling from the driver
  ARM: OMAP2+: serial: Remove the un-used slave idle hooks
  ARM: OMAP2+: hwmod: Remove sysc slave idle and auto idle apis

 arch/arm/mach-omap2/omap_hwmod.c                   |  141 ++++----------------
 arch/arm/mach-omap2/omap_hwmod.h                   |   13 +-
 arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    3 +
 arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |    6 +
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    4 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 +-
 arch/arm/mach-omap2/serial.c                       |   31 -----
 drivers/tty/serial/omap-serial.c                   |   23 ----
 8 files changed, 50 insertions(+), 177 deletions(-)

-- 
1.7.9.5

[PATCH 1/8] ARM: OMAP2+: hwmod: Remove unused _HWMOD_WAKEUP_ENABLED flag

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

_HWMOD_WAKEUP_ENABLED is currently unused across the hwmod
framework. Just get rid of it, so we have one less flag to
worry about.

Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    4 ----
 arch/arm/mach-omap2/omap_hwmod.h |    6 ++----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 4653efb..b45c8fb 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -608,8 +608,6 @@ static int _enable_wakeup(struct omap_hwmod *oh, u32 *v)
 
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 
-	oh->_int_flags |= _HWMOD_WAKEUP_ENABLED;
-
 	return 0;
 }
 
@@ -643,8 +641,6 @@ static int _disable_wakeup(struct omap_hwmod *oh, u32 *v)
 
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 
-	oh->_int_flags &= ~_HWMOD_WAKEUP_ENABLED;
-
 	return 0;
 }
 
diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
index 3ae852a..23f530b 100644
--- a/arch/arm/mach-omap2/omap_hwmod.h
+++ b/arch/arm/mach-omap2/omap_hwmod.h
@@ -468,15 +468,13 @@ struct omap_hwmod_omap4_prcm {
  * These are for internal use only and are managed by the omap_hwmod code.
  *
  * _HWMOD_NO_MPU_PORT: no path exists for the MPU to write to this module
- * _HWMOD_WAKEUP_ENABLED: set when the omap_hwmod code has enabled ENAWAKEUP
  * _HWMOD_SYSCONFIG_LOADED: set when the OCP_SYSCONFIG value has been cached
  * _HWMOD_SKIP_ENABLE: set if hwmod enabled during init (HWMOD_INIT_NO_IDLE) -
  *     causes the first call to _enable() to only update the pinmux
  */
 #define _HWMOD_NO_MPU_PORT			(1 << 0)
-#define _HWMOD_WAKEUP_ENABLED			(1 << 1)
-#define _HWMOD_SYSCONFIG_LOADED			(1 << 2)
-#define _HWMOD_SKIP_ENABLE			(1 << 3)
+#define _HWMOD_SYSCONFIG_LOADED			(1 << 1)
+#define _HWMOD_SKIP_ENABLE			(1 << 2)
 
 /*
  * omap_hwmod._state definitions
-- 
1.7.9.5

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

_enable_wakeup() and _disable_wakeup() are expected to program the
OCP_SYSCONFIG.ENAWAKEUP bit. Get rid of the additional sidle/mstandby
programming in them, as its confusing (this is expected to be handled
elsewhere as part of _enable_sysc()/__idle_sysc()) and unnecessary.

Well, the _enable_sysc()/_idle_sysc() handled only the mstandby modes
correctly. So fix them so they also handle the midle modes correctly
and program HWMOD_IDLEMODE_SMART_WKUP and HWMOD_IDLEMODE_SMART
respectively.

Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
Tested-by: Sourav Poddar <sourav.poddar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   48 ++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index b45c8fb..fdd55cd 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -588,9 +588,7 @@ static void _set_idle_ioring_wakeup(struct omap_hwmod *oh, bool set_wake)
 static int _enable_wakeup(struct omap_hwmod *oh, u32 *v)
 {
 	if (!oh->class->sysc ||
-	    !((oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP) ||
-	      (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP) ||
-	      (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)))
+	    !(oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP))
 		return -EINVAL;
 
 	if (!oh->class->sysc->sysc_fields) {
@@ -601,11 +599,6 @@ static int _enable_wakeup(struct omap_hwmod *oh, u32 *v)
 	if (oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP)
 		*v |= 0x1 << oh->class->sysc->sysc_fields->enwkup_shift;
 
-	if (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP)
-		_set_slave_idlemode(oh, HWMOD_IDLEMODE_SMART_WKUP, v);
-	if (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)
-		_set_master_standbymode(oh, HWMOD_IDLEMODE_SMART_WKUP, v);
-
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 
 	return 0;
@@ -621,9 +614,7 @@ static int _enable_wakeup(struct omap_hwmod *oh, u32 *v)
 static int _disable_wakeup(struct omap_hwmod *oh, u32 *v)
 {
 	if (!oh->class->sysc ||
-	    !((oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP) ||
-	      (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP) ||
-	      (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)))
+	    !(oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP))
 		return -EINVAL;
 
 	if (!oh->class->sysc->sysc_fields) {
@@ -634,11 +625,6 @@ static int _disable_wakeup(struct omap_hwmod *oh, u32 *v)
 	if (oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP)
 		*v &= ~(0x1 << oh->class->sysc->sysc_fields->enwkup_shift);
 
-	if (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP)
-		_set_slave_idlemode(oh, HWMOD_IDLEMODE_SMART, v);
-	if (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)
-		_set_master_standbymode(oh, HWMOD_IDLEMODE_SMART, v);
-
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 
 	return 0;
@@ -1351,13 +1337,24 @@ static void _enable_sysc(struct omap_hwmod *oh)
 
 	clkdm = _get_clkdm(oh);
 	if (sf & SYSC_HAS_SIDLEMODE) {
+		if (oh->flags & HWMOD_SWSUP_SIDLE) {
+			idlemode = HWMOD_IDLEMODE_NO;
+		} else {
+			if (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP)
+				idlemode = HWMOD_IDLEMODE_SMART_WKUP;
+			else
+				idlemode = HWMOD_IDLEMODE_SMART;
+		}
+
+		/*
+		 * This is special handling for some IPs like
+		 * 32k sync timer. Force them to idle!
+		 */
 		clkdm_act = (clkdm && clkdm->flags & CLKDM_ACTIVE_WITH_MPU);
 		if (clkdm_act && !(oh->class->sysc->idlemodes &
 				   (SIDLE_SMART | SIDLE_SMART_WKUP)))
 			idlemode = HWMOD_IDLEMODE_FORCE;
-		else
-			idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
-				HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
+
 		_set_slave_idlemode(oh, idlemode, &v);
 	}
 
@@ -1423,13 +1420,14 @@ static void _idle_sysc(struct omap_hwmod *oh)
 	sf = oh->class->sysc->sysc_flags;
 
 	if (sf & SYSC_HAS_SIDLEMODE) {
-		/* XXX What about HWMOD_IDLEMODE_SMART_WKUP? */
-		if (oh->flags & HWMOD_SWSUP_SIDLE ||
-		    !(oh->class->sysc->idlemodes &
-		      (SIDLE_SMART | SIDLE_SMART_WKUP)))
+		if (oh->flags & HWMOD_SWSUP_SIDLE) {
 			idlemode = HWMOD_IDLEMODE_FORCE;
-		else
-			idlemode = HWMOD_IDLEMODE_SMART;
+		} else {
+			if (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP)
+				idlemode = HWMOD_IDLEMODE_SMART_WKUP;
+			else
+				idlemode = HWMOD_IDLEMODE_SMART;
+		}
 		_set_slave_idlemode(oh, idlemode, &v);
 	}
 
-- 
1.7.9.5

[PATCH 3/8] ARM: OMAP2+: hwmod: Always have OCP_SYSCONFIG.ENAWAKEUP enabled

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

Get rid of all complexities around when to enable OCP_SYSCONFIG.ENAWAKEUP.
It should be safe to have this set *always* for all IP blocks which
have this control. It should be a *dont care* when the IP is in
NO/FORCE modes of sidle/mstandby. As for the SMART/SMART_WKUP modes
of sidle/mstandby the framework anyway sets this always.

Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
Tested-by: Sourav Poddar <sourav.poddar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index fdd55cd..932d5e6 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1362,8 +1362,6 @@ static void _enable_sysc(struct omap_hwmod *oh)
 		if (oh->flags & HWMOD_SWSUP_MSTANDBY) {
 			idlemode = HWMOD_IDLEMODE_NO;
 		} else {
-			if (sf & SYSC_HAS_ENAWAKEUP)
-				_enable_wakeup(oh, &v);
 			if (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)
 				idlemode = HWMOD_IDLEMODE_SMART_WKUP;
 			else
@@ -1381,9 +1379,7 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	    (sf & SYSC_HAS_CLOCKACTIVITY))
 		_set_clockactivity(oh, oh->class->sysc->clockact, &v);
 
-	/* If slave is in SMARTIDLE, also enable wakeup */
-	if ((sf & SYSC_HAS_SIDLEMODE) && !(oh->flags & HWMOD_SWSUP_SIDLE))
-		_enable_wakeup(oh, &v);
+	_enable_wakeup(oh, &v);
 
 	_write_sysconfig(v, oh);
 
@@ -1435,8 +1431,6 @@ static void _idle_sysc(struct omap_hwmod *oh)
 		if (oh->flags & HWMOD_SWSUP_MSTANDBY) {
 			idlemode = HWMOD_IDLEMODE_FORCE;
 		} else {
-			if (sf & SYSC_HAS_ENAWAKEUP)
-				_enable_wakeup(oh, &v);
 			if (oh->class->sysc->idlemodes & MSTANDBY_SMART_WKUP)
 				idlemode = HWMOD_IDLEMODE_SMART_WKUP;
 			else
@@ -1445,9 +1439,7 @@ static void _idle_sysc(struct omap_hwmod *oh)
 		_set_master_standbymode(oh, idlemode, &v);
 	}
 
-	/* If slave is in SMARTIDLE, also enable wakeup */
-	if ((sf & SYSC_HAS_SIDLEMODE) && !(oh->flags & HWMOD_SWSUP_SIDLE))
-		_enable_wakeup(oh, &v);
+	_enable_wakeup(oh, &v);
 
 	_write_sysconfig(v, oh);
 }
@@ -3767,14 +3759,6 @@ int omap_hwmod_enable_wakeup(struct omap_hwmod *oh)
 	u32 v;
 
 	spin_lock_irqsave(&oh->_lock, flags);
-
-	if (oh->class->sysc &&
-	    (oh->class->sysc->sysc_flags & SYSC_HAS_ENAWAKEUP)) {
-		v = oh->_sysc_cache;
-		_enable_wakeup(oh, &v);
-		_write_sysconfig(v, oh);
-	}
-
 	_set_idle_ioring_wakeup(oh, true);
 	spin_unlock_irqrestore(&oh->_lock, flags);
 
-- 
1.7.9.5

[PATCH 4/8] ARM: OMAP2+: hwmod: Add a new flag to handle SIDLE in SWSUP only in active

[Santosh Shilimkar]

From: Rajendra Nayak <rnayak@ti.com>

Some IPs (like UART) need the sidle mode to be controlled in SW only
while they are active. Once they go inactive, they need the IP to be
put back in HW control so they are also wakeup capable.

The flag HWMOD_SWSUP_SIDLE takes care of IPs which need the sidle
mode to be *always* controlled in SWSUP. We now have a need to control
IPs sidle mode in SWSUP only while its active.

So define a new flag 'HWMOD_SWSUP_SIDLE_ACT' to help the framework
know about these new IP requirements.

Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
Tested-by: Sourav Poddar <sourav.poddar@ti.com>
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    3 ++-
 arch/arm/mach-omap2/omap_hwmod.h |    4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 932d5e6..9e1e8c8 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1337,7 +1337,8 @@ static void _enable_sysc(struct omap_hwmod *oh)
 
 	clkdm = _get_clkdm(oh);
 	if (sf & SYSC_HAS_SIDLEMODE) {
-		if (oh->flags & HWMOD_SWSUP_SIDLE) {
+		if (oh->flags & HWMOD_SWSUP_SIDLE ||
+				oh->flags & HWMOD_SWSUP_SIDLE_ACT) {
 			idlemode = HWMOD_IDLEMODE_NO;
 		} else {
 			if (oh->class->sysc->idlemodes & SIDLE_SMART_WKUP)
diff --git a/arch/arm/mach-omap2/omap_hwmod.h b/arch/arm/mach-omap2/omap_hwmod.h
index 23f530b..c191035 100644
--- a/arch/arm/mach-omap2/omap_hwmod.h
+++ b/arch/arm/mach-omap2/omap_hwmod.h
@@ -451,6 +451,9 @@ struct omap_hwmod_omap4_prcm {
  *     enabled.  This prevents the hwmod code from being able to
  *     enable and reset the IP block early.  XXX Eventually it should
  *     be possible to query the clock framework for this information.
+ * HWMOD_SWSUP_SIDLE_ACT: omap_hwmod code should manually bring the module out of
+ *     idle, but rely on smart-idle to the put it back in idle, so the wakeups
+ *     are still functional (Only known case for now is UART)
  */
 #define HWMOD_SWSUP_SIDLE			(1 << 0)
 #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
@@ -462,6 +465,7 @@ struct omap_hwmod_omap4_prcm {
 #define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1 << 7)
 #define HWMOD_16BIT_REG				(1 << 8)
 #define HWMOD_EXT_OPT_MAIN_CLK			(1 << 9)
+#define HWMOD_SWSUP_SIDLE_ACT			(1 << 10)
 
 /*
  * omap_hwmod._int_flags definitions
-- 
1.7.9.5

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Santosh Shilimkar]

+ Felipe ( Sorry I missed you in CC list)

On Wednesday 20 February 2013 03:27 PM, Santosh Shilimkar wrote:
> OMAP UART IP needs manual idle modes based on functional state of the
> IP. Currently this is handled by the driver with function pointers
> implemented in platform code.
>
> This however breaks in case of device tree because of missing
> idle handling.
>
> The series tries to address the issue and tries to remove complete
> sysc handling from serial driver.
>
> Patches has been tested on OMAP4430 SDP and OMAP5 EVM where the console
> sluggishness was observed without idle mode handling. CPUIDLE and
> suspend tested ok on these devices.
>
> Thanks to Vaibhav Bedia for testing on Beaglebone(AM33XX) with suspend and
> also the UART async wakeup from suspend.
>
> Thanks to Sourav Poddar for Beagle XM(OMAP3630) devices testing with
> retention and offmode in Idle and suspend for non-DT builds.
>
> Serial driver platform code still has one more function pointer
> left which is used to trigger io_ring(). This one needs some
> discussion with pincontrol driver folks on how to implement
> such a feature with generic pincontrol driver.
>
> Rajendra Nayak (4):
>    ARM: OMAP2+: hwmod: Remove unused _HWMOD_WAKEUP_ENABLED flag
>    ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming
>    ARM: OMAP2+: hwmod: Always have OCP_SYSCONFIG.ENAWAKEUP enabled
>    ARM: OMAP2+: hwmod: Add a new flag to handle SIDLE in SWSUP only in
>      active
>
> Santosh Shilimkar (4):
>    ARM: OMAP2+: hwmod-data: UART IP needs software control to manage
>      sidle modes
>    SERIAL: OMAP: Remove the slave idle handling from the driver
>    ARM: OMAP2+: serial: Remove the un-used slave idle hooks
>    ARM: OMAP2+: hwmod: Remove sysc slave idle and auto idle apis
>
>   arch/arm/mach-omap2/omap_hwmod.c                   |  141 ++++----------------
>   arch/arm/mach-omap2/omap_hwmod.h                   |   13 +-
>   arch/arm/mach-omap2/omap_hwmod_2xxx_ipblock_data.c |    3 +
>   arch/arm/mach-omap2/omap_hwmod_33xx_data.c         |    6 +
>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c         |    4 +
>   arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |    6 +-
>   arch/arm/mach-omap2/serial.c                       |   31 -----
>   drivers/tty/serial/omap-serial.c                   |   23 ----
>   8 files changed, 50 insertions(+), 177 deletions(-)
>

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Russell King - ARM Linux]

On Wed, Feb 20, 2013 at 03:27:44PM +0530, Santosh Shilimkar wrote:
> OMAP UART IP needs manual idle modes based on functional state of the
> IP. Currently this is handled by the driver with function pointers
> implemented in platform code.

Up until now, the nightly test builds have included a load of test code
for supporting DMA on OMAP serial - only the buggy transmit side which
we've covered the reasons earlier why this can't work properly.

We've also covered why the ripping out of the DMA code from the serial
driver, and then subsequent "cleanup" of the driver has resulted in it
being impossible to resurect receive DMA support without reverting all
those cleanups.

This patch set is yet again another nail in the coffin of any DMA support
in the serial driver; it is removing the final bits of knowledge about
how DMA support interacts with the idle support.  Arguably, the previous
cleanups already did that, but at least the hooks were still clearly
visible as separate stand-alone entities and how only they were affected
by DMA being enabled.

Therefore, tonight I am dropping and discarding what I have left over
from my work on getting DMA support working with the OMAP serial driver
again from the nightly test builds and my git tree, and I intend no
further involvement with this.

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Santosh Shilimkar]

On Wednesday 20 February 2013 03:44 PM, Russell King - ARM Linux wrote:
> On Wed, Feb 20, 2013 at 03:27:44PM +0530, Santosh Shilimkar wrote:
>> OMAP UART IP needs manual idle modes based on functional state of the
>> IP. Currently this is handled by the driver with function pointers
>> implemented in platform code.
>
> Up until now, the nightly test builds have included a load of test code
> for supporting DMA on OMAP serial - only the buggy transmit side which
> we've covered the reasons earlier why this can't work properly.
>
> We've also covered why the ripping out of the DMA code from the serial
> driver, and then subsequent "cleanup" of the driver has resulted in it
> being impossible to resurect receive DMA support without reverting all
> those cleanups.
>
> This patch set is yet again another nail in the coffin of any DMA support
> in the serial driver; it is removing the final bits of knowledge about
> how DMA support interacts with the idle support.  Arguably, the previous
> cleanups already did that, but at least the hooks were still clearly
> visible as separate stand-alone entities and how only they were affected
> by DMA being enabled.
>
Actually the clean-up will remove the serial driver dependency with
idle handling. Infact DMA support need not care about idle handling
anymore.

> Therefore, tonight I am dropping and discarding what I have left over
> from my work on getting DMA support working with the OMAP serial driver
> again from the nightly test builds and my git tree, and I intend no
> further involvement with this.
>
Please please don't do that. We were at a point where we are unable
to use UART has just simple console with DT without the $subject
series. We can help you if some re-basing is needed for your
patches because of the $subject series.

Sorry for another series getting in between your work but this
one was unavoidable due to already broken support.

Regards,
Santosh

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Russell King - ARM Linux]

On Wed, Feb 20, 2013 at 03:53:22PM +0530, Santosh Shilimkar wrote:
> Actually the clean-up will remove the serial driver dependency with
> idle handling. Infact DMA support need not care about idle handling
> anymore.

How is that so?  Looking back at the driver source when it did have full
DMA support shows that the handling of force idle state is different
between PIO and DMA modes.

For example, for transmit (which is the only case I had taken the time
to properly understand before the DMA code was unceremoniously ripped
out):

- On DMA transmit, the noidle setting is cleared.  runtime PM status is put.
- If we switch from DMA to PIO transmit, the noidle setting it set.
  runtime PM status is still put.
- When stopping transmit, if we aren't using DMA, idle is forced.
- In the runtime suspend, if we're using DMA and the i291 errata is in effect,
  then idle state is forced
- In the runtime resume, if we're using DMA and the i291 errata is in effect,
  idle state is cleared.

See - that information has been lost through ripping out the DMA code and
then subsequently cleaning up the code to make these things "invisible".
And now that you've lifted the forceidle/noidle functions out of the driver,
there's now even less clue about how to preserve the above behaviour.

>> Therefore, tonight I am dropping and discarding what I have left over
>> from my work on getting DMA support working with the OMAP serial driver
>> again from the nightly test builds and my git tree, and I intend no
>> further involvement with this.
>>
> Please please don't do that. We were at a point where we are unable
> to use UART has just simple console with DT without the $subject
> series. We can help you if some re-basing is needed for your
> patches because of the $subject series.

Actually, I've already dropped the TX DMA stuff out a while back when it
became (a) just too painful to keep porting the thing forward and (b) it
doesn't work properly anyway.  It's last update was back in October, and
was based on v3.6 because it already became impossible to bring it
forward across the utter buggeration of the OMAP serial driver.

(That explains why I only ever pulled forward two patches - those which
add the hooks into the serial core layer and into the OMAP serial driver
to cleanly allocate a DMA-able transmit buffer, leaving the rest of the
transmit DMA code to rot.)

[PATCH 0/8] ARM; OMAP2+: hwmod and SERIAL: Remove sysc handling from driver

[Santosh Shilimkar]

On Wednesday 20 February 2013 05:21 PM, Russell King - ARM Linux wrote:
> On Wed, Feb 20, 2013 at 03:53:22PM +0530, Santosh Shilimkar wrote:
>> Actually the clean-up will remove the serial driver dependency with
>> idle handling. Infact DMA support need not care about idle handling
>> anymore.
>
> How is that so?  Looking back at the driver source when it did have full
> DMA support shows that the handling of force idle state is different
> between PIO and DMA modes.
>
> For example, for transmit (which is the only case I had taken the time
> to properly understand before the DMA code was unceremoniously ripped
> out):
>
> - On DMA transmit, the noidle setting is cleared.  runtime PM status is put.
> - If we switch from DMA to PIO transmit, the noidle setting it set.
>    runtime PM status is still put.
> - When stopping transmit, if we aren't using DMA, idle is forced.
> - In the runtime suspend, if we're using DMA and the i291 errata is in effect,
>    then idle state is forced
> - In the runtime resume, if we're using DMA and the i291 errata is in effect,
>    idle state is cleared.
>
> See - that information has been lost through ripping out the DMA code and
> then subsequently cleaning up the code to make these things "invisible".
> And now that you've lifted the forceidle/noidle functions out of the driver,
> there's now even less clue about how to preserve the above behaviour.
>
I looked at the history based on above and indeed the i291 DMA
information got lost as part of cleanup. The UART DMA had a different 
requirement though the DMA errata is applicable to only OMAP3 devices 
and got fixed in subsequent OMAP based devices.

So perhaps we can make dma use conditional based on devices
*when* we add the tx DMA support. And the errata information
flag can be passed from DT files.

>>> Therefore, tonight I am dropping and discarding what I have left over
>>> from my work on getting DMA support working with the OMAP serial driver
>>> again from the nightly test builds and my git tree, and I intend no
>>> further involvement with this.
>>>
>> Please please don't do that. We were at a point where we are unable
>> to use UART has just simple console with DT without the $subject
>> series. We can help you if some re-basing is needed for your
>> patches because of the $subject series.
>
> Actually, I've already dropped the TX DMA stuff out a while back when it
> became (a) just too painful to keep porting the thing forward and (b) it
> doesn't work properly anyway.  It's last update was back in October, and
> was based on v3.6 because it already became impossible to bring it
> forward across the utter buggeration of the OMAP serial driver.
>
> (That explains why I only ever pulled forward two patches - those which
> add the hooks into the serial core layer and into the OMAP serial driver
> to cleanly allocate a DMA-able transmit buffer, leaving the rest of the
> transmit DMA code to rot.)
>
Thanks for the information.

Regards,
Santosh

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Paul Walmsley]

Hi

On Wed, 20 Feb 2013, Santosh Shilimkar wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> _enable_wakeup() and _disable_wakeup() are expected to program the
> OCP_SYSCONFIG.ENAWAKEUP bit.

These functions were originally intended to take care of everything needed 
for the IP block to wake up the chip, including the PRCM WKEN programming.  
ENAWAKEUP is simply one part of that.

> Get rid of the additional sidle/mstandby programming in them, as its 
> confusing (this is expected to be handled elsewhere as part of 
> _enable_sysc()/__idle_sysc())

Sorry, why does the expectation exist for the code to enable and disable 
device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in 
functions called by _enable_sysc()/_idle_sysc()?

> and unnecessary.

So here's part of the reason why the module wakeup control functions 
should remain separate.  When the kernel boots, all the PM features should 
be disabled.  Then mach-omap2/pm*.c should enables PM features where 
they're needed.

Right now, mach-omap2/pm34xx.c sets module WKEN bits.  (These direct 
register accesses need to be moved as part of the cleanup work, of moving 
the PM/PRM/CM code into drivers.)  But the list of IP blocks that 
should be allowed to wake the system is board-dependent.

So really, what mach-omap2/pm34xx.c should do is to call into the hwmod 
enable-wakeups code to enable MPU wakeups for all the IP blocks that have 
some DT property set, something like 'enable-wakeups'.  Then the hwmod 
code should ensure that the PRM wakeup-enable and GRPSEL bits are 
programmed (by calling into the PRM driver code) and should then either 
set the ENAWAKEUP bits or put the module into smart_wkup MSTANDBY/MIDLE.

Similarly, when the PM driver is unloaded, it should set no-idle on all 
the IP blocks that were marked as wakeup-capable and disable the PRCM 
wakeup control bits, by calling some hwmod disable-wakeups code.

> Well, the _enable_sysc()/_idle_sysc() handled only the mstandby modes
> correctly. So fix them so they also handle the midle modes correctly

If there's a fix here, please split that out into a separate patch.


- Paul

[PATCH 3/8] ARM: OMAP2+: hwmod: Always have OCP_SYSCONFIG.ENAWAKEUP enabled

[Paul Walmsley]

On Wed, 20 Feb 2013, Santosh Shilimkar wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> Get rid of all complexities around when to enable OCP_SYSCONFIG.ENAWAKEUP.
> It should be safe to have this set *always* for all IP blocks which
> have this control. It should be a *dont care* when the IP is in
> NO/FORCE modes of sidle/mstandby. As for the SMART/SMART_WKUP modes
> of sidle/mstandby the framework anyway sets this always.
> 
> Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> Tested-by: Sourav Poddar <sourav.poddar@ti.com>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Some of the comments here:

http://marc.info/?l=linux-omap&m=136469345727226&w=2

apply to this patch as well.


- Paul

[PATCH 1/8] ARM: OMAP2+: hwmod: Remove unused _HWMOD_WAKEUP_ENABLED flag

[Paul Walmsley]

On Wed, 20 Feb 2013, Santosh Shilimkar wrote:

> From: Rajendra Nayak <rnayak@ti.com>
> 
> _HWMOD_WAKEUP_ENABLED is currently unused across the hwmod
> framework. Just get rid of it, so we have one less flag to
> worry about.
> 
> Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>

Thanks, queued for 3.9.

- Paul

[PATCH 1/8] ARM: OMAP2+: hwmod: Remove unused _HWMOD_WAKEUP_ENABLED flag

[Paul Walmsley]

On Sun, 31 Mar 2013, Paul Walmsley wrote:

> On Wed, 20 Feb 2013, Santosh Shilimkar wrote:
> 
> > From: Rajendra Nayak <rnayak@ti.com>
> > 
> > _HWMOD_WAKEUP_ENABLED is currently unused across the hwmod
> > framework. Just get rid of it, so we have one less flag to
> > worry about.
> > 
> > Tested-by: Vaibhav Bedia <vaibhav.bedia@ti.com>
> > Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> > Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
> 
> Thanks, queued for 3.9.

Err, 3.10, rather.


- Paul

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Rajendra Nayak]

Paul,

>> _enable_wakeup() and _disable_wakeup() are expected to program the
>> OCP_SYSCONFIG.ENAWAKEUP bit.
> 
> These functions were originally intended to take care of everything needed 
> for the IP block to wake up the chip, including the PRCM WKEN programming.  
> ENAWAKEUP is simply one part of that.
> 
>> Get rid of the additional sidle/mstandby programming in them, as its 
>> confusing (this is expected to be handled elsewhere as part of 
>> _enable_sysc()/__idle_sysc())
> 
> Sorry, why does the expectation exist for the code to enable and disable 
> device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in 
> functions called by _enable_sysc()/_idle_sysc()?

It all comes down to if SIDLE_SMART_WKUP/MSTANDBY_SMART_WKUP programming
be considered as 'idlemode' programming or 'enwakeup' programming.
If you consider these are being part of 'enwakeup' progrmming, these should
certainly be handled as part of _enable_wakeup() and _disable_wakeup().

Today, in some cases, these are *also* handled as part of _enable_sysc()
and _idle_sysc(). The way _enable_wakeup() is invoked from _enable_sysc()
is also very inconsistent. For instance, for any IP which supports
SYSC_HAS_MIDLEMODE and SYSC_HAS_ENAWAKEUP, we invoke _enable_wakeup()
regardless of the MIDLEMODE programmmed.
While in case of the IP supporting SYSC_HAS_SIDLEMODE, _enable_wakeup() is
invoked only when the SIDLEMODE programmed is SMART or SMART_WKUP.

I understand the cleanups you are suggesting below as part of the movement
of some of these things outside of mach-omap2.
I was more looking at fixing the existing piece so its readable and does
things more consistently.

regards,
Rajendra

> 
>> and unnecessary.
> 
> So here's part of the reason why the module wakeup control functions 
> should remain separate.  When the kernel boots, all the PM features should 
> be disabled.  Then mach-omap2/pm*.c should enables PM features where 
> they're needed.
> 
> Right now, mach-omap2/pm34xx.c sets module WKEN bits.  (These direct 
> register accesses need to be moved as part of the cleanup work, of moving 
> the PM/PRM/CM code into drivers.)  But the list of IP blocks that 
> should be allowed to wake the system is board-dependent.
> 
> So really, what mach-omap2/pm34xx.c should do is to call into the hwmod 
> enable-wakeups code to enable MPU wakeups for all the IP blocks that have 
> some DT property set, something like 'enable-wakeups'.  Then the hwmod 
> code should ensure that the PRM wakeup-enable and GRPSEL bits are 
> programmed (by calling into the PRM driver code) and should then either 
> set the ENAWAKEUP bits or put the module into smart_wkup MSTANDBY/MIDLE.
> 
> Similarly, when the PM driver is unloaded, it should set no-idle on all 
> the IP blocks that were marked as wakeup-capable and disable the PRCM 
> wakeup control bits, by calling some hwmod disable-wakeups code.
> 
>> Well, the _enable_sysc()/_idle_sysc() handled only the mstandby modes
>> correctly. So fix them so they also handle the midle modes correctly
> 
> If there's a fix here, please split that out into a separate patch.
> 
> 
> - Paul
> 

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Rajendra Nayak]

Hi Paul,

>>> _enable_wakeup() and _disable_wakeup() are expected to program the
>>> OCP_SYSCONFIG.ENAWAKEUP bit.
>>
>> These functions were originally intended to take care of everything needed 
>> for the IP block to wake up the chip, including the PRCM WKEN programming.  
>> ENAWAKEUP is simply one part of that.
>>
>>> Get rid of the additional sidle/mstandby programming in them, as its 
>>> confusing (this is expected to be handled elsewhere as part of 
>>> _enable_sysc()/__idle_sysc())
>>
>> Sorry, why does the expectation exist for the code to enable and disable 
>> device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in 
>> functions called by _enable_sysc()/_idle_sysc()?
> 
> It all comes down to if SIDLE_SMART_WKUP/MSTANDBY_SMART_WKUP programming
> be considered as 'idlemode' programming or 'enwakeup' programming.
> If you consider these are being part of 'enwakeup' progrmming, these should
> certainly be handled as part of _enable_wakeup() and _disable_wakeup().
> 
> Today, in some cases, these are *also* handled as part of _enable_sysc()
> and _idle_sysc(). The way _enable_wakeup() is invoked from _enable_sysc()
> is also very inconsistent. For instance, for any IP which supports
> SYSC_HAS_MIDLEMODE and SYSC_HAS_ENAWAKEUP, we invoke _enable_wakeup()
> regardless of the MIDLEMODE programmmed.
> While in case of the IP supporting SYSC_HAS_SIDLEMODE, _enable_wakeup() is
> invoked only when the SIDLEMODE programmed is SMART or SMART_WKUP.
> 
> I understand the cleanups you are suggesting below as part of the movement
> of some of these things outside of mach-omap2.
> I was more looking at fixing the existing piece so its readable and does
> things more consistently.

Do you have any further thoughts on how we should do about this?

regards,
Rajendra

> 
> regards,
> Rajendra
> 
>>
>>> and unnecessary.
>>
>> So here's part of the reason why the module wakeup control functions 
>> should remain separate.  When the kernel boots, all the PM features should 
>> be disabled.  Then mach-omap2/pm*.c should enables PM features where 
>> they're needed.
>>
>> Right now, mach-omap2/pm34xx.c sets module WKEN bits.  (These direct 
>> register accesses need to be moved as part of the cleanup work, of moving 
>> the PM/PRM/CM code into drivers.)  But the list of IP blocks that 
>> should be allowed to wake the system is board-dependent.
>>
>> So really, what mach-omap2/pm34xx.c should do is to call into the hwmod 
>> enable-wakeups code to enable MPU wakeups for all the IP blocks that have 
>> some DT property set, something like 'enable-wakeups'.  Then the hwmod 
>> code should ensure that the PRM wakeup-enable and GRPSEL bits are 
>> programmed (by calling into the PRM driver code) and should then either 
>> set the ENAWAKEUP bits or put the module into smart_wkup MSTANDBY/MIDLE.
>>
>> Similarly, when the PM driver is unloaded, it should set no-idle on all 
>> the IP blocks that were marked as wakeup-capable and disable the PRCM 
>> wakeup control bits, by calling some hwmod disable-wakeups code.
>>
>>> Well, the _enable_sysc()/_idle_sysc() handled only the mstandby modes
>>> correctly. So fix them so they also handle the midle modes correctly
>>
>> If there's a fix here, please split that out into a separate patch.
>>
>>
>> - Paul
>>
> 

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Paul Walmsley]

Hi Rajendra,

On Thu, 18 Apr 2013, Rajendra Nayak wrote:

> >>> _enable_wakeup() and _disable_wakeup() are expected to program the
> >>> OCP_SYSCONFIG.ENAWAKEUP bit.
> >>
> >> These functions were originally intended to take care of everything needed 
> >> for the IP block to wake up the chip, including the PRCM WKEN programming.  
> >> ENAWAKEUP is simply one part of that.
> >>
> >>> Get rid of the additional sidle/mstandby programming in them, as its 
> >>> confusing (this is expected to be handled elsewhere as part of 
> >>> _enable_sysc()/__idle_sysc())
> >>
> >> Sorry, why does the expectation exist for the code to enable and disable 
> >> device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in 
> >> functions called by _enable_sysc()/_idle_sysc()?
> > 
> > It all comes down to if SIDLE_SMART_WKUP/MSTANDBY_SMART_WKUP programming
> > be considered as 'idlemode' programming or 'enwakeup' programming.
> > If you consider these are being part of 'enwakeup' progrmming, these should
> > certainly be handled as part of _enable_wakeup() and _disable_wakeup().
> > 
> > Today, in some cases, these are *also* handled as part of _enable_sysc()
> > and _idle_sysc(). The way _enable_wakeup() is invoked from _enable_sysc()
> > is also very inconsistent. For instance, for any IP which supports
> > SYSC_HAS_MIDLEMODE and SYSC_HAS_ENAWAKEUP, we invoke _enable_wakeup()
> > regardless of the MIDLEMODE programmmed.
> > While in case of the IP supporting SYSC_HAS_SIDLEMODE, _enable_wakeup() is
> > invoked only when the SIDLEMODE programmed is SMART or SMART_WKUP.
> > 
> > I understand the cleanups you are suggesting below as part of the movement
> > of some of these things outside of mach-omap2.
> > I was more looking at fixing the existing piece so its readable and does
> > things more consistently.
> 
> Do you have any further thoughts on how we should do about this?

Is it possible to implement a solution that preserves _enable_wakeup() and 
_disable_wakeup() as distinct functions, that can be called by separate 
wakeup control entry points?

If it makes sense to change _enable_sysc() so that it doesn't call 
_enable_wakeup(), but does similar work itself, that's fine with me, as 
long as there's not too much code duplication.

It will be good to have the inconsistencies fixed.


- Paul

[PATCH 2/8] ARM: OMAP2+: hwmod: Cleanup sidle/mstandby programming

[Rajendra Nayak]

On Tuesday 23 April 2013 01:49 PM, Paul Walmsley wrote:
> Hi Rajendra,
> 
> On Thu, 18 Apr 2013, Rajendra Nayak wrote:
> 
>>>>> _enable_wakeup() and _disable_wakeup() are expected to program the
>>>>> OCP_SYSCONFIG.ENAWAKEUP bit.
>>>>
>>>> These functions were originally intended to take care of everything needed 
>>>> for the IP block to wake up the chip, including the PRCM WKEN programming.  
>>>> ENAWAKEUP is simply one part of that.
>>>>
>>>>> Get rid of the additional sidle/mstandby programming in them, as its 
>>>>> confusing (this is expected to be handled elsewhere as part of 
>>>>> _enable_sysc()/__idle_sysc())
>>>>
>>>> Sorry, why does the expectation exist for the code to enable and disable 
>>>> device wakeup to be part of _enable_sysc()/_idle_sysc(), rather than in 
>>>> functions called by _enable_sysc()/_idle_sysc()?
>>>
>>> It all comes down to if SIDLE_SMART_WKUP/MSTANDBY_SMART_WKUP programming
>>> be considered as 'idlemode' programming or 'enwakeup' programming.
>>> If you consider these are being part of 'enwakeup' progrmming, these should
>>> certainly be handled as part of _enable_wakeup() and _disable_wakeup().
>>>
>>> Today, in some cases, these are *also* handled as part of _enable_sysc()
>>> and _idle_sysc(). The way _enable_wakeup() is invoked from _enable_sysc()
>>> is also very inconsistent. For instance, for any IP which supports
>>> SYSC_HAS_MIDLEMODE and SYSC_HAS_ENAWAKEUP, we invoke _enable_wakeup()
>>> regardless of the MIDLEMODE programmmed.
>>> While in case of the IP supporting SYSC_HAS_SIDLEMODE, _enable_wakeup() is
>>> invoked only when the SIDLEMODE programmed is SMART or SMART_WKUP.
>>>
>>> I understand the cleanups you are suggesting below as part of the movement
>>> of some of these things outside of mach-omap2.
>>> I was more looking at fixing the existing piece so its readable and does
>>> things more consistently.
>>
>> Do you have any further thoughts on how we should do about this?
> 
> Is it possible to implement a solution that preserves _enable_wakeup() and 
> _disable_wakeup() as distinct functions, that can be called by separate 
> wakeup control entry points?
> 
> If it makes sense to change _enable_sysc() so that it doesn't call 
> _enable_wakeup(), but does similar work itself, that's fine with me, as 
> long as there's not too much code duplication.
> 
> It will be good to have the inconsistencies fixed.

Sure, I'll post something on those lines shortly.

regards,
Rajendra
> 
> 
> - Paul
>