Hi Jean I'm really sorry it's taken me so long to do detailed review of these patches for merging... anyway - On Wed, 14 Dec 2011, jean.pihet@xxxxxxxxxxxxxx wrote: > From: Jean Pihet <j-pihet@xxxxxx> > > . Implement the devices wake-up latency constraints using the global > device PM QoS notification handler which applies the constraints to the > underlying layer > . Implement the low level code which controls the power domains next power > states, through the hwmod and pwrdm layers > . Add cpuidle and power domains wake-up latency figures for OMAP3, cf. > comments in the code and [1] for the details on where the numbers > are magically coming from > . Implement the relation between the cpuidle and per-device PM QoS frameworks > in the OMAP3 specific idle callbacks. > The chosen C-state shall satisfy the following conditions: > . the 'valid' field is enabled, > . it satisfies the enable_off_mode flag, > . the next state for MPU and CORE power domains is not lower than the > state programmed by the per-device PM QoS. I've been reviewing these closely. It looks to me that are some issues that need to be resolved before all of them are mergeable. One issue that I noticed in this series is that there are some locking issues in patch 1. It looks to me that calls to _pwrdm_wakeuplat_update_pwrst() can race with each other, since it's called outside the lock? Another issue is with the latency data, which we've discussed previously. Before we merge the latency data in mainline, we need to ensure that we've minimized the dependency of that data on other register settings, such as the AUTOEXTCLKMODE bits or the external high-frequency oscillator stabilization time. Otherwise any board vendor that ships devices based on that data will need to redo it, which we'd like to avoid. I'll comment more on this in replies to those data patches. But there is a more fundamental issue that we need to deal with before we should merge the low-level powerdomain portion of your patches -- an issue that isn't directly related to your code. We should first switch over to use functional powerstates throughout our PM code. Right now, we have some code that tweaks powerdomain bits directly, and some code that calls omap_set_pwrdm_state(). We should get rid of the former, I think, so that there is one single code path that directly changes the low-level powerdomain bits. Otherwise I think we run the risk of having one of our code paths lose track of what the powerdomain settings are -- and that way lies madness... Anyway, I've got some experimental code to do this, but since it touches a lot of our existing PM code, I'd like to have Kevin's review, testing, and approval of it. (And yours too, ideally ;-) So what I'd like to do is to repost your series with some changes, and with the functional powerstate portion sorted out first. Would you be able to help test those for 3.4? In the meantime, it looks to me like two of your patches are good to go in the short term, with some tweaks - the omap_hwmod changes and the dev_pm_qos callback changes. I'll repost those shortly and queue them up for whenever Tony wants to pull them. - Paul -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
