[PATCH] ARM/sp810: introduce API to change system mode

[PATCH] ARM/sp810: introduce API to change system mode

[Viresh Kumar]

From: Shiraz Hashim <shiraz.hashim@st.com>

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 <shiraz.hashim@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/hardware/sp810.h b/arch/arm/include/asm/hardware/sp810.h
index e0d1c0c..df0960a 100644
--- a/arch/arm/include/asm/hardware/sp810.h
+++ b/arch/arm/include/asm/hardware/sp810.h
@@ -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 */
-- 
1.7.8.110.g4cb5d

[PATCH] ARM/sp810: introduce API to change system mode

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 02:43:44PM +0530, Viresh Kumar wrote:
> From: Shiraz Hashim <shiraz.hashim@st.com>
> 
> 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.

> 
> Signed-off-by: Shiraz Hashim <shiraz.hashim@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
>  1 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/hardware/sp810.h b/arch/arm/include/asm/hardware/sp810.h
> index e0d1c0c..df0960a 100644
> --- a/arch/arm/include/asm/hardware/sp810.h
> +++ b/arch/arm/include/asm/hardware/sp810.h
> @@ -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 */
> -- 
> 1.7.8.110.g4cb5d
> 

[PATCH] ARM/sp810: introduce API to change system mode

[Viresh Kumar]

On 2/23/2012 3:33 PM, Russell King - ARM Linux wrote:
> On Thu, Feb 23, 2012 at 02:43:44PM +0530, Viresh Kumar wrote:
>> > From: Shiraz Hashim <shiraz.hashim@st.com>
>> > 
>> > 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
-- 
viresh

[PATCH] ARM/sp810: introduce API to change system mode

[Linus Walleij]

On Thu, Feb 23, 2012 at 10:13 AM, Viresh Kumar <viresh.kumar@st.com> wrote:

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

OK...

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

[PATCH] ARM/sp810: introduce API to change system mode

[Russell King - ARM Linux]

On Mon, Feb 27, 2012 at 11:38:14AM +0100, Linus Walleij wrote:
> 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.

[PATCH] ARM/sp810: introduce API to change system mode

[Linus Walleij]

On Mon, Feb 27, 2012 at 11:55 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Feb 27, 2012 at 11:38:14AM +0100, Linus Walleij wrote:
>> 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.

> 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

[PATCH] ARM/sp810: introduce API to change system mode

[Russell King - ARM Linux]

On Mon, Feb 27, 2012 at 04:29:58PM +0100, Linus Walleij wrote:
> On Mon, Feb 27, 2012 at 11:55 AM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Feb 27, 2012 at 11:38:14AM +0100, Linus Walleij wrote:
> >> 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.

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.

[PATCH] ARM/sp810: introduce API to change system mode

[Russell King - ARM Linux]

On Thu, Feb 23, 2012 at 02:43:44PM +0530, Viresh Kumar wrote:
> From: Shiraz Hashim <shiraz.hashim@st.com>
> 
> 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 <shiraz.hashim@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  arch/arm/include/asm/hardware/sp810.h |   57 +++++++++++++++++++++++++++++++++
>  1 files changed, 57 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/include/asm/hardware/sp810.h b/arch/arm/include/asm/hardware/sp810.h
> index e0d1c0c..df0960a 100644
> --- a/arch/arm/include/asm/hardware/sp810.h
> +++ b/arch/arm/include/asm/hardware/sp810.h
> @@ -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)

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.

[PATCH] ARM/sp810: introduce API to change system mode

[Viresh Kumar]

On 3/1/2012 1:44 AM, Russell King - ARM Linux wrote:
>> >  #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)
> 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.

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

-- 
viresh