* [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
@ 2024-05-22 8:14 Enlin Mu
2024-06-05 3:15 ` Chunyan Zhang
0 siblings, 1 reply; 4+ messages in thread
From: Enlin Mu @ 2024-05-22 8:14 UTC (permalink / raw)
To: orsonzhai, baolin.wang, zhang.lyra, tglx, enlin.mu, enlinmu
Cc: linux-kernel, linux-hardening
From: Enlin Mu <enlin.mu@unisoc.com>
Using 32 bit for suspend compensation, the max compensation time is 36
hours(working clock is 32k).In some IOT devices, the suspend time may
be long, even exceeding 36 hours. Therefore, a 64 bit timer counter
is needed for counting.
Signed-off-by: Enlin Mu <enlin.mu@unisoc.com>
---
drivers/clocksource/timer-sprd.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
index 430cb99d8d79..936691e27f8b 100644
--- a/drivers/clocksource/timer-sprd.c
+++ b/drivers/clocksource/timer-sprd.c
@@ -30,6 +30,7 @@
#define TIMER_VALUE_SHDW_HI 0x1c
#define TIMER_VALUE_LO_MASK GENMASK(31, 0)
+#define TIMER_VALUE_HI_MASK GENMASK(31, 0)
static void sprd_timer_enable(void __iomem *base, u32 flag)
{
@@ -57,10 +58,11 @@ static void sprd_timer_disable(void __iomem *base)
writel_relaxed(val, base + TIMER_CTL);
}
-static void sprd_timer_update_counter(void __iomem *base, unsigned long cycles)
+static void sprd_timer_update_counter(void __iomem *base, unsigned long cycles_lo,
+ unsigned long cycles_hi)
{
- writel_relaxed(cycles & TIMER_VALUE_LO_MASK, base + TIMER_LOAD_LO);
- writel_relaxed(0, base + TIMER_LOAD_HI);
+ writel_relaxed(cycles_lo & TIMER_VALUE_LO_MASK, base + TIMER_LOAD_LO);
+ writel_relaxed(cycles_hi, base + TIMER_LOAD_HI);
}
static void sprd_timer_enable_interrupt(void __iomem *base)
@@ -82,7 +84,8 @@ static int sprd_timer_set_next_event(unsigned long cycles,
struct timer_of *to = to_timer_of(ce);
sprd_timer_disable(timer_of_base(to));
- sprd_timer_update_counter(timer_of_base(to), cycles);
+ sprd_timer_update_counter(timer_of_base(to), cycles,
+ (u64)cycles >> 32);
sprd_timer_enable(timer_of_base(to), 0);
return 0;
@@ -93,7 +96,8 @@ static int sprd_timer_set_periodic(struct clock_event_device *ce)
struct timer_of *to = to_timer_of(ce);
sprd_timer_disable(timer_of_base(to));
- sprd_timer_update_counter(timer_of_base(to), timer_of_period(to));
+ sprd_timer_update_counter(timer_of_base(to), timer_of_period(to),
+ (u64)timer_of_period(to) >> 32);
sprd_timer_enable(timer_of_base(to), TIMER_CTL_PERIOD_MODE);
return 0;
@@ -162,14 +166,21 @@ static struct timer_of suspend_to = {
static u64 sprd_suspend_timer_read(struct clocksource *cs)
{
- return ~(u64)readl_relaxed(timer_of_base(&suspend_to) +
- TIMER_VALUE_SHDW_LO) & cs->mask;
+ u32 hi, lo;
+
+ lo = readl_relaxed(timer_of_base(&suspend_to) +
+ TIMER_VALUE_SHDW_LO);
+ hi = readl_relaxed(timer_of_base(&suspend_to) +
+ TIMER_VALUE_SHDW_HI);
+
+ return ~((u64)hi << 32 | lo);
}
static int sprd_suspend_timer_enable(struct clocksource *cs)
{
sprd_timer_update_counter(timer_of_base(&suspend_to),
- TIMER_VALUE_LO_MASK);
+ TIMER_VALUE_LO_MASK,
+ TIMER_VALUE_HI_MASK);
sprd_timer_enable(timer_of_base(&suspend_to), TIMER_CTL_PERIOD_MODE);
return 0;
@@ -186,7 +197,7 @@ static struct clocksource suspend_clocksource = {
.read = sprd_suspend_timer_read,
.enable = sprd_suspend_timer_enable,
.disable = sprd_suspend_timer_disable,
- .mask = CLOCKSOURCE_MASK(32),
+ .mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
};
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
2024-05-22 8:14 Enlin Mu
@ 2024-06-05 3:15 ` Chunyan Zhang
0 siblings, 0 replies; 4+ messages in thread
From: Chunyan Zhang @ 2024-06-05 3:15 UTC (permalink / raw)
To: Enlin Mu
Cc: orsonzhai, baolin.wang, tglx, enlin.mu, enlinmu, linux-kernel,
linux-hardening
Hi Enlin,
On Wed, 22 May 2024 at 16:15, Enlin Mu <enlin.mu@outlook.com> wrote:
>
> From: Enlin Mu <enlin.mu@unisoc.com>
>
> Using 32 bit for suspend compensation, the max compensation time is 36
> hours(working clock is 32k).In some IOT devices, the suspend time may
> be long, even exceeding 36 hours. Therefore, a 64 bit timer counter
> is needed for counting.
>
> Signed-off-by: Enlin Mu <enlin.mu@unisoc.com>
> ---
> drivers/clocksource/timer-sprd.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
> index 430cb99d8d79..936691e27f8b 100644
> --- a/drivers/clocksource/timer-sprd.c
> +++ b/drivers/clocksource/timer-sprd.c
> @@ -30,6 +30,7 @@
> #define TIMER_VALUE_SHDW_HI 0x1c
>
> #define TIMER_VALUE_LO_MASK GENMASK(31, 0)
> +#define TIMER_VALUE_HI_MASK GENMASK(31, 0)
>
> static void sprd_timer_enable(void __iomem *base, u32 flag)
> {
> @@ -57,10 +58,11 @@ static void sprd_timer_disable(void __iomem *base)
> writel_relaxed(val, base + TIMER_CTL);
> }
>
> -static void sprd_timer_update_counter(void __iomem *base, unsigned long cycles)
> +static void sprd_timer_update_counter(void __iomem *base, unsigned long cycles_lo,
> + unsigned long cycles_hi)
I would suggest using u64 rather than adding a new parameter. In this
way we can avoid some of the changes below.
> {
> - writel_relaxed(cycles & TIMER_VALUE_LO_MASK, base + TIMER_LOAD_LO);
> - writel_relaxed(0, base + TIMER_LOAD_HI);
> + writel_relaxed(cycles_lo & TIMER_VALUE_LO_MASK, base + TIMER_LOAD_LO);
> + writel_relaxed(cycles_hi, base + TIMER_LOAD_HI);
> }
>
> static void sprd_timer_enable_interrupt(void __iomem *base)
> @@ -82,7 +84,8 @@ static int sprd_timer_set_next_event(unsigned long cycles,
> struct timer_of *to = to_timer_of(ce);
>
> sprd_timer_disable(timer_of_base(to));
> - sprd_timer_update_counter(timer_of_base(to), cycles);
> + sprd_timer_update_counter(timer_of_base(to), cycles,
> + (u64)cycles >> 32);
On 32-bit systems, TIMER_LOAD_HI is still 0.
> sprd_timer_enable(timer_of_base(to), 0);
>
> return 0;
> @@ -93,7 +96,8 @@ static int sprd_timer_set_periodic(struct clock_event_device *ce)
> struct timer_of *to = to_timer_of(ce);
>
> sprd_timer_disable(timer_of_base(to));
> - sprd_timer_update_counter(timer_of_base(to), timer_of_period(to));
> + sprd_timer_update_counter(timer_of_base(to), timer_of_period(to),
> + (u64)timer_of_period(to) >> 32);
Same here, so do you need to consider 32-bit systems?
> sprd_timer_enable(timer_of_base(to), TIMER_CTL_PERIOD_MODE);
>
> return 0;
> @@ -162,14 +166,21 @@ static struct timer_of suspend_to = {
>
> static u64 sprd_suspend_timer_read(struct clocksource *cs)
> {
> - return ~(u64)readl_relaxed(timer_of_base(&suspend_to) +
> - TIMER_VALUE_SHDW_LO) & cs->mask;
> + u32 hi, lo;
> +
> + lo = readl_relaxed(timer_of_base(&suspend_to) +
> + TIMER_VALUE_SHDW_LO);
> + hi = readl_relaxed(timer_of_base(&suspend_to) +
> + TIMER_VALUE_SHDW_HI);
> +
> + return ~((u64)hi << 32 | lo);
> }
>
> static int sprd_suspend_timer_enable(struct clocksource *cs)
> {
> sprd_timer_update_counter(timer_of_base(&suspend_to),
> - TIMER_VALUE_LO_MASK);
> + TIMER_VALUE_LO_MASK,
> + TIMER_VALUE_HI_MASK);
Like I suggested above, not add a new parameter, then pass
CLOCKSOURCE_MASK(64) as the cycles, that would avoid wrongly setting
TIMER_LOAD_HI for 32-bit systems based on the current implementations.
Thanks,
Chunyan
> sprd_timer_enable(timer_of_base(&suspend_to), TIMER_CTL_PERIOD_MODE);
>
> return 0;
> @@ -186,7 +197,7 @@ static struct clocksource suspend_clocksource = {
> .read = sprd_suspend_timer_read,
> .enable = sprd_suspend_timer_enable,
> .disable = sprd_suspend_timer_disable,
> - .mask = CLOCKSOURCE_MASK(32),
> + .mask = CLOCKSOURCE_MASK(64),
> .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> };
>
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
@ 2025-09-11 8:25 Cixi Geng
2025-09-22 3:53 ` Baolin Wang
0 siblings, 1 reply; 4+ messages in thread
From: Cixi Geng @ 2025-09-11 8:25 UTC (permalink / raw)
To: daniel.lezcano, tglx
Cc: orsonzhai, baolin.wang, zhang.lyra, cixi.geng, linux-kernel
From: Enlin Mu <enlin.mu@unisoc.com>
Using 32 bit for suspend compensation, the max compensation time is 36
hours(working clock is 32k).In some IOT devices, the suspend time may
be long, even exceeding 36 hours. Therefore, a 64 bit timer counter
is needed for counting.
Signed-off-by: Enlin Mu <enlin.mu@unisoc.com>
Signed-off-by: Cixi Geng <cixi.geng@linux.dev>
---
drivers/clocksource/timer-sprd.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
index 430cb99d8d79..742ee88b05d0 100644
--- a/drivers/clocksource/timer-sprd.c
+++ b/drivers/clocksource/timer-sprd.c
@@ -30,6 +30,7 @@
#define TIMER_VALUE_SHDW_HI 0x1c
#define TIMER_VALUE_LO_MASK GENMASK(31, 0)
+#define TIMER_VALUE_HI_MASK GENMASK(31, 0)
static void sprd_timer_enable(void __iomem *base, u32 flag)
{
@@ -162,15 +163,23 @@ static struct timer_of suspend_to = {
static u64 sprd_suspend_timer_read(struct clocksource *cs)
{
- return ~(u64)readl_relaxed(timer_of_base(&suspend_to) +
- TIMER_VALUE_SHDW_LO) & cs->mask;
+ u32 lo, hi;
+
+ lo = readl_relaxed(timer_of_base(&suspend_to) +
+ TIMER_VALUE_SHDW_LO);
+ hi = readl_relaxed(timer_of_base(&suspend_to) +
+ TIMER_VALUE_SHDW_HI);
+ return ~(((u64)hi << 32) | lo);
}
static int sprd_suspend_timer_enable(struct clocksource *cs)
{
- sprd_timer_update_counter(timer_of_base(&suspend_to),
- TIMER_VALUE_LO_MASK);
- sprd_timer_enable(timer_of_base(&suspend_to), TIMER_CTL_PERIOD_MODE);
+ writel_relaxed(TIMER_VALUE_LO_MASK,
+ timer_of_base(&suspend_to) + TIMER_LOAD_LO);
+ writel_relaxed(TIMER_VALUE_HI_MASK,
+ timer_of_base(&suspend_to) + TIMER_LOAD_HI);
+ sprd_timer_enable(timer_of_base(&suspend_to),
+ TIMER_CTL_PERIOD_MODE|TIMER_CTL_64BIT_WIDTH);
return 0;
}
@@ -186,7 +195,7 @@ static struct clocksource suspend_clocksource = {
.read = sprd_suspend_timer_read,
.enable = sprd_suspend_timer_enable,
.disable = sprd_suspend_timer_disable,
- .mask = CLOCKSOURCE_MASK(32),
+ .mask = CLOCKSOURCE_MASK(64),
.flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
};
--
2.43.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit
2025-09-11 8:25 [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit Cixi Geng
@ 2025-09-22 3:53 ` Baolin Wang
0 siblings, 0 replies; 4+ messages in thread
From: Baolin Wang @ 2025-09-22 3:53 UTC (permalink / raw)
To: Cixi Geng, daniel.lezcano, tglx; +Cc: orsonzhai, zhang.lyra, linux-kernel
On 2025/9/11 16:25, Cixi Geng wrote:
> From: Enlin Mu <enlin.mu@unisoc.com>
>
> Using 32 bit for suspend compensation, the max compensation time is 36
> hours(working clock is 32k).In some IOT devices, the suspend time may
> be long, even exceeding 36 hours. Therefore, a 64 bit timer counter
> is needed for counting.
>
> Signed-off-by: Enlin Mu <enlin.mu@unisoc.com>
> Signed-off-by: Cixi Geng <cixi.geng@linux.dev>
> ---
> drivers/clocksource/timer-sprd.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/timer-sprd.c b/drivers/clocksource/timer-sprd.c
> index 430cb99d8d79..742ee88b05d0 100644
> --- a/drivers/clocksource/timer-sprd.c
> +++ b/drivers/clocksource/timer-sprd.c
> @@ -30,6 +30,7 @@
> #define TIMER_VALUE_SHDW_HI 0x1c
>
> #define TIMER_VALUE_LO_MASK GENMASK(31, 0)
> +#define TIMER_VALUE_HI_MASK GENMASK(31, 0)
>
> static void sprd_timer_enable(void __iomem *base, u32 flag)
> {
> @@ -162,15 +163,23 @@ static struct timer_of suspend_to = {
>
> static u64 sprd_suspend_timer_read(struct clocksource *cs)
> {
> - return ~(u64)readl_relaxed(timer_of_base(&suspend_to) +
> - TIMER_VALUE_SHDW_LO) & cs->mask;
> + u32 lo, hi;
> +
> + lo = readl_relaxed(timer_of_base(&suspend_to) +
> + TIMER_VALUE_SHDW_LO);
> + hi = readl_relaxed(timer_of_base(&suspend_to) +
> + TIMER_VALUE_SHDW_HI);
Can you align your code like the previous code?
> + return ~(((u64)hi << 32) | lo);
> }
>
> static int sprd_suspend_timer_enable(struct clocksource *cs)
> {
> - sprd_timer_update_counter(timer_of_base(&suspend_to),
> - TIMER_VALUE_LO_MASK);
> - sprd_timer_enable(timer_of_base(&suspend_to), TIMER_CTL_PERIOD_MODE);
> + writel_relaxed(TIMER_VALUE_LO_MASK,
> + timer_of_base(&suspend_to) + TIMER_LOAD_LO);
> + writel_relaxed(TIMER_VALUE_HI_MASK,
> + timer_of_base(&suspend_to) + TIMER_LOAD_HI);
> + sprd_timer_enable(timer_of_base(&suspend_to),
> + TIMER_CTL_PERIOD_MODE|TIMER_CTL_64BIT_WIDTH);
Ditto. Otherwise LGTM.
>
> return 0;
> }
> @@ -186,7 +195,7 @@ static struct clocksource suspend_clocksource = {
> .read = sprd_suspend_timer_read,
> .enable = sprd_suspend_timer_enable,
> .disable = sprd_suspend_timer_disable,
> - .mask = CLOCKSOURCE_MASK(32),
> + .mask = CLOCKSOURCE_MASK(64),
> .flags = CLOCK_SOURCE_IS_CONTINUOUS | CLOCK_SOURCE_SUSPEND_NONSTOP,
> };
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-22 3:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-11 8:25 [PATCH] clocksource/drivers/sprd: Enable register for timer counter from 32 bit to 64 bit Cixi Geng
2025-09-22 3:53 ` Baolin Wang
-- strict thread matches above, loose matches on Subject: below --
2024-05-22 8:14 Enlin Mu
2024-06-05 3:15 ` Chunyan Zhang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.