Hi Rajendra,
On 11/17/2011 8:19 AM, Rajendra Nayak wrote:
[...]
+static int omap_console_hwmod_enable(struct omap_device *od) +{
- console_lock();
- /*
- For early console we prevented hwmod reset and idle
A period is missing. Or maybe it should a comma with not capital letter.
- So before we enable the uart clocks idle the console
- uart clocks, then enable back the console uart hwmod.
- */
- omap_hwmod_idle(od->hwmods[0]);
- omap_hwmod_enable(od->hwmods[0]);
Why do we have to idle -> enable? The module is already enabled, isn't it? The comment is not super clear for me :-) Is the goal is to reset the IP?
Yes, now that I read it, it does sound confusing. I should have at-least read it once before I picked it from serial.c
But anyway, here's what the problem is.
I feel its partly to do with the inability of hwmod to handle some modules which are left enabled post a setup, due to the 'HWMOD_INIT_NO_IDLE' flag set. Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post a setup. Now when a driver for such devices/modules tries to enable the module the first time, hwmod throws up a big WARN stating the hwmod is already in an enabled state.
OK, now, that makes sense :-) We have hwmod in ENABLE state whereas the omap_device is still in IDLE or even DISABLE.
[uart used as console is one such module, which cannot be idled post a setup by hwmod]
If hwmod could be made in some way intelligent enough to know that the module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself, a lot of this hackery might not be needed at all.
Fully agree, the fmwk should handle that.
Simplest way to do it could be to just add another intermediate state, something like '_HWMOD_STATE_ENABLED_AT_INIT'. I will post a patch for this, see if you like it being handled that way.
That seems to be good. I'm just wondering if we need to introduce a new state for that or use a dedicated flag. My concern is just that we will have two flavors of HWMOD_STATE_ENABLED that we will have to check in various places in the hwmod core code.
Maybe that's not such a big deal. Go ahead, and we will see how it looks like.
The other part of the problem is also with the inability to hook up 'custom' omap_device_pm_latency for omap devices created from DT. Maybe a lot of such cases which need custom activate/deactivate functions might have to be handled in some way in the framework itself like the one above.
For the moment, it looks like only the serial is requiring such custom stuff, but anyway, we should have a mechanism to allow that...
Thanks, Benoit