[PATCH] ARM: suspend: use flush range instead of flush all

On Wed, Sep 12, 2012 at 2:28 PM, Lorenzo Pieralisi
[off-list ref] wrote:

On Wed, Sep 12, 2012 at 08:43:33AM +0100, Shilimkar, Santosh wrote:

quoted

+ Lorenzo,

On Wed, Sep 12, 2012 at 12:48 PM, wzch [off-list ref] wrote:

quoted

From: Wenzeng Chen <redacted>

In cpu suspend function __cpu_suspend_save(), we save cp15 registers,
resume function, sp and suspend_pgd, then flush the data to DDR, but
no need to flush all D cache, only need to flush range.

Change-Id: I591a1fde929f3f987c69306b601843ed975d3e41

You should drop above.

quoted

Signed-off-by: Wenzeng Chen <redacted>
---

Lorenzo and myself discussed about the above expensive flush and he
is planning to post a similar patch but with small difference.

quoted

 arch/arm/kernel/suspend.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/suspend.c b/arch/arm/kernel/suspend.c
index 1794cc3..bb582d8 100644
--- a/arch/arm/kernel/suspend.c
+++ b/arch/arm/kernel/suspend.c

@@ -17,6 +17,7 @@ extern void cpu_resume_mmu(void);
  */
 void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)
 {
+       u32 *ptr_orig = ptr;
        *save_ptr = virt_to_phys(ptr);

        /* This must correspond to the LDM in cpu_resume() assembly */

@@ -26,7 +27,8 @@ void __cpu_suspend_save(u32 *ptr, u32 ptrsz, u32 sp, u32 *save_ptr)

        cpu_do_suspend(ptr);

-       flush_cache_all();

Lorenzo's patch was limiting above flush to local cache (LOUs) instead
of dropping
it completely.

Because if we remove it completely we have to make sure that every given
suspend finisher calls flush_cache_all(), hence from my viewpoint this
patch is incomplete. Either we remove it, and add it to ALL suspend
finisher (or just make sure it is there) or we leave it but it should use
the new LoUIS API we are going to add.

Yep.

quoted

quoted

+       __cpuc_flush_dcache_area((void *)ptr_orig, ptrsz);
+       __cpuc_flush_dcache_area((void *)save_ptr, sizeof(*save_ptr));
        outer_clean_range(*save_ptr, *save_ptr + ptrsz);
        outer_clean_range(virt_to_phys(save_ptr),
                          virt_to_phys(save_ptr) + sizeof(*save_ptr));

Just thinking bit more, even in case we drop the flush_cache_all()
completely, it should be safe since the suspend_finisher()  takes
care of the cache maintenance already.

We already discussed this. Fine by me, but we have to make sure it is
called on all suspend finishers in the mainline.

I agree. As mentioned in reply to Russell, am ok to limit this
flush to LoUIS to start with.

Regards
Santosh