diff mbox

[v2] clockevents/tcb_clksrc: implement suspend/resume

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

Commit Message

Alexandre Belloni May 12, 2017, 6:22 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>
---
Changes in v2:
 - use writel instead of __raw_writel
 - Document sequence
 - use ARRAY_SIZE(tcb_cache) instead of 3

 drivers/clocksource/tcb_clksrc.c | 51 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Daniel Lezcano June 6, 2017, 4:48 p.m. UTC | #1
On Fri, May 12, 2017 at 08:22:51PM +0200, 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>
> ---
> Changes in v2:
>  - use writel instead of __raw_writel
>  - Document sequence
>  - use ARRAY_SIZE(tcb_cache) instead of 3
> 
>  drivers/clocksource/tcb_clksrc.c | 51 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c

Hi,

what should I do with this patch as the patched file is supposed to be removed
[1]. 

  -- Daniel

[1] https://www.spinics.net/lists/kernel/msg2520390.html
Alexandre Belloni June 6, 2017, 6:09 p.m. UTC | #2
On 06/06/2017 at 18:48:17 +0200, Daniel Lezcano wrote:
> On Fri, May 12, 2017 at 08:22:51PM +0200, 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>
> > ---
> > Changes in v2:
> >  - use writel instead of __raw_writel
> >  - Document sequence
> >  - use ARRAY_SIZE(tcb_cache) instead of 3
> > 
> >  drivers/clocksource/tcb_clksrc.c | 51 ++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)
> > 
> > diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
> 
> Hi,
> 
> what should I do with this patch as the patched file is supposed to be removed
> [1]. 
> 

I feel like we are far from reaching an agreement on the other series so
it is still worth applying.
Daniel Lezcano June 6, 2017, 6:18 p.m. UTC | #3
On 06/06/2017 20:09, Alexandre Belloni wrote:
> On 06/06/2017 at 18:48:17 +0200, Daniel Lezcano wrote:
>> On Fri, May 12, 2017 at 08:22:51PM +0200, 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>
>>> ---
>>> Changes in v2:
>>>  - use writel instead of __raw_writel
>>>  - Document sequence
>>>  - use ARRAY_SIZE(tcb_cache) instead of 3
>>>
>>>  drivers/clocksource/tcb_clksrc.c | 51 ++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 51 insertions(+)
>>>
>>> diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
>>
>> Hi,
>>
>> what should I do with this patch as the patched file is supposed to be removed
>> [1]. 
>>
> 
> I feel like we are far from reaching an agreement on the other series so
> it is still worth applying.

Ok.
Daniel Lezcano June 13, 2017, 12:21 p.m. UTC | #4
On 12/05/2017 20:22, 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>

The context will be saved/restored on all the platforms (other than
sama5d2) using this timer even if the clock is not powered down. Is that ok?

Should it be tagged for stable@?

Side note: the config option is in drivers/misc, any particular reason
to have it there and not in the clocksource's Kconfig?
Alexandre Belloni June 13, 2017, 12:28 p.m. UTC | #5
On 13/06/2017 at 14:21:10 +0200, Daniel Lezcano wrote:
> On 12/05/2017 20:22, 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>
> 
> The context will be saved/restored on all the platforms (other than
> sama5d2) using this timer even if the clock is not powered down. Is that ok?
> 

Yes, that is fine. There is not much we can do here until we can
determine what is the target suspend state from the drivers.

> Should it be tagged for stable@?
> 

No because support for this suspend mode is not yet upstream.

> Side note: the config option is in drivers/misc, any particular reason
> to have it there and not in the clocksource's Kconfig?
> 

Mostly historical, this is changed in the other series.
Daniel Lezcano June 13, 2017, 12:31 p.m. UTC | #6
On 13/06/2017 14:28, Alexandre Belloni wrote:
> On 13/06/2017 at 14:21:10 +0200, Daniel Lezcano wrote:
>> On 12/05/2017 20:22, 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>
>>
>> The context will be saved/restored on all the platforms (other than
>> sama5d2) using this timer even if the clock is not powered down. Is that ok?
>>
> 
> Yes, that is fine. There is not much we can do here until we can
> determine what is the target suspend state from the drivers.
> 
>> Should it be tagged for stable@?
>>
> 
> No because support for this suspend mode is not yet upstream.
> 
>> Side note: the config option is in drivers/misc, any particular reason
>> to have it there and not in the clocksource's Kconfig?
>>
> 
> Mostly historical, this is changed in the other series.

Ok, thanks.

I've applied this patch for 4.13.

  -- Daniel
diff mbox

Patch

diff --git a/drivers/clocksource/tcb_clksrc.c b/drivers/clocksource/tcb_clksrc.c
index d4ca9962a759..828729c70a0c 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,54 @@  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 < ARRAY_SIZE(tcb_cache); 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 < ARRAY_SIZE(tcb_cache); i++) {
+		/* Restore registers for the channel, RA and RB are not used  */
+		writel(tcb_cache[i].cmr, tcaddr + ATMEL_TC_REG(i, CMR));
+		writel(tcb_cache[i].rc, tcaddr + ATMEL_TC_REG(i, RC));
+		writel(0, tcaddr + ATMEL_TC_REG(i, RA));
+		writel(0, tcaddr + ATMEL_TC_REG(i, RB));
+		/* Disable all the interrupts */
+		writel(0xff, tcaddr + ATMEL_TC_REG(i, IDR));
+		/* Reenable interrupts that were enabled before suspending */
+		writel(tcb_cache[i].imr, tcaddr + ATMEL_TC_REG(i, IER));
+		/* Start the clock if it was used */
+		if (tcb_cache[i].clken)
+			writel(ATMEL_TC_CLKEN, tcaddr + ATMEL_TC_REG(i, CCR));
+	}
+
+	/* Dual channel, chain channels */
+	writel(bmr_cache, tcaddr + ATMEL_TC_BMR);
+	/* Finally, trigger all the channels*/
+	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