[PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init

Will McVicker posted 6 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init
Posted by Will McVicker 2 months, 2 weeks ago
The function register_current_timer_delay() is called from the
exynos_mct clocksource driver at probe time. In the event that the
exynos_mct driver is probed deferred or the platform manually unbinds
and rebinds the driver we need this function available. So drop the
__init tag.

Signed-off-by: Will McVicker <willmcvicker@google.com>
---
 arch/arm/lib/delay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/lib/delay.c b/arch/arm/lib/delay.c
index b7fe84f68bf1..acfb87143f21 100644
--- a/arch/arm/lib/delay.c
+++ b/arch/arm/lib/delay.c
@@ -62,7 +62,7 @@ static void __timer_udelay(unsigned long usecs)
 	__timer_const_udelay(usecs * UDELAY_MULT);
 }
 
-void __init register_current_timer_delay(const struct delay_timer *timer)
+void register_current_timer_delay(const struct delay_timer *timer)
 {
 	u32 new_mult, new_shift;
 	u64 res;
-- 
2.52.0.rc2.455.g230fcf2819-goog
Re: [PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init
Posted by Russell King (Oracle) 2 months, 2 weeks ago
On Thu, Nov 20, 2025 at 06:42:29PM +0000, Will McVicker wrote:
> The function register_current_timer_delay() is called from the
> exynos_mct clocksource driver at probe time. In the event that the
> exynos_mct driver is probed deferred or the platform manually unbinds
> and rebinds the driver we need this function available. So drop the
> __init tag.

First question. Why.

Second, have you analysed the code to check that you _can_ call this
after init time?

Let's look at the code:

        if (!delay_calibrated && (!delay_res || (res < delay_res))) {
        } else {
                pr_info("Ignoring duplicate/late registration of read_current_ti
mer delay\n");
        }

So, if delay_calibrated is set, then this will fail. When is that set?
It's set by calibrate_delay_is_known() and calibration_delay_done().
When are these called? Basically after calibrate_delay() has finished.
When is calibrate_delay() called? It's called by start_kernel(), while
the init sections are still present.

So, calling this _after_ the init sections has been freed will result
in the above pr_info() printed and this function doing *nothing*.
So it's utterly pointless to call if the init sections have been freed.

Please find a different solution.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init
Posted by William McVicker 2 months, 2 weeks ago
Hi Daniel and Russell,

On 11/21/2025, Russell King (Oracle) wrote:
> On Thu, Nov 20, 2025 at 06:42:29PM +0000, Will McVicker wrote:
> > The function register_current_timer_delay() is called from the
> > exynos_mct clocksource driver at probe time. In the event that the
> > exynos_mct driver is probed deferred or the platform manually unbinds
> > and rebinds the driver we need this function available. So drop the
> > __init tag.
> 
> First question. Why.

Sorry for not explaining this very well. Let me fill in the gaps.

> 
> Second, have you analysed the code to check that you _can_ call this
> after init time?
> 
> Let's look at the code:
> 
>         if (!delay_calibrated && (!delay_res || (res < delay_res))) {
>         } else {
>                 pr_info("Ignoring duplicate/late registration of read_current_ti
> mer delay\n");
>         }
> 
> So, if delay_calibrated is set, then this will fail. When is that set?
> It's set by calibrate_delay_is_known() and calibration_delay_done().
> When are these called? Basically after calibrate_delay() has finished.
> When is calibrate_delay() called? It's called by start_kernel(), while
> the init sections are still present.
> 
> So, calling this _after_ the init sections has been freed will result
> in the above pr_info() printed and this function doing *nothing*.
> So it's utterly pointless to call if the init sections have been freed.
> 
> Please find a different solution.

Sorry for wasting your time digging into this! You're right we shouldn't (and
don't) call register_current_timer_delay() after the init sections are freed.
This change was made purely to address the section mismatch compile-time
errors. The Exynos MCT driver cannot be compiled as a module for ARM 32-bit
devices; however, since ARM64 and ARM devices both call the function
exynos4_clocksource_init() and ARM64 configurations can set the MCT driver as
a module, we hit a section mismatch error due to calling an __init tagged
function -- register_current_timer_delay() -- from a non-init tagged function
-- exynos4_clocksource_init(). If you inspect the code, you will see that
register_current_timer_delay() is compiled out of the driver for ARM64. To
avoid hacking up the MCT driver with more `#if defined(CONFIG_ARM)` to handle
this section mismatch, I decided it was cleaner to drop the __init tag.

I'd be happy to re-factor the MCT driver to split up the ARM and ARM64 parts of
exynos4_clocksource_init() to avoid the section mismatch altogher in order to
keep the __init tag for register_current_timer_delay(). Given the comments
here, I think that makes sense.

I hope that explains it. Let me know if I overlooked anything.

Thanks,
Will
Re: [PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init
Posted by Daniel Lezcano 2 months, 2 weeks ago
On 11/20/25 19:42, Will McVicker wrote:
> The function register_current_timer_delay() is called from the
> exynos_mct clocksource driver at probe time. In the event that the
> exynos_mct driver is probed deferred or the platform manually unbinds
> and rebinds the driver we need this function available. So drop the
> __init tag.

Is this change supposed to fix an existing bug or is it a guess it could 
happen ? If the latter, I prefer to not pick this patch until is proven 
it fixes an observed issue.



-- 
<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