On Fri, 2012-05-18 at 11:23 +0530, Shilimkar, Santosh wrote: > On Thu, May 17, 2012 at 10:12 PM, Kevin Hilman <khilman@xxxxxx> wrote: > > "Shilimkar, Santosh" <santosh.shilimkar@xxxxxx> writes: > > > >> On Thu, May 17, 2012 at 4:28 AM, Kevin Hilman <khilman@xxxxxx> wrote: > >>> Tero Kristo <t-kristo@xxxxxx> writes: > >>> > >>>> From: Santosh Shilimkar <santosh.shilimkar@xxxxxx> > >>>> > >>>> The SAR RAM is maintained during Device OFF mode. > >>> > >>> so why is this patch bothering to save and restore it? > >>> > >> SAR RAM is maintained(not powered down) but somebody needs to > >> feel the contents which will be preserved :-) > >> > >>> -ECONFUSED > >>> > >>>> The register layout > >>>> is fixed in SAR ROM. SAR is split into 4 banks with different > >>>> privilege accesses based on device type > >>>> > >>>> --------------------------------------------------------------- > >>>> Access mode Bank Address Range > >>>> --------------------------------------------------------------- > >>>> HS/GP : Public 1 0x4A32_6000 - 0x4A32_6FFF (4kB) > >>>> HS/GP : Public 2 0x4A32_7000 - 0x4A32_73FF (1kB) > >>>> > >>>> HS/EMU : Secured > >>>> GP : Public 3 0x4A32_8000 - 0x4A32_87FF (2kB) > >>>> > >>>> HS/GP :Secure > >>>> write once. 4 0x4A32_9000 - 0x4A32_93FF (1kB) > >>>> --------------------------------------------------------------- > >>>> > >>>> The save process is done entirely by software and restore is done by > >>>> hardware using the auto-restore feature. The restore feature is enabled > >>>> by default and cannot be disabled. The software must save the data > >>>> to be restored in a dedicated location in SAR RAM. > >>> > >>> Some general comments: > >>> > >>> - can the cluster PM notifier be used for the save path? > >>> > >> Nope. This is for device OFF only case. CPU Cluster state as such > >> has no dependency. > > > > Yeah, I see now. I like your idea of a device-off notifier chain > > modeled on the CPU PM notifier chain. > > > >>> - This patch adds lots of data that is immediately removed by the next > >>> patch. Probably the two just need to be combined. > >>> > >> iormap() hunk from this patch can be completely dropped since > >> it is getting fixed in next patch. Other infrastructure is maintained. > > > > Look again at how much stuff is deleted in the next patch that is added > > in this patch, and think about how a reviewer is supposed to follow that > > easily. > > > OK. With me. Actually with proposed split of further patch, things can > be sorted out in different patches quite well. I'll take a look if I'll merge these patches completely or modify the contents. In some work version I had, this patch was completely merged with the next. > > >>> - BUG_ON() should not be used unless there is absolutely no recovery > >>> path, since it casues a full kernel panic. Instead, some error > >>> recovery should be added. > >>> > >> WARN_ON should suffice. > > > > Not really. Error recovery should be added. > > > Agree Will fix that also. -Tero -- 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
