* [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered @ 2025-02-21 3:46 stephen eta zhou 2025-03-07 8:10 ` Daniel Lezcano 0 siblings, 1 reply; 7+ messages in thread From: stephen eta zhou @ 2025-02-21 3:46 UTC (permalink / raw) To: daniel.lezcano; +Cc: tglx, linux-kernel Hi daniel While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. From 9dd9b5bd7ab1638990176f7171417c83ddb7a221 Mon Sep 17 00:00:00 2001 From: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> Date: Fri, 21 Feb 2025 11:15:40 +0800 Subject: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered Fix read_current_timer() on ARM32 by adding support in the SP804 driver. Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> --- drivers/clocksource/timer-sp804.c | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c index cd1916c05325..b98a14d24874 100644 --- a/drivers/clocksource/timer-sp804.c +++ b/drivers/clocksource/timer-sp804.c @@ -21,6 +21,11 @@ #include <linux/of_irq.h> #include <linux/sched_clock.h> +#ifdef CONFIG_ARM +#include <linux/delay.h> +#include "timer-of.h" +#endif + #include "timer-sp.h" /* Hisilicon 64-bit timer(a variant of ARM SP804) */ @@ -59,6 +64,10 @@ static struct sp804_timer hisi_sp804_timer __initdata = { static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; +#ifdef CONFIG_ARM + struct delay_timer delay; +#endif + static long __init sp804_get_clock_rate(struct clk *clk, const char *name) { int err; @@ -102,6 +111,13 @@ static u64 notrace sp804_read(void) return ~readl_relaxed(sched_clkevt->value); } +#ifdef CONFIG_ARM +static unsigned long sp804_read_delay_timer_read(void) +{ + return sp804_read(); +} +#endif + static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, const char *name, struct clk *clk, @@ -259,6 +275,10 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time struct clk *clk1, *clk2; const char *name = of_get_property(np, "compatible", NULL); +#ifdef CONFIG_ARM + struct timer_of to = { .flags = TIMER_OF_CLOCK }; +#endif + if (initialized) { pr_debug("%pOF: skipping further SP804 timer device\n", np); return 0; @@ -318,6 +338,22 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time if (ret) goto err; } + +#ifdef CONFIG_ARM + ret = timer_of_init(np, &to); + if (ret) { + pr_err("Failed to initialize the Timer device tree: %d\n", ret); + return ret; + } + + delay.read_current_timer = sp804_read_delay_timer_read; + delay.freq = timer_of_rate(&to); + if (delay.freq <= 0) + pr_warn("Failed to obtain the freq of the clock source: %d\n", ret); + + register_current_timer_delay(&delay); +#endif + initialized = true; return 0; -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-02-21 3:46 [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered stephen eta zhou @ 2025-03-07 8:10 ` Daniel Lezcano 2025-03-07 11:17 ` Krzysztof Kozlowski 2025-03-08 4:23 ` stephen eta zhou 0 siblings, 2 replies; 7+ messages in thread From: Daniel Lezcano @ 2025-03-07 8:10 UTC (permalink / raw) To: stephen eta zhou; +Cc: tglx, linux-kernel Hi Stephen, thanks for the proposed fix On 21/02/2025 04:46, stephen eta zhou wrote: > Hi daniel > > While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. It is better to put that information in the changelog and provide a fixed format of the patch description. > From 9dd9b5bd7ab1638990176f7171417c83ddb7a221 Mon Sep 17 00:00:00 2001 > From: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > Date: Fri, 21 Feb 2025 11:15:40 +0800 > Subject: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when > clock source is not registered > > Fix read_current_timer() on ARM32 by adding support in the SP804 driver. > > Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > --- > drivers/clocksource/timer-sp804.c | 36 +++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c > index cd1916c05325..b98a14d24874 100644 > --- a/drivers/clocksource/timer-sp804.c > +++ b/drivers/clocksource/timer-sp804.c > @@ -21,6 +21,11 @@ > #include <linux/of_irq.h> > #include <linux/sched_clock.h> > > +#ifdef CONFIG_ARM > +#include <linux/delay.h> > +#include "timer-of.h" > +#endif > + > #include "timer-sp.h" > > /* Hisilicon 64-bit timer(a variant of ARM SP804) */ > @@ -59,6 +64,10 @@ static struct sp804_timer hisi_sp804_timer __initdata = { > > static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; > > +#ifdef CONFIG_ARM > + struct delay_timer delay; static ... > +#endif > + > > static long __init sp804_get_clock_rate(struct clk *clk, const char *name) > { > int err; > @@ -102,6 +111,13 @@ static u64 notrace sp804_read(void) > return ~readl_relaxed(sched_clkevt->value); > } > > +#ifdef CONFIG_ARM > +static unsigned long sp804_read_delay_timer_read(void) > +{ > + return sp804_read(); > +} > +#endif Group this function with the global delay variable above. > static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, > const char *name, > struct clk *clk, > @@ -259,6 +275,10 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > struct clk *clk1, *clk2; > const char *name = of_get_property(np, "compatible", NULL); > > +#ifdef CONFIG_ARM > + struct timer_of to = { .flags = TIMER_OF_CLOCK }; > +#endif > + > if (initialized) { > pr_debug("%pOF: skipping further SP804 timer device\n", np); > return 0; > @@ -318,6 +338,22 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > if (ret) > goto err; > } > + > +#ifdef CONFIG_ARM > + ret = timer_of_init(np, &to); The clock is already retrieved from the initialization code before > + if (ret) { > + pr_err("Failed to initialize the Timer device tree: %d\n", ret); > + return ret; > + } > + > + delay.read_current_timer = sp804_read_delay_timer_read; > + delay.freq = timer_of_rate(&to); > + if (delay.freq <= 0) > + pr_warn("Failed to obtain the freq of the clock source: %d\n", ret); > + > + register_current_timer_delay(&delay); > +#endif> initialized = true; > > return 0; -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-03-07 8:10 ` Daniel Lezcano @ 2025-03-07 11:17 ` Krzysztof Kozlowski 2025-03-07 13:41 ` Daniel Lezcano 2025-03-08 2:32 ` stephen eta zhou 2025-03-08 4:23 ` stephen eta zhou 1 sibling, 2 replies; 7+ messages in thread From: Krzysztof Kozlowski @ 2025-03-07 11:17 UTC (permalink / raw) To: Daniel Lezcano, stephen eta zhou; +Cc: tglx, linux-kernel On 07/03/2025 09:10, Daniel Lezcano wrote: > > Hi Stephen, > > thanks for the proposed fix > > On 21/02/2025 04:46, stephen eta zhou wrote: >> Hi daniel >> >> While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. > > It is better to put that information in the changelog and provide a > fixed format of the patch description. Daniel, In case you actually consider applying this - some of the patches or their descriptions were generated by sort of AI tools, thus approach with caution. It was already pointed out that patches might be a garbage output of AI. Also they were not tested, even though what is claimed here "debugging on ...". Best regards, Krzysztof ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-03-07 11:17 ` Krzysztof Kozlowski @ 2025-03-07 13:41 ` Daniel Lezcano 2025-03-08 3:15 ` stephen eta zhou 2025-03-08 2:32 ` stephen eta zhou 1 sibling, 1 reply; 7+ messages in thread From: Daniel Lezcano @ 2025-03-07 13:41 UTC (permalink / raw) To: Krzysztof Kozlowski, stephen eta zhou; +Cc: tglx, linux-kernel Hi Krzysztof, On 07/03/2025 12:17, Krzysztof Kozlowski wrote: > On 07/03/2025 09:10, Daniel Lezcano wrote: >> >> Hi Stephen, >> >> thanks for the proposed fix >> >> On 21/02/2025 04:46, stephen eta zhou wrote: >>> Hi daniel >>> >>> While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. >> >> It is better to put that information in the changelog and provide a >> fixed format of the patch description. > Daniel, > > In case you actually consider applying this - some of the patches or > their descriptions were generated by sort of AI tools, thus approach > with caution. > > It was already pointed out that patches might be a garbage output of AI. > Also they were not tested, even though what is claimed here "debugging > on ...". Oh! Thanks for letting me know that, I appreciate I'll pay double attention to the patches. Thanks -- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-03-07 13:41 ` Daniel Lezcano @ 2025-03-08 3:15 ` stephen eta zhou 0 siblings, 0 replies; 7+ messages in thread From: stephen eta zhou @ 2025-03-08 3:15 UTC (permalink / raw) To: Daniel Lezcano, Krzysztof Kozlowski; +Cc: tglx, linux-kernel Hi Daniel > Oh! Thanks for letting me know that, I appreciate > I'll pay double attention to the patches. I want to explain to you what Krzysztof said about my patch being generated by AI. I have always had a worship spirit for Linux. I admit that I was a little biased towards AI in my previous email reply, because I was worried that some of my words would make kernel contributors feel unfriendly, so I asked AI to help me polish my answer, but I was indeed misled by AI during the polishing process. I admit that this was my previous fault. But what I want to explain about this patch is that I really debugged it step by step using qemu. If necessary, I can provide complete and detailed debugging records. I can first briefly explain how I found this problem. I was trying to optimize the memblock_add function, and I wanted to know how long it took to run. In the early kernel, I tried to get the tsc timestamp, but I found that all I got were 0, so I started to investigate step by step, and finally located that vexpress-v2p-ca9 used the sp804 driver and did not register the current_timer. I also submitted another patch for this problem, but there is no reply yet. In another patch, I added WARN to read_current_timer to quickly locate the problem, because I did debug for a while at that time. I am not sure if the patch I submitted is the best, but I want to explain that this is indeed the patch I debugged and submitted myself. If you want to verify this problem, it is also very simple. Just use qemu to run vexpress_defconfig, this conf, and then use vexpress-v2p-ca9.dtb, and then breakpoint to the read_current_timer function in qemu+gdb, and you will find that it will return -ENXIO For the previous situation, I apologize to the kernel maintainers, but please give me a chance, can you not list me as AI? ..... Thanks Stephen ________________________________________ From: Daniel Lezcano <daniel.lezcano@linaro.org> Sent: Friday, March 7, 2025 21:41 To: Krzysztof Kozlowski <krzk@kernel.org>; stephen eta zhou <stephen.eta.zhou@outlook.com> Cc: tglx@linutronix.de <tglx@linutronix.de>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered Hi Krzysztof, On 07/03/2025 12:17, Krzysztof Kozlowski wrote: > On 07/03/2025 09:10, Daniel Lezcano wrote: >> >> Hi Stephen, >> >> thanks for the proposed fix >> >> On 21/02/2025 04:46, stephen eta zhou wrote: >>> Hi daniel >>> >>> While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. >> >> It is better to put that information in the changelog and provide a >> fixed format of the patch description. > Daniel, > > In case you actually consider applying this - some of the patches or > their descriptions were generated by sort of AI tools, thus approach > with caution. > > It was already pointed out that patches might be a garbage output of AI. > Also they were not tested, even though what is claimed here "debugging > on ...". Oh! Thanks for letting me know that, I appreciate I'll pay double attention to the patches. Thanks -- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-03-07 11:17 ` Krzysztof Kozlowski 2025-03-07 13:41 ` Daniel Lezcano @ 2025-03-08 2:32 ` stephen eta zhou 1 sibling, 0 replies; 7+ messages in thread From: stephen eta zhou @ 2025-03-08 2:32 UTC (permalink / raw) To: Krzysztof Kozlowski, Daniel Lezcano; +Cc: tglx, linux-kernel > In case you actually consider applying this - some of the patches or > their descriptions were generated by sort of AI tools, thus approach > with caution. > It was already pointed out that patches might be a garbage output of AI. > Also they were not tested, even though what is claimed here "debugging > on ...". Hi Krzysztof I want to point out that this code was debugged by me personally, and I can provide a complete debugging record. I want to point out that the reason why my answers to some of the previous patches are a bit AI-like is because I am a bit reserved about submitting code to the kernel community, and I am worried that some answers may not be friendly, so I used AI to polish some of my answers. But I must point out that this is the result of my own debugging and the solution I came up with after careful consideration. Thanks Stephen ________________________________________ From: Krzysztof Kozlowski <krzk@kernel.org> Sent: Friday, March 7, 2025 19:17 To: Daniel Lezcano <daniel.lezcano@linaro.org>; stephen eta zhou <stephen.eta.zhou@outlook.com> Cc: tglx@linutronix.de <tglx@linutronix.de>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered On 07/03/2025 09:10, Daniel Lezcano wrote: > > Hi Stephen, > > thanks for the proposed fix > > On 21/02/2025 04:46, stephen eta zhou wrote: >> Hi daniel >> >> While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. > > It is better to put that information in the changelog and provide a > fixed format of the patch description. Daniel, In case you actually consider applying this - some of the patches or their descriptions were generated by sort of AI tools, thus approach with caution. It was already pointed out that patches might be a garbage output of AI. Also they were not tested, even though what is claimed here "debugging on ...". Best regards, Krzysztof ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered 2025-03-07 8:10 ` Daniel Lezcano 2025-03-07 11:17 ` Krzysztof Kozlowski @ 2025-03-08 4:23 ` stephen eta zhou 1 sibling, 0 replies; 7+ messages in thread From: stephen eta zhou @ 2025-03-08 4:23 UTC (permalink / raw) To: Daniel Lezcano; +Cc: tglx, linux-kernel > From: Daniel Lezcano <daniel.lezcano@linaro.org> > Sent: Friday, March 7, 2025 16:10 > To: stephen eta zhou <stephen.eta.zhou@outlook.com> > Cc: tglx@linutronix.de <tglx@linutronix.de>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> > Subject: Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered > Hi Stephen, > thanks for the proposed fix > On 21/02/2025 04:46, stephen eta zhou wrote: > > Hi daniel > > > > While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. > It is better to put that information in the changelog and provide a > fixed format of the patch description. > > From 9dd9b5bd7ab1638990176f7171417c83ddb7a221 Mon Sep 17 00:00:00 2001 > > From: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > > Date: Fri, 21 Feb 2025 11:15:40 +0800 > > Subject: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when > > clock source is not registered > > > > Fix read_current_timer() on ARM32 by adding support in the SP804 driver. > > > > Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > > --- > > drivers/clocksource/timer-sp804.c | 36 +++++++++++++++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > > > diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c > > index cd1916c05325..b98a14d24874 100644 > > --- a/drivers/clocksource/timer-sp804.c > > +++ b/drivers/clocksource/timer-sp804.c > > @@ -21,6 +21,11 @@ > > #include <linux/of_irq.h> > > #include <linux/sched_clock.h> > > > > +#ifdef CONFIG_ARM > > +#include <linux/delay.h> > > +#include "timer-of.h" > > +#endif > > + > > #include "timer-sp.h" > > > > /* Hisilicon 64-bit timer(a variant of ARM SP804) */ > > @@ -59,6 +64,10 @@ static struct sp804_timer hisi_sp804_timer __initdata = { > > > > static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; > > > > +#ifdef CONFIG_ARM > > + struct delay_timer delay; > static ... > > +#endif > > + > > > > static long __init sp804_get_clock_rate(struct clk *clk, const char *name) > > { > > int err; > > @@ -102,6 +111,13 @@ static u64 notrace sp804_read(void) > > return ~readl_relaxed(sched_clkevt->value); > > } > > > > +#ifdef CONFIG_ARM > > +static unsigned long sp804_read_delay_timer_read(void) > > +{ > > + return sp804_read(); > > +} > > +#endif > Group this function with the global delay variable above. > > static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, > > const char *name, > > struct clk *clk, > > @@ -259,6 +275,10 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > > struct clk *clk1, *clk2; > > const char *name = of_get_property(np, "compatible", NULL); > > > > +#ifdef CONFIG_ARM > > + struct timer_of to = { .flags = TIMER_OF_CLOCK }; > > +#endif > > + > > if (initialized) { > > pr_debug("%pOF: skipping further SP804 timer device\n", np); > > return 0; > > @@ -318,6 +338,22 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > > if (ret) > > goto err; > > } > > + > > +#ifdef CONFIG_ARM > > + ret = timer_of_init(np, &to); > The clock is already retrieved from the initialization code before > > + if (ret) { > > + pr_err("Failed to initialize the Timer device tree: %d\n", ret); > > + return ret; > > + } > > + > > + delay.read_current_timer = sp804_read_delay_timer_read; > > + delay.freq = timer_of_rate(&to); > > + if (delay.freq <= 0) > > + pr_warn("Failed to obtain the freq of the clock source: %d\n", ret); > > + > > + register_current_timer_delay(&delay); > > +#endif> initialized = true; > > > > return 0; Hi Daniel, Thank you for your feedback. I will submit a v2 patch containing these fixes in the next few days. Thanks Stephen ________________________________________ From: Daniel Lezcano <daniel.lezcano@linaro.org> Sent: Friday, March 7, 2025 16:10 To: stephen eta zhou <stephen.eta.zhou@outlook.com> Cc: tglx@linutronix.de <tglx@linutronix.de>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org> Subject: Re: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered Hi Stephen, thanks for the proposed fix On 21/02/2025 04:46, stephen eta zhou wrote: > Hi daniel > > While debugging on the vexpress-v2p-ca9 platform, I discovered that the read_current_timer API wasn't functioning correctly. The issue was that the SP804 driver lacked ARM32 support and did not register read_current_timer. To add ARM32 compatibility, I’ve submitted this patch. Without it, using SP804 as the timer on ARM32 causes issues with boot_init_stack_canary when inserting the canary value into the interrupt stack, and also affects entropy generation and collection, resulting in incorrect rdseed values. It is better to put that information in the changelog and provide a fixed format of the patch description. > From 9dd9b5bd7ab1638990176f7171417c83ddb7a221 Mon Sep 17 00:00:00 2001 > From: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > Date: Fri, 21 Feb 2025 11:15:40 +0800 > Subject: [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when > clock source is not registered > > Fix read_current_timer() on ARM32 by adding support in the SP804 driver. > > Signed-off-by: Stephen Eta Zhou <stephen.eta.zhou@outlook.com> > --- > drivers/clocksource/timer-sp804.c | 36 +++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/drivers/clocksource/timer-sp804.c b/drivers/clocksource/timer-sp804.c > index cd1916c05325..b98a14d24874 100644 > --- a/drivers/clocksource/timer-sp804.c > +++ b/drivers/clocksource/timer-sp804.c > @@ -21,6 +21,11 @@ > #include <linux/of_irq.h> > #include <linux/sched_clock.h> > > +#ifdef CONFIG_ARM > +#include <linux/delay.h> > +#include "timer-of.h" > +#endif > + > #include "timer-sp.h" > > /* Hisilicon 64-bit timer(a variant of ARM SP804) */ > @@ -59,6 +64,10 @@ static struct sp804_timer hisi_sp804_timer __initdata = { > > static struct sp804_clkevt sp804_clkevt[NR_TIMERS]; > > +#ifdef CONFIG_ARM > + struct delay_timer delay; static ... > +#endif > + > > static long __init sp804_get_clock_rate(struct clk *clk, const char *name) > { > int err; > @@ -102,6 +111,13 @@ static u64 notrace sp804_read(void) > return ~readl_relaxed(sched_clkevt->value); > } > > +#ifdef CONFIG_ARM > +static unsigned long sp804_read_delay_timer_read(void) > +{ > + return sp804_read(); > +} > +#endif Group this function with the global delay variable above. > static int __init sp804_clocksource_and_sched_clock_init(void __iomem *base, > const char *name, > struct clk *clk, > @@ -259,6 +275,10 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > struct clk *clk1, *clk2; > const char *name = of_get_property(np, "compatible", NULL); > > +#ifdef CONFIG_ARM > + struct timer_of to = { .flags = TIMER_OF_CLOCK }; > +#endif > + > if (initialized) { > pr_debug("%pOF: skipping further SP804 timer device\n", np); > return 0; > @@ -318,6 +338,22 @@ static int __init sp804_of_init(struct device_node *np, struct sp804_timer *time > if (ret) > goto err; > } > + > +#ifdef CONFIG_ARM > + ret = timer_of_init(np, &to); The clock is already retrieved from the initialization code before > + if (ret) { > + pr_err("Failed to initialize the Timer device tree: %d\n", ret); > + return ret; > + } > + > + delay.read_current_timer = sp804_read_delay_timer_read; > + delay.freq = timer_of_rate(&to); > + if (delay.freq <= 0) > + pr_warn("Failed to obtain the freq of the clock source: %d\n", ret); > + > + register_current_timer_delay(&delay); > +#endif> initialized = true; > > return 0; -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-03-08 4:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-02-21 3:46 [PATCH] clocksource: timer-sp804: Fix read_current_timer() issue when clock source is not registered stephen eta zhou 2025-03-07 8:10 ` Daniel Lezcano 2025-03-07 11:17 ` Krzysztof Kozlowski 2025-03-07 13:41 ` Daniel Lezcano 2025-03-08 3:15 ` stephen eta zhou 2025-03-08 2:32 ` stephen eta zhou 2025-03-08 4:23 ` stephen eta zhou
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®