From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kevin Hilman Subject: Re: [PATCHv2 06/19] ARM: OMAP4: PM: Add SAR backup support towards device OFF Date: Thu, 17 May 2012 09:42:48 -0700 Message-ID: <87aa1621yv.fsf@ti.com> References: <1336990730-26892-1-git-send-email-t-kristo@ti.com> <1336990730-26892-7-git-send-email-t-kristo@ti.com> <87aa177my3.fsf@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: (Santosh Shilimkar's message of "Thu, 17 May 2012 12:32:00 +0530") List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-arm-kernel-bounces@lists.infradead.org Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: "Shilimkar, Santosh" Cc: Tero Kristo , paul@pwsan.com, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: linux-omap@vger.kernel.org IlNoaWxpbWthciwgU2FudG9zaCIgPHNhbnRvc2guc2hpbGlta2FyQHRpLmNvbT4gd3JpdGVzOgoK PiBPbiBUaHUsIE1heSAxNywgMjAxMiBhdCA0OjI4IEFNLCBLZXZpbiBIaWxtYW4gPGtoaWxtYW5A dGkuY29tPiB3cm90ZToKPj4gVGVybyBLcmlzdG8gPHQta3Jpc3RvQHRpLmNvbT4gd3JpdGVzOgo+ Pgo+Pj4gRnJvbTogU2FudG9zaCBTaGlsaW1rYXIgPHNhbnRvc2guc2hpbGlta2FyQHRpLmNvbT4K Pj4+Cj4+PiBUaGUgU0FSIFJBTSBpcyBtYWludGFpbmVkIGR1cmluZyBEZXZpY2UgT0ZGIG1vZGUu Cj4+Cj4+IHNvIHdoeSBpcyB0aGlzIHBhdGNoIGJvdGhlcmluZyB0byBzYXZlIGFuZCByZXN0b3Jl IGl0Pwo+Pgo+IFNBUiBSQU0gaXMgbWFpbnRhaW5lZChub3QgcG93ZXJlZCBkb3duKSBidXQgc29t ZWJvZHkgbmVlZHMgdG8KPiBmZWVsIHRoZSBjb250ZW50cyB3aGljaCB3aWxsIGJlIHByZXNlcnZl ZCA6LSkKPgo+PiAtRUNPTkZVU0VECj4+Cj4+PiBUaGUgcmVnaXN0ZXIgbGF5b3V0Cj4+PiBpcyBm aXhlZCBpbiBTQVIgUk9NLiBTQVIgaXMgc3BsaXQgaW50byA0IGJhbmtzIHdpdGggZGlmZmVyZW50 Cj4+PiBwcml2aWxlZ2UgYWNjZXNzZXMgYmFzZWQgb24gZGV2aWNlIHR5cGUKPj4+Cj4+PiDCoC0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLQo+Pj4gwqBBY2Nlc3MgbW9kZSDCoCDCoCDCoCDCoCDCoEJhbmsgwqAgwqBBZGRyZXNzIFJh bmdlCj4+PiDCoC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LS0tLS0tLS0tLS0tLS0tLQo+Pj4gwqBIUy9HUCA6IFB1YmxpYyDCoCDCoCDCoCDCoCDCoCDCoCDC oCAxIMKgIMKgIMKgIDB4NEEzMl82MDAwIC0gMHg0QTMyXzZGRkYgKDRrQikKPj4+IMKgSFMvR1Ag OiBQdWJsaWMgwqAgwqAgwqAgwqAgwqAgwqAgwqAgMiDCoCDCoCDCoCAweDRBMzJfNzAwMCAtIDB4 NEEzMl83M0ZGICgxa0IpCj4+Pgo+Pj4gwqBIUy9FTVUgOiBTZWN1cmVkCj4+PiDCoEdQIDogUHVi bGljIMKgIMKgIMKgIMKgIMKgMyDCoCDCoCDCoCAweDRBMzJfODAwMCAtIDB4NEEzMl84N0ZGICgy a0IpCj4+Pgo+Pj4gwqBIUy9HUCA6U2VjdXJlCj4+PiDCoHdyaXRlIG9uY2UuIMKgIMKgIMKgIMKg IMKgNCDCoCDCoCDCoCAweDRBMzJfOTAwMCAtIDB4NEEzMl85M0ZGICgxa0IpCj4+PiDCoC0tLS0t LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t LQo+Pj4KPj4+IFRoZSBzYXZlIHByb2Nlc3MgaXMgZG9uZSBlbnRpcmVseSBieSBzb2Z0d2FyZSBh bmQgcmVzdG9yZSBpcyBkb25lIGJ5Cj4+PiBoYXJkd2FyZSB1c2luZyB0aGUgYXV0by1yZXN0b3Jl IGZlYXR1cmUuIFRoZSByZXN0b3JlIGZlYXR1cmUgaXMgZW5hYmxlZAo+Pj4gYnkgZGVmYXVsdCBh bmQgY2Fubm90IGJlIGRpc2FibGVkLiBUaGUgc29mdHdhcmUgbXVzdCBzYXZlIHRoZSBkYXRhCj4+ PiB0byBiZSByZXN0b3JlZCBpbiBhIGRlZGljYXRlZCBsb2NhdGlvbiBpbiBTQVIgUkFNLgo+Pgo+ PiBTb21lIGdlbmVyYWwgY29tbWVudHM6Cj4+Cj4+IC0gY2FuIHRoZSBjbHVzdGVyIFBNIG5vdGlm aWVyIGJlIHVzZWQgZm9yIHRoZSBzYXZlIHBhdGg/Cj4+Cj4gTm9wZS4gVGhpcyBpcyBmb3IgZGV2 aWNlIE9GRiBvbmx5IGNhc2UuIENQVSBDbHVzdGVyIHN0YXRlIGFzIHN1Y2gKPiBoYXMgbm8gZGVw ZW5kZW5jeS4KClllYWgsIEkgc2VlIG5vdy4gIEkgbGlrZSB5b3VyIGlkZWEgb2YgYSBkZXZpY2Ut b2ZmIG5vdGlmaWVyIGNoYWluCm1vZGVsZWQgb24gdGhlIENQVSBQTSBub3RpZmllciBjaGFpbi4K Cj4+IC0gVGhpcyBwYXRjaCBhZGRzIGxvdHMgb2YgZGF0YSB0aGF0IGlzIGltbWVkaWF0ZWx5IHJl bW92ZWQgYnkgdGhlIG5leHQKPj4gwqBwYXRjaC4gwqBQcm9iYWJseSB0aGUgdHdvIGp1c3QgbmVl ZCB0byBiZSBjb21iaW5lZC4KPj4KPiBpb3JtYXAoKSBodW5rIGZyb20gdGhpcyBwYXRjaCBjYW4g YmUgY29tcGxldGVseSBkcm9wcGVkIHNpbmNlCj4gaXQgaXMgZ2V0dGluZyBmaXhlZCBpbiBuZXh0 IHBhdGNoLiBPdGhlciBpbmZyYXN0cnVjdHVyZSBpcyBtYWludGFpbmVkLgoKTG9vayBhZ2FpbiBh dCBob3cgbXVjaCBzdHVmZiBpcyBkZWxldGVkIGluIHRoZSBuZXh0IHBhdGNoIHRoYXQgaXMgYWRk ZWQKaW4gdGhpcyBwYXRjaCwgYW5kIHRoaW5rIGFib3V0IGhvdyBhIHJldmlld2VyIGlzIHN1cHBv c2VkIHRvIGZvbGxvdyB0aGF0CmVhc2lseS4KCj4+IC0gQlVHX09OKCkgc2hvdWxkIG5vdCBiZSB1 c2VkIHVubGVzcyB0aGVyZSBpcyBhYnNvbHV0ZWx5IG5vIHJlY292ZXJ5Cj4+IMKgcGF0aCwgc2lu Y2UgaXQgY2FzdWVzIGEgZnVsbCBrZXJuZWwgcGFuaWMuIMKgIEluc3RlYWQsIHNvbWUgZXJyb3IK Pj4gwqByZWNvdmVyeSBzaG91bGQgYmUgYWRkZWQuCj4+Cj4gV0FSTl9PTiBzaG91bGQgc3VmZmlj ZS4KCk5vdCByZWFsbHkuICBFcnJvciByZWNvdmVyeSBzaG91bGQgYmUgYWRkZWQuCgpLZXZpbgoK X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KbGludXgtYXJt LWtlcm5lbCBtYWlsaW5nIGxpc3QKbGludXgtYXJtLWtlcm5lbEBsaXN0cy5pbmZyYWRlYWQub3Jn Cmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vbGludXgtYXJtLWtl cm5lbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: khilman@ti.com (Kevin Hilman) Date: Thu, 17 May 2012 09:42:48 -0700 Subject: [PATCHv2 06/19] ARM: OMAP4: PM: Add SAR backup support towards device OFF In-Reply-To: (Santosh Shilimkar's message of "Thu, 17 May 2012 12:32:00 +0530") References: <1336990730-26892-1-git-send-email-t-kristo@ti.com> <1336990730-26892-7-git-send-email-t-kristo@ti.com> <87aa177my3.fsf@ti.com> Message-ID: <87aa1621yv.fsf@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org "Shilimkar, Santosh" writes: > On Thu, May 17, 2012 at 4:28 AM, Kevin Hilman wrote: >> Tero Kristo writes: >> >>> From: Santosh Shilimkar >>> >>> 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. >> - 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. Kevin