ARM/sp810: introduce API to change system mode

PATCH New Viresh Kumar 3 participants 9 messages Feb 23, 2012 – Mar 1, 2012 Arm (32-bit)

lore

Share & tools

lore

Viresh Kumar

author

2012-02-23 09:13 UTC #0 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

From: Shiraz Hashim <[email protected]>

sp810 controller can change system's working mode to various power save states. Introduce an API to accomplish the same.

Signed-off-by Shiraz Hashim <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

arch/arm/include/asm/hardware/sp810.h +57 -0

index e0d1c0c..df0960a 100644
@@ -14,10 +14,25 @@
 #ifndef __ASM_ARM_SP810_H
 #define __ASM_ARM_SP810_H
 
+#include <linux/err.h>
 #include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
 
 /* sysctl registers offset */
 #define SCCTRL			0x000
+	#define SYS_MODE_STS_MASK		(0xF << 3)
+	#define SYS_MODE_STS_SLEEP		(0x0 << 3)
+	#define SYS_MODE_STS_DOZE		(0x1 << 3)
+	#define SYS_MODE_STS_SLOW		(0x2 << 3)
+	#define SYS_MODE_STS_NORMAL		(0x4 << 3)
+
+	#define SYS_MODE_MASK			(0x7 << 0)
+	#define SYS_MODE_SLEEP			(0x0 << 0)
+	#define SYS_MODE_DOZE			(0x1 << 0)
+	#define SYS_MODE_SLOW			(0x2 << 0)
+	#define SYS_MODE_NORMAL			(0x4 << 0)
+
 #define SCSYSSTAT		0x004
 #define SCIMCTRL		0x008
 #define SCIMSTAT		0x00C

@@ -65,4 +80,46 @@ static inline void sysctl_soft_reset(void __iomem *base)
 	writel(0, base + SCSYSSTAT);
 }
 
+static inline int sysctl_change_mode(void __iomem *base, int mode)
+{
+	u32 val, mode_sts;
+	unsigned long finish;
+
+	switch (mode) {
+	case SYS_MODE_SLEEP:
+		mode_sts = SYS_MODE_STS_SLEEP;
+		break;
+	case SYS_MODE_DOZE:
+		mode_sts = SYS_MODE_STS_DOZE;
+		break;
+	case SYS_MODE_SLOW:
+		mode_sts = SYS_MODE_STS_SLOW;
+		break;
+	case SYS_MODE_NORMAL:
+		mode_sts = SYS_MODE_STS_NORMAL;
+		break;
+	default:
+		pr_err("Wrong system mode\n");
+		return -EINVAL;
+	}
+
+	val = readl(base + SCCTRL);
+	if ((val & SYS_MODE_STS_MASK) == mode_sts)
+		return 0;
+
+	val &= ~SYS_MODE_MASK;
+	val |= mode;
+	writel(val, base + SCCTRL);
+
+	/* read back if mode is set */
+	finish = jiffies + 2 * HZ;
+	do {
+		val = readl(base + SCCTRL);
+		if ((val & SYS_MODE_STS_MASK) == mode_sts)
+			return 0;
+		udelay(1000);
+	} while (!time_after_eq(jiffies, finish));
+
+	return -EFAULT;
+}
 #endif	/* __ASM_ARM_SP810_H */

Signature · 1 line

1.7.8.110.g4cb5d

Russell King - ARM Linux

2012-02-23 10:03 UTC #1 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 4 lines · Viresh Kumar

From: Shiraz Hashim <[email protected]>
sp810 controller can change system's working mode to various power save states. Introduce an API to accomplish the same.

Where is the documentation for this peripheral? I've never seen any documentation on ARMs website for it.

Quoted 87 lines

Signed-off-by Shiraz Hashim <[email protected]>

Signed-off-by Viresh Kumar <[email protected]>

---
 arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

arch/arm/include/asm/hardware/sp810.h +57 -0

index e0d1c0c..df0960a 100644
@@ -14,10 +14,25 @@
 #ifndef __ASM_ARM_SP810_H
 #define __ASM_ARM_SP810_H
 
+#include <linux/err.h>
 #include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
 
 /* sysctl registers offset */
 #define SCCTRL			0x000
+	#define SYS_MODE_STS_MASK		(0xF << 3)
+	#define SYS_MODE_STS_SLEEP		(0x0 << 3)
+	#define SYS_MODE_STS_DOZE		(0x1 << 3)
+	#define SYS_MODE_STS_SLOW		(0x2 << 3)
+	#define SYS_MODE_STS_NORMAL		(0x4 << 3)
+
+	#define SYS_MODE_MASK			(0x7 << 0)
+	#define SYS_MODE_SLEEP			(0x0 << 0)
+	#define SYS_MODE_DOZE			(0x1 << 0)
+	#define SYS_MODE_SLOW			(0x2 << 0)
+	#define SYS_MODE_NORMAL			(0x4 << 0)
+
 #define SCSYSSTAT		0x004
 #define SCIMCTRL		0x008
 #define SCIMSTAT		0x00C

@@ -65,4 +80,46 @@ static inline void sysctl_soft_reset(void __iomem *base)
 	writel(0, base + SCSYSSTAT);
 }
 
+static inline int sysctl_change_mode(void __iomem *base, int mode)
+{
+	u32 val, mode_sts;
+	unsigned long finish;
+
+	switch (mode) {
+	case SYS_MODE_SLEEP:
+		mode_sts = SYS_MODE_STS_SLEEP;
+		break;
+	case SYS_MODE_DOZE:
+		mode_sts = SYS_MODE_STS_DOZE;
+		break;
+	case SYS_MODE_SLOW:
+		mode_sts = SYS_MODE_STS_SLOW;
+		break;
+	case SYS_MODE_NORMAL:
+		mode_sts = SYS_MODE_STS_NORMAL;
+		break;
+	default:
+		pr_err("Wrong system mode\n");
+		return -EINVAL;
+	}
+
+	val = readl(base + SCCTRL);
+	if ((val & SYS_MODE_STS_MASK) == mode_sts)
+		return 0;
+
+	val &= ~SYS_MODE_MASK;
+	val |= mode;
+	writel(val, base + SCCTRL);
+
+	/* read back if mode is set */
+	finish = jiffies + 2 * HZ;
+	do {
+		val = readl(base + SCCTRL);
+		if ((val & SYS_MODE_STS_MASK) == mode_sts)
+			return 0;
+		udelay(1000);
+	} while (!time_after_eq(jiffies, finish));
+
+	return -EFAULT;
+}
 #endif	/* __ASM_ARM_SP810_H */

Signature · 1 line

1.7.8.110.g4cb5d

Viresh Kumar

author

2012-02-23 10:31 UTC #2 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 8 lines · Russell King - ARM Linux

Quoted 4 lines · Viresh Kumar
Quoted 4 lines
From: Shiraz Hashim <[email protected]>
sp810 controller can change system's working mode to various power save states. Introduce an API to accomplish the same.
Where is the documentation for this peripheral? I've never seen any documentation on ARMs website for it.

Even i couldn't locate it now :(

Documentation does mention it at several places, but didn't give a link.

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0440b/CHDJAIGE.html

Signature · 1 line

viresh

Linus Walleij

2012-02-27 10:38 UTC #3 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

In reply to Viresh Kumar

Quoted 2 lines · Viresh Kumar

sp810 controller can change system's working mode to various power save states. Introduce an API to accomplish the same.

OK...

Quoted 1 line

+static inline int sysctl_change_mode(void __iomem *base, int mode)

Why do you make this a static inline?

And why is the present soft reset code also static inlined?

When I look at how this is used in arch/arm/mach-vexpress/v2m.c it's apparently glued back-to-back with the SP804
so I think it should be some CONFIG_HAS_SP810
and put directly into common/timer-sp.c.

#ifdef CONFIG_HAS_SP810

/* Put SP810 specific support functions here */

void __init sp810_clockevents_init(void __iomem *sp804_base, void __iomem *sp810_base, unsigned int irq, const char *name) {
/* Do some SP810 magic stuff */
/* Calls sp804_clockevents_init() */
}

#endif

Maybe I'm delusional but I think this could help centralizing the use of the SP* cells, since they're obviously used in both SPEAr and versatile express.

Yours,
Linus Walleij

Russell King - ARM Linux

2012-02-27 10:55 UTC #4 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 3 lines · Linus Walleij

Maybe I'm delusional but I think this could help centralizing the use of the SP* cells, since they're obviously used in both SPEAr and versatile express.

They can be used in any PrimeXsys based system, and I don't see that there's anything which guarantees having both a SP810 and SP804 together.

There are platforms which have SP804 without SP810 - Integrator CP and bcmring seem to fall into that category.

But... the register concerned does a whole load of other things, and I'm not sure we want to have accesses to it de-centralized.

Linus Walleij

2012-02-27 15:29 UTC #5 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 11 lines · Russell King - ARM Linux

Quoted 3 lines · Linus Walleij
Maybe I'm delusional but I think this could help centralizing the use of the SP* cells, since they're obviously used in both SPEAr and versatile express.
They can be used in any PrimeXsys based system, and I don't see that there's anything which guarantees having both a SP810 and SP804 together.
There are platforms which have SP804 without SP810 - Integrator CP and bcmring seem to fall into that category.

I was more thinking the other way around - I think that all systems with an SP810 also have an SP804.

I'm led to thinking this because of these registers:

#define SCCTRL_TIMEREN0SEL_REFCLK       (0 << 15)
#define SCCTRL_TIMEREN0SEL_TIMCLK       (1 << 15)
#define SCCTRL_TIMEREN1SEL_REFCLK       (0 << 17)
#define SCCTRL_TIMEREN1SEL_TIMCLK       (1 << 17)

TIMER0 and TIMER1 seems to indicate TIMER0 and TIMER1 of the SP804 block.

In any case it seems to depend on CONFIG_ARM_TIMER_SP804.

Quoted 2 lines

But... the register concerned does a whole load of other things, and I'm not sure we want to have accesses to it de-centralized.

Sure, just arch/arm/common/sc-810.c works fine as well.

Now I got incentive to finally write that letter to Greg about what to do with system controllers.

Yours,
Linus Walleij

Russell King - ARM Linux

2012-02-27 15:42 UTC #6 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 19 lines · Linus Walleij

Quoted 11 lines · Russell King - ARM Linux
Quoted 3 lines · Linus Walleij
Maybe I'm delusional but I think this could help centralizing the use of the SP* cells, since they're obviously used in both SPEAr and versatile express.
They can be used in any PrimeXsys based system, and I don't see that there's anything which guarantees having both a SP810 and SP804 together.
There are platforms which have SP804 without SP810 - Integrator CP and bcmring seem to fall into that category.
I was more thinking the other way around - I think that all systems with an SP810 also have an SP804.
I'm led to thinking this because of these registers:

Quoted 7 lines

#define SCCTRL_TIMEREN0SEL_REFCLK       (0 << 15)
#define SCCTRL_TIMEREN0SEL_TIMCLK       (1 << 15)
#define SCCTRL_TIMEREN1SEL_REFCLK       (0 << 17)
#define SCCTRL_TIMEREN1SEL_TIMCLK       (1 << 17)

TIMER0 and TIMER1 seems to indicate TIMER0 and TIMER1 of the SP804 block.

Except... there's four timers, and we may not be necessarily using timer 0 and 1. So, you'd need to know independently which are the two timers you're using.

Russell King - ARM Linux

2012-02-29 20:14 UTC #7 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

In reply to Viresh Kumar

Quoted 27 lines · Viresh Kumar

From: Shiraz Hashim <[email protected]>
sp810 controller can change system's working mode to various power save states. Introduce an API to accomplish the same.
Signed-off-by Shiraz Hashim <[email protected]>
Signed-off-by Viresh Kumar <[email protected]>
---
 arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)
arch/arm/include/asm/hardware/sp810.h +4 -0
index e0d1c0c..df0960a 100644
@@ -14,10 +14,25 @@
 #ifndef __ASM_ARM_SP810_H
 #define __ASM_ARM_SP810_H
 
+#include <linux/err.h>
 #include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/jiffies.h>
 
 /* sysctl registers offset */
 #define SCCTRL			0x000
+	#define SYS_MODE_STS_MASK		(0xF << 3)

arch/arm/include/asm/hardware/sp810.h line 34

Quoted 10 lines

arch/arm/include/asm/hardware/sp810.h +10 -0

+	#define SYS_MODE_STS_SLEEP		(0x0 << 3)
+	#define SYS_MODE_STS_DOZE		(0x1 << 3)
+	#define SYS_MODE_STS_SLOW		(0x2 << 3)
+	#define SYS_MODE_STS_NORMAL		(0x4 << 3)
+
+	#define SYS_MODE_MASK			(0x7 << 0)
+	#define SYS_MODE_SLEEP			(0x0 << 0)
+	#define SYS_MODE_DOZE			(0x1 << 0)
+	#define SYS_MODE_SLOW			(0x2 << 0)
+	#define SYS_MODE_NORMAL			(0x4 << 0)

Please don't indent #defines like this, I find it abhorrent that,@least, the '#' is not in column 1.

Also, please name these constants SCCTRL_xxx so that the prefix matches the register which they're related to.

So, I'd much prefer these to be:

#define SCCTRL_MODE_STS_MASK (0xf << 3)
...

Thanks.

Viresh Kumar

author

2012-03-01 03:51 UTC #8 lore raw

PATCH 1/1 ARM/sp810: introduce API to change system mode

Quoted 6 lines · Russell King - ARM Linux

Quoted 6 lines

#define SCCTRL 0x000

arch/arm/include/asm/hardware/sp810.h +5 -0

+	#define SYS_MODE_STS_MASK		(0xF << 3)
+	#define SYS_MODE_STS_SLEEP		(0x0 << 3)
+	#define SYS_MODE_STS_DOZE		(0x1 << 3)
+	#define SYS_MODE_STS_SLOW		(0x2 << 3)
+	#define SYS_MODE_STS_NORMAL		(0x4 << 3)

Quoted 9 lines

Quoted 6 lines
arch/arm/include/asm/hardware/sp810.h line 34
Quoted 6 lines
arch/arm/include/asm/hardware/sp810.h +6 -0
+
+	#define SYS_MODE_MASK			(0x7 << 0)
+	#define SYS_MODE_SLEEP			(0x0 << 0)
+	#define SYS_MODE_DOZE			(0x1 << 0)
+	#define SYS_MODE_SLOW			(0x2 << 0)
+	#define SYS_MODE_NORMAL			(0x4 << 0)
Please don't indent #defines like this, I find it abhorrent that, at least, the '#' is not in column 1.

I will change it if you want, but this one looked more readable to me.

Quoted 6 lines

Also, please name these constants SCCTRL_xxx so that the prefix matches the register which they're related to.
So, I'd much prefer these to be:
#define SCCTRL_MODE_STS_MASK (0xf << 3)

Sure.

Signature · 1 line

viresh