diff mbox

clockevents/tcb_clksrc: implement suspend/resume

Message ID 20170411154826.5883-1-alexandre.belloni@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Alexandre Belloni April 11, 2017, 3:48 p.m. UTC 
On sama5d2, power to the core may be cut while entering suspend mode. It is
necessary to save and restore the TCB registers.

Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
 drivers/clocksource/tcb_clksrc.c | 46 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

Comments

Daniel Lezcano April 14, 2017, 7:13 p.m. UTC | #1 
On 11/04/2017 17:48, Alexandre Belloni wrote:
> On sama5d2, power to the core may be cut while entering suspend mode. It is
> necessary to save and restore the TCB registers.
> 
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
>  drivers/clocksource/tcb_clksrc.c | 46 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
> index d4ca9962a759..57f5d72328f4 100644
> --- a/drivers/clocksource/tcb_clksrc.c
> +++ b/drivers/clocksource/tcb_clksrc.c
> @@ -9,6 +9,7 @@
>  #include <linux/ioport.h>
>  #include <linux/io.h>
>  #include <linux/platform_device.h>
> +#include <linux/syscore_ops.h>
>  #include <linux/atmel_tc.h>
>  
>  
> @@ -40,6 +41,14 @@
>   */
>  
>  static void __iomem *tcaddr;
> +static struct
> +{
> +	u32 cmr;
> +	u32 imr;
> +	u32 rc;
> +	bool clken;
> +} tcb_cache[3];
> +static u32 bmr_cache;
>  
>  static u64 tc_get_cycles(struct clocksource *cs)
>  {
> @@ -61,12 +70,49 @@ static u64 tc_get_cycles32(struct clocksource *cs)
>  	return __raw_readl(tcaddr + ATMEL_TC_REG(0, CV));
>  }
>  
> +void tc_clksrc_suspend(struct clocksource *cs)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++) {

s/3/ARRAY_SIZE(tcb_cache)/

> +		tcb_cache[i].cmr = readl(tcaddr + ATMEL_TC_REG(i, CMR));
> +		tcb_cache[i].imr = readl(tcaddr + ATMEL_TC_REG(i, IMR));
> +		tcb_cache[i].rc = readl(tcaddr + ATMEL_TC_REG(i, RC));
> +		tcb_cache[i].clken = !!(readl(tcaddr + ATMEL_TC_REG(i, SR)) &
> +					ATMEL_TC_CLKSTA);
> +	}
> +
> +	bmr_cache = readl(tcaddr + ATMEL_TC_BMR);
> +}
> +
> +void tc_clksrc_resume(struct clocksource *cs)
> +{
> +	int i;
> +
> +	for (i = 0; i < 3; i++) {

s/3/ARRAY_SIZE(tcb_cache)/

> +		__raw_writel(tcb_cache[i].cmr, tcaddr + ATMEL_TC_REG(i, CMR));

Why __raw_writel?

> +		__raw_writel(0, tcaddr + ATMEL_TC_REG(i, RA));
> +		__raw_writel(0, tcaddr + ATMEL_TC_REG(i, RB));
> +		__raw_writel(tcb_cache[i].rc, tcaddr + ATMEL_TC_REG(i, RC));
> +		__raw_writel(0xff, tcaddr + ATMEL_TC_REG(i, IDR));
> +		__raw_writel(tcb_cache[i].imr, tcaddr + ATMEL_TC_REG(i, IER));
> +		if (tcb_cache[i].clken)
> +			__raw_writel(ATMEL_TC_CLKEN, tcaddr +
> +				     ATMEL_TC_REG(i, CCR));
> +	}
> +
> +	writel(bmr_cache, tcaddr + ATMEL_TC_BMR);
> +	writel(ATMEL_TC_SYNC, tcaddr + ATMEL_TC_BCR);

Do you mind to add a description of the restore sequence?

Thanks!

  -- Daniel

> +}
> +
>  static struct clocksource clksrc = {
>  	.name           = "tcb_clksrc",
>  	.rating         = 200,
>  	.read           = tc_get_cycles,
>  	.mask           = CLOCKSOURCE_MASK(32),
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> +	.suspend	= tc_clksrc_suspend,
> +	.resume		= tc_clksrc_resume,
>  };
>  
>  #ifdef CONFIG_GENERIC_CLOCKEVENTS
>
Alexandre Belloni June 15, 2017, 7:40 p.m. UTC | #2 
On 14/04/2017 at 21:13:36 +0200, Daniel Lezcano wrote:
> > +void tc_clksrc_resume(struct clocksource *cs)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < 3; i++) {
> 
> s/3/ARRAY_SIZE(tcb_cache)/
> 
> > +		__raw_writel(tcb_cache[i].cmr, tcaddr + ATMEL_TC_REG(i, CMR));
> 
> Why __raw_writel?
> 

Ok, I got to the bottom of that question and I think it is worth
answering it. __raw_{read,write}l were necessary to make the driver work
on AVR32, because its core is BE and the IP LE and the regular
readl/writel are (were) not doing the proper conversion.

This was supposed to be changed in a patch that was never applied:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/331775.html

But everything is fine, as AVR32 is now removed from the kernel. I think
I'll switch the driver to regular readl/writel, using the _relaxed
version in the hot path. Is that fine for you?

I'll also do so in the rework if at some point we can agree on some
bindings, I'll try to address that soon too.
Daniel Lezcano June 15, 2017, 8:52 p.m. UTC | #3 
On 15/06/2017 21:40, Alexandre Belloni wrote:
> On 14/04/2017 at 21:13:36 +0200, Daniel Lezcano wrote:
>>> +void tc_clksrc_resume(struct clocksource *cs)
>>> +{
>>> +	int i;
>>> +
>>> +	for (i = 0; i < 3; i++) {
>>
>> s/3/ARRAY_SIZE(tcb_cache)/
>>
>>> +		__raw_writel(tcb_cache[i].cmr, tcaddr + ATMEL_TC_REG(i, CMR));
>>
>> Why __raw_writel?
>>
> 
> Ok, I got to the bottom of that question and I think it is worth
> answering it. __raw_{read,write}l were necessary to make the driver work
> on AVR32, because its core is BE and the IP LE and the regular
> readl/writel are (were) not doing the proper conversion.
> 
> This was supposed to be changed in a patch that was never applied:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-March/331775.html
> 
> But everything is fine, as AVR32 is now removed from the kernel. I think
> I'll switch the driver to regular readl/writel, using the _relaxed
> version in the hot path. Is that fine for you?

Yes.

> I'll also do so in the rework if at some point we can agree on some
> bindings, I'll try to address that soon too.

Ok, thanks.

  -- Daniel
diff mbox

Patch

diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index d4ca9962a759..57f5d72328f4 100644
--- a/drivers/clocksource/tcb_clksrc.c
+++ b/drivers/clocksource/tcb_clksrc.c
@@ -9,6 +9,7 @@ 
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <linux/platform_device.h>
+#include <linux/syscore_ops.h>
 #include <linux/atmel_tc.h>
 
 
@@ -40,6 +41,14 @@ 
  */
 
 static void __iomem *tcaddr;
+static struct
+{
+	u32 cmr;
+	u32 imr;
+	u32 rc;
+	bool clken;
+} tcb_cache[3];
+static u32 bmr_cache;
 
 static u64 tc_get_cycles(struct clocksource *cs)
 {
@@ -61,12 +70,49 @@  static u64 tc_get_cycles32(struct clocksource *cs)
 	return __raw_readl(tcaddr + ATMEL_TC_REG(0, CV));
 }
 
+void tc_clksrc_suspend(struct clocksource *cs)
+{
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		tcb_cache[i].cmr = readl(tcaddr + ATMEL_TC_REG(i, CMR));
+		tcb_cache[i].imr = readl(tcaddr + ATMEL_TC_REG(i, IMR));
+		tcb_cache[i].rc = readl(tcaddr + ATMEL_TC_REG(i, RC));
+		tcb_cache[i].clken = !!(readl(tcaddr + ATMEL_TC_REG(i, SR)) &
+					ATMEL_TC_CLKSTA);
+	}
+
+	bmr_cache = readl(tcaddr + ATMEL_TC_BMR);
+}
+
+void tc_clksrc_resume(struct clocksource *cs)
+{
+	int i;
+
+	for (i = 0; i < 3; i++) {
+		__raw_writel(tcb_cache[i].cmr, tcaddr + ATMEL_TC_REG(i, CMR));
+		__raw_writel(0, tcaddr + ATMEL_TC_REG(i, RA));
+		__raw_writel(0, tcaddr + ATMEL_TC_REG(i, RB));
+		__raw_writel(tcb_cache[i].rc, tcaddr + ATMEL_TC_REG(i, RC));
+		__raw_writel(0xff, tcaddr + ATMEL_TC_REG(i, IDR));
+		__raw_writel(tcb_cache[i].imr, tcaddr + ATMEL_TC_REG(i, IER));
+		if (tcb_cache[i].clken)
+			__raw_writel(ATMEL_TC_CLKEN, tcaddr +
+				     ATMEL_TC_REG(i, CCR));
+	}
+
+	writel(bmr_cache, tcaddr + ATMEL_TC_BMR);
+	writel(ATMEL_TC_SYNC, tcaddr + ATMEL_TC_BCR);
+}
+
 static struct clocksource clksrc = {
 	.name           = "tcb_clksrc",
 	.rating         = 200,
 	.read           = tc_get_cycles,
 	.mask           = CLOCKSOURCE_MASK(32),
 	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
+	.suspend	= tc_clksrc_suspend,
+	.resume		= tc_clksrc_resume,
 };
 
 #ifdef CONFIG_GENERIC_CLOCKEVENTS