| Message ID | 20170411154826.5883-1-alexandre.belloni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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.
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 --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
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(+)