linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: William McVicker <willmcvicker@google.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: "Catalin Marinas" <catalin.marinas@arm.com>,
	"Will Deacon" <will@kernel.org>,
	"Daniel Lezcano" <daniel.lezcano@linaro.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Krzysztof Kozlowski" <krzk@kernel.org>,
	"Alim Akhtar" <alim.akhtar@samsung.com>,
	"Donghoon Yu" <hoony.yu@samsung.com>,
	"Hosung Kim" <hosung0.kim@samsung.com>,
	"Rob Herring" <robh@kernel.org>,
	"John Stultz" <jstultz@google.com>,
	"Youngmin Nam" <youngmin.nam@samsung.com>,
	"Peter Griffin" <peter.griffin@linaro.org>,
	"Tudor Ambarus" <tudor.ambarus@linaro.org>,
	"André Draszik" <andre.draszik@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Marek Szyprowski" <m.szyprowski@samsung.com>,
	linux-samsung-soc@vger.kernel.org, kernel-team@android.com,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init
Date: Fri, 21 Nov 2025 17:51:58 +0000	[thread overview]
Message-ID: <aSCmvsOc13Z1TS6y@google.com> (raw)
In-Reply-To: <aSB_bUbfjenD1fjZ@shell.armlinux.org.uk>

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


  reply	other threads:[~2025-11-21 17:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20251120184253eucas1p17d9dea24d56e4eefc283a738a94f2634@eucas1p1.samsung.com>
2025-11-20 18:42 ` [PATCH v6 0/6] Add module support for Arm64 Exynos MCT driver Will McVicker
2025-11-20 18:42   ` [PATCH v6 1/6] ARM: make register_current_timer_delay() accessible after init Will McVicker
2025-11-21 14:37     ` Daniel Lezcano
2025-11-21 15:04     ` Russell King (Oracle)
2025-11-21 17:51       ` William McVicker [this message]
2025-11-20 18:42   ` [PATCH v6 2/6] clocksource/drivers/exynos_mct: Don't register as a sched_clock on arm64 Will McVicker
2025-11-20 18:42   ` [PATCH v6 3/6] clocksource/drivers/exynos_mct: Set percpu local timer interrupts for ARM64 Will McVicker
2025-11-20 18:42   ` [PATCH v6 4/6] clocksource/drivers/exynos_mct: Fix uninitialized irq name warning Will McVicker
2025-11-20 18:42   ` [PATCH v6 5/6] clocksource/drivers/exynos_mct: Add module support for ARM64 Will McVicker
2025-11-20 18:42   ` [PATCH v6 6/6] arm64: exynos: Drop select CLKSRC_EXYNOS_MCT Will McVicker
2025-11-21 12:16   ` [PATCH v6 0/6] Add module support for Arm64 Exynos MCT driver Marek Szyprowski
2025-11-21 15:11   ` Russell King (Oracle)
2025-11-21 17:55     ` William McVicker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aSCmvsOc13Z1TS6y@google.com \
    --to=willmcvicker@google.com \
    --cc=alim.akhtar@samsung.com \
    --cc=andre.draszik@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=conor+dt@kernel.org \
    --cc=daniel.lezcano@linaro.org \
    --cc=hoony.yu@samsung.com \
    --cc=hosung0.kim@samsung.com \
    --cc=jstultz@google.com \
    --cc=kernel-team@android.com \
    --cc=krzk@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=m.szyprowski@samsung.com \
    --cc=peter.griffin@linaro.org \
    --cc=robh@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tudor.ambarus@linaro.org \
    --cc=will@kernel.org \
    --cc=youngmin.nam@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).