* [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 01/11] ARM: OMAP2+: hwmod: add setup_preprogram hook
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Péter Ujfalusi, Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 01/11] ARM: OMAP2+: hwmod: add setup_preprogram hook
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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);
};
/**
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Péter Ujfalusi, Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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 */
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 7:19 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:19 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Péter Ujfalusi, Benoît Cousson
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 7:19 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:19 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 7:19 ` Tony Lindgren
@ 2012-06-07 7:31 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:31 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Péter Ujfalusi, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 7:31 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:31 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 7:31 ` Paul Walmsley
@ 2012-06-07 7:48 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:48 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Péter Ujfalusi, Benoît Cousson
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 7:48 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:48 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 7:48 ` Tony Lindgren
@ 2012-06-07 10:45 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:45 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Péter Ujfalusi, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 10:45 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:45 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
2012-06-07 10:45 ` Paul Walmsley
@ 2012-06-07 11:08 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 11:08 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Péter Ujfalusi, Benoît Cousson
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 02/11] ARM: OMAP4+: AESS: enable internal auto-gating during initial setup
@ 2012-06-07 11:08 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 11:08 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 03/11] ARM: OMAP4: hwmod data: add SL2IF hardreset line
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Tero Kristo
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,
},
},
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 03/11] ARM: OMAP4: hwmod data: add SL2IF hardreset line
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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,
},
},
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel
Cc: Tero Kristo, Benoît Cousson, Felipe Balbi
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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);
+
/*-------------------------------------------------------------------------*/
/*
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 7:31 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:31 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 7:31 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:31 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:31 ` Tony Lindgren
@ 2012-06-07 7:33 ` Felipe Balbi
-1 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 7:33 UTC (permalink / raw)
To: Tony Lindgren
Cc: Paul Walmsley, linux-omap, linux-arm-kernel, Tero Kristo,
Benoît Cousson, 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 --]
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 7:33 ` Felipe Balbi
0 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 7:33 UTC (permalink / raw)
To: linux-arm-kernel
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>
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:33 ` Felipe Balbi
@ 2012-06-07 8:00 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 8:00 UTC (permalink / raw)
To: Felipe Balbi
Cc: Paul Walmsley, linux-omap, linux-arm-kernel, Tero Kristo,
Benoît Cousson
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 8:00 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 8:00 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:31 ` Tony Lindgren
@ 2012-06-07 7:40 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:40 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 7:40 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:40 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:40 ` Paul Walmsley
@ 2012-06-07 7:51 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:51 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 7:51 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 7:51 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:51 ` Tony Lindgren
@ 2012-06-07 7:55 ` Felipe Balbi
-1 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 7:55 UTC (permalink / raw)
To: Tony Lindgren
Cc: Paul Walmsley, linux-omap, linux-arm-kernel, Tero Kristo,
Benoît Cousson, 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 --]
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 7:55 ` Felipe Balbi
0 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 7:55 UTC (permalink / raw)
To: linux-arm-kernel
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>
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:55 ` Felipe Balbi
@ 2012-06-07 8:02 ` Cousson, Benoit
-1 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-07 8:02 UTC (permalink / raw)
To: balbi
Cc: Tony Lindgren, Paul Walmsley, linux-omap, linux-arm-kernel, Tero Kristo
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 8:02 ` Cousson, Benoit
0 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-07 8:02 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 8:02 ` Cousson, Benoit
@ 2012-06-07 8:10 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 8:10 UTC (permalink / raw)
To: Cousson, Benoit
Cc: balbi, Paul Walmsley, linux-omap, linux-arm-kernel, Tero Kristo
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 8:10 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 8:10 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 8:02 ` Cousson, Benoit
@ 2012-06-07 8:14 ` Felipe Balbi
-1 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 8:14 UTC (permalink / raw)
To: Cousson, Benoit
Cc: balbi, Tony Lindgren, Paul Walmsley, linux-omap,
linux-arm-kernel, Tero Kristo
[-- 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 --]
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 8:14 ` Felipe Balbi
0 siblings, 0 replies; 120+ messages in thread
From: Felipe Balbi @ 2012-06-07 8:14 UTC (permalink / raw)
To: linux-arm-kernel
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>
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 8:02 ` Cousson, Benoit
@ 2012-06-07 10:52 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:52 UTC (permalink / raw)
To: Cousson, Benoit
Cc: balbi, Tony Lindgren, linux-omap, linux-arm-kernel, Tero Kristo
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 10:52 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:52 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 10:52 ` Paul Walmsley
@ 2012-06-07 12:30 ` Cousson, Benoit
-1 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-07 12:30 UTC (permalink / raw)
To: Paul Walmsley
Cc: balbi, Tony Lindgren, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
+ 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 12:30 ` Cousson, Benoit
0 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-07 12:30 UTC (permalink / raw)
To: linux-arm-kernel
+ 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 12:30 ` Cousson, Benoit
@ 2012-06-08 1:11 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 1:11 UTC (permalink / raw)
To: Cousson, Benoit
Cc: balbi, Tony Lindgren, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-08 1:11 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 1:11 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-08 1:11 ` Paul Walmsley
@ 2012-06-08 13:13 ` Cousson, Benoit
-1 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-08 13:13 UTC (permalink / raw)
To: Paul Walmsley
Cc: balbi, Tony Lindgren, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-08 13:13 ` Cousson, Benoit
0 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-08 13:13 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-08 13:13 ` Cousson, Benoit
@ 2012-06-08 13:28 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 13:28 UTC (permalink / raw)
To: Cousson, Benoit
Cc: balbi, Tony Lindgren, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-08 13:28 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 13:28 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-08 13:28 ` Paul Walmsley
@ 2012-06-08 19:32 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-08 19:32 UTC (permalink / raw)
To: Paul Walmsley, Cousson, Benoit
Cc: Balbi, Felipe, Tony Lindgren, linux-omap, linux-arm-kernel,
Kristo, Tero, Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-08 19:32 ` Hiremath, Vaibhav
0 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-08 19:32 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
2012-06-08 19:32 ` Hiremath, Vaibhav
@ 2012-06-08 23:10 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 23:10 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: Cousson, Benoit, Balbi, Felipe, Tony Lindgren, linux-omap,
linux-arm-kernel, Kristo, Tero, Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
@ 2012-06-08 23:10 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 23:10 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
2012-06-08 23:10 ` Paul Walmsley
@ 2012-06-09 8:39 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-09 8:39 UTC (permalink / raw)
To: Paul Walmsley
Cc: Cousson, Benoit, Balbi, Felipe, Tony Lindgren, linux-omap,
linux-arm-kernel, Kristo, Tero, Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
@ 2012-06-09 8:39 ` Hiremath, Vaibhav
0 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-09 8:39 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
2012-06-09 8:39 ` Hiremath, Vaibhav
@ 2012-06-09 16:05 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 16:05 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: Cousson, Benoit, Balbi, Felipe, Tony Lindgren, linux-omap,
linux-arm-kernel, Kristo, Tero, Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* AM335x CPSW reset (was "RE: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)")
@ 2012-06-09 16:05 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 16:05 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-08 13:28 ` Paul Walmsley
@ 2012-06-11 6:15 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-11 6:15 UTC (permalink / raw)
To: Paul Walmsley
Cc: Cousson, Benoit, balbi, linux-omap, linux-arm-kernel,
Tero Kristo, Ohad Ben-Cohen
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-11 6:15 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-11 6:15 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-11 6:15 ` Tony Lindgren
@ 2012-06-11 8:04 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-11 8:04 UTC (permalink / raw)
To: Tony Lindgren
Cc: Cousson, Benoit, balbi, linux-omap, linux-arm-kernel,
Tero Kristo, Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-11 8:04 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-11 8:04 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-11 8:04 ` Paul Walmsley
@ 2012-06-11 9:24 ` Cousson, Benoit
-1 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-11 9:24 UTC (permalink / raw)
To: Paul Walmsley
Cc: Tony Lindgren, balbi, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-11 9:24 ` Cousson, Benoit
0 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-11 9:24 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-11 9:24 ` Cousson, Benoit
@ 2012-06-11 16:20 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-11 16:20 UTC (permalink / raw)
To: Cousson, Benoit
Cc: Tony Lindgren, balbi, linux-omap, linux-arm-kernel, Tero Kristo,
Ohad Ben-Cohen
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-11 16:20 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-11 16:20 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 7:51 ` Tony Lindgren
@ 2012-06-07 10:20 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:20 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 10:20 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 10:20 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 10:20 ` Paul Walmsley
@ 2012-06-07 10:52 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 10:52 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 10:52 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-07 10:52 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 10:52 ` Tony Lindgren
@ 2012-06-07 22:05 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 22:05 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-07 22:05 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 22:05 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-07 22:05 ` Paul Walmsley
@ 2012-06-08 6:38 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-08 6:38 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-08 6:38 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-08 6:38 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-08 6:38 ` Tony Lindgren
@ 2012-06-09 1:31 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 1:31 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-09 1:31 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 1:31 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
2012-06-09 1:31 ` Paul Walmsley
@ 2012-06-11 6:21 ` Tony Lindgren
-1 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-11 6:21 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tero Kristo, Benoît Cousson,
Felipe Balbi
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 04/11] ARM: OMAP2+: usb_host_fs: add custom reset for usb_host_fs (fsusb)
@ 2012-06-11 6:21 ` Tony Lindgren
0 siblings, 0 replies; 120+ messages in thread
From: Tony Lindgren @ 2012-06-11 6:21 UTC (permalink / raw)
To: linux-arm-kernel
* 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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel
Cc: Tony Lindgren, Kevin Hilman, Benoît Cousson, Vaibhav Hiremath
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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);
}
^ permalink raw reply related [flat|nested] 120+ messages in thread* RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:59 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-07 6:59 UTC (permalink / raw)
To: Paul Walmsley, linux-omap, linux-arm-kernel
Cc: Tony Lindgren, Hilman, Kevin, Cousson, Benoit
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-07 6:59 ` Hiremath, Vaibhav
0 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-07 6:59 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread* RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 6:59 ` Hiremath, Vaibhav
@ 2012-06-07 7:08 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:08 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Hilman, Kevin,
Cousson, Benoit
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-07 7:08 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 7:08 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 7:08 ` Paul Walmsley
@ 2012-06-07 18:09 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-07 18:09 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Hilman, Kevin,
Cousson, Benoit
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-07 18:09 ` Hiremath, Vaibhav
0 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-07 18:09 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 18:09 ` Hiremath, Vaibhav
@ 2012-06-07 20:03 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 20:03 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Hilman, Kevin,
Cousson, Benoit
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-07 20:03 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 20:03 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* RE: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 20:03 ` Paul Walmsley
@ 2012-06-08 19:10 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-08 19:10 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Hilman, Kevin,
Cousson, Benoit
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-08 19:10 ` Hiremath, Vaibhav
0 siblings, 0 replies; 120+ messages in thread
From: Hiremath, Vaibhav @ 2012-06-08 19:10 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-08 19:10 ` Hiremath, Vaibhav
@ 2012-06-11 9:12 ` Cousson, Benoit
-1 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-11 9:12 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: Paul Walmsley, linux-omap, linux-arm-kernel, Tony Lindgren,
Hilman, Kevin
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-11 9:12 ` Cousson, Benoit
0 siblings, 0 replies; 120+ messages in thread
From: Cousson, Benoit @ 2012-06-11 9:12 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-08 13:22 ` Tero Kristo
-1 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-08 13:22 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Kevin Hilman,
Benoît Cousson, Vaibhav Hiremath
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-08 13:22 ` Tero Kristo
0 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-08 13:22 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
2012-06-08 13:22 ` Tero Kristo
@ 2012-06-08 23:18 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 23:18 UTC (permalink / raw)
To: Tero Kristo
Cc: linux-omap, linux-arm-kernel, Tony Lindgren, Kevin Hilman,
Benoît Cousson, Vaibhav Hiremath
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 05/11] ARM: OMAP2+: hwmod code/data: fix 32K sync timer
@ 2012-06-08 23:18 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-08 23:18 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 06/11] ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Djamil Elaidi
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 */
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 06/11] ARM: OMAP4+: hwmod: fix issue causing IPs not going back to Smart-Standby
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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 */
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 07/11] ARM: OMAP: PM: Lock clocks list while generating summary
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Tony Lindgren, Nishanth Menon, Todd Poynor
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;
}
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 07/11] ARM: OMAP: PM: Lock clocks list while generating summary
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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;
}
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 08/11] ARM: OMAP2+: CM: increase the module disable timeout
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Tero Kristo
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"
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 08/11] ARM: OMAP2+: CM: increase the module disable timeout
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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"
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 10/11] ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 10/11] ARM: OMAP2+: hwmod: add flag to prevent hwmod code from touching IP block during init
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 120+ messages in thread
* [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Rajendra Nayak, Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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,
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:39 ` Rajendra Nayak
-1 siblings, 0 replies; 120+ messages in thread
From: Rajendra Nayak @ 2012-06-07 6:39 UTC (permalink / raw)
To: Paul Walmsley; +Cc: linux-omap, linux-arm-kernel, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
@ 2012-06-07 6:39 ` Rajendra Nayak
0 siblings, 0 replies; 120+ messages in thread
From: Rajendra Nayak @ 2012-06-07 6:39 UTC (permalink / raw)
To: linux-arm-kernel
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,
>
>
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
2012-06-07 6:39 ` Rajendra Nayak
@ 2012-06-18 17:41 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-18 17:41 UTC (permalink / raw)
To: Rajendra Nayak; +Cc: linux-omap, linux-arm-kernel, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
@ 2012-06-18 17:41 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-18 17:41 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread* Re: [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
2012-06-18 17:41 ` Paul Walmsley
@ 2012-06-19 5:15 ` Rajendra Nayak
-1 siblings, 0 replies; 120+ messages in thread
From: Rajendra Nayak @ 2012-06-19 5:15 UTC (permalink / raw)
To: Paul Walmsley; +Cc: linux-omap, linux-arm-kernel, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 09/11] ARM: OMAP4: clock data: add clockdomains for clocks used as main clocks
@ 2012-06-19 5:15 ` Rajendra Nayak
0 siblings, 0 replies; 120+ messages in thread
From: Rajendra Nayak @ 2012-06-19 5:15 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 11/11] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-07 6:13 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-omap, linux-arm-kernel; +Cc: Péter Ujfalusi, Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 11/11] ARM: OMAP4: hwmod data: do not enable or reset the McPDM during kernel init
@ 2012-06-07 6:13 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-07 6:13 UTC (permalink / raw)
To: linux-arm-kernel
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",
^ permalink raw reply related [flat|nested] 120+ messages in thread
* Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
2012-06-07 6:13 ` Paul Walmsley
@ 2012-06-08 13:30 ` Tero Kristo
-1 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-08 13:30 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Kevin Hilman, Benoît Cousson
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
@ 2012-06-08 13:30 ` Tero Kristo
0 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-08 13:30 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 120+ messages in thread* Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
2012-06-08 13:30 ` Tero Kristo
@ 2012-06-09 1:15 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 1:15 UTC (permalink / raw)
To: Tero Kristo
Cc: linux-omap, linux-arm-kernel, Kevin Hilman, Benoît Cousson
[-- 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
^ permalink raw reply [flat|nested] 120+ messages in thread* [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
@ 2012-06-09 1:15 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-09 1:15 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
2012-06-08 13:30 ` Tero Kristo
@ 2012-06-13 23:55 ` Paul Walmsley
-1 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-13 23:55 UTC (permalink / raw)
To: Tero Kristo
Cc: linux-omap, linux-arm-kernel, Kevin Hilman, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
@ 2012-06-13 23:55 ` Paul Walmsley
0 siblings, 0 replies; 120+ messages in thread
From: Paul Walmsley @ 2012-06-13 23:55 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* Re: [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
2012-06-13 23:55 ` Paul Walmsley
@ 2012-06-14 7:36 ` Tero Kristo
-1 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-14 7:36 UTC (permalink / raw)
To: Paul Walmsley
Cc: linux-omap, linux-arm-kernel, Kevin Hilman, Benoît Cousson
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
^ permalink raw reply [flat|nested] 120+ messages in thread
* [PATCH 00/11] ARM: OMAP: core/hwmod: first set of fixes for 3.5-rc
@ 2012-06-14 7:36 ` Tero Kristo
0 siblings, 0 replies; 120+ messages in thread
From: Tero Kristo @ 2012-06-14 7:36 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 120+ messages in thread