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. >>> - 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 -- 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
