arm: mm: cache-l2x0: add l2x0 suspend and resume functions

RFC PATCH New Daniel Walker 5 participants 14 messages Jan 28, 2010 – Jan 29, 2010 Arm (32-bit)

lore

Share & tools

lore

Daniel Walker

author

2010-01-28 22:59 UTC #0 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

From: Willie Ruan <[email protected]>

The application processor in Qualcomm's MSM chips can be powered off (also called power collapse) during suspend and idle time. L2 cache power will be collapsed too on current chips. To handle this scenario, we need to add a function (l2x0_suspend here) to flush L2 cache before the power loss and another function (l2x0_resume) to restore the cache settings after the power is restored.

To minimize performance impact these two functions should be called as close as possible to the point where power is collapsed and restored. Since it is possible that the power could not be turned off, e.g. due to a pending interrupt, the parameter 'collapsed' should be used for the l2x0_resume function to indicate the actual power collapse state.

Signed-off-by Willie Ruan <[email protected]>

Signed-off-by Daniel Walker <[email protected]>

---
 arch/arm/include/asm/hardware/cache-l2x0.h |    3 ++
 arch/arm/mm/cache-l2x0.c                   |   46 ++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)

arch/arm/include/asm/hardware/cache-l2x0.h +3 -0

index cdb9022..a86b948 100644
@@ -55,4 +55,7 @@
 extern void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask);
 #endif
 
+extern void l2x0_suspend(void);
+extern void l2x0_resume(int collapsed);
+
 #endif

arch/arm/mm/cache-l2x0.c +46 -0

67 lines not on this page · raw message

Signature · 1 line

1.6.3.3

Pavel Machek

2010-01-29 06:49 UTC #1 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 8 lines · Daniel Walker

From: Willie Ruan <[email protected]>
The application processor in Qualcomm's MSM chips can be powered off (also called power collapse) during suspend and idle time. L2 cache power will be collapsed too on current chips. To handle this scenario, we need to add a function (l2x0_suspend here) to flush L2 cache before the power loss and another function (l2x0_resume) to restore the cache settings after the power is restored.

So this fixes suspend/resume? Ack...

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Catalin Marinas

2010-01-29 11:08 UTC #3 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resumefunctions

In reply to Daniel Walker

Quoted 2 lines · Daniel Walker

arch/arm/mm/cache-l2x0.c +0 -0
index cb8fc65..c746878 100644

[...]

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_all(void) · line 64

Quoted 6 lines

arch/arm/mm/cache-l2x0.c +6 -0

+static inline void l2x0_flush_all(void)
+{
+       /* clean and invalidate all ways */
+       sync_writel(0xff, L2X0_CLEAN_INV_WAY, 0xff);
+       cache_sync();
+}

Do we need any for of locking around this code (as we do for l2x0_inv_all) or races are not possible during suspend?

Signature · 1 line

Catalin

Russell King - ARM Linux

2010-01-29 14:14 UTC #12 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

In reply to Daniel Walker

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_all(void) · line 64

Quoted 6 lines

arch/arm/mm/cache-l2x0.c +6 -0

+static inline void l2x0_flush_all(void)
+{
+	/* clean and invalidate all ways */
+	sync_writel(0xff, L2X0_CLEAN_INV_WAY, 0xff);
+	cache_sync();
+}

Note that this patch is out of date; it won't apply to current kernels and build.

Daniel Walker

author

2010-01-29 14:35 UTC #13 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 9 lines · Russell King - ARM Linux

arch/arm/mm/cache-l2x0.c static inline void l2x0_inv_all(void) · line 64
Quoted 6 lines
arch/arm/mm/cache-l2x0.c +6 -0
+static inline void l2x0_flush_all(void)
+{
+	/* clean and invalidate all ways */
+	sync_writel(0xff, L2X0_CLEAN_INV_WAY, 0xff);
+	cache_sync();
+}
Note that this patch is out of date; it won't apply to current kernels and build.

Do you generally prefer patches against current -rc kernels, or against -next (or you development tree)?

Daniel

Ruan, Willie

2010-01-29 17:54 UTC #2 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

In reply to Pavel Machek

Quoted 4 lines

From: Pavel Machek [mailto:pavel at ucw.cz]
Sent: Thursday, January 28, 2010 10:49 PM
So this fixes suspend/resume?

Yes

~Willie

Ruan, Willie

2010-01-29 17:55 UTC #4 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

In reply to Catalin Marinas

Quoted 5 lines

From: Catalin Marinas [mailto:catalin.marinas at arm.com]
Sent: Friday, January 29, 2010 3:09 AM
Do we need any for of locking around this code (as we do for l2x0_inv_all) or races are not possible during suspend?

Local interrupts have been disabled in suspend_ops->enter and in arch_idle. Thus, no other locking is necessary.

~Willie

Daniel Walker

author

2010-01-29 17:59 UTC #5 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 8 lines · Willie

Quoted 5 lines
From: Catalin Marinas [mailto:catalin.marinas at arm.com]
Sent: Friday, January 29, 2010 3:09 AM
Do we need any for of locking around this code (as we do for l2x0_inv_all) or races are not possible during suspend?
Local interrupts have been disabled in suspend_ops->enter and in arch_idle. Thus, no other locking is necessary.

What if there are multiple cpu's calling cache_sync() at the same time? Disable interrupt wouldn't prevent it ..

Daniel

Ruan, Willie

2010-01-29 18:23 UTC #6 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 5 lines

From: Daniel Walker [mailto:dwalker at codeaurora.org]
Sent: Friday, January 29, 2010 9:59 AM
What if there are multiple cpu's calling cache_sync() at the same time? Disable interrupt wouldn't prevent it ..

cache_sync() is calling sync_writel(), which is using spin_lock_irqsave(). So, each call of cache_sync() and sync_writel() is SMP safe individually in l2x0_flush_all() as in l2x0_inv_all(), unless we need to protect the two calls together, which seems not necessary to me.

Thanks,
~Willie

Daniel Walker

author

2010-01-29 18:29 UTC #7 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 10 lines · Willie

Quoted 5 lines
From: Daniel Walker [mailto:dwalker at codeaurora.org]
Sent: Friday, January 29, 2010 9:59 AM
What if there are multiple cpu's calling cache_sync() at the same time? Disable interrupt wouldn't prevent it ..
cache_sync() is calling sync_writel(), which is using spin_lock_irqsave(). So, each call of cache_sync() and sync_writel() is SMP safe individually in l2x0_flush_all() as in l2x0_inv_all(), unless we need to protect the two calls together, which seems not necessary to me.

This is the current version,

static inline void cache_wait(void __iomem *reg, unsigned long mask)

{
        /* wait for the operation to complete */
        while (readl(reg) & mask)
                ;
}

static inline void cache_sync(void)

{
        void __iomem *base = l2x0_base;
        writel(0, base + L2X0_CACHE_SYNC);
        cache_wait(base + L2X0_CACHE_SYNC, 1);
}

Maybe cache_sync was recently changed to "writel" instead sync_writel() due it getting called with the lock already held.

Daniel

Russell King - ARM Linux

2010-01-29 19:03 UTC #8 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 22 lines · Daniel Walker

Quoted 10 lines · Willie
Quoted 5 lines
From: Daniel Walker [mailto:dwalker at codeaurora.org]
Sent: Friday, January 29, 2010 9:59 AM
What if there are multiple cpu's calling cache_sync() at the same time? Disable interrupt wouldn't prevent it ..
cache_sync() is calling sync_writel(), which is using spin_lock_irqsave(). So, each call of cache_sync() and sync_writel() is SMP safe individually in l2x0_flush_all() as in l2x0_inv_all(), unless we need to protect the two calls together, which seems not necessary to me.
This is the current version,
static inline void cache_wait(void __iomem *reg, unsigned long mask)
{
        /* wait for the operation to complete */
        while (readl(reg) & mask)
                ;
}
static inline void cache_sync(void)

Quoted 8 lines

{
        void __iomem *base = l2x0_base;
        writel(0, base + L2X0_CACHE_SYNC);
        cache_wait(base + L2X0_CACHE_SYNC, 1);
}
Maybe cache_sync was recently changed to "writel" instead sync_writel() due it getting called with the lock already held.

It's pointless discussing what the current code does, because that's not what mainline does _today_ - the locking in this file has completely changed.

Daniel Walker

author

2010-01-29 19:12 UTC #9 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 7 lines · Russell King - ARM Linux

Quoted 3 lines
Maybe cache_sync was recently changed to "writel" instead sync_writel() due it getting called with the lock already held.
It's pointless discussing what the current code does, because that's not what mainline does _today_ - the locking in this file has completely changed.

Where is the code we should be looking at (what I quoted is from -next)? Or is it in -next and we just need to look closer ?

Daniel

Russell King - ARM Linux

2010-01-29 19:43 UTC #10 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 11 lines · Daniel Walker

Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines
Maybe cache_sync was recently changed to "writel" instead sync_writel() due it getting called with the lock already held.
It's pointless discussing what the current code does, because that's not what mainline does _today_ - the locking in this file has completely changed.
Where is the code we should be looking at (what I quoted is from -next)?

Umm, I think whatever you're looking at is outdated by at least one month. What you're trying to tell me is in -next hasn't been there for at least an entire month:

http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/sfr/linux-next.git;a=blob;f=arch/arm/mm/cache-l2x0.c;h=cb8fc6573b1b2c9dedbeeec0f9a87c491a78ba85;hb=HEAD

This is the same as what was merged into Linus' tree for 2.6.33-rc1 on 17 Dec, and was in -next well before that date.

If your tree has sync_writel() then it's out of date.

Daniel Walker

author

2010-01-29 20:01 UTC #11 lore raw

RFC PATCH 4/12 arm: mm: cache-l2x0: add l2x0 suspend and resume functions

Quoted 10 lines · Russell King - ARM Linux

Umm, I think whatever you're looking at is outdated by at least one month. What you're trying to tell me is in -next hasn't been there for at least an entire month:
http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/sfr/linux-next.git;a=blob;f=arch/arm/mm/cache-l2x0.c;h=cb8fc6573b1b2c9dedbeeec0f9a87c491a78ba85;hb=HEAD
This is the same as what was merged into Linus' tree for 2.6.33-rc1 on 17 Dec, and was in -next well before that date.
If your tree has sync_writel() then it's out of date.

I was quoting code from next cause I was porting this patch to next (due to your prior comments). The version of next that I have doesn't have sync_writel() (it's next as of yesterday) ..

I'm updating the locking in this patches also.

Daniel