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

[Paul Walmsley]

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

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

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Tero Kristo <t-***@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,
},
},


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

[Paul Walmsley]

From: Djamil Elaidi <d-***@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-***@ti.com>
Signed-off-by: Paul Walmsley <***@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 */



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

[Paul Walmsley]

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

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Beno=C3=AEt Cousson <b-***@ti.com>
Cc: P=C3=A9ter 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/oma=
p_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 o=
map_hwmod *oh)
}
=20
/**
+ * _setup_preprogram - Pre-program an IP block during the _setup() pro=
cess
+ * @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)
=20
_setup_iclk_autoidle(oh);
=20
- if (!_setup_reset(oh))
+ if (!_setup_reset(oh)) {
+ _setup_preprogram(oh);
_setup_postsetup(oh);
+ }
=20
return 0;
}
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/pl=
at-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 res=
et fn
+ * @setup_preprogram: ptr to fn to be executed after the reset in _set=
up()
*
* 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 bloc=
ks
+ * 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);
};
=20
/**


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

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

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

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

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

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

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

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

This patch is a collaboration between Kevin Hilman <***@ti.com>
and Paul Walmsley <***@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 <***@atomide.com>
Cc: Vaibhav Hiremath <***@ti.com>
Cc: Beno=C3=AEt Cousson <b-***@ti.com>
Tested-by: Kevin Hilman <***@ti.com>
Signed-off-by: Kevin Hilman <***@ti.com>
Signed-off-by: Paul Walmsley <***@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/oma=
p_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 _f=
ind_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 =3D oh->class->sysc->sysc_flags;
=20
if (sf & SYSC_HAS_SIDLEMODE) {
- idlemode =3D (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 =3D HWMOD_IDLEMODE_FORCE;
+ else
+ idlemode =3D HWMOD_IDLEMODE_NO;
+ } else {
+ idlemode =3D HWMOD_IDLEMODE_SMART;
+ }
_set_slave_idlemode(oh, idlemode, &v);
}
=20


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hiremath, Vaibhav]

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

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

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

[Paul Walmsley]

Hi

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hiremath, Vaibhav]

Post by Paul Walmsley
Hi

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Hi

Post by Hiremath, Vaibhav
I couldn't finish my testing today, got into continuous meetings.

No worries, I understand.

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hiremath, Vaibhav]

Post by Paul Walmsley
Hi

Post by Hiremath, Vaibhav
I couldn't finish my testing today, got into continuous meetings.

No worries, I understand.

Post by Hiremath, Vaibhav
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
"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
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

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

[Cousson, Benoit]

Post by Hiremath, Vaibhav

Post by Paul Walmsley
Hi

Post by Hiremath, Vaibhav
I couldn't finish my testing today, got into continuous meetings.

No worries, I understand.

Post by Hiremath, Vaibhav
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
"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
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?

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tero Kristo]

Hi Paul,

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

<clip>

Post by Paul Walmsley
{
@@ -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


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

[Paul Walmsley]

Post by Tero Kristo
There's a bug in this patch, see below.

...

Post by Tero Kristo
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

=46rom: Tero Kristo <t-***@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-***@ti.com>
[***@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 <***@pwsan.com>
Cc: Beno=C3=AEt Cousson <b-***@ti.com>
Cc: Felipe 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=20
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/Makefil=
e
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) :=3D hsmmc.o
obj-y +=3D $(omap-hsmmc-m) $(omap-hsmmc-y)
=20
=20
-usbfs-$(CONFIG_ARCH_OMAP_OTG) :=3D usb-fs.o
-obj-y +=3D $(usbfs-m) $(usbfs-y)
-obj-y +=3D usb-musb.o
+obj-y +=3D usb-fs.o usb-musb.o
obj-y +=3D omap_phy_internal.o
=20
obj-$(CONFIG_MACH_OMAP2_TUSB6010) +=3D 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
=20
/*
+ * 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 o=
f
* 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 =3D {
static struct omap_hwmod_class omap44xx_usb_host_fs_hwmod_class =3D {
.name =3D "usb_host_fs",
.sysc =3D &omap44xx_usb_host_fs_sysc,
+ .reset =3D omap_usb_host_fs_reset,
};
=20
/* 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 omap=
1 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 modif=
y
* it under the terms of the GNU General Public License as published b=
y
@@ -32,6 +32,8 @@
#include <plat/usb.h>
#include <plat/board.h>
=20
+#include "cm.h"
+#include "common.h"
#include "control.h"
#include "mux.h"
=20
@@ -41,6 +43,64 @@
#define INT_USB_IRQ_HGEN INT_24XX_USB_IRQ_HGEN
#define INT_USB_IRQ_OTG INT_24XX_USB_IRQ_OTG
=20
+/* HCCOMMANDSTATUS: the register offset of the HCCOMMANDSTATUS registe=
r */
+#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 blo=
ck
+ * @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 =3D=3D 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 =3D=3D 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)
=20
#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>
=20
#define OMAP3_HS_USB_PORTS 3
=20
@@ -181,6 +182,8 @@ static inline void omap2_usbfs_init(struct omap_usb=
_config *pdata)
}
#endif
=20
+extern int omap_usb_host_fs_reset(struct omap_hwmod *oh);
+
/*--------------------------------------------------------------------=
-----*/
=20
/*


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley
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
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.

Post by Paul Walmsley
+/* 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.

Post by Paul Walmsley
+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Felipe Balbi]

Hi,

Post by Tony Lindgren

Post by Paul Walmsley
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
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.

Post by Tony Lindgren

Post by Paul Walmsley
+/* 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.

Post by Paul Walmsley
+ 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.

[Tony Lindgren]

Post by Felipe Balbi
Hi,

Post by Tony Lindgren

Post by Paul Walmsley
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
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Felipe Balbi]

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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...

[Cousson, Benoit]

Post by Felipe Balbi

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Cousson, Benoit

Post by Felipe Balbi

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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

Post by Cousson, Benoit
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Felipe Balbi]

Post by Cousson, Benoit

Post by Felipe Balbi

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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.

[Paul Walmsley]

Post by Cousson, Benoit
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Cousson, Benoit]

+ Ohad

Post by Paul Walmsley

Post by Cousson, Benoit
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.

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Cousson, Benoit
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Cousson, Benoit]

Post by Paul Walmsley

Post by Cousson, Benoit
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Paul Walmsley

Post by Cousson, Benoit
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hiremath, Vaibhav]

Post by Paul Walmsley

Post by Paul Walmsley

Post by Cousson, Benoit
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

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

[Paul Walmsley]

Hi

Post by Hiremath, Vaibhav
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" ?

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hiremath, Vaibhav]

Post by Paul Walmsley
Hi

Post by Hiremath, Vaibhav
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.

Post by Paul Walmsley

Post by Hiremath, Vaibhav
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

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

[Paul Walmsley]

Post by Hiremath, Vaibhav

Post by Paul Walmsley

Post by Hiremath, Vaibhav
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.

Post by Hiremath, Vaibhav

Post by Paul Walmsley

Post by Hiremath, Vaibhav
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Paul Walmsley

Post by Cousson, Benoit
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.

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Cousson, Benoit]

Hi Paul,

Post by Paul Walmsley

Post by Tony Lindgren

Post by Paul Walmsley
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

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

[Paul Walmsley]

Post by Cousson, Benoit

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren

Post by Paul Walmsley

Post by Tony Lindgren
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.

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren
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/.

Post by Tony Lindgren
That way the hwmod code can include those functions using the driver register
static inline int xyz_driver_reset(void __iomem *base, int flags)
{
...
}
Then instead of having a separate platform init file for each driver,
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.

Post by Tony Lindgren
{
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren
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/.

Post by Tony Lindgren
That way the hwmod code can include those functions using the driver register
static inline int xyz_driver_reset(void __iomem *base, int flags)
{
...
}
Then instead of having a separate platform init file for each driver,
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.

Post by Paul Walmsley
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.

Post by Paul Walmsley

Post by Tony Lindgren
{
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..

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren
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.

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

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

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Rajendra Nayak <***@ti.com>
Cc: Beno=C3=AEt Cousson <b-***@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 =3D {
=20
static struct clk sys_32k_ck =3D {
.name =3D "sys_32k_ck",
+ .clkdm_name =3D "prm_clkdm",
.rate =3D 32768,
.ops =3D &clkops_null,
};
@@ -512,6 +513,7 @@ static struct clk ddrphy_ck =3D {
.name =3D "ddrphy_ck",
.parent =3D &dpll_core_m2_ck,
.ops =3D &clkops_null,
+ .clkdm_name =3D "l3_emif_clkdm",
.fixed_div =3D 2,
.recalc =3D &omap_fixed_divisor_recalc,
};
@@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] =3D {
static struct clk dpll_mpu_m2_ck =3D {
.name =3D "dpll_mpu_m2_ck",
.parent =3D &dpll_mpu_ck,
+ .clkdm_name =3D "cm_clkdm",
.clksel =3D dpll_mpu_m2_div,
.clksel_reg =3D OMAP4430_CM_DIV_M2_DPLL_MPU,
.clksel_mask =3D OMAP4430_DPLL_CLKOUT_DIV_MASK,
@@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] =3D {
static struct clk l3_div_ck =3D {
.name =3D "l3_div_ck",
.parent =3D &div_core_ck,
+ .clkdm_name =3D "cm_clkdm",
.clksel =3D l3_div_div,
.clksel_reg =3D OMAP4430_CM_CLKSEL_CORE,
.clksel_mask =3D OMAP4430_CLKSEL_L3_MASK,
@@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] =3D=
{
static struct clk trace_clk_div_ck =3D {
.name =3D "trace_clk_div_ck",
.parent =3D &pmd_trace_clk_mux_ck,
+ .clkdm_name =3D "emu_sys_clkdm",
.clksel =3D trace_clk_div_div,
.clksel_reg =3D OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
.clksel_mask =3D OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rajendra Nayak]

Post by Paul Walmsley
Until the OMAP4 code is converted to disable the use of the clock
framework-based clockdomain enable/disable sequence, any clock used a=

s

Post by Paul Walmsley
a hwmod main_clk must have a clockdomain associated with it. This
patch populates some clock structure clockdomain names to resolve the

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.

Post by Paul Walmsley
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_c=

k.

Post by Paul Walmsley
omap_hwmod: l3_div_ck: missing clockdomain for l3_div_ck.
omap_hwmod: ddrphy_ck: missing clockdomain for ddrphy_ck.
---
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-oma=

p2/clock44xx_data.c

Post by Paul Walmsley
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 =3D {
static struct clk sys_32k_ck =3D {
.name =3D "sys_32k_ck",
+ .clkdm_name =3D "prm_clkdm",
.rate =3D 32768,
.ops =3D&clkops_null,
};
@@ -512,6 +513,7 @@ static struct clk ddrphy_ck =3D {
.name =3D "ddrphy_ck",
.parent =3D&dpll_core_m2_ck,
.ops =3D&clkops_null,
+ .clkdm_name =3D "l3_emif_clkdm",
.fixed_div =3D 2,
.recalc =3D&omap_fixed_divisor_recalc,
};
@@ -769,6 +771,7 @@ static const struct clksel dpll_mpu_m2_div[] =3D =

{

Post by Paul Walmsley
static struct clk dpll_mpu_m2_ck =3D {
.name =3D "dpll_mpu_m2_ck",
.parent =3D&dpll_mpu_ck,
+ .clkdm_name =3D "cm_clkdm",
.clksel =3D dpll_mpu_m2_div,
.clksel_reg =3D OMAP4430_CM_DIV_M2_DPLL_MPU,
.clksel_mask =3D OMAP4430_DPLL_CLKOUT_DIV_MASK,
@@ -1149,6 +1152,7 @@ static const struct clksel l3_div_div[] =3D {
static struct clk l3_div_ck =3D {
.name =3D "l3_div_ck",
.parent =3D&div_core_ck,
+ .clkdm_name =3D "cm_clkdm",
.clksel =3D l3_div_div,
.clksel_reg =3D OMAP4430_CM_CLKSEL_CORE,
.clksel_mask =3D OMAP4430_CLKSEL_L3_MASK,
@@ -2824,6 +2828,7 @@ static const struct clksel trace_clk_div_div[] =

=3D {

Post by Paul Walmsley
static struct clk trace_clk_div_ck =3D {
.name =3D "trace_clk_div_ck",
.parent =3D&pmd_trace_clk_mux_ck,
+ .clkdm_name =3D "emu_sys_clkdm",
.clksel =3D trace_clk_div_div,
.clksel_reg =3D OMAP4430_CM_EMU_DEBUGSS_CLKCTRL,
.clksel_mask =3D OMAP4430_CLKSEL_PMD_TRACE_CLK_MASK,

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Rajendra Nayak

Post by Paul Walmsley
Until the OMAP4 code is converted to disable the use of the clock
framework-based clockdomain enable/disable sequence, any clock used as
a hwmod main_clk must have a clockdomain associated with it. This
patch populates some clock structure clockdomain names to resolve the

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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Rajendra Nayak]

Post by Paul Walmsley

Post by Rajendra Nayak

Post by Paul Walmsley
Until the OMAP4 code is converted to disable the use of the clock
framework-based clockdomain enable/disable sequence, any clock used as
a hwmod main_clk must have a clockdomain associated with it. This
patch populates some clock structure clockdomain names to resolve the

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

Post by Paul Walmsley
- Paul

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

[Paul Walmsley]

From: Todd Poynor <***@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 <***@pwsan.com>
Cc: Tony Lindgren <***@atomide.com>
Signed-off-by: Todd Poynor <***@google.com>
[***@ti.com: added commit message]
Signed-off-by: Nishanth Menon <***@ti.com>
[***@pwsan.com: minor edits to commit message]
Signed-off-by: Paul Walmsley <***@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;
}


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

[Paul Walmsley]

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

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

omap_hwmod: usb_host_fs: _wait_target_disable failed

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Tero Kristo <t-***@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"



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

[Paul Walmsley]

Resolve this kernel boot message:

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

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

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

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: P=C3=A9ter Ujfalusi <***@ti.com>
Cc: Beno=C3=AEt Cousson <b-***@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 =3D=
{
.name =3D "mcpdm",
.class =3D &omap44xx_mcpdm_hwmod_class,
.clkdm_name =3D "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 =3D HWMOD_EXT_OPT_MAIN_CLK,
.mpu_irqs =3D omap44xx_mcpdm_irqs,
.sdma_reqs =3D omap44xx_mcpdm_sdma_reqs,
.main_clk =3D "mcpdm_fck",


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

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

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

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Beno=C3=AEt Cousson <b-***@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/oma=
p_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 !=3D _HWMOD_STATE_INITIALIZED)
return -EINVAL;
=20
+ if (oh->flags & HWMOD_EXT_OPT_MAIN_CLK)
+ return -EPERM;
+
if (oh->rst_lines_cnt =3D=3D 0) {
r =3D _enable(oh);
if (r) {
diff --git a/arch/arm/plat-omap/include/plat/omap_hwmod.h b/arch/arm/pl=
at-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 disable=
d
* 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)
=20
/*
* omap_hwmod._int_flags definitions


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

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

omap_hwmod: aess: _wait_target_disable failed

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

Signed-off-by: Paul Walmsley <***@pwsan.com>
Cc: Beno=C3=AEt Cousson <b-***@ti.com>
Cc: P=C3=A9ter 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/Makefil=
e
index fa742f3..bc2ac4f 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -4,7 +4,7 @@
=20
# Common support
obj-y :=3D 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
=20
omap-2-3-common =3D irq.o sdrc.o
hwmod-common =3D 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 =3D 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 *controlle=
rs);
=20
+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"
=20
/* 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_a=
ess_sysc =3D {
static struct omap_hwmod_class omap44xx_aess_hwmod_class =3D {
.name =3D "aess",
.sysc =3D &omap44xx_aess_sysc,
+ .setup_preprogram =3D omap_aess_preprogram,
};
=20
/* aess */


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley
Enable the AESS auto-gating control bit during AESS hwmod setup. This
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren
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.

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren
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
$ 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.

Post by Paul Walmsley

Post by Tony Lindgren
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.

Post by Paul Walmsley
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Paul Walmsley]

Post by Tony Lindgren
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().

Post by Tony Lindgren
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tony Lindgren]

Post by Paul Walmsley

Post by Tony Lindgren
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().

Post by Tony Lindgren
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.

Post by Paul Walmsley
The main reasons why these integration registers are handled now in common
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.

Post by Paul Walmsley
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.

Post by Paul Walmsley
...
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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tero Kristo]

Hi Paul,

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

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



-Tero



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

[Paul Walmsley]

Hi Tero,

Post by Tero Kristo
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
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

[Paul Walmsley]

Hi Tero,

Post by Tero Kristo
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

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


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

[Tero Kristo]

Post by Paul Walmsley
Hi Tero,

Post by Tero Kristo
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

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


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