[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

Hi,

Here is an initial set of fixes for power management and OMAP core
infrastructure for 3.5-rc.  Most of these patches address boot
warnings and power management problems on OMAP4, although a few of
them address more general issues.

Compile-test information is below.  The patches been boot-tested on
OMAP3530 ES3.0 BeagleBoard and OMAP4430 ES2.0 PandaBoard.  Suspend and
idle are broken on both of these platforms right now.  Unfortunately
my board farm is down at the moment, so I'm unable to do further
testing on other boards.  Boot logs, such as they are, are available
from:

http://www.pwsan.com/omap/bootlogs/20120606/omap_fixes_a_3.5rc__23a432a1860498d0dfbe36319de3b86d9832c6e7/

These patches are available via git from git://git.pwsan.com/linux-2.6
in the branch 'omap_fixes_a_3.5rc'.  Will plan to send a pull request
tomorrow unless there are actionable comments.


- Paul

---

object size (delta in bytes from v3.5-rc1 (f8f5701bdaf9134b1f90e5044a82c66324d2073f)):
 text 	 data 	  bss 	total 	kernel
    0 	    0 	    0 	    0 	5912osk_testconfig/vmlinux
 +352 	  +88 	    0 	 +440 	n800_multi_omap2xxx/vmlinux
 +384 	  +96 	    0 	 +480 	n800_testconfig/vmlinux
    0 	    0 	    0 	    0 	omap1510_defconfig/vmlinux
    0 	    0 	    0 	    0 	omap1_defconfig/vmlinux
 +352 	 +312 	    0 	 +664 	omap2_4_testconfig/vmlinux
 +384 	 +448 	    0 	 +832 	omap2plus_defconfig/vmlinux
 +384 	 +384 	    0 	 +768 	omap2plus_no_pm/vmlinux
 +320 	 +320 	    0 	 +640 	omap3_4_testconfig/vmlinux
 +320 	  +88 	    0 	 +408 	omap3_testconfig/vmlinux
 +320 	 +256 	    0 	 +576 	omap4_testconfig/vmlinux


Djamil Elaidi (1):
      ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby

Paul Walmsley (8):
      ARM: OMAP2+: hwmod: add setup_preprogram hook
      ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
      ARM: OMAP4: hwmod data: add SL2IF hardreset line
      ARM: OMAP2+: hwmod code/data: fix 32K sync timer
      ARM: OMAP2+: CM: increase the module disable timeout
      ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
      ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init
      ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init

Tero Kristo (1):
      ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

Todd Poynor (1):
      ARM: OMAP: PM: Lock clocks list while generating summary


 arch/arm/mach-omap2/Makefile                 |    6 +--
 arch/arm/mach-omap2/aess.c                   |   55 +++++++++++++++++++++++
 arch/arm/mach-omap2/clock44xx_data.c         |    5 ++
 arch/arm/mach-omap2/cm.h                     |   19 ++++++++
 arch/arm/mach-omap2/cminst44xx.c             |    4 +-
 arch/arm/mach-omap2/common.h                 |    2 +
 arch/arm/mach-omap2/omap_hwmod.c             |   58 +++++++++++++++++++++---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c   |   23 ++++++++++
 arch/arm/mach-omap2/usb-fs.c                 |   62 ++++++++++++++++++++++++++
 arch/arm/plat-omap/clock.c                   |    2 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h |   15 ++++++
 arch/arm/plat-omap/include/plat/usb.h        |    3 +
 12 files changed, 238 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm/mach-omap2/aess.c

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

Hi,

Here is an initial set of fixes for power management and OMAP core
infrastructure for 3.5-rc.  Most of these patches address boot
warnings and power management problems on OMAP4, although a few of
them address more general issues.

Compile-test information is below.  The patches been boot-tested on
OMAP3530 ES3.0 BeagleBoard and OMAP4430 ES2.0 PandaBoard.  Suspend and
idle are broken on both of these platforms right now.  Unfortunately
my board farm is down at the moment, so I'm unable to do further
testing on other boards.  Boot logs, such as they are, are available
from:

http://www.pwsan.com/omap/bootlogs/20120606/omap_fixes_a_3.5rc__23a432a1860498d0dfbe36319de3b86d9832c6e7/

These patches are available via git from git://git.pwsan.com/linux-2.6
in the branch 'omap_fixes_a_3.5rc'.  Will plan to send a pull request
tomorrow unless there are actionable comments.


- Paul

---

object size (delta in bytes from v3.5-rc1 (f8f5701bdaf9134b1f90e5044a82c66324d2073f)):
 text 	 data 	  bss 	total 	kernel
    0 	    0 	    0 	    0 	5912osk_testconfig/vmlinux
 +352 	  +88 	    0 	 +440 	n800_multi_omap2xxx/vmlinux
 +384 	  +96 	    0 	 +480 	n800_testconfig/vmlinux
    0 	    0 	    0 	    0 	omap1510_defconfig/vmlinux
    0 	    0 	    0 	    0 	omap1_defconfig/vmlinux
 +352 	 +312 	    0 	 +664 	omap2_4_testconfig/vmlinux
 +384 	 +448 	    0 	 +832 	omap2plus_defconfig/vmlinux
 +384 	 +384 	    0 	 +768 	omap2plus_no_pm/vmlinux
 +320 	 +320 	    0 	 +640 	omap3_4_testconfig/vmlinux
 +320 	  +88 	    0 	 +408 	omap3_testconfig/vmlinux
 +320 	 +256 	    0 	 +576 	omap4_testconfig/vmlinux


Djamil Elaidi (1):
      ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby

Paul Walmsley (8):
      ARM: OMAP2+: hwmod: add setup_preprogram hook
      ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
      ARM: OMAP4: hwmod data: add SL2IF hardreset line
      ARM: OMAP2+: hwmod code/data: fix 32K sync timer
      ARM: OMAP2+: CM: increase the module disable timeout
      ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
      ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init
      ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init

Tero Kristo (1):
      ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

Todd Poynor (1):
      ARM: OMAP: PM: Lock clocks list while generating summary


 arch/arm/mach-omap2/Makefile                 |    6 +--
 arch/arm/mach-omap2/aess.c                   |   55 +++++++++++++++++++++++
 arch/arm/mach-omap2/clock44xx_data.c         |    5 ++
 arch/arm/mach-omap2/cm.h                     |   19 ++++++++
 arch/arm/mach-omap2/cminst44xx.c             |    4 +-
 arch/arm/mach-omap2/common.h                 |    2 +
 arch/arm/mach-omap2/omap_hwmod.c             |   58 +++++++++++++++++++++---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c   |   23 ++++++++++
 arch/arm/mach-omap2/usb-fs.c                 |   62 ++++++++++++++++++++++++++
 arch/arm/plat-omap/clock.c                   |    2 +
 arch/arm/plat-omap/include/plat/omap_hwmod.h |   15 ++++++
 arch/arm/plat-omap/include/plat/usb.h        |    3 +
 12 files changed, 238 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm/mach-omap2/aess.c

[PATCH 01/11] ARM: OMAP2+: hwmod: add setup_preprogram hook

[Paul Walmsley]

After reset, some IP blocks cannot indicate to the PRCM that they are
inactive until they are configured.  One example on OMAP4 is the AESS
IP block.

To fix this cleanly, this patch adds another optional function
pointer, setup_preprogram, to the IP block's hwmod data.  The function
that is pointed to is called by the hwmod code immediately after the
IP block is reset.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Péter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   21 ++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    9 +++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index bf86f7e..b0d3064 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2103,6 +2103,23 @@ static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 }
 
 /**
+ * _setup_preprogram - Pre-program an IP block during the _setup() process
+ * @oh: struct omap_hwmod *
+ *
+ * Some IP blocks (such as AESS) require some additional programming
+ * after reset before they can enter idle.  If a function pointer to
+ * do so is present in the hwmod data, then call it and pass along the
+ * return value; otherwise, return 0.
+ */
+static int __init _setup_preprogram(struct omap_hwmod *oh)
+{
+	if (!oh->class->setup_preprogram)
+		return 0;
+
+	return oh->class->setup_preprogram(oh);
+}
+
+/**
  * _setup_reset - reset an IP block during the setup process
  * @oh: struct omap_hwmod *
  *
@@ -2224,8 +2241,10 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
 
 	_setup_iclk_autoidle(oh);
 
-	if (!_setup_reset(oh))
+	if (!_setup_reset(oh)) {
+		_setup_preprogram(oh);
 		_setup_postsetup(oh);
+	}
 
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index c835b71..fdc4b2a 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -458,6 +458,7 @@ struct omap_hwmod_omap4_prcm {
  * @rev: revision of the IP class
  * @pre_shutdown: ptr to fn to be executed immediately prior to device shutdown
  * @reset: ptr to fn to be executed in place of the standard hwmod reset fn
+ * @setup_preprogram: ptr to fn to be executed after the reset in _setup()
  *
  * Represent the class of a OMAP hardware "modules" (e.g. timer,
  * smartreflex, gpio, uart...)
@@ -474,6 +475,13 @@ struct omap_hwmod_omap4_prcm {
  * executed in place of the standard hwmod _reset() code in
  * mach-omap2/omap_hwmod.c.  This is needed for IP blocks which have
  * unusual reset sequences - usually processor IP blocks like the IVA.
+ *
+ * @setup_preprogram is called between the calls to _setup_reset() and
+ * _setup_postsetup().  It is intended to be used for IP blocks that
+ * require some initial configuration during their first
+ * initialization.  (For example, the AESS IP block on OMAP4+ cannot
+ * enter idle until its internal autogating bit is set.)  Most IP blocks
+ * will not need this.
  */
 struct omap_hwmod_class {
 	const char				*name;
@@ -481,6 +489,7 @@ struct omap_hwmod_class {
 	u32					rev;
 	int					(*pre_shutdown)(struct omap_hwmod *oh);
 	int					(*reset)(struct omap_hwmod *oh);
+	int					(*setup_preprogram)(struct omap_hwmod *oh);
 };
 
 /**


--
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

[PATCH 01/11] ARM: OMAP2+: hwmod: add setup_preprogram hook

[Paul Walmsley]

After reset, some IP blocks cannot indicate to the PRCM that they are
inactive until they are configured.  One example on OMAP4 is the AESS
IP block.

To fix this cleanly, this patch adds another optional function
pointer, setup_preprogram, to the IP block's hwmod data.  The function
that is pointed to is called by the hwmod code immediately after the
IP block is reset.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: P?ter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |   21 ++++++++++++++++++++-
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    9 +++++++++
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index bf86f7e..b0d3064 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2103,6 +2103,23 @@ static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 }
 
 /**
+ * _setup_preprogram - Pre-program an IP block during the _setup() process
+ * @oh: struct omap_hwmod *
+ *
+ * Some IP blocks (such as AESS) require some additional programming
+ * after reset before they can enter idle.  If a function pointer to
+ * do so is present in the hwmod data, then call it and pass along the
+ * return value; otherwise, return 0.
+ */
+static int __init _setup_preprogram(struct omap_hwmod *oh)
+{
+	if (!oh->class->setup_preprogram)
+		return 0;
+
+	return oh->class->setup_preprogram(oh);
+}
+
+/**
  * _setup_reset - reset an IP block during the setup process
  * @oh: struct omap_hwmod *
  *
@@ -2224,8 +2241,10 @@ static int __init _setup(struct omap_hwmod *oh, void *data)
 
 	_setup_iclk_autoidle(oh);
 
-	if (!_setup_reset(oh))
+	if (!_setup_reset(oh)) {
+		_setup_preprogram(oh);
 		_setup_postsetup(oh);
+	}
 
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index c835b71..fdc4b2a 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -458,6 +458,7 @@ struct omap_hwmod_omap4_prcm {
  * @rev: revision of the IP class
  * @pre_shutdown: ptr to fn to be executed immediately prior to device shutdown
  * @reset: ptr to fn to be executed in place of the standard hwmod reset fn
+ * @setup_preprogram: ptr to fn to be executed after the reset in _setup()
  *
  * Represent the class of a OMAP hardware "modules" (e.g. timer,
  * smartreflex, gpio, uart...)
@@ -474,6 +475,13 @@ struct omap_hwmod_omap4_prcm {
  * executed in place of the standard hwmod _reset() code in
  * mach-omap2/omap_hwmod.c.  This is needed for IP blocks which have
  * unusual reset sequences - usually processor IP blocks like the IVA.
+ *
+ * @setup_preprogram is called between the calls to _setup_reset() and
+ * _setup_postsetup().  It is intended to be used for IP blocks that
+ * require some initial configuration during their first
+ * initialization.  (For example, the AESS IP block on OMAP4+ cannot
+ * enter idle until its internal autogating bit is set.)  Most IP blocks
+ * will not need this.
  */
 struct omap_hwmod_class {
 	const char				*name;
@@ -481,6 +489,7 @@ struct omap_hwmod_class {
 	u32					rev;
 	int					(*pre_shutdown)(struct omap_hwmod *oh);
 	int					(*reset)(struct omap_hwmod *oh);
+	int					(*setup_preprogram)(struct omap_hwmod *oh);
 };
 
 /**

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

Enable the AESS auto-gating control bit during AESS hwmod setup.  This
fixes the following boot warning on OMAP4:

omap_hwmod: aess: _wait_target_disable failed

Without this patch, the AESS IP block does not indicate to the PRCM
that it is idle after it is reset.  This prevents some types of SoC
power management until something sets the auto-gating control bit.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Péter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/Makefile               |    2 +
 arch/arm/mach-omap2/aess.c                 |   55 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/common.h               |    2 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 +
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-omap2/aess.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fa742f3..bc2ac4f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -4,7 +4,7 @@
 
 # Common support
 obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer.o pm.o \
-	 common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o
+	 common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o aess.o
 
 omap-2-3-common				= irq.o sdrc.o
 hwmod-common				= omap_hwmod.o \
diff --git a/arch/arm/mach-omap2/aess.c b/arch/arm/mach-omap2/aess.c
new file mode 100644
index 0000000..d5ca3d2
--- /dev/null
+++ b/arch/arm/mach-omap2/aess.c
@@ -0,0 +1,55 @@
+/*
+ * AESS IP block integration
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ * Paul Walmsley
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+
+#include <plat/omap_hwmod.h>
+
+#include "common.h"
+
+/*
+ * AESS_AUTO_GATING_ENABLE_OFFSET: offset in bytes of the AESS IP
+ *     block's AESS_AUTO_GATING_ENABLE__1 register from the IP block's
+ *     base address
+ */
+#define AESS_AUTO_GATING_ENABLE_OFFSET				0x07c
+
+/* Register bitfields in the AESS_AUTO_GATING_ENABLE__1 register */
+#define AESS_AUTO_GATING_ENABLE_SHIFT				0
+
+/**
+ * omap_aess_preprogram - enable AESS internal autogating
+ * @oh: struct omap_hwmod *
+ *
+ * Since the AESS will not IdleAck to the PRCM until its internal
+ * autogating is enabled, we must enable autogating during the initial
+ * AESS hwmod setup.  Returns 0.
+ */
+int omap_aess_preprogram(struct omap_hwmod *oh)
+{
+	u32 v;
+
+	/* Set AESS_AUTO_GATING_ENABLE__1.ENABLE to allow idle entry */
+	v = 1 << AESS_AUTO_GATING_ENABLE_SHIFT;
+	omap_hwmod_write(v, oh, AESS_AUTO_GATING_ENABLE_OFFSET);
+
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index be9dfd1..d9b8b06 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -304,5 +304,7 @@ extern void omap_sdrc_init(struct omap_sdrc_params *sdrc_cs0,
 struct omap2_hsmmc_info;
 extern int omap4_twl6030_hsmmc_init(struct omap2_hsmmc_info *controllers);
 
+extern int omap_aess_preprogram(struct omap_hwmod *oh);
+
 #endif /* __ASSEMBLER__ */
 #endif /* __ARCH_ARM_MACH_OMAP2PLUS_COMMON_H */
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 950454a..4a2f2cc 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -39,6 +39,7 @@
 #include "prm44xx.h"
 #include "prm-regbits-44xx.h"
 #include "wd_timer.h"
+#include "common.h"
 
 /* Base offset for all OMAP4 interrupts external to MPUSS */
 #define OMAP44XX_IRQ_GIC_START	32
@@ -313,6 +314,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_aess_sysc = {
 static struct omap_hwmod_class omap44xx_aess_hwmod_class = {
 	.name	= "aess",
 	.sysc	= &omap44xx_aess_sysc,
+	.setup_preprogram = omap_aess_preprogram,
 };
 
 /* aess */


--
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

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

Enable the AESS auto-gating control bit during AESS hwmod setup.  This
fixes the following boot warning on OMAP4:

omap_hwmod: aess: _wait_target_disable failed

Without this patch, the AESS IP block does not indicate to the PRCM
that it is idle after it is reset.  This prevents some types of SoC
power management until something sets the auto-gating control bit.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: P?ter Ujfalusi <peter.ujfalusi@ti.com>
---
 arch/arm/mach-omap2/Makefile               |    2 +
 arch/arm/mach-omap2/aess.c                 |   55 ++++++++++++++++++++++++++++
 arch/arm/mach-omap2/common.h               |    2 +
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    2 +
 4 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-omap2/aess.c

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index fa742f3..bc2ac4f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -4,7 +4,7 @@
 
 # Common support
 obj-y := id.o io.o control.o mux.o devices.o serial.o gpmc.o timer.o pm.o \
-	 common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o
+	 common.o gpio.o dma.o wd_timer.o display.o i2c.o hdq1w.o aess.o
 
 omap-2-3-common				= irq.o sdrc.o
 hwmod-common				= omap_hwmod.o \
diff --git a/arch/arm/mach-omap2/aess.c b/arch/arm/mach-omap2/aess.c
new file mode 100644
index 0000000..d5ca3d2
--- /dev/null
+++ b/arch/arm/mach-omap2/aess.c
@@ -0,0 +1,55 @@
+/*
+ * AESS IP block integration
+ *
+ * Copyright (C) 2012 Texas Instruments, Inc.
+ * Paul Walmsley
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+
+#include <plat/omap_hwmod.h>
+
+#include "common.h"
+
+/*
+ * AESS_AUTO_GATING_ENABLE_OFFSET: offset in bytes of the AESS IP
+ *     block's AESS_AUTO_GATING_ENABLE__1 register from the IP block's
+ *     base address
+ */
+#define AESS_AUTO_GATING_ENABLE_OFFSET				0x07c
+
+/* Register bitfields in the AESS_AUTO_GATING_ENABLE__1 register */
+#define AESS_AUTO_GATING_ENABLE_SHIFT				0
+
+/**
+ * omap_aess_preprogram - enable AESS internal autogating
+ * @oh: struct omap_hwmod *
+ *
+ * Since the AESS will not IdleAck to the PRCM until its internal
+ * autogating is enabled, we must enable autogating during the initial
+ * AESS hwmod setup.  Returns 0.
+ */
+int omap_aess_preprogram(struct omap_hwmod *oh)
+{
+	u32 v;
+
+	/* Set AESS_AUTO_GATING_ENABLE__1.ENABLE to allow idle entry */
+	v = 1 << AESS_AUTO_GATING_ENABLE_SHIFT;
+	omap_hwmod_write(v, oh, AESS_AUTO_GATING_ENABLE_OFFSET);
+
+	return 0;
+}
diff --git a/arch/arm/mach-omap2/common.h b/arch/arm/mach-omap2/common.h
index be9dfd1..d9b8b06 100644
--- a/arch/arm/mach-omap2/common.h
+++ b/arch/arm/mach-omap2/common.h
@@ -304,5 +304,7 @@ extern void omap_sdrc_init(struct omap_sdrc_params *sdrc_cs0,
 struct omap2_hsmmc_info;
 extern int omap4_twl6030_hsmmc_init(struct omap2_hsmmc_info *controllers);
 
+extern int omap_aess_preprogram(struct omap_hwmod *oh);
+
 #endif /* __ASSEMBLER__ */
 #endif /* __ARCH_ARM_MACH_OMAP2PLUS_COMMON_H */
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 950454a..4a2f2cc 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -39,6 +39,7 @@
 #include "prm44xx.h"
 #include "prm-regbits-44xx.h"
 #include "wd_timer.h"
+#include "common.h"
 
 /* Base offset for all OMAP4 interrupts external to MPUSS */
 #define OMAP44XX_IRQ_GIC_START	32
@@ -313,6 +314,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_aess_sysc = {
 static struct omap_hwmod_class omap44xx_aess_hwmod_class = {
 	.name	= "aess",
 	.sysc	= &omap44xx_aess_sysc,
+	.setup_preprogram = omap_aess_preprogram,
 };
 
 /* aess */

[PATCH 03/11] ARM: OMAP4: hwmod data: add SL2IF hardreset line

[Paul Walmsley]

On boot, the sl2if module can't be enabled.  The following message is
logged:

omap_hwmod: sl2if: cannot be enabled for reset (3)

This is probably because the SL2IF is still being held in hardreset.
The SL2IF's hardreset line is shared with one of the IVAHD's hardreset
lines; see for example Table 3-536 "RM_IVAHD_RSTCTRL" in the OMAP4430
TRM Rev. AA (SWPU231AA).  To work around this, add the SL2IF's
hardreset line to the hwmod data.  This is correct from a hardware
perspective and also will prevent the hwmod from attempting to enable
the SL2IF during reset.  The driver for this IP block will need to
handle its integration until the appropriate way to handle the IVAHD
integration can be elucidated.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 4a2f2cc..a93ce48 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2597,15 +2597,22 @@ static struct omap_hwmod_class omap44xx_sl2if_hwmod_class = {
 	.name	= "sl2if",
 };
 
+static struct omap_hwmod_rst_info omap44xx_sl2if_resets[] = {
+	{ .name = "logic", .rst_shift = 2 },
+};
+
 /* sl2if */
 static struct omap_hwmod omap44xx_sl2if_hwmod = {
 	.name		= "sl2if",
 	.class		= &omap44xx_sl2if_hwmod_class,
 	.clkdm_name	= "ivahd_clkdm",
+	.rst_lines	= omap44xx_sl2if_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_sl2if_resets),
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_IVAHD_SL2_CLKCTRL_OFFSET,
 			.context_offs = OMAP4_RM_IVAHD_SL2_CONTEXT_OFFSET,
+			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},

[PATCH 03/11] ARM: OMAP4: hwmod data: add SL2IF hardreset line

[Paul Walmsley]

On boot, the sl2if module can't be enabled.  The following message is
logged:

omap_hwmod: sl2if: cannot be enabled for reset (3)

This is probably because the SL2IF is still being held in hardreset.
The SL2IF's hardreset line is shared with one of the IVAHD's hardreset
lines; see for example Table 3-536 "RM_IVAHD_RSTCTRL" in the OMAP4430
TRM Rev. AA (SWPU231AA).  To work around this, add the SL2IF's
hardreset line to the hwmod data.  This is correct from a hardware
perspective and also will prevent the hwmod from attempting to enable
the SL2IF during reset.  The driver for this IP block will need to
handle its integration until the appropriate way to handle the IVAHD
integration can be elucidated.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 4a2f2cc..a93ce48 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2597,15 +2597,22 @@ static struct omap_hwmod_class omap44xx_sl2if_hwmod_class = {
 	.name	= "sl2if",
 };
 
+static struct omap_hwmod_rst_info omap44xx_sl2if_resets[] = {
+	{ .name = "logic", .rst_shift = 2 },
+};
+
 /* sl2if */
 static struct omap_hwmod omap44xx_sl2if_hwmod = {
 	.name		= "sl2if",
 	.class		= &omap44xx_sl2if_hwmod_class,
 	.clkdm_name	= "ivahd_clkdm",
+	.rst_lines	= omap44xx_sl2if_resets,
+	.rst_lines_cnt	= ARRAY_SIZE(omap44xx_sl2if_resets),
 	.prcm = {
 		.omap4 = {
 			.clkctrl_offs = OMAP4_CM_IVAHD_SL2_CLKCTRL_OFFSET,
 			.context_offs = OMAP4_RM_IVAHD_SL2_CONTEXT_OFFSET,
+			.rstctrl_offs = OMAP4_RM_IVAHD_RSTCTRL_OFFSET,
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

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

Add a custom reset function for the usb_host_fs/fsusb IP block, and
connect it to the OMAP4 FSUSB block.

This is the first of two fixes required to get rid of the boot
warning:

omap_hwmod: usb_host_fs: _wait_target_disable failed

and to allow the module to idle.

It may be necessary to use this reset method for OMAP2xxx SoCs as
well; this is left for a future patch.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
[paul@pwsan.com: rewrote the custom reset function, documented it and
 updated the commit message, and moved the code to mach-omap2/fs-usb.c]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/Makefile               |    4 --
 arch/arm/mach-omap2/cm.h                   |    8 +++-
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 
 arch/arm/mach-omap2/usb-fs.c               |   62 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/usb.h      |    3 +
 5 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index bc2ac4f..fc2ff91 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -245,9 +245,7 @@ omap-hsmmc-$(CONFIG_MMC_OMAP_HS)	:= hsmmc.o
 obj-y					+= $(omap-hsmmc-m) $(omap-hsmmc-y)
 
 
-usbfs-$(CONFIG_ARCH_OMAP_OTG)		:= usb-fs.o
-obj-y					+= $(usbfs-m) $(usbfs-y)
-obj-y					+= usb-musb.o
+obj-y					+= usb-fs.o usb-musb.o
 obj-y					+= omap_phy_internal.o
 
 obj-$(CONFIG_MACH_OMAP2_TUSB6010)	+= usb-tusb6010.o
diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
index a7bc096..99978c7 100644
--- a/arch/arm/mach-omap2/cm.h
+++ b/arch/arm/mach-omap2/cm.h
@@ -1,7 +1,7 @@
 /*
  * OMAP2+ Clock Management prototypes
  *
- * Copyright (C) 2007-2009 Texas Instruments, Inc.
+ * Copyright (C) 2007-2012 Texas Instruments, Inc.
  * Copyright (C) 2007-2009 Nokia Corporation
  *
  * Written by Paul Walmsley
@@ -14,6 +14,12 @@
 #define __ARCH_ASM_MACH_OMAP2_CM_H
 
 /*
+ * MAX_MODULE_SOFTRESET_TIME: maximum time in microseconds to wait for
+ * an IP block to finish an OCP SOFTRESET.
+ */
+#define MAX_MODULE_SOFTRESET_WAIT	10000
+
+/*
  * MAX_MODULE_READY_TIME: max duration in microseconds to wait for the
  * PRCM to request that a module exit the inactive state in the case of
  * OMAP2 & 3.
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index a93ce48..02daacc 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -3314,6 +3314,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_usb_host_fs_sysc = {
 static struct omap_hwmod_class omap44xx_usb_host_fs_hwmod_class = {
 	.name	= "usb_host_fs",
 	.sysc	= &omap44xx_usb_host_fs_sysc,
+	.reset	= omap_usb_host_fs_reset,
 };
 
 /* usb_host_fs */
diff --git a/arch/arm/mach-omap2/usb-fs.c b/arch/arm/mach-omap2/usb-fs.c
index 1481078..4faf0f7 100644
--- a/arch/arm/mach-omap2/usb-fs.c
+++ b/arch/arm/mach-omap2/usb-fs.c
@@ -1,7 +1,7 @@
 /*
  * Platform level USB initialization for FS USB OTG controller on omap1 and 24xx
  *
- * Copyright (C) 2004 Texas Instruments, Inc.
+ * Copyright (C) 2004, 2012 Texas Instruments, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -32,6 +32,8 @@
 #include <plat/usb.h>
 #include <plat/board.h>
 
+#include "cm.h"
+#include "common.h"
 #include "control.h"
 #include "mux.h"
 
@@ -41,6 +43,64 @@
 #define INT_USB_IRQ_HGEN	INT_24XX_USB_IRQ_HGEN
 #define INT_USB_IRQ_OTG		INT_24XX_USB_IRQ_OTG
 
+/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
+#define HCCOMMANDSTATUS			0x0008
+
+/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
+#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
+
+/**
+ * omap_usb_host_fs_reset - custom reset function for the FSUSB IP block
+ * @oh: struct omap_hwmod * of the usb_host_fs IP block
+ *
+ * Reset the FSUSB IP block.  This IP block requires a custom
+ * two-stage reset; otherwise the IP block won't idle-ack to the PRCM.
+ * First the OCP SOFTRESET method must be used.  Next, the IP block's
+ * internal reset bit must be toggled.  This will place the OHCI
+ * controller state into UsbSuspend, which allows the IP block to
+ * idle-ack to the PRCM.  Note that the FSUSB still takes almost 4
+ * milliseconds to idle-ack after this function returns.  Returns 0
+ * upon success, -EINVAL if the IP block softreset data wasn't
+ * supplied, or -EBUSY if the IP block reset times out.
+ */
+int omap_usb_host_fs_reset(struct omap_hwmod *oh)
+{
+	int c;
+
+	if (omap_hwmod_softreset(oh))
+		return -EINVAL;
+
+	omap_test_timeout(!(omap_hwmod_read(oh, oh->class->sysc->sysc_offs)
+			   & SYSC_TYPE2_SOFTRESET_MASK),
+			  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (c == MAX_MODULE_SOFTRESET_WAIT) {
+		pr_warn("%s: %s: softreset failed (waited %d usec)\n",
+			__func__, oh->name, MAX_MODULE_SOFTRESET_WAIT);
+		return -EBUSY;
+	} else {
+		pr_debug("%s: %s: softreset in %d usec\n", __func__,
+			 oh->name, c);
+	}
+
+	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
+
+	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
+			    & HCCOMMANDSTATUS_HCR_MASK),
+			  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (c == MAX_MODULE_SOFTRESET_WAIT) {
+		pr_warn("%s: %s: host controller reset failed (waited %d usec)\n",
+			__func__, oh->name, MAX_MODULE_SOFTRESET_WAIT);
+		return -EBUSY;
+	} else {
+		pr_debug("%s: %s: host controller reset in %d usec\n", __func__,
+			 oh->name, c);
+	}
+
+	return 0;
+}
+
 #if defined(CONFIG_ARCH_OMAP2)
 
 #ifdef	CONFIG_USB_GADGET_OMAP
diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 762eeb0..ac7db50 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -6,6 +6,7 @@
 #include <linux/io.h>
 #include <linux/usb/musb.h>
 #include <plat/board.h>
+#include <plat/omap_hwmod.h>
 
 #define OMAP3_HS_USB_PORTS	3
 
@@ -181,6 +182,8 @@ static inline void omap2_usbfs_init(struct omap_usb_config *pdata)
 }
 #endif
 
+extern int omap_usb_host_fs_reset(struct omap_hwmod *oh);
+
 /*-------------------------------------------------------------------------*/
 
 /*


--
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

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

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

Add a custom reset function for the usb_host_fs/fsusb IP block, and
connect it to the OMAP4 FSUSB block.

This is the first of two fixes required to get rid of the boot
warning:

omap_hwmod: usb_host_fs: _wait_target_disable failed

and to allow the module to idle.

It may be necessary to use this reset method for OMAP2xxx SoCs as
well; this is left for a future patch.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
[paul at pwsan.com: rewrote the custom reset function, documented it and
 updated the commit message, and moved the code to mach-omap2/fs-usb.c]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: Felipe Balbi <balbi@ti.com>
---
 arch/arm/mach-omap2/Makefile               |    4 --
 arch/arm/mach-omap2/cm.h                   |    8 +++-
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 
 arch/arm/mach-omap2/usb-fs.c               |   62 ++++++++++++++++++++++++++++
 arch/arm/plat-omap/include/plat/usb.h      |    3 +
 5 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index bc2ac4f..fc2ff91 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -245,9 +245,7 @@ omap-hsmmc-$(CONFIG_MMC_OMAP_HS)	:= hsmmc.o
 obj-y					+= $(omap-hsmmc-m) $(omap-hsmmc-y)
 
 
-usbfs-$(CONFIG_ARCH_OMAP_OTG)		:= usb-fs.o
-obj-y					+= $(usbfs-m) $(usbfs-y)
-obj-y					+= usb-musb.o
+obj-y					+= usb-fs.o usb-musb.o
 obj-y					+= omap_phy_internal.o
 
 obj-$(CONFIG_MACH_OMAP2_TUSB6010)	+= usb-tusb6010.o
diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
index a7bc096..99978c7 100644
--- a/arch/arm/mach-omap2/cm.h
+++ b/arch/arm/mach-omap2/cm.h
@@ -1,7 +1,7 @@
 /*
  * OMAP2+ Clock Management prototypes
  *
- * Copyright (C) 2007-2009 Texas Instruments, Inc.
+ * Copyright (C) 2007-2012 Texas Instruments, Inc.
  * Copyright (C) 2007-2009 Nokia Corporation
  *
  * Written by Paul Walmsley
@@ -14,6 +14,12 @@
 #define __ARCH_ASM_MACH_OMAP2_CM_H
 
 /*
+ * MAX_MODULE_SOFTRESET_TIME: maximum time in microseconds to wait for
+ * an IP block to finish an OCP SOFTRESET.
+ */
+#define MAX_MODULE_SOFTRESET_WAIT	10000
+
+/*
  * MAX_MODULE_READY_TIME: max duration in microseconds to wait for the
  * PRCM to request that a module exit the inactive state in the case of
  * OMAP2 & 3.
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index a93ce48..02daacc 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -3314,6 +3314,7 @@ static struct omap_hwmod_class_sysconfig omap44xx_usb_host_fs_sysc = {
 static struct omap_hwmod_class omap44xx_usb_host_fs_hwmod_class = {
 	.name	= "usb_host_fs",
 	.sysc	= &omap44xx_usb_host_fs_sysc,
+	.reset	= omap_usb_host_fs_reset,
 };
 
 /* usb_host_fs */
diff --git a/arch/arm/mach-omap2/usb-fs.c b/arch/arm/mach-omap2/usb-fs.c
index 1481078..4faf0f7 100644
--- a/arch/arm/mach-omap2/usb-fs.c
+++ b/arch/arm/mach-omap2/usb-fs.c
@@ -1,7 +1,7 @@
 /*
  * Platform level USB initialization for FS USB OTG controller on omap1 and 24xx
  *
- * Copyright (C) 2004 Texas Instruments, Inc.
+ * Copyright (C) 2004, 2012 Texas Instruments, Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -32,6 +32,8 @@
 #include <plat/usb.h>
 #include <plat/board.h>
 
+#include "cm.h"
+#include "common.h"
 #include "control.h"
 #include "mux.h"
 
@@ -41,6 +43,64 @@
 #define INT_USB_IRQ_HGEN	INT_24XX_USB_IRQ_HGEN
 #define INT_USB_IRQ_OTG		INT_24XX_USB_IRQ_OTG
 
+/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
+#define HCCOMMANDSTATUS			0x0008
+
+/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
+#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
+
+/**
+ * omap_usb_host_fs_reset - custom reset function for the FSUSB IP block
+ * @oh: struct omap_hwmod * of the usb_host_fs IP block
+ *
+ * Reset the FSUSB IP block.  This IP block requires a custom
+ * two-stage reset; otherwise the IP block won't idle-ack to the PRCM.
+ * First the OCP SOFTRESET method must be used.  Next, the IP block's
+ * internal reset bit must be toggled.  This will place the OHCI
+ * controller state into UsbSuspend, which allows the IP block to
+ * idle-ack to the PRCM.  Note that the FSUSB still takes almost 4
+ * milliseconds to idle-ack after this function returns.  Returns 0
+ * upon success, -EINVAL if the IP block softreset data wasn't
+ * supplied, or -EBUSY if the IP block reset times out.
+ */
+int omap_usb_host_fs_reset(struct omap_hwmod *oh)
+{
+	int c;
+
+	if (omap_hwmod_softreset(oh))
+		return -EINVAL;
+
+	omap_test_timeout(!(omap_hwmod_read(oh, oh->class->sysc->sysc_offs)
+			   & SYSC_TYPE2_SOFTRESET_MASK),
+			  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (c == MAX_MODULE_SOFTRESET_WAIT) {
+		pr_warn("%s: %s: softreset failed (waited %d usec)\n",
+			__func__, oh->name, MAX_MODULE_SOFTRESET_WAIT);
+		return -EBUSY;
+	} else {
+		pr_debug("%s: %s: softreset in %d usec\n", __func__,
+			 oh->name, c);
+	}
+
+	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
+
+	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
+			    & HCCOMMANDSTATUS_HCR_MASK),
+			  MAX_MODULE_SOFTRESET_WAIT, c);
+
+	if (c == MAX_MODULE_SOFTRESET_WAIT) {
+		pr_warn("%s: %s: host controller reset failed (waited %d usec)\n",
+			__func__, oh->name, MAX_MODULE_SOFTRESET_WAIT);
+		return -EBUSY;
+	} else {
+		pr_debug("%s: %s: host controller reset in %d usec\n", __func__,
+			 oh->name, c);
+	}
+
+	return 0;
+}
+
 #if defined(CONFIG_ARCH_OMAP2)
 
 #ifdef	CONFIG_USB_GADGET_OMAP
diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 762eeb0..ac7db50 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -6,6 +6,7 @@
 #include <linux/io.h>
 #include <linux/usb/musb.h>
 #include <plat/board.h>
+#include <plat/omap_hwmod.h>
 
 #define OMAP3_HS_USB_PORTS	3
 
@@ -181,6 +182,8 @@ static inline void omap2_usbfs_init(struct omap_usb_config *pdata)
 }
 #endif
 
+extern int omap_usb_host_fs_reset(struct omap_hwmod *oh);
+
 /*-------------------------------------------------------------------------*/
 
 /*

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Kevin discovered that commit c8d82ff68fb6873691536cf33021977efbf5593c
("ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod
database") broke CORE idle on OMAP3.  This prevents device low power
states.

The root cause is that the 32K sync timer IP block does not support
smart-idle mode[1], and so the hwmod code keeps the IP block in
no-idle mode while it is active.  This in turn prevents the WKUP
clockdomain from transitioning to idle.  There is a hardcoded sleep
dependency that prevents the CORE_L3 and CORE_CM clockdomains from
transitioning to idle when the WKUP clockdomain is active[2], so the
chip cannot enter any device low power states.

It turns out that there is no need to take the 32k sync timer out of
idle.  The IP block itself probably does not have any native idle
handling at all, due to its simplicity.  Furthermore, the PRCM will
never request target idle for this IP block while the kernel is
running, due to the sleep dependency that prevents the WKUP
clockdomain from idling while the CORE_L3 clockdomain is active.  So
we can safely leave the 32k sync timer in target-no-idle mode, even
while we continue to access it.

This workaround is implemented by programming the force-idle mode for
any IP block that only supports the force-idle and no-idle modes.  If
an IP block is ever released that doesn't support smart-idle and
requires no-idle mode to be programmed while it's in use, we'll have
to change this behavior.

Another theoretically clean fix for this problem would be to implement
PM runtime-based control for 32k sync timer accesses.  These PM
runtime calls would need to located in a custom clocksource, since the
32k sync timer is currently used as an MMIO clocksource.  But in
practice, there would be little benefit to doing so; and there would
be some cost, due to the addition of unnecessary lines of code and the
additional CPU overhead of the PM runtime and hwmod code - unnecessary
in this case.

Another possible fix would have been to modify the pm34xx.c code to
force the IP block idle before entering WFI.  But this would not have
been an acceptable approach: we are trying to remove this type of
centralized IP block idle control from the PM code.

This patch is effectively a workaround for a hardware problem.  A
better hardware approach would have been to implement a smart-idle
target idle mode for this IP block.  The smart-idle mode in this case
would behave identically to the force-idle mode.  We consider the
force-idle and no-idle target idle mode settings to be intended for
debugging and automatic idle management bug workarounds only[4].

This patch is a collaboration between Kevin Hilman <khilman@ti.com>
and Paul Walmsley <paul@pwsan.com>.

References:

1. Table 16-96 "REG_32KSYNCNT_SYSCONFIG" of the OMAP34xx TRM Rev. ZU
   (SWPU223U), available from:
   http://www.ti.com/pdfs/wtbu/OMAP34x_ES3.1.x_PUBLIC_TRM_vzU.zip

2. Table 4-72 "Sleep Dependencies" of the OMAP34xx TRM Rev. ZU
   (SWPU223U)

3. ibid.

4. Section 3.1.1.1.2 "Module-Level Clock Management" of The
   OMAP4430 TRM Rev. vAA (SWPU231AA).

Cc: Tony Lindgren <tony@atomide.com>
Cc: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Benoît Cousson <b-cousson@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index b0d3064..6b4ae31 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1124,10 +1124,12 @@ static struct omap_hwmod_addr_space * __init _find_mpu_rt_addr_space(struct omap
  * _enable_sysc - try to bring a module out of idle via OCP_SYSCONFIG
  * @oh: struct omap_hwmod *
  *
- * If module is marked as SWSUP_SIDLE, force the module out of slave
- * idle; otherwise, configure it for smart-idle.  If module is marked
- * as SWSUP_MSUSPEND, force the module out of master standby;
- * otherwise, configure it for smart-standby.  No return value.
+ * Ensure that the OCP_SYSCONFIG register for the IP block represented
+ * by @oh is set to indicate to the PRCM that the IP block is active.
+ * Usually this means placing the module into smart-idle mode and
+ * smart-standby, but if there is a bug in the automatic idle handling
+ * for the IP block, it may need to be placed into the force-idle or
+ * no-idle variants of these modes.  No return value.
  */
 static void _enable_sysc(struct omap_hwmod *oh)
 {
@@ -1141,8 +1143,26 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	sf = oh->class->sysc->sysc_flags;
 
 	if (sf & SYSC_HAS_SIDLEMODE) {
-		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
-			HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
+		if (oh->flags & HWMOD_SWSUP_SIDLE) {
+			/*
+			 * IP blocks without smart idle should be left
+			 * in force-idle.  Currently this only applies
+			 * to 32k sync "timer" which is guaranteed to
+			 * be accessible when the kernel is running.
+			 * HWMOD_SWSUP_IDLE must also be set on these
+			 * IP blocks to indicate a hardware problem.
+			 * XXX Not an ideal workaround.
+			 */
+			if (oh->class->sysc->idlemodes &
+			    (SIDLE_NO | SIDLE_FORCE) &&
+			    !(oh->class->sysc->idlemodes &
+			      (SIDLE_SMART || SIDLE_SMART_WKUP)))
+				idlemode = HWMOD_IDLEMODE_FORCE;
+			else
+				idlemode = HWMOD_IDLEMODE_NO;
+		} else {
+			idlemode = HWMOD_IDLEMODE_SMART;
+		}
 		_set_slave_idlemode(oh, idlemode, &v);
 	}
 


--
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

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Kevin discovered that commit c8d82ff68fb6873691536cf33021977efbf5593c
("ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod
database") broke CORE idle on OMAP3.  This prevents device low power
states.

The root cause is that the 32K sync timer IP block does not support
smart-idle mode[1], and so the hwmod code keeps the IP block in
no-idle mode while it is active.  This in turn prevents the WKUP
clockdomain from transitioning to idle.  There is a hardcoded sleep
dependency that prevents the CORE_L3 and CORE_CM clockdomains from
transitioning to idle when the WKUP clockdomain is active[2], so the
chip cannot enter any device low power states.

It turns out that there is no need to take the 32k sync timer out of
idle.  The IP block itself probably does not have any native idle
handling at all, due to its simplicity.  Furthermore, the PRCM will
never request target idle for this IP block while the kernel is
running, due to the sleep dependency that prevents the WKUP
clockdomain from idling while the CORE_L3 clockdomain is active.  So
we can safely leave the 32k sync timer in target-no-idle mode, even
while we continue to access it.

This workaround is implemented by programming the force-idle mode for
any IP block that only supports the force-idle and no-idle modes.  If
an IP block is ever released that doesn't support smart-idle and
requires no-idle mode to be programmed while it's in use, we'll have
to change this behavior.

Another theoretically clean fix for this problem would be to implement
PM runtime-based control for 32k sync timer accesses.  These PM
runtime calls would need to located in a custom clocksource, since the
32k sync timer is currently used as an MMIO clocksource.  But in
practice, there would be little benefit to doing so; and there would
be some cost, due to the addition of unnecessary lines of code and the
additional CPU overhead of the PM runtime and hwmod code - unnecessary
in this case.

Another possible fix would have been to modify the pm34xx.c code to
force the IP block idle before entering WFI.  But this would not have
been an acceptable approach: we are trying to remove this type of
centralized IP block idle control from the PM code.

This patch is effectively a workaround for a hardware problem.  A
better hardware approach would have been to implement a smart-idle
target idle mode for this IP block.  The smart-idle mode in this case
would behave identically to the force-idle mode.  We consider the
force-idle and no-idle target idle mode settings to be intended for
debugging and automatic idle management bug workarounds only[4].

This patch is a collaboration between Kevin Hilman <khilman@ti.com>
and Paul Walmsley <paul@pwsan.com>.

References:

1. Table 16-96 "REG_32KSYNCNT_SYSCONFIG" of the OMAP34xx TRM Rev. ZU
   (SWPU223U), available from:
   http://www.ti.com/pdfs/wtbu/OMAP34x_ES3.1.x_PUBLIC_TRM_vzU.zip

2. Table 4-72 "Sleep Dependencies" of the OMAP34xx TRM Rev. ZU
   (SWPU223U)

3. ibid.

4. Section 3.1.1.1.2 "Module-Level Clock Management" of The
   OMAP4430 TRM Rev. vAA (SWPU231AA).

Cc: Tony Lindgren <tony@atomide.com>
Cc: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
Tested-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Kevin Hilman <khilman@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |   32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index b0d3064..6b4ae31 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -1124,10 +1124,12 @@ static struct omap_hwmod_addr_space * __init _find_mpu_rt_addr_space(struct omap
  * _enable_sysc - try to bring a module out of idle via OCP_SYSCONFIG
  * @oh: struct omap_hwmod *
  *
- * If module is marked as SWSUP_SIDLE, force the module out of slave
- * idle; otherwise, configure it for smart-idle.  If module is marked
- * as SWSUP_MSUSPEND, force the module out of master standby;
- * otherwise, configure it for smart-standby.  No return value.
+ * Ensure that the OCP_SYSCONFIG register for the IP block represented
+ * by @oh is set to indicate to the PRCM that the IP block is active.
+ * Usually this means placing the module into smart-idle mode and
+ * smart-standby, but if there is a bug in the automatic idle handling
+ * for the IP block, it may need to be placed into the force-idle or
+ * no-idle variants of these modes.  No return value.
  */
 static void _enable_sysc(struct omap_hwmod *oh)
 {
@@ -1141,8 +1143,26 @@ static void _enable_sysc(struct omap_hwmod *oh)
 	sf = oh->class->sysc->sysc_flags;
 
 	if (sf & SYSC_HAS_SIDLEMODE) {
-		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
-			HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
+		if (oh->flags & HWMOD_SWSUP_SIDLE) {
+			/*
+			 * IP blocks without smart idle should be left
+			 * in force-idle.  Currently this only applies
+			 * to 32k sync "timer" which is guaranteed to
+			 * be accessible when the kernel is running.
+			 * HWMOD_SWSUP_IDLE must also be set on these
+			 * IP blocks to indicate a hardware problem.
+			 * XXX Not an ideal workaround.
+			 */
+			if (oh->class->sysc->idlemodes &
+			    (SIDLE_NO | SIDLE_FORCE) &&
+			    !(oh->class->sysc->idlemodes &
+			      (SIDLE_SMART || SIDLE_SMART_WKUP)))
+				idlemode = HWMOD_IDLEMODE_FORCE;
+			else
+				idlemode = HWMOD_IDLEMODE_NO;
+		} else {
+			idlemode = HWMOD_IDLEMODE_SMART;
+		}
 		_set_slave_idlemode(oh, idlemode, &v);
 	}
 

[PATCH 06/11] ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby

[Paul Walmsley]

From: Djamil Elaidi <d-elaidi@ti.com>

If an IP is configured in Smart-Standby-Wakeup, when disabling wakeup feature the
IP will not go back to Smart-Standby, but will remain in Smart-Standby-Wakeup.

Signed-off-by: Djamil Elaidi <d-elaidi@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 6b4ae31..d3afac5 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -530,7 +530,7 @@ static int _disable_wakeup(struct omap_hwmod *oh, u32 *v)
 	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_WKUP, v);
+		_set_master_standbymode(oh, HWMOD_IDLEMODE_SMART, v);
 
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 

[PATCH 06/11] ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby

[Paul Walmsley]

From: Djamil Elaidi <d-elaidi@ti.com>

If an IP is configured in Smart-Standby-Wakeup, when disabling wakeup feature the
IP will not go back to Smart-Standby, but will remain in Smart-Standby-Wakeup.

Signed-off-by: Djamil Elaidi <d-elaidi@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 6b4ae31..d3afac5 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -530,7 +530,7 @@ static int _disable_wakeup(struct omap_hwmod *oh, u32 *v)
 	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_WKUP, v);
+		_set_master_standbymode(oh, HWMOD_IDLEMODE_SMART, v);
 
 	/* XXX test pwrdm_get_wken for this hwmod's subsystem */
 

[PATCH 07/11] ARM: OMAP: PM: Lock clocks list while generating summary

[Paul Walmsley]

From: Todd Poynor <toddpoynor@google.com>

Commit a53025724052b2b1edbc982a4a248784638f563d (OMAP: Add debugfs
node to show the summary of all clocks) introduced clock summary,
however, we are interested in seeing snapshot of the clock state, not
in dynamically changing clock configurations as the data provided by
clock summary will then be useless for debugging configuration
issues. So, hold the common lock when dumping the clock summary.

Cc: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
[nm@ti.com: added commit message]
Signed-off-by: Nishanth Menon <nm@ti.com>
[paul@pwsan.com: minor edits to commit message]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/clock.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 62ec5c4..706b7e2 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -461,6 +461,7 @@ static int clk_dbg_show_summary(struct seq_file *s, void *unused)
 	struct clk *c;
 	struct clk *pa;
 
+	mutex_lock(&clocks_mutex);
 	seq_printf(s, "%-30s %-30s %-10s %s\n",
 		"clock-name", "parent-name", "rate", "use-count");
 
@@ -469,6 +470,7 @@ static int clk_dbg_show_summary(struct seq_file *s, void *unused)
 		seq_printf(s, "%-30s %-30s %-10lu %d\n",
 			c->name, pa ? pa->name : "none", c->rate, c->usecount);
 	}
+	mutex_unlock(&clocks_mutex);
 
 	return 0;
 }

[PATCH 07/11] ARM: OMAP: PM: Lock clocks list while generating summary

[Paul Walmsley]

From: Todd Poynor <toddpoynor@google.com>

Commit a53025724052b2b1edbc982a4a248784638f563d (OMAP: Add debugfs
node to show the summary of all clocks) introduced clock summary,
however, we are interested in seeing snapshot of the clock state, not
in dynamically changing clock configurations as the data provided by
clock summary will then be useless for debugging configuration
issues. So, hold the common lock when dumping the clock summary.

Cc: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
[nm at ti.com: added commit message]
Signed-off-by: Nishanth Menon <nm@ti.com>
[paul at pwsan.com: minor edits to commit message]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/plat-omap/clock.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index 62ec5c4..706b7e2 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -461,6 +461,7 @@ static int clk_dbg_show_summary(struct seq_file *s, void *unused)
 	struct clk *c;
 	struct clk *pa;
 
+	mutex_lock(&clocks_mutex);
 	seq_printf(s, "%-30s %-30s %-10s %s\n",
 		"clock-name", "parent-name", "rate", "use-count");
 
@@ -469,6 +470,7 @@ static int clk_dbg_show_summary(struct seq_file *s, void *unused)
 		seq_printf(s, "%-30s %-30s %-10lu %d\n",
 			c->name, pa ? pa->name : "none", c->rate, c->usecount);
 	}
+	mutex_unlock(&clocks_mutex);
 
 	return 0;
 }

[PATCH 08/11] ARM: OMAP2+: CM: increase the module disable timeout

[Paul Walmsley]

Increase the timeout for disabling an IP block to five milliseconds.
This is to handle the usb_host_fs idle latency, which takes almost
four milliseconds after a host controller reset.

This is the second of two patches needed to resolve the following
boot warning:

omap_hwmod: usb_host_fs: _wait_target_disable failed

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/cm.h                   |   11 +++++++++++
 arch/arm/mach-omap2/cminst44xx.c           |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
index 99978c7..cf67617 100644
--- a/arch/arm/mach-omap2/cm.h
+++ b/arch/arm/mach-omap2/cm.h
@@ -28,4 +28,15 @@
  */
 #define MAX_MODULE_READY_TIME		2000
 
+/*
+ * MAX_MODULE_DISABLE_TIME: max duration in microseconds to wait for
+ * the PRCM to request that a module enter the inactive state in the
+ * case of OMAP2 & 3.  In the case of OMAP4 this is the max duration
+ * in microseconds for the module to reach the inactive state from
+ * a functional state.
+ * XXX FSUSB on OMAP4430 takes ~4ms to idle after reset during
+ * kernel init.
+ */
+#define MAX_MODULE_DISABLE_TIME		5000
+
 #endif
diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 8c86d29..1a39945 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -313,9 +313,9 @@ int omap4_cminst_wait_module_idle(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_off
 
 	omap_test_timeout((_clkctrl_idlest(part, inst, cdoffs, clkctrl_offs) ==
 			   CLKCTRL_IDLEST_DISABLED),
-			  MAX_MODULE_READY_TIME, i);
+			  MAX_MODULE_DISABLE_TIME, i);
 
-	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
+	return (i < MAX_MODULE_DISABLE_TIME) ? 0 : -EBUSY;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 02daacc..20e45a6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -30,6 +30,7 @@
 #include <plat/mmc.h>
 #include <plat/dmtimer.h>
 #include <plat/common.h>
+#include <plat/usb.h>
 
 #include "omap_hwmod_common_data.h"
 

[PATCH 08/11] ARM: OMAP2+: CM: increase the module disable timeout

[Paul Walmsley]

Increase the timeout for disabling an IP block to five milliseconds.
This is to handle the usb_host_fs idle latency, which takes almost
four milliseconds after a host controller reset.

This is the second of two patches needed to resolve the following
boot warning:

omap_hwmod: usb_host_fs: _wait_target_disable failed

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/cm.h                   |   11 +++++++++++
 arch/arm/mach-omap2/cminst44xx.c           |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |    1 +
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/cm.h b/arch/arm/mach-omap2/cm.h
index 99978c7..cf67617 100644
--- a/arch/arm/mach-omap2/cm.h
+++ b/arch/arm/mach-omap2/cm.h
@@ -28,4 +28,15 @@
  */
 #define MAX_MODULE_READY_TIME		2000
 
+/*
+ * MAX_MODULE_DISABLE_TIME: max duration in microseconds to wait for
+ * the PRCM to request that a module enter the inactive state in the
+ * case of OMAP2 & 3.  In the case of OMAP4 this is the max duration
+ * in microseconds for the module to reach the inactive state from
+ * a functional state.
+ * XXX FSUSB on OMAP4430 takes ~4ms to idle after reset during
+ * kernel init.
+ */
+#define MAX_MODULE_DISABLE_TIME		5000
+
 #endif
diff --git a/arch/arm/mach-omap2/cminst44xx.c b/arch/arm/mach-omap2/cminst44xx.c
index 8c86d29..1a39945 100644
--- a/arch/arm/mach-omap2/cminst44xx.c
+++ b/arch/arm/mach-omap2/cminst44xx.c
@@ -313,9 +313,9 @@ int omap4_cminst_wait_module_idle(u8 part, u16 inst, s16 cdoffs, u16 clkctrl_off
 
 	omap_test_timeout((_clkctrl_idlest(part, inst, cdoffs, clkctrl_offs) ==
 			   CLKCTRL_IDLEST_DISABLED),
-			  MAX_MODULE_READY_TIME, i);
+			  MAX_MODULE_DISABLE_TIME, i);
 
-	return (i < MAX_MODULE_READY_TIME) ? 0 : -EBUSY;
+	return (i < MAX_MODULE_DISABLE_TIME) ? 0 : -EBUSY;
 }
 
 /**
diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 02daacc..20e45a6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -30,6 +30,7 @@
 #include <plat/mmc.h>
 #include <plat/dmtimer.h>
 #include <plat/common.h>
+#include <plat/usb.h>
 
 #include "omap_hwmod_common_data.h"
 

[PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Paul Walmsley]

Until the OMAP4 code is converted to disable the use of the clock
framework-based clockdomain enable/disable sequence, any clock used as
a hwmod main_clk must have a clockdomain associated with it.  This
patch populates some clock structure clockdomain names to resolve the
following warnings during kernel init:

omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck.
omap_hwmod: trace_clk_div_ck: missing clockdomain for trace_clk_div_ck.
omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck.
omap_hwmod: ddrphy_ck: missing clockdomain for ddrphy_ck.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/clock44xx_data.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 2172f66..e2b701e 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -84,6 +84,7 @@ static struct clk slimbus_clk = {
 
 static struct clk sys_32k_ck = {
 	.name		= "sys_32k_ck",
+	.clkdm_name	= "prm_clkdm",
 	.rate		= 32768,
 	.ops		= &clkops_null,
 };
@@ -512,6 +513,7 @@ static struct clk ddrphy_ck = {
 	.name		= "ddrphy_ck",
 	.parent		= &dpll_core_m2_ck,
 	.ops		= &clkops_null,
+	.clkdm_name	= "l3_emif_clkdm",
 	.fixed_div	= 2,
 	.recalc		= &omap_fixed_divisor_recalc,
 };
@@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] = {
 static struct clk dpll_mpu_m2_ck = {
 	.name		= "dpll_mpu_m2_ck",
 	.parent		= &dpll_mpu_ck,
+	.clkdm_name	= "cm_clkdm",
 	.clksel		= dpll_mpu_m2_div,
 	.clksel_reg	= OMAP4430_CM_DIV_M2_DPLL_MPU,
 	.clksel_mask	= OMAP4430_DPLL_CLKOUT_DIV_MASK,
@@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] = {
 static struct clk l3_div_ck = {
 	.name		= "l3_div_ck",
 	.parent		= &div_core_ck,
+	.clkdm_name	= "cm_clkdm",
 	.clksel		= l3_div_div,
 	.clksel_reg	= OMAP4430_CM_CLKSEL_CORE,
 	.clksel_mask	= OMAP4430_CLKSEL_L3_MASK,
@@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] = {
 static struct clk trace_clk_div_ck = {
 	.name		= "trace_clk_div_ck",
 	.parent		= &pmd_trace_clk_mux_ck,
+	.clkdm_name	= "emu_sys_clkdm",
 	.clksel		= trace_clk_div_div,
 	.clksel_reg	= OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
 	.clksel_mask	= OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,


--
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

[PATCH 10/11] ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init

[Paul Walmsley]

Add HWMOD_EXT_OPT_MAIN_CLK flag to indicate that this IP block is
dependent on an off-chip functional clock that is not guaranteed to be
present during initialization.  IP blocks marked with this flag are
left in the INITIALIZED state during kernel init.

This is a workaround for a hardware problem.  It should be possible to
guarantee that at least one clock source will be present and active
for any IP block's main functional clock.  This ensures that the hwmod
code can enable and reset the IP block.  Resetting the IP block during
kernel init prevents any bogus bootloader, ROM code, or previous OS
configuration from affecting the kernel.  Hopefully a clock
multiplexer can be added on future SoCs.

N.B., at some point in the future, it should be possible to query the
clock framework for this type of information.  Then this flag should
no longer be needed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |    3 +++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index d3afac5..c8655c0 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2155,6 +2155,9 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
+	if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK)
+		return -EPERM;
+
 	if (oh->rst_lines_cnt == 0) {
 		r = _enable(oh);
 		if (r) {
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index fdc4b2a..d117931 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -409,6 +409,11 @@ struct omap_hwmod_omap4_prcm {
  *     in order to complete the reset. Optional clocks will be disabled
  *     again after the reset.
  * HWMOD_16BIT_REG: Module has 16bit registers
+ * HWMOD_EXT_OPT_MAIN_CLK: The only main functional clock source for
+ *     this IP block comes from an off-chip source and is not always
+ *     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.
  */
 #define HWMOD_SWSUP_SIDLE			(1 << 0)
 #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
@@ -419,6 +424,7 @@ struct omap_hwmod_omap4_prcm {
 #define HWMOD_NO_IDLEST				(1 << 6)
 #define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1 << 7)
 #define HWMOD_16BIT_REG				(1 << 8)
+#define HWMOD_EXT_OPT_MAIN_CLK			(1 << 9)
 
 /*
  * omap_hwmod._int_flags definitions


--
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

[PATCH 10/11] ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init

[Paul Walmsley]

Add HWMOD_EXT_OPT_MAIN_CLK flag to indicate that this IP block is
dependent on an off-chip functional clock that is not guaranteed to be
present during initialization.  IP blocks marked with this flag are
left in the INITIALIZED state during kernel init.

This is a workaround for a hardware problem.  It should be possible to
guarantee that at least one clock source will be present and active
for any IP block's main functional clock.  This ensures that the hwmod
code can enable and reset the IP block.  Resetting the IP block during
kernel init prevents any bogus bootloader, ROM code, or previous OS
configuration from affecting the kernel.  Hopefully a clock
multiplexer can be added on future SoCs.

N.B., at some point in the future, it should be possible to query the
clock framework for this type of information.  Then this flag should
no longer be needed.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c             |    3 +++
 arch/arm/plat-omap/include/plat/omap_hwmod.h |    6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index d3afac5..c8655c0 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -2155,6 +2155,9 @@ static int __init _setup_reset(struct omap_hwmod *oh)
 	if (oh->_state != _HWMOD_STATE_INITIALIZED)
 		return -EINVAL;
 
+	if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK)
+		return -EPERM;
+
 	if (oh->rst_lines_cnt == 0) {
 		r = _enable(oh);
 		if (r) {
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/plat-omap/include/plat/omap_hwmod.h
index fdc4b2a..d117931 100644
--- a/arch/arm/plat-omap/include/plat/omap_hwmod.h
+++ b/arch/arm/plat-omap/include/plat/omap_hwmod.h
@@ -409,6 +409,11 @@ struct omap_hwmod_omap4_prcm {
  *     in order to complete the reset. Optional clocks will be disabled
  *     again after the reset.
  * HWMOD_16BIT_REG: Module has 16bit registers
+ * HWMOD_EXT_OPT_MAIN_CLK: The only main functional clock source for
+ *     this IP block comes from an off-chip source and is not always
+ *     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.
  */
 #define HWMOD_SWSUP_SIDLE			(1 << 0)
 #define HWMOD_SWSUP_MSTANDBY			(1 << 1)
@@ -419,6 +424,7 @@ struct omap_hwmod_omap4_prcm {
 #define HWMOD_NO_IDLEST				(1 << 6)
 #define HWMOD_CONTROL_OPT_CLKS_IN_RESET		(1 << 7)
 #define HWMOD_16BIT_REG				(1 << 8)
+#define HWMOD_EXT_OPT_MAIN_CLK			(1 << 9)
 
 /*
  * omap_hwmod._int_flags definitions

[PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Paul Walmsley]

Until the OMAP4 code is converted to disable the use of the clock
framework-based clockdomain enable/disable sequence, any clock used as
a hwmod main_clk must have a clockdomain associated with it.  This
patch populates some clock structure clockdomain names to resolve the
following warnings during kernel init:

omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck.
omap_hwmod: trace_clk_div_ck: missing clockdomain for trace_clk_div_ck.
omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck.
omap_hwmod: ddrphy_ck: missing clockdomain for ddrphy_ck.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Rajendra Nayak <rnayak@ti.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/clock44xx_data.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
index 2172f66..e2b701e 100644
--- a/arch/arm/mach-omap2/clock44xx_data.c
+++ b/arch/arm/mach-omap2/clock44xx_data.c
@@ -84,6 +84,7 @@ static struct clk slimbus_clk = {
 
 static struct clk sys_32k_ck = {
 	.name		= "sys_32k_ck",
+	.clkdm_name	= "prm_clkdm",
 	.rate		= 32768,
 	.ops		= &clkops_null,
 };
@@ -512,6 +513,7 @@ static struct clk ddrphy_ck = {
 	.name		= "ddrphy_ck",
 	.parent		= &dpll_core_m2_ck,
 	.ops		= &clkops_null,
+	.clkdm_name	= "l3_emif_clkdm",
 	.fixed_div	= 2,
 	.recalc		= &omap_fixed_divisor_recalc,
 };
@@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] = {
 static struct clk dpll_mpu_m2_ck = {
 	.name		= "dpll_mpu_m2_ck",
 	.parent		= &dpll_mpu_ck,
+	.clkdm_name	= "cm_clkdm",
 	.clksel		= dpll_mpu_m2_div,
 	.clksel_reg	= OMAP4430_CM_DIV_M2_DPLL_MPU,
 	.clksel_mask	= OMAP4430_DPLL_CLKOUT_DIV_MASK,
@@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] = {
 static struct clk l3_div_ck = {
 	.name		= "l3_div_ck",
 	.parent		= &div_core_ck,
+	.clkdm_name	= "cm_clkdm",
 	.clksel		= l3_div_div,
 	.clksel_reg	= OMAP4430_CM_CLKSEL_CORE,
 	.clksel_mask	= OMAP4430_CLKSEL_L3_MASK,
@@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] = {
 static struct clk trace_clk_div_ck = {
 	.name		= "trace_clk_div_ck",
 	.parent		= &pmd_trace_clk_mux_ck,
+	.clkdm_name	= "emu_sys_clkdm",
 	.clksel		= trace_clk_div_div,
 	.clksel_reg	= OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
 	.clksel_mask	= OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,

[PATCH 11/11] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init

[Paul Walmsley]

Resolve this kernel boot message:

omap_hwmod: mcpdm: cannot be enabled for reset (3)

It appears that the McPDM on OMAP4 can only receive its functional
clock from an off-chip source.  This source is not guaranteed to be
present on the board, and when present, it is controlled by I2C.  This
would introduce a board dependency to the early hwmod code which it
was not designed to handle.  Also, neither the driver for this
off-chip clock provider nor the I2C code is available early in boot
when the hwmod code is attempting to enable and reset IP blocks.  This
effectively makes it impossible to enable and reset this device during
hwmod init.

At its core, this patch is a workaround for an OMAP hardware problem.
It should be possible to configure the OMAP to provide any IP block's
functional clock from an on-chip source.  (This is true for almost
every IP block on the chip.  As far as I know, McPDM is the only
exception.)  If the kernel cannot reset and configure IP blocks, it
cannot guarantee a sane SoC state.  Relying on an optional off-chip
clock also creates a board dependency which is beyond the scope of the
early hwmod code.

This patch works around the issue by marking the McPDM hwmod record
with the HWMOD_UNKNOWN_MAIN_FCLK_STATE flag.  This prevents the hwmod
code from touching the device early during boot.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Péter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Benoît Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 20e45a6..cc1310a 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2090,6 +2090,18 @@ static struct omap_hwmod omap44xx_mcpdm_hwmod = {
 	.name		= "mcpdm",
 	.class		= &omap44xx_mcpdm_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
+	/*
+	 * It's suspected that the McPDM requires an off-chip main
+	 * functional clock, controlled via I2C.  This IP block is
+	 * currently reset very early during boot, before I2C is
+	 * available, so it doesn't seem that we have any choice in
+	 * the kernel other than to avoid resetting it.  XXX This is
+	 * really a hardware issue workaround: every IP block should
+	 * be able to source its main functional clock from either
+	 * on-chip or off-chip sources.  McPDM seems to be the only
+	 * current exception.
+	 */
+	.flags		= HWMOD_EXT_OPT_MAIN_CLK,
 	.mpu_irqs	= omap44xx_mcpdm_irqs,
 	.sdma_reqs	= omap44xx_mcpdm_sdma_reqs,
 	.main_clk	= "mcpdm_fck",


--
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

[PATCH 11/11] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init

[Paul Walmsley]

Resolve this kernel boot message:

omap_hwmod: mcpdm: cannot be enabled for reset (3)

It appears that the McPDM on OMAP4 can only receive its functional
clock from an off-chip source.  This source is not guaranteed to be
present on the board, and when present, it is controlled by I2C.  This
would introduce a board dependency to the early hwmod code which it
was not designed to handle.  Also, neither the driver for this
off-chip clock provider nor the I2C code is available early in boot
when the hwmod code is attempting to enable and reset IP blocks.  This
effectively makes it impossible to enable and reset this device during
hwmod init.

At its core, this patch is a workaround for an OMAP hardware problem.
It should be possible to configure the OMAP to provide any IP block's
functional clock from an on-chip source.  (This is true for almost
every IP block on the chip.  As far as I know, McPDM is the only
exception.)  If the kernel cannot reset and configure IP blocks, it
cannot guarantee a sane SoC state.  Relying on an optional off-chip
clock also creates a board dependency which is beyond the scope of the
early hwmod code.

This patch works around the issue by marking the McPDM hwmod record
with the HWMOD_UNKNOWN_MAIN_FCLK_STATE flag.  This prevents the hwmod
code from touching the device early during boot.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: P?ter Ujfalusi <peter.ujfalusi@ti.com>
Cc: Beno?t Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 20e45a6..cc1310a 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -2090,6 +2090,18 @@ static struct omap_hwmod omap44xx_mcpdm_hwmod = {
 	.name		= "mcpdm",
 	.class		= &omap44xx_mcpdm_hwmod_class,
 	.clkdm_name	= "abe_clkdm",
+	/*
+	 * It's suspected that the McPDM requires an off-chip main
+	 * functional clock, controlled via I2C.  This IP block is
+	 * currently reset very early during boot, before I2C is
+	 * available, so it doesn't seem that we have any choice in
+	 * the kernel other than to avoid resetting it.  XXX This is
+	 * really a hardware issue workaround: every IP block should
+	 * be able to source its main functional clock from either
+	 * on-chip or off-chip sources.  McPDM seems to be the only
+	 * current exception.
+	 */
+	.flags		= HWMOD_EXT_OPT_MAIN_CLK,
 	.mpu_irqs	= omap44xx_mcpdm_irqs,
 	.sdma_reqs	= omap44xx_mcpdm_sdma_reqs,
 	.main_clk	= "mcpdm_fck",

Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Rajendra Nayak]

On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
> Until the OMAP4 code is converted to disable the use of the clock
> framework-based clockdomain enable/disable sequence, any clock used as
> a hwmod main_clk must have a clockdomain associated with it.  This
> patch populates some clock structure clockdomain names to resolve the
> following warnings during kernel init:

But these associations are useless if the clock is not a 'gate' clock,
except for getting rid of these warnings.
Maybe we should make hwmod understand that not all clocks need to have
a clockdomain associated with it and stop complaining.

>
> omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck.
> omap_hwmod: trace_clk_div_ck: missing clockdomain for trace_clk_div_ck.
> omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck.
> omap_hwmod: ddrphy_ck: missing clockdomain for ddrphy_ck.
>
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> Cc: Rajendra Nayak<rnayak@ti.com>
> Cc: Benoît Cousson<b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/clock44xx_data.c |    5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
> index 2172f66..e2b701e 100644
> --- a/arch/arm/mach-omap2/clock44xx_data.c
> +++ b/arch/arm/mach-omap2/clock44xx_data.c
> @@ -84,6 +84,7 @@ static struct clk slimbus_clk = {
>
>   static struct clk sys_32k_ck = {
>   	.name		= "sys_32k_ck",
> +	.clkdm_name	= "prm_clkdm",
>   	.rate		= 32768,
>   	.ops		=&clkops_null,
>   };
> @@ -512,6 +513,7 @@ static struct clk ddrphy_ck = {
>   	.name		= "ddrphy_ck",
>   	.parent		=&dpll_core_m2_ck,
>   	.ops		=&clkops_null,
> +	.clkdm_name	= "l3_emif_clkdm",
>   	.fixed_div	= 2,
>   	.recalc		=&omap_fixed_divisor_recalc,
>   };
> @@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] = {
>   static struct clk dpll_mpu_m2_ck = {
>   	.name		= "dpll_mpu_m2_ck",
>   	.parent		=&dpll_mpu_ck,
> +	.clkdm_name	= "cm_clkdm",
>   	.clksel		= dpll_mpu_m2_div,
>   	.clksel_reg	= OMAP4430_CM_DIV_M2_DPLL_MPU,
>   	.clksel_mask	= OMAP4430_DPLL_CLKOUT_DIV_MASK,
> @@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] = {
>   static struct clk l3_div_ck = {
>   	.name		= "l3_div_ck",
>   	.parent		=&div_core_ck,
> +	.clkdm_name	= "cm_clkdm",
>   	.clksel		= l3_div_div,
>   	.clksel_reg	= OMAP4430_CM_CLKSEL_CORE,
>   	.clksel_mask	= OMAP4430_CLKSEL_L3_MASK,
> @@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] = {
>   static struct clk trace_clk_div_ck = {
>   	.name		= "trace_clk_div_ck",
>   	.parent		=&pmd_trace_clk_mux_ck,
> +	.clkdm_name	= "emu_sys_clkdm",
>   	.clksel		= trace_clk_div_div,
>   	.clksel_reg	= OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
>   	.clksel_mask	= OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,
>
>

--
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

[PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Rajendra Nayak]

On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
> Until the OMAP4 code is converted to disable the use of the clock
> framework-based clockdomain enable/disable sequence, any clock used as
> a hwmod main_clk must have a clockdomain associated with it.  This
> patch populates some clock structure clockdomain names to resolve the
> following warnings during kernel init:

But these associations are useless if the clock is not a 'gate' clock,
except for getting rid of these warnings.
Maybe we should make hwmod understand that not all clocks need to have
a clockdomain associated with it and stop complaining.

>
> omap_hwmod: dpll_mpu_m2_ck: missing clockdomain for dpll_mpu_m2_ck.
> omap_hwmod: trace_clk_div_ck: missing clockdomain for trace_clk_div_ck.
> omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck.
> omap_hwmod: ddrphy_ck: missing clockdomain for ddrphy_ck.
>
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> Cc: Rajendra Nayak<rnayak@ti.com>
> Cc: Beno?t Cousson<b-cousson@ti.com>
> ---
>   arch/arm/mach-omap2/clock44xx_data.c |    5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/clock44xx_data.c b/arch/arm/mach-omap2/clock44xx_data.c
> index 2172f66..e2b701e 100644
> --- a/arch/arm/mach-omap2/clock44xx_data.c
> +++ b/arch/arm/mach-omap2/clock44xx_data.c
> @@ -84,6 +84,7 @@ static struct clk slimbus_clk = {
>
>   static struct clk sys_32k_ck = {
>   	.name		= "sys_32k_ck",
> +	.clkdm_name	= "prm_clkdm",
>   	.rate		= 32768,
>   	.ops		=&clkops_null,
>   };
> @@ -512,6 +513,7 @@ static struct clk ddrphy_ck = {
>   	.name		= "ddrphy_ck",
>   	.parent		=&dpll_core_m2_ck,
>   	.ops		=&clkops_null,
> +	.clkdm_name	= "l3_emif_clkdm",
>   	.fixed_div	= 2,
>   	.recalc		=&omap_fixed_divisor_recalc,
>   };
> @@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] = {
>   static struct clk dpll_mpu_m2_ck = {
>   	.name		= "dpll_mpu_m2_ck",
>   	.parent		=&dpll_mpu_ck,
> +	.clkdm_name	= "cm_clkdm",
>   	.clksel		= dpll_mpu_m2_div,
>   	.clksel_reg	= OMAP4430_CM_DIV_M2_DPLL_MPU,
>   	.clksel_mask	= OMAP4430_DPLL_CLKOUT_DIV_MASK,
> @@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] = {
>   static struct clk l3_div_ck = {
>   	.name		= "l3_div_ck",
>   	.parent		=&div_core_ck,
> +	.clkdm_name	= "cm_clkdm",
>   	.clksel		= l3_div_div,
>   	.clksel_reg	= OMAP4430_CM_CLKSEL_CORE,
>   	.clksel_mask	= OMAP4430_CLKSEL_L3_MASK,
> @@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] = {
>   static struct clk trace_clk_div_ck = {
>   	.name		= "trace_clk_div_ck",
>   	.parent		=&pmd_trace_clk_mux_ck,
> +	.clkdm_name	= "emu_sys_clkdm",
>   	.clksel		= trace_clk_div_div,
>   	.clksel_reg	= OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
>   	.clksel_mask	= OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,
>
>

RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Thu, Jun 07, 2012 at 11:43:10, Paul Walmsley wrote:
> Kevin discovered that commit c8d82ff68fb6873691536cf33021977efbf5593c
> ("ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod
> database") broke CORE idle on OMAP3.  This prevents device low power
> states.
> 
> The root cause is that the 32K sync timer IP block does not support
> smart-idle mode[1], and so the hwmod code keeps the IP block in
> no-idle mode while it is active.  This in turn prevents the WKUP
> clockdomain from transitioning to idle.  There is a hardcoded sleep
> dependency that prevents the CORE_L3 and CORE_CM clockdomains from
> transitioning to idle when the WKUP clockdomain is active[2], so the
> chip cannot enter any device low power states.
> 
> It turns out that there is no need to take the 32k sync timer out of
> idle.  The IP block itself probably does not have any native idle
> handling at all, due to its simplicity.  Furthermore, the PRCM will
> never request target idle for this IP block while the kernel is
> running, due to the sleep dependency that prevents the WKUP
> clockdomain from idling while the CORE_L3 clockdomain is active.  So
> we can safely leave the 32k sync timer in target-no-idle mode, even
> while we continue to access it.
> 
> This workaround is implemented by programming the force-idle mode for
> any IP block that only supports the force-idle and no-idle modes.  If
> an IP block is ever released that doesn't support smart-idle and
> requires no-idle mode to be programmed while it's in use, we'll have
> to change this behavior.
> 

Isn't this impact AM33xx devices, where we do not support smart idle mode???
Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...

Thanks,
Vaibhav

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Thu, Jun 07, 2012 at 11:43:10, Paul Walmsley wrote:
> Kevin discovered that commit c8d82ff68fb6873691536cf33021977efbf5593c
> ("ARM: OMAP2/3: hwmod data: Add 32k-sync timer data to hwmod
> database") broke CORE idle on OMAP3.  This prevents device low power
> states.
> 
> The root cause is that the 32K sync timer IP block does not support
> smart-idle mode[1], and so the hwmod code keeps the IP block in
> no-idle mode while it is active.  This in turn prevents the WKUP
> clockdomain from transitioning to idle.  There is a hardcoded sleep
> dependency that prevents the CORE_L3 and CORE_CM clockdomains from
> transitioning to idle when the WKUP clockdomain is active[2], so the
> chip cannot enter any device low power states.
> 
> It turns out that there is no need to take the 32k sync timer out of
> idle.  The IP block itself probably does not have any native idle
> handling at all, due to its simplicity.  Furthermore, the PRCM will
> never request target idle for this IP block while the kernel is
> running, due to the sleep dependency that prevents the WKUP
> clockdomain from idling while the CORE_L3 clockdomain is active.  So
> we can safely leave the 32k sync timer in target-no-idle mode, even
> while we continue to access it.
> 
> This workaround is implemented by programming the force-idle mode for
> any IP block that only supports the force-idle and no-idle modes.  If
> an IP block is ever released that doesn't support smart-idle and
> requires no-idle mode to be programmed while it's in use, we'll have
> to change this behavior.
> 

Isn't this impact AM33xx devices, where we do not support smart idle mode???
Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...

Thanks,
Vaibhav

RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Hi

On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:

> Isn't this impact AM33xx devices, where we do not support smart idle mode???
> Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...

Thanks, please let me know how your tests go.  If it doesn't work, we'll 
go back to the flag-based approach in the patch I posted already.  


- Paul

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Hi

On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:

> Isn't this impact AM33xx devices, where we do not support smart idle mode???
> Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...

Thanks, please let me know how your tests go.  If it doesn't work, we'll 
go back to the flag-based approach in the patch I posted already.  


- Paul

Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> Enable the AESS auto-gating control bit during AESS hwmod setup.  This
> fixes the following boot warning on OMAP4:
> 
> omap_hwmod: aess: _wait_target_disable failed
> 
> Without this patch, the AESS IP block does not indicate to the PRCM
> that it is idle after it is reset.  This prevents some types of SoC
> power management until something sets the auto-gating control bit.

I don't like the idea of having custom platform init code for every driver
to reset it, let's see if there's some better way to deal with stuff like
this.

It seems that most/many IP blocks need their custom reset hacks, and it's
not limited to just few instances?

Maybe we can distribute custom hacks like this to the drivers? If there is
no generic way to reset some IP block, then the driver should pass it's
whatever workaround bits/methdod/ to the bus level (hwmod) code during the
init rather than piling up all the possible hacks to the bus level code.

This way the driver init can still do the bus level reset during it's init,
and bail out after that if the module is not in use for the board in question.
That is already being flagged in device tree with status = "disable" flag,
and can also passed in platform data if necessary.

AFAIK there's no need to reset the IP blocks before the driver init, it's
really needed for PM. So it's not needed early on, and it's OK to require
running  the driver init for driver modules that are not in use to reset
them properly. After all, the hardware is on the device, even if it's not
being used.

Regards,

Tony

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> Enable the AESS auto-gating control bit during AESS hwmod setup.  This
> fixes the following boot warning on OMAP4:
> 
> omap_hwmod: aess: _wait_target_disable failed
> 
> Without this patch, the AESS IP block does not indicate to the PRCM
> that it is idle after it is reset.  This prevents some types of SoC
> power management until something sets the auto-gating control bit.

I don't like the idea of having custom platform init code for every driver
to reset it, let's see if there's some better way to deal with stuff like
this.

It seems that most/many IP blocks need their custom reset hacks, and it's
not limited to just few instances?

Maybe we can distribute custom hacks like this to the drivers? If there is
no generic way to reset some IP block, then the driver should pass it's
whatever workaround bits/methdod/ to the bus level (hwmod) code during the
init rather than piling up all the possible hacks to the bus level code.

This way the driver init can still do the bus level reset during it's init,
and bail out after that if the module is not in use for the board in question.
That is already being flagged in device tree with status = "disable" flag,
and can also passed in platform data if necessary.

AFAIK there's no need to reset the IP blocks before the driver init, it's
really needed for PM. So it's not needed early on, and it's OK to require
running  the driver init for driver modules that are not in use to reset
them properly. After all, the hardware is on the device, even if it's not
being used.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> From: Tero Kristo <t-kristo@ti.com>
> 
> Add a custom reset function for the usb_host_fs/fsusb IP block, and
> connect it to the OMAP4 FSUSB block.
> 
> This is the first of two fixes required to get rid of the boot
> warning:
> 
> omap_hwmod: usb_host_fs: _wait_target_disable failed
> 
> and to allow the module to idle.
> 
> It may be necessary to use this reset method for OMAP2xxx SoCs as
> well; this is left for a future patch.

Here too I think driver like features like this should live in the
driver init for omap OHCI driver. In the likely case that FS OHCI is
not in use on the board, the OHCI glue can just reset it.
 
> +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> +#define HCCOMMANDSTATUS			0x0008
> +
> +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)

I don't think the bus layer code should need to know anything about driver
specific registers.

> +	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
> +
> +	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
> +			    & HCCOMMANDSTATUS_HCR_MASK),
> +			  MAX_MODULE_SOFTRESET_WAIT, c);
> +

These should be accessed by the driver in a standard way after ioremapping
the device.

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> From: Tero Kristo <t-kristo@ti.com>
> 
> Add a custom reset function for the usb_host_fs/fsusb IP block, and
> connect it to the OMAP4 FSUSB block.
> 
> This is the first of two fixes required to get rid of the boot
> warning:
> 
> omap_hwmod: usb_host_fs: _wait_target_disable failed
> 
> and to allow the module to idle.
> 
> It may be necessary to use this reset method for OMAP2xxx SoCs as
> well; this is left for a future patch.

Here too I think driver like features like this should live in the
driver init for omap OHCI driver. In the likely case that FS OHCI is
not in use on the board, the OHCI glue can just reset it.
 
> +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> +#define HCCOMMANDSTATUS			0x0008
> +
> +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)

I don't think the bus layer code should need to know anything about driver
specific registers.

> +	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
> +
> +	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
> +			    & HCCOMMANDSTATUS_HCR_MASK),
> +			  MAX_MODULE_SOFTRESET_WAIT, c);
> +

These should be accessed by the driver in a standard way after ioremapping
the device.

Tony

Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> It seems that most/many IP blocks need their custom reset hacks, and 
> it's not limited to just few instances?

Only four out of the fifty-seven omap_hwmod_classes defined in 
mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
functions used:

$ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
57
$ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
4

That's 7% of the classes.  In terms of the total number of IP block 
instances that use custom reset functions viewed against the total number 
of instances on the chip, the percentage is even smaller.

> AFAIK there's no need to reset the IP blocks before the driver init, 
> it's really needed for PM. So it's not needed early on, and it's OK to 
> require running the driver init for driver modules that are not in use 
> to reset them properly. After all, the hardware is on the device, even 
> if it's not being used.

I don't think I'm following you.  It's not just PM; the problem is also 
with kexec or buggy bootloaders.  If an IP block isn't reset when the 
kernel boots, and is doing DMA or anything else that could affect the 
reset of the system, it could easily cause unpredictable behavior or 
crashes in unrelated kernel code.

It's also worth mentioning that many IP blocks, such as AESS, don't have 
Linux drivers.


- Paul

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> It seems that most/many IP blocks need their custom reset hacks, and 
> it's not limited to just few instances?

Only four out of the fifty-seven omap_hwmod_classes defined in 
mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
functions used:

$ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
57
$ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
4

That's 7% of the classes.  In terms of the total number of IP block 
instances that use custom reset functions viewed against the total number 
of instances on the chip, the percentage is even smaller.

> AFAIK there's no need to reset the IP blocks before the driver init, 
> it's really needed for PM. So it's not needed early on, and it's OK to 
> require running the driver init for driver modules that are not in use 
> to reset them properly. After all, the hardware is on the device, even 
> if it's not being used.

I don't think I'm following you.  It's not just PM; the problem is also 
with kexec or buggy bootloaders.  If an IP block isn't reset when the 
kernel boots, and is doing DMA or anything else that could affect the 
reset of the system, it could easily cause unpredictable behavior or 
crashes in unrelated kernel code.

It's also worth mentioning that many IP blocks, such as AESS, don't have 
Linux drivers.


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

Hi,

On Thu, Jun 07, 2012 at 12:31:13AM -0700, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> > From: Tero Kristo <t-kristo@ti.com>
> > 
> > Add a custom reset function for the usb_host_fs/fsusb IP block, and
> > connect it to the OMAP4 FSUSB block.
> > 
> > This is the first of two fixes required to get rid of the boot
> > warning:
> > 
> > omap_hwmod: usb_host_fs: _wait_target_disable failed
> > 
> > and to allow the module to idle.
> > 
> > It may be necessary to use this reset method for OMAP2xxx SoCs as
> > well; this is left for a future patch.
> 
> Here too I think driver like features like this should live in the
> driver init for omap OHCI driver. In the likely case that FS OHCI is
> not in use on the board, the OHCI glue can just reset it.

this I don't agree. It means we will have to add some OHCI code even
when OHCI is disabled.

> > +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> > +#define HCCOMMANDSTATUS			0x0008
> > +
> > +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> > +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
> 
> I don't think the bus layer code should need to know anything about driver
> specific registers.
> 
> > +	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
> > +
> > +	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
> > +			    & HCCOMMANDSTATUS_HCR_MASK),
> > +			  MAX_MODULE_SOFTRESET_WAIT, c);
> > +
> 
> These should be accessed by the driver in a standard way after ioremapping
> the device.

agree.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

Hi,

On Thu, Jun 07, 2012 at 12:31:13AM -0700, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> > From: Tero Kristo <t-kristo@ti.com>
> > 
> > Add a custom reset function for the usb_host_fs/fsusb IP block, and
> > connect it to the OMAP4 FSUSB block.
> > 
> > This is the first of two fixes required to get rid of the boot
> > warning:
> > 
> > omap_hwmod: usb_host_fs: _wait_target_disable failed
> > 
> > and to allow the module to idle.
> > 
> > It may be necessary to use this reset method for OMAP2xxx SoCs as
> > well; this is left for a future patch.
> 
> Here too I think driver like features like this should live in the
> driver init for omap OHCI driver. In the likely case that FS OHCI is
> not in use on the board, the OHCI glue can just reset it.

this I don't agree. It means we will have to add some OHCI code even
when OHCI is disabled.

> > +/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS register */
> > +#define HCCOMMANDSTATUS			0x0008
> > +
> > +/* HCCOMMANDSTATUS_HCR: the bitmask of the host controller reset flag */
> > +#define HCCOMMANDSTATUS_HCR_MASK	(1 << 0)
> 
> I don't think the bus layer code should need to know anything about driver
> specific registers.
> 
> > +	omap_hwmod_write(HCCOMMANDSTATUS_HCR_MASK, oh, HCCOMMANDSTATUS);
> > +
> > +	omap_test_timeout(!(omap_hwmod_read(oh, HCCOMMANDSTATUS)
> > +			    & HCCOMMANDSTATUS_HCR_MASK),
> > +			  MAX_MODULE_SOFTRESET_WAIT, c);
> > +
> 
> These should be accessed by the driver in a standard way after ioremapping
> the device.

agree.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120607/f6e637b0/attachment.sig>

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Here too I think driver like features like this should live in the
> driver init for omap OHCI driver. In the likely case that FS OHCI is
> not in use on the board, the OHCI glue can just reset it.

What if the driver is not compiled into the kernel, but instead is built 
as a loadable module?


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Here too I think driver like features like this should live in the
> driver init for omap OHCI driver. In the likely case that FS OHCI is
> not in use on the board, the OHCI glue can just reset it.

What if the driver is not compiled into the kernel, but instead is built 
as a loadable module?


- Paul

Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 00:35]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > It seems that most/many IP blocks need their custom reset hacks, and 
> > it's not limited to just few instances?
> 
> Only four out of the fifty-seven omap_hwmod_classes defined in 
> mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
> functions used:
> 
> $ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 57
> $ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 4
> 
> That's 7% of the classes.  In terms of the total number of IP block 
> instances that use custom reset functions viewed against the total number 
> of instances on the chip, the percentage is even smaller.

OK so that's not too bad then. But there's also the omap2_wd_timer_disable
pre_shutdown too. And there's also the sysconfig autoidle bit for each driver
that we're tweaking in the bus level code?

If we can remove the ioremapping and accessing driver registers in the bus
level code things get much simpler for the bus level code.

> > AFAIK there's no need to reset the IP blocks before the driver init, 
> > it's really needed for PM. So it's not needed early on, and it's OK to 
> > require running the driver init for driver modules that are not in use 
> > to reset them properly. After all, the hardware is on the device, even 
> > if it's not being used.
> 
> I don't think I'm following you.  It's not just PM; the problem is also 
> with kexec or buggy bootloaders.  If an IP block isn't reset when the 
> kernel boots, and is doing DMA or anything else that could affect the 
> reset of the system, it could easily cause unpredictable behavior or 
> crashes in unrelated kernel code.

Still sounds like the responsibility of the bootloaders and drivers to
set things up properly, that's the standard behaviour.
 
> It's also worth mentioning that many IP blocks, such as AESS, don't have 
> Linux drivers.

Sounds like it should have a minimal driver then, just to reset it if
nothing else.

Regards,

Tony

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 00:35]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > It seems that most/many IP blocks need their custom reset hacks, and 
> > it's not limited to just few instances?
> 
> Only four out of the fifty-seven omap_hwmod_classes defined in 
> mach-omap2/omap_hwmod_44xx_data.c after this series have custom reset 
> functions used:
> 
> $ fgrep 'struct omap_hwmod_class ' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 57
> $ fgrep '.reset' arch/arm/mach-omap2/omap_hwmod_44xx_data.c | wc -l
> 4
> 
> That's 7% of the classes.  In terms of the total number of IP block 
> instances that use custom reset functions viewed against the total number 
> of instances on the chip, the percentage is even smaller.

OK so that's not too bad then. But there's also the omap2_wd_timer_disable
pre_shutdown too. And there's also the sysconfig autoidle bit for each driver
that we're tweaking in the bus level code?

If we can remove the ioremapping and accessing driver registers in the bus
level code things get much simpler for the bus level code.

> > AFAIK there's no need to reset the IP blocks before the driver init, 
> > it's really needed for PM. So it's not needed early on, and it's OK to 
> > require running the driver init for driver modules that are not in use 
> > to reset them properly. After all, the hardware is on the device, even 
> > if it's not being used.
> 
> I don't think I'm following you.  It's not just PM; the problem is also 
> with kexec or buggy bootloaders.  If an IP block isn't reset when the 
> kernel boots, and is doing DMA or anything else that could affect the 
> reset of the system, it could easily cause unpredictable behavior or 
> crashes in unrelated kernel code.

Still sounds like the responsibility of the bootloaders and drivers to
set things up properly, that's the standard behaviour.
 
> It's also worth mentioning that many IP blocks, such as AESS, don't have 
> Linux drivers.

Sounds like it should have a minimal driver then, just to reset it if
nothing else.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Here too I think driver like features like this should live in the
> > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > not in use on the board, the OHCI glue can just reset it.
> 
> What if the driver is not compiled into the kernel, but instead is built 
> as a loadable module?

You can still have a core piece of the driver that's always built in, such
as omap-ohci-common. But it should live under drivers, not in the bus level
code. Or you can insmod/rmmod it to reset things properly.

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Here too I think driver like features like this should live in the
> > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > not in use on the board, the OHCI glue can just reset it.
> 
> What if the driver is not compiled into the kernel, but instead is built 
> as a loadable module?

You can still have a core piece of the driver that's always built in, such
as omap-ohci-common. But it should live under drivers, not in the bus level
code. Or you can insmod/rmmod it to reset things properly.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1040 bytes --]

On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > 
> > > Here too I think driver like features like this should live in the
> > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > not in use on the board, the OHCI glue can just reset it.
> > 
> > What if the driver is not compiled into the kernel, but instead is built 
> > as a loadable module?
> 
> You can still have a core piece of the driver that's always built in, such
> as omap-ohci-common. But it should live under drivers, not in the bus level
> code. Or you can insmod/rmmod it to reset things properly.

that's such a hack... both solutions are quite hacky. The only problem
here is because some bootloaders are leaving controller in an unknown
state and I guess to be completely safe, resets should be done before
any driver kicks in.

But, driver will probably reset again the IP block during probe...

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > 
> > > Here too I think driver like features like this should live in the
> > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > not in use on the board, the OHCI glue can just reset it.
> > 
> > What if the driver is not compiled into the kernel, but instead is built 
> > as a loadable module?
> 
> You can still have a core piece of the driver that's always built in, such
> as omap-ohci-common. But it should live under drivers, not in the bus level
> code. Or you can insmod/rmmod it to reset things properly.

that's such a hack... both solutions are quite hacky. The only problem
here is because some bootloaders are leaving controller in an unknown
state and I guess to be completely safe, resets should be done before
any driver kicks in.

But, driver will probably reset again the IP block during probe...

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120607/71c77cbd/attachment.sig>

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120607 00:39]:
> Hi,
> 
> On Thu, Jun 07, 2012 at 12:31:13AM -0700, Tony Lindgren wrote:
> > * Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> > > From: Tero Kristo <t-kristo@ti.com>
> > > 
> > > Add a custom reset function for the usb_host_fs/fsusb IP block, and
> > > connect it to the OMAP4 FSUSB block.
> > > 
> > > This is the first of two fixes required to get rid of the boot
> > > warning:
> > > 
> > > omap_hwmod: usb_host_fs: _wait_target_disable failed
> > > 
> > > and to allow the module to idle.
> > > 
> > > It may be necessary to use this reset method for OMAP2xxx SoCs as
> > > well; this is left for a future patch.
> > 
> > Here too I think driver like features like this should live in the
> > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > not in use on the board, the OHCI glue can just reset it.
> 
> this I don't agree. It means we will have to add some OHCI code even
> when OHCI is disabled.

That's because the bus level code alone is not enough for the reset and
driver features are needed to reset it. Nothing wrong with the code itself,
it should be just something that's part of the driver rather than the bus
level code. It could be always built in for sure even if it lives under
drivers.

Tony 

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [120607 00:39]:
> Hi,
> 
> On Thu, Jun 07, 2012 at 12:31:13AM -0700, Tony Lindgren wrote:
> > * Paul Walmsley <paul@pwsan.com> [120606 23:26]:
> > > From: Tero Kristo <t-kristo@ti.com>
> > > 
> > > Add a custom reset function for the usb_host_fs/fsusb IP block, and
> > > connect it to the OMAP4 FSUSB block.
> > > 
> > > This is the first of two fixes required to get rid of the boot
> > > warning:
> > > 
> > > omap_hwmod: usb_host_fs: _wait_target_disable failed
> > > 
> > > and to allow the module to idle.
> > > 
> > > It may be necessary to use this reset method for OMAP2xxx SoCs as
> > > well; this is left for a future patch.
> > 
> > Here too I think driver like features like this should live in the
> > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > not in use on the board, the OHCI glue can just reset it.
> 
> this I don't agree. It means we will have to add some OHCI code even
> when OHCI is disabled.

That's because the bus level code alone is not enough for the reset and
driver features are needed to reset it. Nothing wrong with the code itself,
it should be just something that's part of the driver rather than the bus
level code. It could be always built in for sure even if it lives under
drivers.

Tony 

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
>> * Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
>>> On Thu, 7 Jun 2012, Tony Lindgren wrote:
>>>
>>>> Here too I think driver like features like this should live in the
>>>> driver init for omap OHCI driver. In the likely case that FS OHCI is
>>>> not in use on the board, the OHCI glue can just reset it.
>>>
>>> What if the driver is not compiled into the kernel, but instead is built
>>> as a loadable module?
>>
>> You can still have a core piece of the driver that's always built in, such
>> as omap-ohci-common. But it should live under drivers, not in the bus level
>> code. Or you can insmod/rmmod it to reset things properly.
>
> that's such a hack... both solutions are quite hacky. The only problem
> here is because some bootloaders are leaving controller in an unknown
> state and I guess to be completely safe, resets should be done before
> any driver kicks in.
>
> But, driver will probably reset again the IP block during probe...

Ideally it should be done only in the probe if needed. In the case of 
the DSS, the bootloader can init it with a splash screen and we do not 
want to blindly reset it and thus produce some ugly artifact on the screen.

In fact we should delay the reset to the very last moment and 
potentially reset the IPs not under driver control later after a couple 
of second for example.
It will avoid reseting every IP that will be handled properly by drivers.

Regards,
Benoit

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
>> * Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
>>> On Thu, 7 Jun 2012, Tony Lindgren wrote:
>>>
>>>> Here too I think driver like features like this should live in the
>>>> driver init for omap OHCI driver. In the likely case that FS OHCI is
>>>> not in use on the board, the OHCI glue can just reset it.
>>>
>>> What if the driver is not compiled into the kernel, but instead is built
>>> as a loadable module?
>>
>> You can still have a core piece of the driver that's always built in, such
>> as omap-ohci-common. But it should live under drivers, not in the bus level
>> code. Or you can insmod/rmmod it to reset things properly.
>
> that's such a hack... both solutions are quite hacky. The only problem
> here is because some bootloaders are leaving controller in an unknown
> state and I guess to be completely safe, resets should be done before
> any driver kicks in.
>
> But, driver will probably reset again the IP block during probe...

Ideally it should be done only in the probe if needed. In the case of 
the DSS, the bootloader can init it with a splash screen and we do not 
want to blindly reset it and thus produce some ugly artifact on the screen.

In fact we should delay the reset to the very last moment and 
potentially reset the IPs not under driver control later after a couple 
of second for example.
It will avoid reseting every IP that will be handled properly by drivers.

Regards,
Benoit

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Cousson, Benoit <b-cousson@ti.com> [120607 01:07]:
> On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> >On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> >>* Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
> >>>On Thu, 7 Jun 2012, Tony Lindgren wrote:
> >>>
> >>>>Here too I think driver like features like this should live in the
> >>>>driver init for omap OHCI driver. In the likely case that FS OHCI is
> >>>>not in use on the board, the OHCI glue can just reset it.
> >>>
> >>>What if the driver is not compiled into the kernel, but instead is built
> >>>as a loadable module?
> >>
> >>You can still have a core piece of the driver that's always built in, such
> >>as omap-ohci-common. But it should live under drivers, not in the bus level
> >>code. Or you can insmod/rmmod it to reset things properly.
> >
> >that's such a hack... both solutions are quite hacky. The only problem
> >here is because some bootloaders are leaving controller in an unknown
> >state and I guess to be completely safe, resets should be done before
> >any driver kicks in.
> >
> >But, driver will probably reset again the IP block during probe...

Well I see two advantages moving the reset and idle responsibility to
the drivers:

1. We can avoid ioremapping the devices in the bus level code and
   simplify things

2. We don't need to duplicate driver code into the bus level code
 
> Ideally it should be done only in the probe if needed. In the case
> of the DSS, the bootloader can init it with a splash screen and we
> do not want to blindly reset it and thus produce some ugly artifact
> on the screen.
> 
> In fact we should delay the reset to the very last moment and
> potentially reset the IPs not under driver control later after a
> couple of second for example.
> It will avoid reseting every IP that will be handled properly by drivers.

Good point.

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Cousson, Benoit <b-cousson@ti.com> [120607 01:07]:
> On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> >On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> >>* Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
> >>>On Thu, 7 Jun 2012, Tony Lindgren wrote:
> >>>
> >>>>Here too I think driver like features like this should live in the
> >>>>driver init for omap OHCI driver. In the likely case that FS OHCI is
> >>>>not in use on the board, the OHCI glue can just reset it.
> >>>
> >>>What if the driver is not compiled into the kernel, but instead is built
> >>>as a loadable module?
> >>
> >>You can still have a core piece of the driver that's always built in, such
> >>as omap-ohci-common. But it should live under drivers, not in the bus level
> >>code. Or you can insmod/rmmod it to reset things properly.
> >
> >that's such a hack... both solutions are quite hacky. The only problem
> >here is because some bootloaders are leaving controller in an unknown
> >state and I guess to be completely safe, resets should be done before
> >any driver kicks in.
> >
> >But, driver will probably reset again the IP block during probe...

Well I see two advantages moving the reset and idle responsibility to
the drivers:

1. We can avoid ioremapping the devices in the bus level code and
   simplify things

2. We don't need to duplicate driver code into the bus level code
 
> Ideally it should be done only in the probe if needed. In the case
> of the DSS, the bootloader can init it with a splash screen and we
> do not want to blindly reset it and thus produce some ugly artifact
> on the screen.
> 
> In fact we should delay the reset to the very last moment and
> potentially reset the IPs not under driver control later after a
> couple of second for example.
> It will avoid reseting every IP that will be handled properly by drivers.

Good point.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1900 bytes --]

On Thu, Jun 07, 2012 at 10:02:51AM +0200, Cousson, Benoit wrote:
> On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> >On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> >>* Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
> >>>On Thu, 7 Jun 2012, Tony Lindgren wrote:
> >>>
> >>>>Here too I think driver like features like this should live in the
> >>>>driver init for omap OHCI driver. In the likely case that FS OHCI is
> >>>>not in use on the board, the OHCI glue can just reset it.
> >>>
> >>>What if the driver is not compiled into the kernel, but instead is built
> >>>as a loadable module?
> >>
> >>You can still have a core piece of the driver that's always built in, such
> >>as omap-ohci-common. But it should live under drivers, not in the bus level
> >>code. Or you can insmod/rmmod it to reset things properly.
> >
> >that's such a hack... both solutions are quite hacky. The only problem
> >here is because some bootloaders are leaving controller in an unknown
> >state and I guess to be completely safe, resets should be done before
> >any driver kicks in.
> >
> >But, driver will probably reset again the IP block during probe...
> 
> Ideally it should be done only in the probe if needed. In the case of
> the DSS, the bootloader can init it with a splash screen and we do
> not want to blindly reset it and thus produce some ugly artifact on
> the screen.
> 
> In fact we should delay the reset to the very last moment and
> potentially reset the IPs not under driver control later after a
> couple of second for example.
> It will avoid reseting every IP that will be handled properly by drivers.

you could have a late_initcall() that will iterate over hwmods and reset
the ones which aren't used. That would mean adding some extra code to
omap_device_build_ss() which would set a flag on each hwmod, or
something similar.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Felipe Balbi]

On Thu, Jun 07, 2012 at 10:02:51AM +0200, Cousson, Benoit wrote:
> On 6/7/2012 9:55 AM, Felipe Balbi wrote:
> >On Thu, Jun 07, 2012 at 12:51:58AM -0700, Tony Lindgren wrote:
> >>* Paul Walmsley<paul@pwsan.com>  [120607 00:44]:
> >>>On Thu, 7 Jun 2012, Tony Lindgren wrote:
> >>>
> >>>>Here too I think driver like features like this should live in the
> >>>>driver init for omap OHCI driver. In the likely case that FS OHCI is
> >>>>not in use on the board, the OHCI glue can just reset it.
> >>>
> >>>What if the driver is not compiled into the kernel, but instead is built
> >>>as a loadable module?
> >>
> >>You can still have a core piece of the driver that's always built in, such
> >>as omap-ohci-common. But it should live under drivers, not in the bus level
> >>code. Or you can insmod/rmmod it to reset things properly.
> >
> >that's such a hack... both solutions are quite hacky. The only problem
> >here is because some bootloaders are leaving controller in an unknown
> >state and I guess to be completely safe, resets should be done before
> >any driver kicks in.
> >
> >But, driver will probably reset again the IP block during probe...
> 
> Ideally it should be done only in the probe if needed. In the case of
> the DSS, the bootloader can init it with a splash screen and we do
> not want to blindly reset it and thus produce some ugly artifact on
> the screen.
> 
> In fact we should delay the reset to the very last moment and
> potentially reset the IPs not under driver control later after a
> couple of second for example.
> It will avoid reseting every IP that will be handled properly by drivers.

you could have a late_initcall() that will iterate over hwmods and reset
the ones which aren't used. That would mean adding some extra code to
omap_device_build_ss() which would set a flag on each hwmod, or
something similar.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120607/7388dedb/attachment.sig>

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > 
> > > Here too I think driver like features like this should live in the
> > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > not in use on the board, the OHCI glue can just reset it.
> > 
> > What if the driver is not compiled into the kernel, but instead is built 
> > as a loadable module?
> 
> You can still have a core piece of the driver that's always built in, such
> as omap-ohci-common. But it should live under drivers, not in the bus level
> code. Or you can insmod/rmmod it to reset things properly.

Do you know of any device drivers that do this now, with a core built-in 
piece separate from a dynamically loadable part?

Seems like it would be tricky to avoid linking in the entire driver, due 
to the symbol dependencies.  Either that, or an extra, largely useless, 
layer of indirection would be needed in the shim layer.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > 
> > > Here too I think driver like features like this should live in the
> > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > not in use on the board, the OHCI glue can just reset it.
> > 
> > What if the driver is not compiled into the kernel, but instead is built 
> > as a loadable module?
> 
> You can still have a core piece of the driver that's always built in, such
> as omap-ohci-common. But it should live under drivers, not in the bus level
> code. Or you can insmod/rmmod it to reset things properly.

Do you know of any device drivers that do this now, with a core built-in 
piece separate from a dynamically loadable part?

Seems like it would be tricky to avoid linking in the entire driver, due 
to the symbol dependencies.  Either that, or an extra, largely useless, 
layer of indirection would be needed in the shim layer.


- Paul

Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> OK so that's not too bad then. But there's also the 
> omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> autoidle bit for each driver that we're tweaking in the bus level code?

I think I lost your point here.  The ioremap() issue is separate from the 
reset functions, etc., in my view.  Moving the reset functions out to 
drivers/ seems potentially more reasonable than dropping the ioremap().

> If we can remove the ioremapping and accessing driver registers in the 
> bus level code things get much simpler for the bus level code.

That's like saying if PCI Configuration Header handling were to be moved 
into the driver code, then the PCI bus-level code would be much simpler 
:-)

The hwmod code ioremaps the device registers to handle the 
integration-level registers at the beginning of the device's address 
space.  These registers can be thought of as part of the PRCM, not part of 
the IP block.  It would have been better if TI had put these integration 
registers in a separate address space like PCI does.  But we are stuck 
with the existing hardware design.  The integration registers also differ 
from chip to chip even with the same underlying IP block, see for example 
the 32k sync timer.

The main reasons why these integration registers are handled now in common 
code are:

1. to avoid duplicating integration code between lots of different drivers 
   that is unrelated to the driver itself, such as bus-level reset

2. to ensure consistency of the OCP registers with the rest of the PM
   state

3. to avoid callbacks into drivers that might otherwise be needed for 
   bitfields like CLOCKACTIVITY 

4. to make it easier to debug integration problems with drivers

If we don't handle those registers in common code, the number of SoC 
integration workarounds that need to be placed into the drivers will 
increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
be changed.  If those integration-level details were still in the drivers, 
a large number of files would need to be changed.  And $DEITY help us if 
the code sequence for dealing with those bits were to ever change in the 
future - we'd need to change a bunch of drivers, rather than just one or 
two files.  Also some people are going to need to audit the driver code 
from an integration level pretty carefully for PM to work consistently.

I suppose one option, if we were to have a real omap_device, would be to 
define callbacks for each driver to implement that would read and write 
the OCP header registers.  Then the omap_bus code could call those 
callbacks to handle the OCP register accesses, when called from the 
driver's PM runtime calls.  Adds another layer of indirection, but would 
localize IP block register accesses to the IP block's driver.

...

As far as the reset and preconfiguration aspects of the hwmod code go, 
they just happen to be possible since we're doing the ioremap anyway.  It 
can be ensured that no matter what drivers are present, or what the 
bootloader or previous OS did or didn't do, a minimal kernel should behave 
predictably.

It seems like it might be reasonable to move these to some built-in driver 
shim layer as you suggest in your other E-mail.  But that is assuming that 
it can be made to work without needless layers of indirection.  I don't 
know of any driver that does this now.  Maybe you know of one?


- Paul

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> OK so that's not too bad then. But there's also the 
> omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> autoidle bit for each driver that we're tweaking in the bus level code?

I think I lost your point here.  The ioremap() issue is separate from the 
reset functions, etc., in my view.  Moving the reset functions out to 
drivers/ seems potentially more reasonable than dropping the ioremap().

> If we can remove the ioremapping and accessing driver registers in the 
> bus level code things get much simpler for the bus level code.

That's like saying if PCI Configuration Header handling were to be moved 
into the driver code, then the PCI bus-level code would be much simpler 
:-)

The hwmod code ioremaps the device registers to handle the 
integration-level registers at the beginning of the device's address 
space.  These registers can be thought of as part of the PRCM, not part of 
the IP block.  It would have been better if TI had put these integration 
registers in a separate address space like PCI does.  But we are stuck 
with the existing hardware design.  The integration registers also differ 
from chip to chip even with the same underlying IP block, see for example 
the 32k sync timer.

The main reasons why these integration registers are handled now in common 
code are:

1. to avoid duplicating integration code between lots of different drivers 
   that is unrelated to the driver itself, such as bus-level reset

2. to ensure consistency of the OCP registers with the rest of the PM
   state

3. to avoid callbacks into drivers that might otherwise be needed for 
   bitfields like CLOCKACTIVITY 

4. to make it easier to debug integration problems with drivers

If we don't handle those registers in common code, the number of SoC 
integration workarounds that need to be placed into the drivers will 
increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
be changed.  If those integration-level details were still in the drivers, 
a large number of files would need to be changed.  And $DEITY help us if 
the code sequence for dealing with those bits were to ever change in the 
future - we'd need to change a bunch of drivers, rather than just one or 
two files.  Also some people are going to need to audit the driver code 
from an integration level pretty carefully for PM to work consistently.

I suppose one option, if we were to have a real omap_device, would be to 
define callbacks for each driver to implement that would read and write 
the OCP header registers.  Then the omap_bus code could call those 
callbacks to handle the OCP register accesses, when called from the 
driver's PM runtime calls.  Adds another layer of indirection, but would 
localize IP block register accesses to the IP block's driver.

...

As far as the reset and preconfiguration aspects of the hwmod code go, 
they just happen to be possible since we're doing the ioremap anyway.  It 
can be ensured that no matter what drivers are present, or what the 
bootloader or previous OS did or didn't do, a minimal kernel should behave 
predictably.

It seems like it might be reasonable to move these to some built-in driver 
shim layer as you suggest in your other E-mail.  But that is assuming that 
it can be made to work without needless layers of indirection.  I don't 
know of any driver that does this now.  Maybe you know of one?


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 03:24]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > > 
> > > > Here too I think driver like features like this should live in the
> > > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > > not in use on the board, the OHCI glue can just reset it.
> > > 
> > > What if the driver is not compiled into the kernel, but instead is built 
> > > as a loadable module?
> > 
> > You can still have a core piece of the driver that's always built in, such
> > as omap-ohci-common. But it should live under drivers, not in the bus level
> > code. Or you can insmod/rmmod it to reset things properly.
> 
> Do you know of any device drivers that do this now, with a core built-in 
> piece separate from a dynamically loadable part?

Hmm yeah good point, only driver frameworks tend to do that. It would require
the module registering with the core driver.
 
> Seems like it would be tricky to avoid linking in the entire driver, due 
> to the symbol dependencies.  Either that, or an extra, largely useless, 
> layer of indirection would be needed in the shim layer.

Yes probably better approach would be to only build in the reset and idle
part of the driver in the minimal case. And that too can get messy as the
makefiles may not even be included.

Until we have something generic in place to deal with stuff like unused driver
reset and idle, how about we set up the driver specific reset parts as inline
functions in the driver header?

That way the hwmod code can include those functions using the driver register
defines. Something like:

static inline int xyz_driver_reset(void __iomem *base, int flags)
{
	...
}

Then instead of having a separate platform init file for each driver,
we could just have a list of reset functions:

static int hwmod_xyz_driver_reset(void __iomem *base, int flags)
{
	int res;

	/* do bus related reset here */
	...

	/* call the driver reset */
	res = xyz_driver_reset(base, flags)


	/* do more bus related reset here */
	...
}

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 03:24]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > * Paul Walmsley <paul@pwsan.com> [120607 00:44]:
> > > On Thu, 7 Jun 2012, Tony Lindgren wrote:
> > > 
> > > > Here too I think driver like features like this should live in the
> > > > driver init for omap OHCI driver. In the likely case that FS OHCI is
> > > > not in use on the board, the OHCI glue can just reset it.
> > > 
> > > What if the driver is not compiled into the kernel, but instead is built 
> > > as a loadable module?
> > 
> > You can still have a core piece of the driver that's always built in, such
> > as omap-ohci-common. But it should live under drivers, not in the bus level
> > code. Or you can insmod/rmmod it to reset things properly.
> 
> Do you know of any device drivers that do this now, with a core built-in 
> piece separate from a dynamically loadable part?

Hmm yeah good point, only driver frameworks tend to do that. It would require
the module registering with the core driver.
 
> Seems like it would be tricky to avoid linking in the entire driver, due 
> to the symbol dependencies.  Either that, or an extra, largely useless, 
> layer of indirection would be needed in the shim layer.

Yes probably better approach would be to only build in the reset and idle
part of the driver in the minimal case. And that too can get messy as the
makefiles may not even be included.

Until we have something generic in place to deal with stuff like unused driver
reset and idle, how about we set up the driver specific reset parts as inline
functions in the driver header?

That way the hwmod code can include those functions using the driver register
defines. Something like:

static inline int xyz_driver_reset(void __iomem *base, int flags)
{
	...
}

Then instead of having a separate platform init file for each driver,
we could just have a list of reset functions:

static int hwmod_xyz_driver_reset(void __iomem *base, int flags)
{
	int res;

	/* do bus related reset here */
	...

	/* call the driver reset */
	res = xyz_driver_reset(base, flags)


	/* do more bus related reset here */
	...
}

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Cousson, Benoit wrote:

> In fact we should delay the reset to the very last moment and 
> potentially reset the IPs not under driver control later after a couple 
> of second for example. It will avoid reseting every IP that will be 
> handled properly by drivers.

We discussed this a couple of years ago on the list and aligned on late 
and lazy resets, from my recollection.  The main reason why it wasn't 
implemented was because there were some drivers that were not yet PM 
runtime-converted.  This would have caused crashes, since the core code 
would have no idea that the non-PM-runtime drivers had initialized their 
devices, and so the core just went ahead and reset those anyway.

It might actually be safe now to switch to the late reset arrangement, 
depending on whether the rest of the drivers have been PM 
runtime-converted by now.

Of course, it would all be moot if the reset is moved away from the hwmod 
code into the drivers.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Cousson, Benoit wrote:

> In fact we should delay the reset to the very last moment and 
> potentially reset the IPs not under driver control later after a couple 
> of second for example. It will avoid reseting every IP that will be 
> handled properly by drivers.

We discussed this a couple of years ago on the list and aligned on late 
and lazy resets, from my recollection.  The main reason why it wasn't 
implemented was because there were some drivers that were not yet PM 
runtime-converted.  This would have caused crashes, since the core code 
would have no idea that the non-PM-runtime drivers had initialized their 
devices, and so the core just went ahead and reset those anyway.

It might actually be safe now to switch to the late reset arrangement, 
depending on whether the rest of the drivers have been PM 
runtime-converted by now.

Of course, it would all be moot if the reset is moved away from the hwmod 
code into the drivers.


- Paul

Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 03:49]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > OK so that's not too bad then. But there's also the 
> > omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> > autoidle bit for each driver that we're tweaking in the bus level code?
> 
> I think I lost your point here.  The ioremap() issue is separate from the 
> reset functions, etc., in my view.  Moving the reset functions out to 
> drivers/ seems potentially more reasonable than dropping the ioremap().
> 
> > If we can remove the ioremapping and accessing driver registers in the 
> > bus level code things get much simpler for the bus level code.
> 
> That's like saying if PCI Configuration Header handling were to be moved 
> into the driver code, then the PCI bus-level code would be much simpler 
> :-)
> 
> The hwmod code ioremaps the device registers to handle the 
> integration-level registers at the beginning of the device's address 
> space.  These registers can be thought of as part of the PRCM, not part of 
> the IP block.  It would have been better if TI had put these integration 
> registers in a separate address space like PCI does.  But we are stuck 
> with the existing hardware design.  The integration registers also differ 
> from chip to chip even with the same underlying IP block, see for example 
> the 32k sync timer.

Yes having these registers in the device address space sucks.
 
> The main reasons why these integration registers are handled now in common 
> code are:
> 
> 1. to avoid duplicating integration code between lots of different drivers 
>    that is unrelated to the driver itself, such as bus-level reset
> 
> 2. to ensure consistency of the OCP registers with the rest of the PM
>    state
> 
> 3. to avoid callbacks into drivers that might otherwise be needed for 
>    bitfields like CLOCKACTIVITY 
> 
> 4. to make it easier to debug integration problems with drivers

Sure that all makes sense.
 
> If we don't handle those registers in common code, the number of SoC 
> integration workarounds that need to be placed into the drivers will 
> increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
> smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
> be changed.  If those integration-level details were still in the drivers, 
> a large number of files would need to be changed.  And $DEITY help us if 
> the code sequence for dealing with those bits were to ever change in the 
> future - we'd need to change a bunch of drivers, rather than just one or 
> two files.  Also some people are going to need to audit the driver code 
> from an integration level pretty carefully for PM to work consistently.
> 
> I suppose one option, if we were to have a real omap_device, would be to 
> define callbacks for each driver to implement that would read and write 
> the OCP header registers.  Then the omap_bus code could call those 
> callbacks to handle the OCP register accesses, when called from the 
> driver's PM runtime calls.  Adds another layer of indirection, but would 
> localize IP block register accesses to the IP block's driver.

Yes there may be some way to deal with this cleanly while keeping the
driver registers in the drivers. Maybe inline functions in the driver
headers that hwmod code could include would be enough here.
 
> ...
> 
> As far as the reset and preconfiguration aspects of the hwmod code go, 
> they just happen to be possible since we're doing the ioremap anyway.  It 
> can be ensured that no matter what drivers are present, or what the 
> bootloader or previous OS did or didn't do, a minimal kernel should behave 
> predictably.
> 
> It seems like it might be reasonable to move these to some built-in driver 
> shim layer as you suggest in your other E-mail.  But that is assuming that 
> it can be made to work without needless layers of indirection.  I don't 
> know of any driver that does this now.  Maybe you know of one?

Yes good point, see the suggestion on the driver header inline functions
in the other email I just sent.

Regards,

Tony

[PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 03:49]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > OK so that's not too bad then. But there's also the 
> > omap2_wd_timer_disable pre_shutdown too. And there's also the sysconfig 
> > autoidle bit for each driver that we're tweaking in the bus level code?
> 
> I think I lost your point here.  The ioremap() issue is separate from the 
> reset functions, etc., in my view.  Moving the reset functions out to 
> drivers/ seems potentially more reasonable than dropping the ioremap().
> 
> > If we can remove the ioremapping and accessing driver registers in the 
> > bus level code things get much simpler for the bus level code.
> 
> That's like saying if PCI Configuration Header handling were to be moved 
> into the driver code, then the PCI bus-level code would be much simpler 
> :-)
> 
> The hwmod code ioremaps the device registers to handle the 
> integration-level registers at the beginning of the device's address 
> space.  These registers can be thought of as part of the PRCM, not part of 
> the IP block.  It would have been better if TI had put these integration 
> registers in a separate address space like PCI does.  But we are stuck 
> with the existing hardware design.  The integration registers also differ 
> from chip to chip even with the same underlying IP block, see for example 
> the 32k sync timer.

Yes having these registers in the device address space sucks.
 
> The main reasons why these integration registers are handled now in common 
> code are:
> 
> 1. to avoid duplicating integration code between lots of different drivers 
>    that is unrelated to the driver itself, such as bus-level reset
> 
> 2. to ensure consistency of the OCP registers with the rest of the PM
>    state
> 
> 3. to avoid callbacks into drivers that might otherwise be needed for 
>    bitfields like CLOCKACTIVITY 
> 
> 4. to make it easier to debug integration problems with drivers

Sure that all makes sense.
 
> If we don't handle those registers in common code, the number of SoC 
> integration workarounds that need to be placed into the drivers will 
> increase.  For example, when OMAP4 added the smart-idle-with-wakeup and 
> smart-standby-with-wakeup OCP idle modes, only a couple of files needed to 
> be changed.  If those integration-level details were still in the drivers, 
> a large number of files would need to be changed.  And $DEITY help us if 
> the code sequence for dealing with those bits were to ever change in the 
> future - we'd need to change a bunch of drivers, rather than just one or 
> two files.  Also some people are going to need to audit the driver code 
> from an integration level pretty carefully for PM to work consistently.
> 
> I suppose one option, if we were to have a real omap_device, would be to 
> define callbacks for each driver to implement that would read and write 
> the OCP header registers.  Then the omap_bus code could call those 
> callbacks to handle the OCP register accesses, when called from the 
> driver's PM runtime calls.  Adds another layer of indirection, but would 
> localize IP block register accesses to the IP block's driver.

Yes there may be some way to deal with this cleanly while keeping the
driver registers in the drivers. Maybe inline functions in the driver
headers that hwmod code could include would be enough here.
 
> ...
> 
> As far as the reset and preconfiguration aspects of the hwmod code go, 
> they just happen to be possible since we're doing the ioremap anyway.  It 
> can be ensured that no matter what drivers are present, or what the 
> bootloader or previous OS did or didn't do, a minimal kernel should behave 
> predictably.
> 
> It seems like it might be reasonable to move these to some built-in driver 
> shim layer as you suggest in your other E-mail.  But that is assuming that 
> it can be made to work without needless layers of indirection.  I don't 
> know of any driver that does this now.  Maybe you know of one?

Yes good point, see the suggestion on the driver header inline functions
in the other email I just sent.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

+ Ohad

On 6/7/2012 12:52 PM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Cousson, Benoit wrote:
>
>> In fact we should delay the reset to the very last moment and
>> potentially reset the IPs not under driver control later after a couple
>> of second for example. It will avoid reseting every IP that will be
>> handled properly by drivers.
>
> We discussed this a couple of years ago on the list and aligned on late
> and lazy resets, from my recollection.

Indeed, what I did not mention is that potentially the whole device init 
should be done ondemand as well. Meaning the whole hwmod setup phase 
should be done only when the driver will probe the device.

> The main reason why it wasn't
> implemented was because there were some drivers that were not yet PM
> runtime-converted.  This would have caused crashes, since the core code
> would have no idea that the non-PM-runtime drivers had initialized their
> devices, and so the core just went ahead and reset those anyway.
>
> It might actually be safe now to switch to the late reset arrangement,
> depending on whether the rest of the drivers have been PM
> runtime-converted by now.
>
> Of course, it would all be moot if the reset is moved away from the hwmod
> code into the drivers.

I do think that moving that to driver code seems much better since all 
these customs reset code (I2C, DSS, USB...) does anyway require access 
to the device itself. The is driver responsibility to do that.

So ideally it should not be in OMAP architecture code.

Regards,
Benoit

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

+ Ohad

On 6/7/2012 12:52 PM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Cousson, Benoit wrote:
>
>> In fact we should delay the reset to the very last moment and
>> potentially reset the IPs not under driver control later after a couple
>> of second for example. It will avoid reseting every IP that will be
>> handled properly by drivers.
>
> We discussed this a couple of years ago on the list and aligned on late
> and lazy resets, from my recollection.

Indeed, what I did not mention is that potentially the whole device init 
should be done ondemand as well. Meaning the whole hwmod setup phase 
should be done only when the driver will probe the device.

> The main reason why it wasn't
> implemented was because there were some drivers that were not yet PM
> runtime-converted.  This would have caused crashes, since the core code
> would have no idea that the non-PM-runtime drivers had initialized their
> devices, and so the core just went ahead and reset those anyway.
>
> It might actually be safe now to switch to the late reset arrangement,
> depending on whether the rest of the drivers have been PM
> runtime-converted by now.
>
> Of course, it would all be moot if the reset is moved away from the hwmod
> code into the drivers.

I do think that moving that to driver code seems much better since all 
these customs reset code (I2C, DSS, USB...) does anyway require access 
to the device itself. The is driver responsibility to do that.

So ideally it should not be in OMAP architecture code.

Regards,
Benoit

RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Thu, Jun 07, 2012 at 12:38:50, Paul Walmsley wrote:
> Hi
> 
> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > Isn't this impact AM33xx devices, where we do not support smart idle mode???
> > Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...
> 
> Thanks, please let me know how your tests go.  If it doesn't work, we'll 
> go back to the flag-based approach in the patch I posted already.  
> 

Paul,

I couldn't finish my testing today, got into continuous meetings.
Tomorrow, I will test it and update you on this.

Thanks,
Vaibhav

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Thu, Jun 07, 2012 at 12:38:50, Paul Walmsley wrote:
> Hi
> 
> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > Isn't this impact AM33xx devices, where we do not support smart idle mode???
> > Anyway, I will also test it on both OMAP3EVM and AM33xx platform now...
> 
> Thanks, please let me know how your tests go.  If it doesn't work, we'll 
> go back to the flag-based approach in the patch I posted already.  
> 

Paul,

I couldn't finish my testing today, got into continuous meetings.
Tomorrow, I will test it and update you on this.

Thanks,
Vaibhav

RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Hi

On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:

> I couldn't finish my testing today, got into continuous meetings.

No worries, I understand.

> Tomorrow, I will test it and update you on this.

That would be great.

I took a look at SPRUH73F and sure enough, at least one module (CONTROL) 
doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field 
Descriptions".  I'd guess that the PRCM won't idle-req this IP block until 
the kernel is not running, so we might be able to get away with the 
existing approach; but the TRM also says:

"By definition, initiator may generate read/write transaction as long as
it is out of IDLE state."

Which pretty much matches my understanding too of the implicit interface 
contract here.

So I think we'd better go back to the flag approach as implemented in this 
patch:

http://www.spinics.net/lists/arm-kernel/msg176836.html

The WBU 32k sync timer's behavior is what relies on quirks of the 
integration that are hard to identify via other means, so it seems to be 
safest to tag it explicitly.


- Paul

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

Hi

On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:

> I couldn't finish my testing today, got into continuous meetings.

No worries, I understand.

> Tomorrow, I will test it and update you on this.

That would be great.

I took a look at SPRUH73F and sure enough, at least one module (CONTROL) 
doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field 
Descriptions".  I'd guess that the PRCM won't idle-req this IP block until 
the kernel is not running, so we might be able to get away with the 
existing approach; but the TRM also says:

"By definition, initiator may generate read/write transaction as long as
it is out of IDLE state."

Which pretty much matches my understanding too of the implicit interface 
contract here.

So I think we'd better go back to the flag approach as implemented in this 
patch:

http://www.spinics.net/lists/arm-kernel/msg176836.html

The WBU 32k sync timer's behavior is what relies on quirks of the 
integration that are hard to identify via other means, so it seems to be 
safest to tag it explicitly.


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Until we have something generic in place to deal with stuff like unused driver
> reset and idle, how about we set up the driver specific reset parts as inline
> functions in the driver header?

You're referring to the driver integration header files in 
arch/arm/plat-omap/include/ and arch/arm/mach-omap2/, right?  That would 
avoid the need for "sideways" includes from drivers/.

> That way the hwmod code can include those functions using the driver register
> defines. Something like:
> 
> static inline int xyz_driver_reset(void __iomem *base, int flags)
> {
> 	...
> }
> 
> Then instead of having a separate platform init file for each driver,
> we could just have a list of reset functions:
> 
> static int hwmod_xyz_driver_reset(void __iomem *base, int flags)

This should probably be passed a struct omap_hwmod * instead of base so 
it can call the existing hwmod bus related reset functions like 
omap_hwmod_softreset().  Or were you thinking about open-coding those into 
this reset function?

Just as an aside, this function will probably need to be marked 
__maybe_unused, so the compiler doesn't warn when other files include this 
header, but don't call the function.

> {
> 	int res;
> 
> 	/* do bus related reset here */
> 	...
>
> 	/* call the driver reset */
> 	res = xyz_driver_reset(base, flags)
> 
> 
> 	/* do more bus related reset here */
> 	...
> }

That's fine with me.  It doesn't matter to me where that code lives as 
long as it makes technical sense.

I'm hoping that in the near future that we can get rid of these 
hwmod_xyz_driver_reset() functions.  Instead it should be possible to have 
the hwmod reset code call functions like xyz_driver_reset_pre_wait() and 
xyz_driver_reset_post_ocpreset() via optional function pointers at 
different points in the reset process.  That should allow us to remove the 
omap_hwmod function calls from the I2C, MSDI, etc. reset functions, and 
also remove some needlessly duplicated code.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Until we have something generic in place to deal with stuff like unused driver
> reset and idle, how about we set up the driver specific reset parts as inline
> functions in the driver header?

You're referring to the driver integration header files in 
arch/arm/plat-omap/include/ and arch/arm/mach-omap2/, right?  That would 
avoid the need for "sideways" includes from drivers/.

> That way the hwmod code can include those functions using the driver register
> defines. Something like:
> 
> static inline int xyz_driver_reset(void __iomem *base, int flags)
> {
> 	...
> }
> 
> Then instead of having a separate platform init file for each driver,
> we could just have a list of reset functions:
> 
> static int hwmod_xyz_driver_reset(void __iomem *base, int flags)

This should probably be passed a struct omap_hwmod * instead of base so 
it can call the existing hwmod bus related reset functions like 
omap_hwmod_softreset().  Or were you thinking about open-coding those into 
this reset function?

Just as an aside, this function will probably need to be marked 
__maybe_unused, so the compiler doesn't warn when other files include this 
header, but don't call the function.

> {
> 	int res;
> 
> 	/* do bus related reset here */
> 	...
>
> 	/* call the driver reset */
> 	res = xyz_driver_reset(base, flags)
> 
> 
> 	/* do more bus related reset here */
> 	...
> }

That's fine with me.  It doesn't matter to me where that code lives as 
long as it makes technical sense.

I'm hoping that in the near future that we can get rid of these 
hwmod_xyz_driver_reset() functions.  Instead it should be possible to have 
the hwmod reset code call functions like xyz_driver_reset_pre_wait() and 
xyz_driver_reset_post_ocpreset() via optional function pointers at 
different points in the reset process.  That should allow us to remove the 
omap_hwmod function calls from the I2C, MSDI, etc. reset functions, and 
also remove some needlessly duplicated code.


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Cousson, Benoit wrote:

> Indeed, what I did not mention is that potentially the whole device init
> should be done ondemand as well. Meaning the whole hwmod setup phase should be
> done only when the driver will probe the device.

That means if no driver exists for an IP block, or if the driver isn't 
using PM runtime, the IP block won't be reset.  And somehow we still are 
missing drivers in mainline.  We also still have drivers that aren't yet 
PM runtime converted.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Cousson, Benoit wrote:

> Indeed, what I did not mention is that potentially the whole device init
> should be done ondemand as well. Meaning the whole hwmod setup phase should be
> done only when the driver will probe the device.

That means if no driver exists for an IP block, or if the driver isn't 
using PM runtime, the IP block won't be reset.  And somehow we still are 
missing drivers in mainline.  We also still have drivers that aren't yet 
PM runtime converted.


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 15:09]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Until we have something generic in place to deal with stuff like unused driver
> > reset and idle, how about we set up the driver specific reset parts as inline
> > functions in the driver header?
> 
> You're referring to the driver integration header files in 
> arch/arm/plat-omap/include/ and arch/arm/mach-omap2/, right?  That would 
> avoid the need for "sideways" includes from drivers/.
> 
> > That way the hwmod code can include those functions using the driver register
> > defines. Something like:
> > 
> > static inline int xyz_driver_reset(void __iomem *base, int flags)
> > {
> > 	...
> > }
> > 
> > Then instead of having a separate platform init file for each driver,
> > we could just have a list of reset functions:
> > 
> > static int hwmod_xyz_driver_reset(void __iomem *base, int flags)
> 
> This should probably be passed a struct omap_hwmod * instead of base so 
> it can call the existing hwmod bus related reset functions like 
> omap_hwmod_softreset().  Or were you thinking about open-coding those into 
> this reset function?

Oh OK yeah makes sense as that's hwmod internal function. Then the driver
specific part should use just void __iomem *base and use readl/writel and
live under include/linux/platform_data/omap-usb.h.

> Just as an aside, this function will probably need to be marked 
> __maybe_unused, so the compiler doesn't warn when other files include this 
> header, but don't call the function.

Yeah probably.
 
> > {
> > 	int res;
> > 
> > 	/* do bus related reset here */
> > 	...
> >
> > 	/* call the driver reset */
> > 	res = xyz_driver_reset(base, flags)
> > 
> > 
> > 	/* do more bus related reset here */
> > 	...
> > }
> 
> That's fine with me.  It doesn't matter to me where that code lives as 
> long as it makes technical sense.

OK good. That way we can separate the driver specific part from the bus
code. And the driver maintainers can review the driver specific part of the
idle/reset function. And maybe at some point we'll have device_reset and
device_idle functions and some generic framework in place for it..
 
> I'm hoping that in the near future that we can get rid of these 
> hwmod_xyz_driver_reset() functions.  Instead it should be possible to have 
> the hwmod reset code call functions like xyz_driver_reset_pre_wait() and 
> xyz_driver_reset_post_ocpreset() via optional function pointers at 
> different points in the reset process.  That should allow us to remove the 
> omap_hwmod function calls from the I2C, MSDI, etc. reset functions, and 
> also remove some needlessly duplicated code.

Sounds good to me. Also sounds like that does not need changes to the
driver specific xyz_driver_reset functions.

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120607 15:09]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Until we have something generic in place to deal with stuff like unused driver
> > reset and idle, how about we set up the driver specific reset parts as inline
> > functions in the driver header?
> 
> You're referring to the driver integration header files in 
> arch/arm/plat-omap/include/ and arch/arm/mach-omap2/, right?  That would 
> avoid the need for "sideways" includes from drivers/.
> 
> > That way the hwmod code can include those functions using the driver register
> > defines. Something like:
> > 
> > static inline int xyz_driver_reset(void __iomem *base, int flags)
> > {
> > 	...
> > }
> > 
> > Then instead of having a separate platform init file for each driver,
> > we could just have a list of reset functions:
> > 
> > static int hwmod_xyz_driver_reset(void __iomem *base, int flags)
> 
> This should probably be passed a struct omap_hwmod * instead of base so 
> it can call the existing hwmod bus related reset functions like 
> omap_hwmod_softreset().  Or were you thinking about open-coding those into 
> this reset function?

Oh OK yeah makes sense as that's hwmod internal function. Then the driver
specific part should use just void __iomem *base and use readl/writel and
live under include/linux/platform_data/omap-usb.h.

> Just as an aside, this function will probably need to be marked 
> __maybe_unused, so the compiler doesn't warn when other files include this 
> header, but don't call the function.

Yeah probably.
 
> > {
> > 	int res;
> > 
> > 	/* do bus related reset here */
> > 	...
> >
> > 	/* call the driver reset */
> > 	res = xyz_driver_reset(base, flags)
> > 
> > 
> > 	/* do more bus related reset here */
> > 	...
> > }
> 
> That's fine with me.  It doesn't matter to me where that code lives as 
> long as it makes technical sense.

OK good. That way we can separate the driver specific part from the bus
code. And the driver maintainers can review the driver specific part of the
idle/reset function. And maybe at some point we'll have device_reset and
device_idle functions and some generic framework in place for it..
 
> I'm hoping that in the near future that we can get rid of these 
> hwmod_xyz_driver_reset() functions.  Instead it should be possible to have 
> the hwmod reset code call functions like xyz_driver_reset_pre_wait() and 
> xyz_driver_reset_post_ocpreset() via optional function pointers at 
> different points in the reset process.  That should allow us to remove the 
> omap_hwmod function calls from the I2C, MSDI, etc. reset functions, and 
> also remove some needlessly duplicated code.

Sounds good to me. Also sounds like that does not need changes to the
driver specific xyz_driver_reset functions.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Cousson, Benoit wrote:
>
>> Indeed, what I did not mention is that potentially the whole device init
>> should be done ondemand as well. Meaning the whole hwmod setup phase should be
>> done only when the driver will probe the device.
>
> That means if no driver exists for an IP block, or if the driver isn't
> using PM runtime, the IP block won't be reset.  And somehow we still are
> missing drivers in mainline.  We also still have drivers that aren't yet
> PM runtime converted.

No the point is still the same as before. You let the drivers do the job 
if they are there, and then do a pass at very late time during the boot 
process to handle the ones that were not probed by any driver.

At least you will avoid the enable -> reset -> idle -> enable sequence 
we are doing right now for most of the active drivers when it is not 
necessary.

Regards,
Benoit

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Cousson, Benoit wrote:
>
>> Indeed, what I did not mention is that potentially the whole device init
>> should be done ondemand as well. Meaning the whole hwmod setup phase should be
>> done only when the driver will probe the device.
>
> That means if no driver exists for an IP block, or if the driver isn't
> using PM runtime, the IP block won't be reset.  And somehow we still are
> missing drivers in mainline.  We also still have drivers that aren't yet
> PM runtime converted.

No the point is still the same as before. You let the drivers do the job 
if they are there, and then do a pass at very late time during the boot 
process to handle the ones that were not probed by any driver.

At least you will avoid the enable -> reset -> idle -> enable sequence 
we are doing right now for most of the active drivers when it is not 
necessary.

Regards,
Benoit

Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Tero Kristo]

Hi Paul,

There's a bug in this patch, see below.

<clip>

>  {
> @@ -1141,8 +1143,26 @@ static void _enable_sysc(struct omap_hwmod *oh)
>  	sf = oh->class->sysc->sysc_flags;
>  
>  	if (sf & SYSC_HAS_SIDLEMODE) {
> -		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
> -			HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
> +		if (oh->flags & HWMOD_SWSUP_SIDLE) {
> +			/*
> +			 * IP blocks without smart idle should be left
> +			 * in force-idle.  Currently this only applies
> +			 * to 32k sync "timer" which is guaranteed to
> +			 * be accessible when the kernel is running.
> +			 * HWMOD_SWSUP_IDLE must also be set on these
> +			 * IP blocks to indicate a hardware problem.
> +			 * XXX Not an ideal workaround.
> +			 */
> +			if (oh->class->sysc->idlemodes &
> +			    (SIDLE_NO | SIDLE_FORCE) &&
> +			    !(oh->class->sysc->idlemodes &
> +			      (SIDLE_SMART || SIDLE_SMART_WKUP)))

There should by binary or, not logical here.

-Tero

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Tero Kristo]

Hi Paul,

There's a bug in this patch, see below.

<clip>

>  {
> @@ -1141,8 +1143,26 @@ static void _enable_sysc(struct omap_hwmod *oh)
>  	sf = oh->class->sysc->sysc_flags;
>  
>  	if (sf & SYSC_HAS_SIDLEMODE) {
> -		idlemode = (oh->flags & HWMOD_SWSUP_SIDLE) ?
> -			HWMOD_IDLEMODE_NO : HWMOD_IDLEMODE_SMART;
> +		if (oh->flags & HWMOD_SWSUP_SIDLE) {
> +			/*
> +			 * IP blocks without smart idle should be left
> +			 * in force-idle.  Currently this only applies
> +			 * to 32k sync "timer" which is guaranteed to
> +			 * be accessible when the kernel is running.
> +			 * HWMOD_SWSUP_IDLE must also be set on these
> +			 * IP blocks to indicate a hardware problem.
> +			 * XXX Not an ideal workaround.
> +			 */
> +			if (oh->class->sysc->idlemodes &
> +			    (SIDLE_NO | SIDLE_FORCE) &&
> +			    !(oh->class->sysc->idlemodes &
> +			      (SIDLE_SMART || SIDLE_SMART_WKUP)))

There should by binary or, not logical here.

-Tero

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Fri, 8 Jun 2012, Cousson, Benoit wrote:

> On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > 
> > > Indeed, what I did not mention is that potentially the whole device 
> > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > phase should be done only when the driver will probe the device.
> > 
> > That means if no driver exists for an IP block, or if the driver isn't
> > using PM runtime, the IP block won't be reset.  And somehow we still are
> > missing drivers in mainline.  We also still have drivers that aren't yet
> > PM runtime converted.
> 
> No the point is still the same as before. You let the drivers do the job if
> they are there, and then do a pass at very late time during the boot process
> to handle the ones that were not probed by any driver.

Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
would be done only when the driver will probe the device.  But you also 
mean that it should also be done for the remaining devices before starting 
userspace.

> At least you will avoid the enable -> reset -> idle -> enable sequence 
> we are doing right now for most of the active drivers when it is not 
> necessary.

It must not be widely known, but early reset was implemented 
intentionally.  The goal was to keep any configuration damage from 
out-of-date or broken bootloaders or previous OSes to a minimum length of 
time during the boot process.

I don't really have a huge problem with switching to a late reset, 
but there are disadvantages to it.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Fri, 8 Jun 2012, Cousson, Benoit wrote:

> On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > 
> > > Indeed, what I did not mention is that potentially the whole device 
> > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > phase should be done only when the driver will probe the device.
> > 
> > That means if no driver exists for an IP block, or if the driver isn't
> > using PM runtime, the IP block won't be reset.  And somehow we still are
> > missing drivers in mainline.  We also still have drivers that aren't yet
> > PM runtime converted.
> 
> No the point is still the same as before. You let the drivers do the job if
> they are there, and then do a pass at very late time during the boot process
> to handle the ones that were not probed by any driver.

Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
would be done only when the driver will probe the device.  But you also 
mean that it should also be done for the remaining devices before starting 
userspace.

> At least you will avoid the enable -> reset -> idle -> enable sequence 
> we are doing right now for most of the active drivers when it is not 
> necessary.

It must not be widely known, but early reset was implemented 
intentionally.  The goal was to keep any configuration damage from 
out-of-date or broken bootloaders or previous OSes to a minimum length of 
time during the boot process.

I don't really have a huge problem with switching to a late reset, 
but there are disadvantages to it.


- Paul

Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Tero Kristo]

Hi Paul,

Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
core retention / dev-off patches. There are a couple of minor issues,
like the bug in patch 5, and the fact that counter_32k hwmod data is
broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
this is not done, device off does not work:

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 66da92d..6ad64c6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -397,8 +397,7 @@ static struct omap_hwmod_class_sysconfig
omap44xx_counter_sysc = {
 	.rev_offs	= 0x0000,
 	.sysc_offs	= 0x0004,
 	.sysc_flags	= SYSC_HAS_SIDLEMODE,
-	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
-			   SIDLE_SMART_WKUP),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
 	.sysc_fields	= &omap_hwmod_sysc_type1,
 };
 


-Tero

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Tero Kristo]

Hi Paul,

Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
core retention / dev-off patches. There are a couple of minor issues,
like the bug in patch 5, and the fact that counter_32k hwmod data is
broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
this is not done, device off does not work:

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 66da92d..6ad64c6 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -397,8 +397,7 @@ static struct omap_hwmod_class_sysconfig
omap44xx_counter_sysc = {
 	.rev_offs	= 0x0000,
 	.sysc_offs	= 0x0004,
 	.sysc_flags	= SYSC_HAS_SIDLEMODE,
-	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
-			   SIDLE_SMART_WKUP),
+	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
 	.sysc_fields	= &omap_hwmod_sysc_type1,
 };
 


-Tero

RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Fri, Jun 08, 2012 at 01:33:46, Paul Walmsley wrote:
> Hi
> 
> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > I couldn't finish my testing today, got into continuous meetings.
> 
> No worries, I understand.
> 
> > Tomorrow, I will test it and update you on this.
> 
> That would be great.
> 
> I took a look at SPRUH73F and sure enough, at least one module (CONTROL) 
> doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field 
> Descriptions".  I'd guess that the PRCM won't idle-req this IP block until 
> the kernel is not running, so we might be able to get away with the 
> existing approach; but the TRM also says:
> 
> "By definition, initiator may generate read/write transaction as long as
> it is out of IDLE state."
> 
> Which pretty much matches my understanding too of the implicit interface 
> contract here.
> 
> So I think we'd better go back to the flag approach as implemented in this 
> patch:
> 
> http://www.spinics.net/lists/arm-kernel/msg176836.html
> 
> The WBU 32k sync timer's behavior is what relies on quirks of the 
> integration that are hard to identify via other means, so it seems to be 
> safest to tag it explicitly.
> 

Paul,

I tested it on AM335x platform just now, it booted up to the Linux prompt, 
but I am sure it is going to impact low power state usecases on AM33xx.

So, I also feel that, flag based approach should be used here. 

Thanks,
Vaibhav

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Hiremath, Vaibhav]

On Fri, Jun 08, 2012 at 01:33:46, Paul Walmsley wrote:
> Hi
> 
> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > I couldn't finish my testing today, got into continuous meetings.
> 
> No worries, I understand.
> 
> > Tomorrow, I will test it and update you on this.
> 
> That would be great.
> 
> I took a look at SPRUH73F and sure enough, at least one module (CONTROL) 
> doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field 
> Descriptions".  I'd guess that the PRCM won't idle-req this IP block until 
> the kernel is not running, so we might be able to get away with the 
> existing approach; but the TRM also says:
> 
> "By definition, initiator may generate read/write transaction as long as
> it is out of IDLE state."
> 
> Which pretty much matches my understanding too of the implicit interface 
> contract here.
> 
> So I think we'd better go back to the flag approach as implemented in this 
> patch:
> 
> http://www.spinics.net/lists/arm-kernel/msg176836.html
> 
> The WBU 32k sync timer's behavior is what relies on quirks of the 
> integration that are hard to identify via other means, so it seems to be 
> safest to tag it explicitly.
> 

Paul,

I tested it on AM335x platform just now, it booted up to the Linux prompt, 
but I am sure it is going to impact low power state usecases on AM33xx.

So, I also feel that, flag based approach should be used here. 

Thanks,
Vaibhav

RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Hiremath, Vaibhav]

On Fri, Jun 08, 2012 at 18:58:51, Paul Walmsley wrote:
> On Fri, 8 Jun 2012, Cousson, Benoit wrote:
> 
> > On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > > 
> > > > Indeed, what I did not mention is that potentially the whole device 
> > > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > > phase should be done only when the driver will probe the device.
> > > 
> > > That means if no driver exists for an IP block, or if the driver isn't
> > > using PM runtime, the IP block won't be reset.  And somehow we still are
> > > missing drivers in mainline.  We also still have drivers that aren't yet
> > > PM runtime converted.
> > 
> > No the point is still the same as before. You let the drivers do the job if
> > they are there, and then do a pass at very late time during the boot process
> > to handle the ones that were not probed by any driver.
> 
> Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
> would be done only when the driver will probe the device.  But you also 
> mean that it should also be done for the remaining devices before starting 
> userspace.
> 
> > At least you will avoid the enable -> reset -> idle -> enable sequence 
> > we are doing right now for most of the active drivers when it is not 
> > necessary.
> 
> It must not be widely known, but early reset was implemented 
> intentionally.  The goal was to keep any configuration damage from 
> out-of-date or broken bootloaders or previous OSes to a minimum length of 
> time during the boot process.
> 
> I don't really have a huge problem with switching to a late reset, 
> but there are disadvantages to it.
> 
> 

I was reading all these discussion and was holding myself back, so that I 
digest before adding another flavor to this discussion,

In case of AM33xx, recently I came across similar issue (rather more than this) with CPSW module.

The issue is,

We have observed that, in order to disable the CPSW module (MODULEMODE=0x0),
We have to assert OCP level reset, before disabling it; else module enters into unknown state, where register status shows, MODULEMODE turns 0x0, but idlests says module is busy.

This has to be done everytime you try to disable the module.

The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
We have to assert reset signal to each submodules.

So the approach I had taken is, I had implemented almost similar function specific to cpsw and introduced flag called HWMOD_SWSUP_RESET_BEFORE_IDLE.


Now the question here would be, should we consider this IP bug or 
integration bug? If it is integration bug, then isn't this device issue?

Thanks,
Vaibhav

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Hiremath, Vaibhav]

On Fri, Jun 08, 2012 at 18:58:51, Paul Walmsley wrote:
> On Fri, 8 Jun 2012, Cousson, Benoit wrote:
> 
> > On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > > 
> > > > Indeed, what I did not mention is that potentially the whole device 
> > > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > > phase should be done only when the driver will probe the device.
> > > 
> > > That means if no driver exists for an IP block, or if the driver isn't
> > > using PM runtime, the IP block won't be reset.  And somehow we still are
> > > missing drivers in mainline.  We also still have drivers that aren't yet
> > > PM runtime converted.
> > 
> > No the point is still the same as before. You let the drivers do the job if
> > they are there, and then do a pass at very late time during the boot process
> > to handle the ones that were not probed by any driver.
> 
> Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
> would be done only when the driver will probe the device.  But you also 
> mean that it should also be done for the remaining devices before starting 
> userspace.
> 
> > At least you will avoid the enable -> reset -> idle -> enable sequence 
> > we are doing right now for most of the active drivers when it is not 
> > necessary.
> 
> It must not be widely known, but early reset was implemented 
> intentionally.  The goal was to keep any configuration damage from 
> out-of-date or broken bootloaders or previous OSes to a minimum length of 
> time during the boot process.
> 
> I don't really have a huge problem with switching to a late reset, 
> but there are disadvantages to it.
> 
> 

I was reading all these discussion and was holding myself back, so that I 
digest before adding another flavor to this discussion,

In case of AM33xx, recently I came across similar issue (rather more than this) with CPSW module.

The issue is,

We have observed that, in order to disable the CPSW module (MODULEMODE=0x0),
We have to assert OCP level reset, before disabling it; else module enters into unknown state, where register status shows, MODULEMODE turns 0x0, but idlests says module is busy.

This has to be done everytime you try to disable the module.

The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
We have to assert reset signal to each submodules.

So the approach I had taken is, I had implemented almost similar function specific to cpsw and introduced flag called HWMOD_SWSUP_RESET_BEFORE_IDLE.


Now the question here would be, should we consider this IP bug or 
integration bug? If it is integration bug, then isn't this device issue?

Thanks,
Vaibhav

AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Paul Walmsley]

Hi

On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:

> In case of AM33xx, recently I came across similar issue (rather more 
> than this) with CPSW module.
> 
> The issue is,
> 
> We have observed that, in order to disable the CPSW module 
> (MODULEMODE=0x0), We have to assert OCP level reset, before disabling 
> it; else module enters into unknown state, where register status shows, 
> MODULEMODE turns 0x0, but idlests says module is busy.
> 
> This has to be done everytime you try to disable the module.
> 
> The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
> We have to assert reset signal to each submodules.
> 
> So the approach I had taken is, I had implemented almost similar 
> function specific to cpsw and introduced flag called 
> HWMOD_SWSUP_RESET_BEFORE_IDLE.

Why "SWSUP" ? 

> Now the question here would be, should we consider this IP bug or 
> integration bug? If it is integration bug, then isn't this device issue?

I don't know if it's a bug in either place, but it sounds like something 
that needs to be handled in the _am335x_disable_module() code in 
mach-omap2/omap_hwmod.c.


- Paul

AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Paul Walmsley]

Hi

On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:

> In case of AM33xx, recently I came across similar issue (rather more 
> than this) with CPSW module.
> 
> The issue is,
> 
> We have observed that, in order to disable the CPSW module 
> (MODULEMODE=0x0), We have to assert OCP level reset, before disabling 
> it; else module enters into unknown state, where register status shows, 
> MODULEMODE turns 0x0, but idlests says module is busy.
> 
> This has to be done everytime you try to disable the module.
> 
> The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
> We have to assert reset signal to each submodules.
> 
> So the approach I had taken is, I had implemented almost similar 
> function specific to cpsw and introduced flag called 
> HWMOD_SWSUP_RESET_BEFORE_IDLE.

Why "SWSUP" ? 

> Now the question here would be, should we consider this IP bug or 
> integration bug? If it is integration bug, then isn't this device issue?

I don't know if it's a bug in either place, but it sounds like something 
that needs to be handled in the _am335x_disable_module() code in 
mach-omap2/omap_hwmod.c.


- Paul

Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

On Fri, 8 Jun 2012, Tero Kristo wrote:

> There's a bug in this patch, see below.

...

> There should by binary or, not logical here.

Thanks for reporting this, I've added some credit in the patch 
description.  This has been fixed by switching back to using a flag-based 
approach based on some comments from Vaibhav Hiremath.


- Paul

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Paul Walmsley]

On Fri, 8 Jun 2012, Tero Kristo wrote:

> There's a bug in this patch, see below.

...

> There should by binary or, not logical here.

Thanks for reporting this, I've added some credit in the patch 
description.  This has been fixed by switching back to using a flag-based 
approach based on some comments from Vaibhav Hiremath.


- Paul

Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

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

Hi Tero,

On Fri, 8 Jun 2012, Tero Kristo wrote:

> Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> core retention / dev-off patches. There are a couple of minor issues,
> like the bug in patch 5, and the fact that counter_32k hwmod data is
> broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> this is not done, device off does not work:
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 66da92d..6ad64c6 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -397,8 +397,7 @@ static struct omap_hwmod_class_sysconfig
> omap44xx_counter_sysc = {
>  	.rev_offs	= 0x0000,
>  	.sysc_offs	= 0x0004,
>  	.sysc_flags	= SYSC_HAS_SIDLEMODE,
> -	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> -			   SIDLE_SMART_WKUP),
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
>  	.sysc_fields	= &omap_hwmod_sysc_type1,
>  };

Thanks, Benoît mentioned this too.  Just added a patch for this to the 
second version of this fixes series.


- Paul

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

Hi Tero,

On Fri, 8 Jun 2012, Tero Kristo wrote:

> Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> core retention / dev-off patches. There are a couple of minor issues,
> like the bug in patch 5, and the fact that counter_32k hwmod data is
> broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> this is not done, device off does not work:
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 66da92d..6ad64c6 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -397,8 +397,7 @@ static struct omap_hwmod_class_sysconfig
> omap44xx_counter_sysc = {
>  	.rev_offs	= 0x0000,
>  	.sysc_offs	= 0x0004,
>  	.sysc_flags	= SYSC_HAS_SIDLEMODE,
> -	.idlemodes	= (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> -			   SIDLE_SMART_WKUP),
> +	.idlemodes	= (SIDLE_FORCE | SIDLE_NO),
>  	.sysc_fields	= &omap_hwmod_sysc_type1,
>  };

Thanks, Beno?t mentioned this too.  Just added a patch for this to the 
second version of this fixes series.


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Oh OK yeah makes sense as that's hwmod internal function. Then the driver
> specific part should use just void __iomem *base and use readl/writel and
> live under include/linux/platform_data/omap-usb.h.

This sounds like something that might be flame-bait, since these functions 
aren't platform_data.

How about putting these functions in arch/arm/plat-omap/include/plat?  
Drivers are able to include those files easily.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Thu, 7 Jun 2012, Tony Lindgren wrote:

> Oh OK yeah makes sense as that's hwmod internal function. Then the driver
> specific part should use just void __iomem *base and use readl/writel and
> live under include/linux/platform_data/omap-usb.h.

This sounds like something that might be flame-bait, since these functions 
aren't platform_data.

How about putting these functions in arch/arm/plat-omap/include/plat?  
Drivers are able to include those files easily.


- Paul

RE: AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Hiremath, Vaibhav]

On Sat, Jun 09, 2012 at 04:40:03, Paul Walmsley wrote:
> Hi
> 
> On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > In case of AM33xx, recently I came across similar issue (rather more 
> > than this) with CPSW module.
> > 
> > The issue is,
> > 
> > We have observed that, in order to disable the CPSW module 
> > (MODULEMODE=0x0), We have to assert OCP level reset, before disabling 
> > it; else module enters into unknown state, where register status shows, 
> > MODULEMODE turns 0x0, but idlests says module is busy.
> > 
> > This has to be done everytime you try to disable the module.
> > 
> > The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
> > We have to assert reset signal to each submodules.
> > 
> > So the approach I had taken is, I had implemented almost similar 
> > function specific to cpsw and introduced flag called 
> > HWMOD_SWSUP_RESET_BEFORE_IDLE.
> 
> Why "SWSUP" ? 
> 

Since it was SW initiated reset assertion so I added this prefix.

> > Now the question here would be, should we consider this IP bug or 
> > integration bug? If it is integration bug, then isn't this device issue?
> 
> I don't know if it's a bug in either place, but it sounds like something 
> that needs to be handled in the _am335x_disable_module() code in 
> mach-omap2/omap_hwmod.c.
> 

Yes, certainly it should be part of _am335x_disable_module().

Thanks,
Vaibhav

AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Hiremath, Vaibhav]

On Sat, Jun 09, 2012 at 04:40:03, Paul Walmsley wrote:
> Hi
> 
> On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:
> 
> > In case of AM33xx, recently I came across similar issue (rather more 
> > than this) with CPSW module.
> > 
> > The issue is,
> > 
> > We have observed that, in order to disable the CPSW module 
> > (MODULEMODE=0x0), We have to assert OCP level reset, before disabling 
> > it; else module enters into unknown state, where register status shows, 
> > MODULEMODE turns 0x0, but idlests says module is busy.
> > 
> > This has to be done everytime you try to disable the module.
> > 
> > The worst part here is, CPSW has 4 submodules (SS, SL1, SL2, CPDMA),
> > We have to assert reset signal to each submodules.
> > 
> > So the approach I had taken is, I had implemented almost similar 
> > function specific to cpsw and introduced flag called 
> > HWMOD_SWSUP_RESET_BEFORE_IDLE.
> 
> Why "SWSUP" ? 
> 

Since it was SW initiated reset assertion so I added this prefix.

> > Now the question here would be, should we consider this IP bug or 
> > integration bug? If it is integration bug, then isn't this device issue?
> 
> I don't know if it's a bug in either place, but it sounds like something 
> that needs to be handled in the _am335x_disable_module() code in 
> mach-omap2/omap_hwmod.c.
> 

Yes, certainly it should be part of _am335x_disable_module().

Thanks,
Vaibhav

RE: AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Paul Walmsley]

On Sat, 9 Jun 2012, Hiremath, Vaibhav wrote:

> On Sat, Jun 09, 2012 at 04:40:03, Paul Walmsley wrote:
> > On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:
> > 
> > > So the approach I had taken is, I had implemented almost similar 
> > > function specific to cpsw and introduced flag called 
> > > HWMOD_SWSUP_RESET_BEFORE_IDLE.
> > 
> > Why "SWSUP" ? 
> 
> Since it was SW initiated reset assertion so I added this prefix.

Maybe just use HWMOD_RESET_BEFORE_IDLE.  The code use SWSUP to mean 
"software-supervised" in the context of idle.

> > > Now the question here would be, should we consider this IP bug or 
> > > integration bug? If it is integration bug, then isn't this device issue?
> > 
> > I don't know if it's a bug in either place, but it sounds like something 
> > that needs to be handled in the _am335x_disable_module() code in 
> > mach-omap2/omap_hwmod.c.
> 
> Yes, certainly it should be part of _am335x_disable_module().

Great. Care to send a patch?


- Paul

AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")

[Paul Walmsley]

On Sat, 9 Jun 2012, Hiremath, Vaibhav wrote:

> On Sat, Jun 09, 2012 at 04:40:03, Paul Walmsley wrote:
> > On Fri, 8 Jun 2012, Hiremath, Vaibhav wrote:
> > 
> > > So the approach I had taken is, I had implemented almost similar 
> > > function specific to cpsw and introduced flag called 
> > > HWMOD_SWSUP_RESET_BEFORE_IDLE.
> > 
> > Why "SWSUP" ? 
> 
> Since it was SW initiated reset assertion so I added this prefix.

Maybe just use HWMOD_RESET_BEFORE_IDLE.  The code use SWSUP to mean 
"software-supervised" in the context of idle.

> > > Now the question here would be, should we consider this IP bug or 
> > > integration bug? If it is integration bug, then isn't this device issue?
> > 
> > I don't know if it's a bug in either place, but it sounds like something 
> > that needs to be handled in the _am335x_disable_module() code in 
> > mach-omap2/omap_hwmod.c.
> 
> Yes, certainly it should be part of _am335x_disable_module().

Great. Care to send a patch?


- Paul

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120608 06:33]:
> On Fri, 8 Jun 2012, Cousson, Benoit wrote:
> 
> > On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > > 
> > > > Indeed, what I did not mention is that potentially the whole device 
> > > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > > phase should be done only when the driver will probe the device.
> > > 
> > > That means if no driver exists for an IP block, or if the driver isn't
> > > using PM runtime, the IP block won't be reset.  And somehow we still are
> > > missing drivers in mainline.  We also still have drivers that aren't yet
> > > PM runtime converted.
> > 
> > No the point is still the same as before. You let the drivers do the job if
> > they are there, and then do a pass at very late time during the boot process
> > to handle the ones that were not probed by any driver.
> 
> Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
> would be done only when the driver will probe the device.  But you also 
> mean that it should also be done for the remaining devices before starting 
> userspace.
> 
> > At least you will avoid the enable -> reset -> idle -> enable sequence 
> > we are doing right now for most of the active drivers when it is not 
> > necessary.
> 
> It must not be widely known, but early reset was implemented 
> intentionally.  The goal was to keep any configuration damage from 
> out-of-date or broken bootloaders or previous OSes to a minimum length of 
> time during the boot process.

These devices should get reset as the device drivers initialize. Some parts
of course need to be initialized properly early like caches and DMA controller.
 
> I don't really have a huge problem with switching to a late reset, 
> but there are disadvantages to it.

I think the early reset actually has more disadvantages to it compared
to driver reset. We don't see any errors when things go wrong, and we
may even kill the only debug console in the reset process.

We are already doing what Benoit describes with clocks where we only
reset the unclaimed ones at late_initcall level, and that has proven
to work well.

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120608 06:33]:
> On Fri, 8 Jun 2012, Cousson, Benoit wrote:
> 
> > On 6/8/2012 3:11 AM, Paul Walmsley wrote:
> > > On Thu, 7 Jun 2012, Cousson, Benoit wrote:
> > > 
> > > > Indeed, what I did not mention is that potentially the whole device 
> > > > init should be done ondemand as well. Meaning the whole hwmod setup 
> > > > phase should be done only when the driver will probe the device.
> > > 
> > > That means if no driver exists for an IP block, or if the driver isn't
> > > using PM runtime, the IP block won't be reset.  And somehow we still are
> > > missing drivers in mainline.  We also still have drivers that aren't yet
> > > PM runtime converted.
> > 
> > No the point is still the same as before. You let the drivers do the job if
> > they are there, and then do a pass at very late time during the boot process
> > to handle the ones that were not probed by any driver.
> 
> Ah, I see what you mean.  Above you wrote that the the hwmod setup phase 
> would be done only when the driver will probe the device.  But you also 
> mean that it should also be done for the remaining devices before starting 
> userspace.
> 
> > At least you will avoid the enable -> reset -> idle -> enable sequence 
> > we are doing right now for most of the active drivers when it is not 
> > necessary.
> 
> It must not be widely known, but early reset was implemented 
> intentionally.  The goal was to keep any configuration damage from 
> out-of-date or broken bootloaders or previous OSes to a minimum length of 
> time during the boot process.

These devices should get reset as the device drivers initialize. Some parts
of course need to be initialized properly early like caches and DMA controller.
 
> I don't really have a huge problem with switching to a late reset, 
> but there are disadvantages to it.

I think the early reset actually has more disadvantages to it compared
to driver reset. We don't see any errors when things go wrong, and we
may even kill the only debug console in the reset process.

We are already doing what Benoit describes with clocks where we only
reset the unclaimed ones at late_initcall level, and that has proven
to work well.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120608 18:35]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Oh OK yeah makes sense as that's hwmod internal function. Then the driver
> > specific part should use just void __iomem *base and use readl/writel and
> > live under include/linux/platform_data/omap-usb.h.
> 
> This sounds like something that might be flame-bait, since these functions 
> aren't platform_data.

They at least should be as both platform init code and the driver will
potentially use these functions.
 
> How about putting these functions in arch/arm/plat-omap/include/plat?  
> Drivers are able to include those files easily.

We need to pretty much get rid of all those headers and make them driver
specific for the multi-zimage support. Drivers should be arch independent,
and whatever parts need to be shared between platform init code and drivers
should follow the standard platform driver stuff.

The other place would be just the standard driver include at somewhere
like include/linux/usb/omap-usb.h etc.

Regards,

Tony

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Tony Lindgren]

* Paul Walmsley <paul@pwsan.com> [120608 18:35]:
> On Thu, 7 Jun 2012, Tony Lindgren wrote:
> 
> > Oh OK yeah makes sense as that's hwmod internal function. Then the driver
> > specific part should use just void __iomem *base and use readl/writel and
> > live under include/linux/platform_data/omap-usb.h.
> 
> This sounds like something that might be flame-bait, since these functions 
> aren't platform_data.

They at least should be as both platform init code and the driver will
potentially use these functions.
 
> How about putting these functions in arch/arm/plat-omap/include/plat?  
> Drivers are able to include those files easily.

We need to pretty much get rid of all those headers and make them driver
specific for the multi-zimage support. Drivers should be arch independent,
and whatever parts need to be shared between platform init code and drivers
should follow the standard platform driver stuff.

The other place would be just the standard driver include at somewhere
like include/linux/usb/omap-usb.h etc.

Regards,

Tony

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Sun, 10 Jun 2012, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [120608 06:33]:
>  
> > I don't really have a huge problem with switching to a late reset, 
> > but there are disadvantages to it.
> 
> I think the early reset actually has more disadvantages to it compared
> to driver reset. We don't see any errors when things go wrong, and we
> may even kill the only debug console in the reset process.
> 
> We are already doing what Benoit describes with clocks where we only
> reset the unclaimed ones at late_initcall level, and that has proven
> to work well.

The difference though between the clock and the IP block init is that 
leaving the clocks on until later has no effect on system stability.  The 
chip simply consumes more energy.

But if the IP blocks are reset later, and the bootloader or previous OS 
gets some register settings wrong, there's a greater risk of system 
instability or unpredictable behavior during the boot process.

I agree that it is probably easier to debug a late reset approach.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Sun, 10 Jun 2012, Tony Lindgren wrote:

> * Paul Walmsley <paul@pwsan.com> [120608 06:33]:
>  
> > I don't really have a huge problem with switching to a late reset, 
> > but there are disadvantages to it.
> 
> I think the early reset actually has more disadvantages to it compared
> to driver reset. We don't see any errors when things go wrong, and we
> may even kill the only debug console in the reset process.
> 
> We are already doing what Benoit describes with clocks where we only
> reset the unclaimed ones at late_initcall level, and that has proven
> to work well.

The difference though between the clock and the IP block init is that 
leaving the clocks on until later has no effect on system stability.  The 
chip simply consumes more energy.

But if the IP blocks are reset later, and the bootloader or previous OS 
gets some register settings wrong, there's a greater risk of system 
instability or unpredictable behavior during the boot process.

I agree that it is probably easier to debug a late reset approach.


- Paul

Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Cousson, Benoit]

On 6/8/2012 9:10 PM, Hiremath, Vaibhav wrote:
> On Fri, Jun 08, 2012 at 01:33:46, Paul Walmsley wrote:
>> Hi
>>
>> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
>>
>>> I couldn't finish my testing today, got into continuous meetings.
>>
>> No worries, I understand.
>>
>>> Tomorrow, I will test it and update you on this.
>>
>> That would be great.
>>
>> I took a look at SPRUH73F and sure enough, at least one module (CONTROL)
>> doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field
>> Descriptions".  I'd guess that the PRCM won't idle-req this IP block until
>> the kernel is not running, so we might be able to get away with the
>> existing approach; but the TRM also says:
>>
>> "By definition, initiator may generate read/write transaction as long as
>> it is out of IDLE state."
>>
>> Which pretty much matches my understanding too of the implicit interface
>> contract here.
>>
>> So I think we'd better go back to the flag approach as implemented in this
>> patch:
>>
>> http://www.spinics.net/lists/arm-kernel/msg176836.html
>>
>> The WBU 32k sync timer's behavior is what relies on quirks of the
>> integration that are hard to identify via other means, so it seems to be
>> safest to tag it explicitly.
>>
>
> Paul,
>
> I tested it on AM335x platform just now, it booted up to the Linux prompt,
> but I am sure it is going to impact low power state usecases on AM33xx.

Why are you sure about that? As explained to Paul, using the force-idle 
will do the same as smart-idle most of the time.

So what power impact are you expecting?

> So, I also feel that, flag based approach should be used here.

Why? Why adding a new flag to detect a condition that is already 
captured somewhere else?

Regards,
Benoit

[PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer

[Cousson, Benoit]

On 6/8/2012 9:10 PM, Hiremath, Vaibhav wrote:
> On Fri, Jun 08, 2012 at 01:33:46, Paul Walmsley wrote:
>> Hi
>>
>> On Thu, 7 Jun 2012, Hiremath, Vaibhav wrote:
>>
>>> I couldn't finish my testing today, got into continuous meetings.
>>
>> No worries, I understand.
>>
>>> Tomorrow, I will test it and update you on this.
>>
>> That would be great.
>>
>> I took a look at SPRUH73F and sure enough, at least one module (CONTROL)
>> doesn't support smart-idle -- per Table 14-217 "CONTROL Register Field
>> Descriptions".  I'd guess that the PRCM won't idle-req this IP block until
>> the kernel is not running, so we might be able to get away with the
>> existing approach; but the TRM also says:
>>
>> "By definition, initiator may generate read/write transaction as long as
>> it is out of IDLE state."
>>
>> Which pretty much matches my understanding too of the implicit interface
>> contract here.
>>
>> So I think we'd better go back to the flag approach as implemented in this
>> patch:
>>
>> http://www.spinics.net/lists/arm-kernel/msg176836.html
>>
>> The WBU 32k sync timer's behavior is what relies on quirks of the
>> integration that are hard to identify via other means, so it seems to be
>> safest to tag it explicitly.
>>
>
> Paul,
>
> I tested it on AM335x platform just now, it booted up to the Linux prompt,
> but I am sure it is going to impact low power state usecases on AM33xx.

Why are you sure about that? As explained to Paul, using the force-idle 
will do the same as smart-idle most of the time.

So what power impact are you expecting?

> So, I also feel that, flag based approach should be used here.

Why? Why adding a new flag to detect a condition that is already 
captured somewhere else?

Regards,
Benoit

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

Hi Paul,

On 6/11/2012 10:04 AM, Paul Walmsley wrote:
> On Sun, 10 Jun 2012, Tony Lindgren wrote:
>
>> * Paul Walmsley <paul@pwsan.com> [120608 06:33]:
>>
>>> I don't really have a huge problem with switching to a late reset,
>>> but there are disadvantages to it.
>>
>> I think the early reset actually has more disadvantages to it compared
>> to driver reset. We don't see any errors when things go wrong, and we
>> may even kill the only debug console in the reset process.
>>
>> We are already doing what Benoit describes with clocks where we only
>> reset the unclaimed ones at late_initcall level, and that has proven
>> to work well.
>
> The difference though between the clock and the IP block init is that
> leaving the clocks on until later has no effect on system stability.  The
> chip simply consumes more energy.
>
> But if the IP blocks are reset later, and the bootloader or previous OS
> gets some register settings wrong, there's a greater risk of system
> instability or unpredictable behavior during the boot process.

Mmm, I'm not convince by that. If we delay the PM init at the very last 
time, at least after the modules are properly reset and init, I do not 
think we will have more issues than today.

Regards,
Benoit

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Cousson, Benoit]

Hi Paul,

On 6/11/2012 10:04 AM, Paul Walmsley wrote:
> On Sun, 10 Jun 2012, Tony Lindgren wrote:
>
>> * Paul Walmsley <paul@pwsan.com> [120608 06:33]:
>>
>>> I don't really have a huge problem with switching to a late reset,
>>> but there are disadvantages to it.
>>
>> I think the early reset actually has more disadvantages to it compared
>> to driver reset. We don't see any errors when things go wrong, and we
>> may even kill the only debug console in the reset process.
>>
>> We are already doing what Benoit describes with clocks where we only
>> reset the unclaimed ones at late_initcall level, and that has proven
>> to work well.
>
> The difference though between the clock and the IP block init is that
> leaving the clocks on until later has no effect on system stability.  The
> chip simply consumes more energy.
>
> But if the IP blocks are reset later, and the bootloader or previous OS
> gets some register settings wrong, there's a greater risk of system
> instability or unpredictable behavior during the boot process.

Mmm, I'm not convince by that. If we delay the PM init at the very last 
time, at least after the modules are properly reset and init, I do not 
think we will have more issues than today.

Regards,
Benoit

Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Mon, 11 Jun 2012, Cousson, Benoit wrote:

> On 6/11/2012 10:04 AM, Paul Walmsley wrote:
> 
> > But if the IP blocks are reset later, and the bootloader or previous OS
> > gets some register settings wrong, there's a greater risk of system
> > instability or unpredictable behavior during the boot process.
> 
> Mmm, I'm not convince by that. If we delay the PM init at the very last 
> time, at least after the modules are properly reset and init, I do not 
> think we will have more issues than today.

My intent was not to convince, but to explain.

Certainly back in the OMAP3 days there were bootloaders that got SDRAM and 
GPMC timings wrong.  No way did I want to be chasing down kernel "bugs" 
based on those.

Anyway, once people finish fixing the drivers, then we should be able to 
switch to late hwmod reset.


- Paul

[PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)

[Paul Walmsley]

On Mon, 11 Jun 2012, Cousson, Benoit wrote:

> On 6/11/2012 10:04 AM, Paul Walmsley wrote:
> 
> > But if the IP blocks are reset later, and the bootloader or previous OS
> > gets some register settings wrong, there's a greater risk of system
> > instability or unpredictable behavior during the boot process.
> 
> Mmm, I'm not convince by that. If we delay the PM init at the very last 
> time, at least after the modules are properly reset and init, I do not 
> think we will have more issues than today.

My intent was not to convince, but to explain.

Certainly back in the OMAP3 days there were bootloaders that got SDRAM and 
GPMC timings wrong.  No way did I want to be chasing down kernel "bugs" 
based on those.

Anyway, once people finish fixing the drivers, then we should be able to 
switch to late hwmod reset.


- Paul

Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

Hi Tero,

On Fri, 8 Jun 2012, Tero Kristo wrote:

> Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> core retention / dev-off patches. There are a couple of minor issues,
> like the bug in patch 5, and the fact that counter_32k hwmod data is
> broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> this is not done, device off does not work:

Thanks for the testing and the reminder.  Will send the next version of 
this soon to address more comments from Tony.


- Paul

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Paul Walmsley]

Hi Tero,

On Fri, 8 Jun 2012, Tero Kristo wrote:

> Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> core retention / dev-off patches. There are a couple of minor issues,
> like the bug in patch 5, and the fact that counter_32k hwmod data is
> broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> this is not done, device off does not work:

Thanks for the testing and the reminder.  Will send the next version of 
this soon to address more comments from Tony.


- Paul

Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Tero Kristo]

On Wed, 2012-06-13 at 17:55 -0600, Paul Walmsley wrote:
> Hi Tero,
> 
> On Fri, 8 Jun 2012, Tero Kristo wrote:
> 
> > Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> > core retention / dev-off patches. There are a couple of minor issues,
> > like the bug in patch 5, and the fact that counter_32k hwmod data is
> > broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> > this is not done, device off does not work:
> 
> Thanks for the testing and the reminder.  Will send the next version of 
> this soon to address more comments from Tony.

I also tested the v2 with my last dev-off branch and it works perfectly
without any additional hwmod related tweaks.

-Tero

[PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc

[Tero Kristo]

On Wed, 2012-06-13 at 17:55 -0600, Paul Walmsley wrote:
> Hi Tero,
> 
> On Fri, 8 Jun 2012, Tero Kristo wrote:
> 
> > Tested this set on top of v3.5-rc1 with omap3 / omap4 suspend + my omap4
> > core retention / dev-off patches. There are a couple of minor issues,
> > like the bug in patch 5, and the fact that counter_32k hwmod data is
> > broken for omap4. This fix is needed on omap4 to fix the counter_32k, if
> > this is not done, device off does not work:
> 
> Thanks for the testing and the reminder.  Will send the next version of 
> this soon to address more comments from Tony.

I also tested the v2 with my last dev-off branch and it works perfectly
without any additional hwmod related tweaks.

-Tero

Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Paul Walmsley]

On Thu, 7 Jun 2012, Rajendra Nayak wrote:

> On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
> > Until the OMAP4 code is converted to disable the use of the clock
> > framework-based clockdomain enable/disable sequence, any clock used as
> > a hwmod main_clk must have a clockdomain associated with it.  This
> > patch populates some clock structure clockdomain names to resolve the
> > following warnings during kernel init:
> 
> But these associations are useless if the clock is not a 'gate' clock, 
> except for getting rid of these warnings. Maybe we should make hwmod 
> understand that not all clocks need to have a clockdomain associated 
> with it and stop complaining.

In retrospect, I think I should have made clockdomains mandatory for all 
clocks for OMAP4 back then, rather than only allowing them for some 
clocks.  As I understand it, that would have saved a lot of time and 
debugging frustration on the bug fixed by commit 
6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data: Force a 
DPLL clkdm/pwrdm ON before a relock").  Oh well.


- Paul

[PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Paul Walmsley]

On Thu, 7 Jun 2012, Rajendra Nayak wrote:

> On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
> > Until the OMAP4 code is converted to disable the use of the clock
> > framework-based clockdomain enable/disable sequence, any clock used as
> > a hwmod main_clk must have a clockdomain associated with it.  This
> > patch populates some clock structure clockdomain names to resolve the
> > following warnings during kernel init:
> 
> But these associations are useless if the clock is not a 'gate' clock, 
> except for getting rid of these warnings. Maybe we should make hwmod 
> understand that not all clocks need to have a clockdomain associated 
> with it and stop complaining.

In retrospect, I think I should have made clockdomains mandatory for all 
clocks for OMAP4 back then, rather than only allowing them for some 
clocks.  As I understand it, that would have saved a lot of time and 
debugging frustration on the bug fixed by commit 
6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data: Force a 
DPLL clkdm/pwrdm ON before a relock").  Oh well.


- Paul

Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Rajendra Nayak]

On Monday 18 June 2012 11:11 PM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Rajendra Nayak wrote:
>
>> On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
>>> Until the OMAP4 code is converted to disable the use of the clock
>>> framework-based clockdomain enable/disable sequence, any clock used as
>>> a hwmod main_clk must have a clockdomain associated with it.  This
>>> patch populates some clock structure clockdomain names to resolve the
>>> following warnings during kernel init:
>>
>> But these associations are useless if the clock is not a 'gate' clock,
>> except for getting rid of these warnings. Maybe we should make hwmod
>> understand that not all clocks need to have a clockdomain associated
>> with it and stop complaining.
>
> In retrospect, I think I should have made clockdomains mandatory for all
> clocks for OMAP4 back then, rather than only allowing them for some
> clocks.  As I understand it, that would have saved a lot of time and
> debugging frustration on the bug fixed by commit
> 6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data: Force a
> DPLL clkdm/pwrdm ON before a relock").  Oh well.

We should certainly have a better way for PM code to WARN() users if
some clocks which need clockdomains to be programmed together with
the clocks, have the clockdomain information missing. One way to do
that is make it mandatory for *all* clocks to have an associated
clockdomain, but that would mean we populate dummy and unnecessary
data atleast in some cases wherein it never gets used, just to get
rid of the WARN(). That certainly does not seem right.
The other way is really to make our frameworks understand and WARN()
*intelligently*.

Today we WARN() users only for main_clks used in hwmod. I feel this
WARN() should instead come from the clock framework, because there
are clearly clocks outside of what is handled by hwmod (like the one
in the commit above) which need this information.
We should also look at making the clock framework intelligent to
identify which clocks really need a clockdomain associated instead
of adding a WARN for every other clock. just my 2 cents..

>
>
> - Paul

[PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks

[Rajendra Nayak]

On Monday 18 June 2012 11:11 PM, Paul Walmsley wrote:
> On Thu, 7 Jun 2012, Rajendra Nayak wrote:
>
>> On Thursday 07 June 2012 11:43 AM, Paul Walmsley wrote:
>>> Until the OMAP4 code is converted to disable the use of the clock
>>> framework-based clockdomain enable/disable sequence, any clock used as
>>> a hwmod main_clk must have a clockdomain associated with it.  This
>>> patch populates some clock structure clockdomain names to resolve the
>>> following warnings during kernel init:
>>
>> But these associations are useless if the clock is not a 'gate' clock,
>> except for getting rid of these warnings. Maybe we should make hwmod
>> understand that not all clocks need to have a clockdomain associated
>> with it and stop complaining.
>
> In retrospect, I think I should have made clockdomains mandatory for all
> clocks for OMAP4 back then, rather than only allowing them for some
> clocks.  As I understand it, that would have saved a lot of time and
> debugging frustration on the bug fixed by commit
> 6c4a057bffe9823221eab547e11fac181dc18a2b ("ARM: OMAP4: clock data: Force a
> DPLL clkdm/pwrdm ON before a relock").  Oh well.

We should certainly have a better way for PM code to WARN() users if
some clocks which need clockdomains to be programmed together with
the clocks, have the clockdomain information missing. One way to do
that is make it mandatory for *all* clocks to have an associated
clockdomain, but that would mean we populate dummy and unnecessary
data atleast in some cases wherein it never gets used, just to get
rid of the WARN(). That certainly does not seem right.
The other way is really to make our frameworks understand and WARN()
*intelligently*.

Today we WARN() users only for main_clks used in hwmod. I feel this
WARN() should instead come from the clock framework, because there
are clearly clocks outside of what is handled by hwmod (like the one
in the commit above) which need this information.
We should also look at making the clock framework intelligent to
identify which clocks really need a clockdomain associated instead
of adding a WARN for every other clock. just my 2 cents..

>
>
> - Paul