Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module context is lost

[Kevin Hilman <khilman@deeprootsystems.com>]

"Shilimkar, Santosh" <santosh.shilimkar@ti.com> writes:

>> -----Original Message-----
>> From: Kevin Hilman [mailto:khilman@deeprootsystems.com]
>> Sent: Friday, October 15, 2010 3:44 AM
>> To: Nayak, Rajendra
>> Cc: linux-omap@vger.kernel.org; Paul Walmsley; Cousson, Benoit; Shilimkar,
>> Santosh
>> Subject: Re: [PATCH] OMAP: hmwod: Update the sysc_cache in case module
>> context is lost
>> 
>> Rajendra Nayak <rnayak@ti.com> writes:
>> 
>> > Do not skip the sysc programming in the hmwod framework based
>> > on the cached value alone, since at times the module might have lost
>> > context (due to the Powerdomain in which the module belongs
>> > transitions to either Open Switch RET or OFF).
>> 
>> Shouldn't the driver for each IP be responsible for restoring it's
>> register contents after context loss, including it's SYSC?
>> 
>> Seems to me that if SYSC is lost, it means the driver's save/restore
>> is buggy.
> 
> I am glad you asked this question. I had a same argument with Benoit
> that driver anyway does context save restore for other registers and
> it can do SYSC as well.
>
> But Benoit's point was that "sysconfig is a part of the PRCM located
> in the IP, but this is purely TI implementation specific. The same
> IP in another platform will not have this sysconfig entry. That's why
> its important to hide them from the driver "

OK, but this patch still doesn't address the real problem.  Namely, that
*somebody* needs to save/restore the SYSC reg for the IP.  

Otherwise, all this patch does is refresh the _sysc_cache with
completely unknown contents.  It also somewhat defeats the purpose of
having a cache.  If you're going to read SYSC in order to determine
whether or not you can avoid a write, you might as well just blindly
write.

One option to fix the save/restore would be that TI specific context
save/restore could be done in the device layer by adding some additional
save/restore do the functions calling omap_device_[idle|enable].

IOW, most devices just call omap_device directly by doing something like:

struct omap_device_pm_latency omap_wdt_latency[] = {
	[0] = {
		.deactivate_func = omap_device_idle_hwmods,
		.activate_func   = omap_device_enable_hwmods,
		.flags		 = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
	},
};


But these [de]activate functions can be anything, and could be made into
functions that do some additional save/restore and then call
omap_device_*

Kevin