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

On 6/14/2012 8:04 PM, Paul Walmsley wrote:

quoted

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

(attribution lost)

quoted

quoted

quoted

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.

quoted

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.

quoted

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?

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

considering that the implementation cost of either approach is identical?


- Paul