[PATCH 1/1] time/sched_clock: move sched_clock_register() out of .init section

[PATCH 1/1] time/sched_clock: move sched_clock_register() out of .init section

[Maxim Kochetkov]

The sched_clock_register() is widely used by clocksource timer
drivers. The __init prefix forces them to be initialized using
macro TIMER_OF_DECLARE with __init prefixed function.

Clocksource devices can be consumers of some external irq, clocks,
resets, e.t.c. Such devices can't be correctly probed if this
dependencies are provided by platform drivers. Because of regular
platform devices are not probed at this moment.

We can convert clocksource drivers to platform device drivers to
fix this issue, but __init prefix in sched_clock_register()
prevents it.

So lets drop __init prefix to allow platform device drivers to use
sched_clock_register().

Signed-off-by: Maxim Kochetkov <fido_max@inbox.ru>
---
 kernel/time/sched_clock.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/kernel/time/sched_clock.c b/kernel/time/sched_clock.c
index cc15fe293719..07f28e0e2716 100644
--- a/kernel/time/sched_clock.c
+++ b/kernel/time/sched_clock.c
@@ -174,8 +174,7 @@ static enum hrtimer_restart sched_clock_poll(struct hrtimer *hrt)
 	return HRTIMER_RESTART;
 }
 
-void __init
-sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
+void sched_clock_register(u64 (*read)(void), int bits, unsigned long rate)
 {
 	u64 res, wrap, new_mask, new_epoch, cyc, ns;
 	u32 new_mult, new_shift;
-- 
2.48.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/1] time/sched_clock: move sched_clock_register() out of .init section

[Thomas Gleixner]

On Fri, Apr 04 2025 at 08:05, Maxim Kochetkov wrote:
> The sched_clock_register() is widely used by clocksource timer
> drivers. The __init prefix forces them to be initialized using
> macro TIMER_OF_DECLARE with __init prefixed function.

No, it does not. It requires that they are built in, not more.

> Clocksource devices can be consumers of some external irq, clocks,
> resets, e.t.c. Such devices can't be correctly probed if this
> dependencies are provided by platform drivers. Because of regular
> platform devices are not probed at this moment.
>
> We can convert clocksource drivers to platform device drivers to
> fix this issue, but __init prefix in sched_clock_register()
> prevents it.

Again. It does not. What the __init prefix prevents is that the driver
can be built as a module and loaded late.

> So lets drop __init prefix to allow platform device drivers to use
> sched_clock_register().

s/So let's//

"So let's" means nothing.

Also this has nothing to do with platform device drivers. It's all about
modules and nothing else.

Thanks,

        tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/1] time/sched_clock: move sched_clock_register() out of .init section

[Maxim Kochetkov]

07.04.2025 09:26, Thomas Gleixner wrote:
> On Fri, Apr 04 2025 at 08:05, Maxim Kochetkov wrote:
>> The sched_clock_register() is widely used by clocksource timer
>> drivers. The __init prefix forces them to be initialized using
>> macro TIMER_OF_DECLARE with __init prefixed function.
> 
> No, it does not. It requires that they are built in, not more.

Thank you for review.

Let me explain some more. I'm trying to solve similar problem, as 
described at 
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240312192519.1602493-1-samuel.holland@sifive.com/#25759271

I have both PLIC and clocksource module configured as Y (not m) in 
Kconfig. So both of them are included in kernel Image binary. But I 
still unable to probe clocksource device because it depends of PLIC irq.
And PLIC probes much later than TIMER_OF_DECLARE part of the clocksource 
driver. I tried to convert clocksource driver to regular platform device 
and it works fine except warning:

WARNING: modpost: vmlinux: section mismatch in reference: 
dw_apb_timer_probe+0x136 (section: .text.unlikely) -> 
sched_clock_register (section: .init.text)

Dropping __init from sched_clock_register() helps to solve this issue.

Is there any real point to keep __init in sched_clock_register()? I see 
no issues to call this function at any time later after kernel boot.

Is there better way to solve this issue?

>> So lets drop __init prefix to allow platform device drivers to use
>> sched_clock_register().
> 
> s/So let's//
> 
> "So let's" means nothing.
> 
> Also this has nothing to do with platform device drivers. It's all about
> modules and nothing else.

Anyway, this patch opens opportunity to compile clocksource drivers as 
modules and probe them much later.

Thanks.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 1/1] time/sched_clock: move sched_clock_register() out of .init section

[Thomas Gleixner]

On Mon, Apr 07 2025 at 10:25, Maxim Kochetkov wrote:
> 07.04.2025 09:26, Thomas Gleixner wrote:
>> On Fri, Apr 04 2025 at 08:05, Maxim Kochetkov wrote:
>>> The sched_clock_register() is widely used by clocksource timer
>>> drivers. The __init prefix forces them to be initialized using
>>> macro TIMER_OF_DECLARE with __init prefixed function.
>> 
>> No, it does not. It requires that they are built in, not more.
>
> Thank you for review.
>
> Let me explain some more. I'm trying to solve similar problem, as 
> described at 
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20240312192519.1602493-1-samuel.holland@sifive.com/#25759271
>
> I have both PLIC and clocksource module configured as Y (not m) in 
> Kconfig. So both of them are included in kernel Image binary. But I 
> still unable to probe clocksource device because it depends of PLIC irq.
> And PLIC probes much later than TIMER_OF_DECLARE part of the clocksource 
> driver.

Which is not a problem as all built-in drivers probe _before_ the init
section is discarded.

> I tried to convert clocksource driver to regular platform device and
> it works fine except warning:
>
> WARNING: modpost: vmlinux: section mismatch in reference: 
> dw_apb_timer_probe+0x136 (section: .text.unlikely) -> 
> sched_clock_register (section: .init.text)

Of course. The warning is because you invoke sched_clock_register()
from dw_apb_timer_probe(), which is regular text. See

drivers/clocksource/ingenic-ost.c
drivers/clocksource/timer-cadence-ttc.c

how to implement a builtin platform driver, which does not suffer from
that problem despite invoking sched_clock_register() from their init
functions.

> Dropping __init from sched_clock_register() helps to solve this issue.

It solves it at the wrong point for a builtin platform driver

> Anyway, this patch opens opportunity to compile clocksource drivers as 
> modules and probe them much later.

That's an orthogonal issue and needs to be discussed seperately from the
problem at hand.

Thanks,

        tglx

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv