Hi Paul,

On 6/15/2012 2:18 AM, Paul Walmsley wrote:
On Thu, 14 Jun 2012, Cousson, Benoit wrote:

On 6/14/2012 8:04 PM, Paul Walmsley wrote:
On Thu, 14 Jun 2012, Cousson, Benoit wrote:

(attribution lost)

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.

Do you mean force-idle? Because accessing a module in no-idle is always
possible.

Thanks, that's indeed a description bug.

I'm not sure to follow you? My point was it should be: "So we can safely leave
the 32k sync timer in force-idle mode, even while we continue to access it."
This is what the WA is doing.

I am expressing appreciation to you for pointing out something incorrect
in the patch description, which has been fixed in the current version of
the patch.

OK, thanks for the clarification, I was confused by the sentence :-(.

SIDLE_NO is the option that makes more sense to me.>>>
Consider an IP block with SIDLE_NO and SIDLE_FORCE but without
SIDLE_SMART.  When an initiator will access it, the default setting should
be SIDLE_NO, for the reason that you identified above: "because accessing
a module in no-idle is always possible."  On the other hand, we don't know
when it's safe to access a module in SIDLE_FORCE unless we have additional
information as to how the IP block handles the SIdleReq signal internally,
and the characteristics of the clock domain in which it's integrated.

...

This is the modulemode and clkdm static dependency / dynamic on OMAP3/4 that
will guaranty that the OCP clock will be enabled upon any access to this
L4_WKUP clock domain IPs. This has nothing to do with the SIDLE mode.

I want to implement a behavior that does not implicitly assume that an IP
block without smart-idle will only exist inside clockdomains which are
guaranteed to be active when the kernel is running.  That might be true
for current WBU chips, but it seems unwise to make that assumption in
general.

1- The fact that the IP does not support smart-idle does not change anything regarding the clock domain behavior. 2- So far the assumption is true for all the existing OMAP devices. Let's not anticipate something that will potentially never ever happen.

It might be reasonable to remove the HWMOD_SWSUP_SIDLE flag from the 32k
counter and just test again for HWMOD_ALWAYS_FORCE_SIDLE in the module
disable.  I'll take a look at this.

Both should be removed as explained before. There is clearly no need for
HWMOD_ALWAYS_FORCE_SIDLE.

We are already explicitly listing the limitation through the idlemodes
attribute. Only SIDLE_FORCE and SIDLE_NO are supported, so we already
know that SIDLE_FORCE is the proper way to fix that limitation for the
current OMAPs. Since there is no other IP with such limitation, we know
as well that there will be no side effect. If, in the future, some more
IPs will have that limitation and will not work as expected, it will
mean that some other HW bugs will be there, and only these ones will
have to be flagged.

But my point is let's keep it simple and not try to anticipate future
bugs. This flag is not require today, let's not add it.

Which of these two behaviors do you feel is more "future-proof," in
general:

1. Implementing a target idle policy that could break if a hardware
    designer were to skip adding a target smart-idle mode to a module in
    a clockdomain that might go idle while the kernel was active?

That's not fully accurate. Even with smart-idle implemented in this module, it will still go to idle automatically without any clock domain dependency properly handled.

The goal of this fix is to adapt the SIDLE management for such module, it will not solve the overall idle management that is anyway handled at clock domain level whatever the SIDLE implementation.

2. Implementing a target idle policy that is guaranteed to allow
    initiator accesses to succeed by definition?

It will not, as explained before. This is the clock domain settings that will make the whole thing working properly.

considering that the implementation cost of either approach is identical?

My point is simple, we should not add any new custom flag when all the information to detect such exception are already there in the current data and represent accurately what the HW is doing.

So far every IPs that do not support SIDLE_SMART can/should use SIDLE_FORCE instead.

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

Reply via email to