RE: [EXT] [PATCH v1 resend 0/7] Timer driver module support

[Zhipeng Wang]

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@xxxxxxxxxx>
> Sent: 2026年3月28日 2:06
> To: daniel.lezcano@xxxxxxxxxx; tglx@xxxxxxxxxx; Zhipeng Wang
> <zhipeng.wang_1@xxxxxxx>
> Cc: shawnguo@xxxxxxxxxx; jstultz@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> Matthias Brugger <matthias.bgg@xxxxxxxxx>; AngeloGioacchino Del Regno
> <angelogioacchino.delregno@xxxxxxxxxxxxx>; moderated list:ARM/Mediatek
> SoC support <linux-arm-kernel@xxxxxxxxxxxxxxxxxxx>; moderated
> list:ARM/Mediatek SoC support <linux-mediatek@xxxxxxxxxxxxxxxxxxx>
> Subject: [EXT] [PATCH v1 resend 0/7] Timer driver module support
> 
> [You don't often get email from daniel.lezcano@xxxxxxxxxx. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> Converting the timer driver modules requires a particular care because,
> depending on the platform, that may be not supported.
> 
> A previous study showed we are safe regarding how the module refcount is
> held and if THIS_MODULE is set for the clockevent and the clocksource when
> they are registered.
> 
> It won't be possible to unload a module if a clockevent is registered.
> 
> It will be possible to unload a module if only a clocksource is registered and it
> is not the current one.
> 
> However platforms without architected timers may need the timer driver to be
> initialized very early and others can be initialized later. The former can not be a
> module and the init function receives a device_node pointer, there is no device
> associated and devres is not used. That results in a lot of rollbacking code
> where usually it is where we find bug and resource leaks. The latter can be
> converted to a module and uses a module_platform_driver(), thus the init
> function is a probe function receiving a struct platform_device pointer
> parameter.
> 
> We end up with two approaches and duplicate code for the init functions. This
> is not optimal.
> 
> Finally, we have the driver having to be initialized very early on some platforms
> and be built as a module on other platforms, resulting on having two init
> functions co-existing in the same driver.
> 
> This series provides what is needed to move to the same probe function for
> early init, builtin and module timers.
> 
> A new macro is introduced: TIMER_PDEV_DECLARE() and a new Kconfig option
> is added CONFIG_EARLY_TIMER. TIMER_PDEV_DECLARE() will have different
> behavior depending on the context:
> 
>  - The driver is a module and CONFIG_EARLY_TIMER=no
>    --> the driver is a module
> 
>  - The driver is builtin and CONFIG_EARLY_TIMER=no
>    --> the driver is loaded later
> 
>  - The driver is builtin or a module but CONFIG_EARLY_TIMER=yes
>    --> the driver is initialized through the timer-probe function
> 
> The different timer driver framework functions have their __init sections
> removed and the symbols exported in order to be compatible with the drivers
> converted into modules.
> 
> The series provides a couple of drivers changed. The Mediatek as a recent
> requested target which is only compiled-tested. The Rockchip timer which was
> tested on a rk3588 in the three different configurations.
> 
> Daniel Lezcano (7):
>   clocksource/drivers/timer-probe: Create a platform_device before the
>     framework is initialized
>   drivers/clocksource/rockchip: Use the TIMER_PDEV_DECLARE() macro
>   clocksource/drivers/mmio: Make the code compatible with modules
>   clocksource/drivers/timer-of: Make the code compatible with modules
>   clocksource/drivers/timer-probe: Add the module support for the
>     TIMER_PDEV_DECLARE() macro
>   clocksource/drivers/rockchip: Add rockchip timer module support
>   clocksource/drivers/mediatek: Convert to module support
> 
>  drivers/clocksource/Kconfig          |   7 +-
>  drivers/clocksource/mmio.c           |  11 ++-
>  drivers/clocksource/timer-mediatek.c |  29 ++++++--
>  drivers/clocksource/timer-of.c       |  24 ++++---
>  drivers/clocksource/timer-of.h       |   5 +-
>  drivers/clocksource/timer-probe.c    |  69 ++++++++++++++++--
>  drivers/clocksource/timer-rockchip.c | 101 ++++++++++-----------------
>  include/asm-generic/vmlinux.lds.h    |  10 +++
>  include/linux/clocksource.h          |  31 ++++++++
>  9 files changed, 194 insertions(+), 93 deletions(-)
> 
> --
> 2.43.0
Hi Daniel,

Apologies for the delayed response. Thank you for working on the timer driver module support series. I've 
tested your patches on i.MX platforms and they work well with the TPM 
timer driver.

Based on your framework, I've converted the i.MX TPM timer driver to 
use TIMER_PDEV_DECLARE. The driver has been tested on:
- i.MX7ULP (ARM, no alternative timer): built-in with CONFIG_EARLY_TIMER=y
- i.MX8ULP (ARM64, has ARM Generic Timer): built-in, module, and 
  CONFIG_EARLY_TIMER=y configurations

All test cases passed without issues.

I'm attaching the patch below:

Author: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>
Date:   Thu Apr 2 15:46:53 2026 +0900

    clocksource/drivers/imx-tpm: Convert to TIMER_PDEV_DECLARE and improve resource management

    Convert the i.MX TPM timer driver from TIMER_OF_DECLARE to
    TIMER_PDEV_DECLARE to support both early initialization and
    platform device probing, aligning with the timer driver
    modernization effort.

    This driver is used on two different platforms with the same
    hardware:
    - ARM platforms (i.MX7ULP) without alternative timers require
      early initialization for delay_timer and sched_clock
    - ARM64 platforms (i.MX8ULP) with ARM Generic Timer can use
      TPM as a backup timer with deferred initialization

    Key changes:
    1. Replace TIMER_OF_DECLARE with TIMER_PDEV_DECLARE to enable
       flexible initialization based on platform requirements

    2. Convert tpm_timer_init() to tpm_timer_probe() with platform
       device parameter for proper device model integration

    3. Use devm_clk_get_enabled() for ipg clock management instead
       of manual clk_prepare_enable/clk_put, fixing potential
       resource leaks in error paths

    4. Remove __init annotations from tpm_clocksource_init() and
       tpm_clockevent_init() to support deferred probing

    5. Add proper of_device_id table for device matching

    Tested on i.MX7ULP (ARM) and i.MX8ULP (ARM64) platforms.

    Signed-off-by: Zhipeng Wang <zhipeng.wang_1@xxxxxxx>

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 00a5c3a682de9..6e753141b3628 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -618,8 +618,9 @@ config CLKSRC_IMX_GPT
        select CLKSRC_MMIO

 config CLKSRC_IMX_TPM
-       bool "Clocksource using i.MX TPM" if COMPILE_TEST
+       tristate "Clocksource using i.MX TPM"
        depends on (ARM || ARM64) && HAVE_CLK
+       default ARCH_MXC
        select CLKSRC_MMIO
        select TIMER_OF
        help
diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index 92c025b70eb62..13ebb6b627a5b 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -8,6 +8,8 @@
 #include <linux/clocksource.h>
 #include <linux/delay.h>
 #include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/sched_clock.h>

 #include "timer-of.h"
@@ -152,7 +154,7 @@ static struct timer_of to_tpm = {
        },
 };

-static int __init tpm_clocksource_init(void)
+static int tpm_clocksource_init(void)
 {
 #if defined(CONFIG_ARM)
        tpm_delay_timer.read_current_timer = &tpm_read_current_timer;
@@ -171,7 +173,7 @@ static int __init tpm_clocksource_init(void)
                                     clocksource_mmio_readl_up);
 }

-static void __init tpm_clockevent_init(void)
+static void tpm_clockevent_init(void)
 {
        clockevents_config_and_register(&to_tpm.clkevt,
                                        timer_of_rate(&to_tpm) >> 3,
@@ -180,23 +182,21 @@ static void __init tpm_clockevent_init(void)
                                        1));
 }

-static int __init tpm_timer_init(struct device_node *np)
+static int tpm_timer_probe(struct platform_device *pdev)
 {
+       struct device *dev = &pdev->dev;
+       struct device_node *np = dev->of_node;
        struct clk *ipg;
        int ret;

-       ipg = of_clk_get_by_name(np, "ipg");
-       if (IS_ERR(ipg)) {
-               pr_err("tpm: failed to get ipg clk\n");
-               return -ENODEV;
-       }
-       /* enable clk before accessing registers */
-       ret = clk_prepare_enable(ipg);
-       if (ret) {
-               pr_err("tpm: ipg clock enable failed (%d)\n", ret);
-               clk_put(ipg);
-               return ret;
-       }
+       /*
+        * Get and enable ipg clock before accessing registers.
+        * Use devm variant to ensure automatic cleanup on error paths.
+        */
+       ipg = devm_clk_get_enabled(dev, "ipg");
+       if (IS_ERR(ipg))
+               return dev_err_probe(dev, PTR_ERR(ipg),
+                                    "failed to get ipg clock\n");

        ret = timer_of_init(np, &to_tpm);
        if (ret)
@@ -241,4 +241,13 @@ static int __init tpm_timer_init(struct device_node *np)

        return tpm_clocksource_init();
 }
-TIMER_OF_DECLARE(imx7ulp, "fsl,imx7ulp-tpm", tpm_timer_init);
+
+static const struct of_device_id imx_tpm_dt_ids[] = {
+       { .compatible = "fsl,imx7ulp-tpm", },
+       { /* sentinel */ }
+};
+
+TIMER_PDEV_DECLARE(imx_tpm, tpm_timer_probe, NULL, imx_tpm_dt_ids);
+
+MODULE_DESCRIPTION("i.MX TPM Timer Driver");
+MODULE_LICENSE("GPL");


BRs,
Zhipeng