[PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Daniel Lezcano]

In the context of the time keeping and the timers, some platforms have
timers which need to be initialized very early. It is the case of the
ARM platform which do not have the architected timers.

The macro TIMER_OF_DECLARE adds an entry in the timer init functions
array at compile time and the function timer_probe is called from the
timer_init() function in kernel/time.c

This array contains a t-uple with the init function and the compatible
string.

The init function has a device node pointer parameter.

The timer_probe() function browses the of nodes and find the ones
matching the compatible string given when using the TIMER_OF_DECLARE
macro. It then calls the init function with the device node as a
pointer.

But there are some platforms where there are multiple timers like the
ARM64 with the architected timers. Those are always initialized very
early and the other timers can be initialized later.

For this reason we find timer drivers with the platform_driver
incarnation. Consequently their init functions are different, they
have a platform_device pointer parameter and rely on the devm_
function for rollbacking.

To summarize, we have:
 - TIMER_OF_DECLARE with init function prototype:
   int (*init)(struct device_node *np);

 - module_platform_driver (and variant) with the probe function
   prototype:
   int (*init)(struct platform_device *pdev);

The current situation with the timers is the following:

 - Two platforms can have the same timer hardware, hence the same
   driver but one without alternate timers and the other with multiple
   timers. For example, the Exynos platform has only the Exynos MCT on
   ARM but has the architeched timers in addition on the ARM64.

 - The timer drivers can be modules now which was not the case until
   recently. TIMER_OF_DECLARE do not allow the build as a module.

It results in duplicate init functions (one with rollback and one with
devm_) and different way to declare the driver (TIMER_OF_DECLARE and
module_platform_driver).

This proposed change is to unify the prototyping of the init functions
to receive a platform_device pointer as parameter. Consequently, it
will allow a smoother and nicer module conversion and a huge cleanup
of the init functions by removing all the rollback code from all the
timer drivers. It introduces a TIMER_OF_DECLARE_PDEV macro.

If the macro is used a platform_device is manually allocated and
initialized with the needed information for the probe
function. Otherwise module_platform_driver can be use instead with the
same probe function without the timer_probe() initialization.

I don't have an expert knowledge of the platform_device internal
subtilitie so I'm not sure if this approach is valid. However, it has
been tested on a Rockchip board with the "rockchip,rk3288-timer" and
verified the macro and the devm_ rollback work correctly.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Hans de Goede <hansg@kernel.org>
Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/clocksource/timer-probe.c | 61 ++++++++++++++++++++++++++++++-
 include/asm-generic/vmlinux.lds.h |  2 +
 include/linux/clocksource.h       |  3 ++
 include/linux/of.h                |  5 +++
 4 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-probe.c b/drivers/clocksource/timer-probe.c
index b7860bc0db4b..6b2b341b8c95 100644
--- a/drivers/clocksource/timer-probe.c
+++ b/drivers/clocksource/timer-probe.c
@@ -7,13 +7,18 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/clocksource.h>
+#include <linux/platform_device.h>
 
 extern struct of_device_id __timer_of_table[];
+extern struct of_device_id __timer_pdev_of_table[];
 
 static const struct of_device_id __timer_of_table_sentinel
 	__used __section("__timer_of_table_end");
 
-void __init timer_probe(void)
+static const struct of_device_id __timer_pdev_of_table_sentinel
+	__used __section("__timer_pdev_of_table_end");
+
+static int __init timer_of_probe(void)
 {
 	struct device_node *np;
 	const struct of_device_id *match;
@@ -38,6 +43,60 @@ void __init timer_probe(void)
 		timers++;
 	}
 
+	return timers;
+}
+
+static int __init timer_pdev_of_probe(void)
+{
+	struct device_node *np;
+	struct platform_device *pdev;
+	const struct of_device_id *match;
+	of_init_fn_pdev init_func;
+	unsigned int timers = 0;
+	int ret;
+
+	for_each_matching_node_and_match(np, __timer_pdev_of_table, &match) {
+		if (!of_device_is_available(np))
+			continue;
+
+		init_func = match->data;
+
+		pdev = platform_device_alloc(of_node_full_name(np), -1);
+		if (!pdev)
+			continue;
+
+		ret = device_add_of_node(&pdev->dev, np);
+		if (ret) {
+			platform_device_put(pdev);
+			continue;
+		}
+
+		dev_set_name(&pdev->dev, pdev->name);
+
+		ret = init_func(pdev);
+		if (!ret) {
+			timers++;
+			continue;
+		}
+
+		if (ret != -EPROBE_DEFER)
+			pr_err("Failed to initialize '%pOF': %d\n", np,
+			       ret);
+
+		device_remove_of_node(&pdev->dev);
+
+		platform_device_put(pdev);
+	}
+
+	return timers;
+}
+
+void __init timer_probe(void)
+{
+	unsigned timers = 0;
+
+	timers += timer_of_probe();
+	timers += timer_pdev_of_probe();
 	timers += acpi_probe_device_table(timer);
 
 	if (!timers)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index fa5f19b8d53a..97606499c8d7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -318,6 +318,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 	KEEP(*(__##name##_of_table_end))
 
 #define TIMER_OF_TABLES()	OF_TABLE(CONFIG_TIMER_OF, timer)
+#define TIMER_PDEV_OF_TABLES()	OF_TABLE(CONFIG_TIMER_OF, timer_pdev)
 #define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
 #define CLK_OF_TABLES()		OF_TABLE(CONFIG_COMMON_CLK, clk)
 #define RESERVEDMEM_OF_TABLES()	OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
@@ -714,6 +715,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 	CLK_OF_TABLES()							\
 	RESERVEDMEM_OF_TABLES()						\
 	TIMER_OF_TABLES()						\
+	TIMER_PDEV_OF_TABLES()						\
 	CPU_METHOD_OF_TABLES()						\
 	CPUIDLE_METHOD_OF_TABLES()					\
 	KERNEL_DTB()							\
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 65b7c41471c3..0eeabd207040 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -289,6 +289,9 @@ extern int clocksource_i8253_init(void);
 #define TIMER_OF_DECLARE(name, compat, fn) \
 	OF_DECLARE_1_RET(timer, name, compat, fn)
 
+#define TIMER_OF_DECLARE_PDEV(name, compat, fn) \
+	OF_DECLARE_PDEV(timer_pdev, name, compat, fn)
+
 #ifdef CONFIG_TIMER_PROBE
 extern void timer_probe(void);
 #else
diff --git a/include/linux/of.h b/include/linux/of.h
index a62154aeda1b..a312a6f5ecc1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1540,9 +1540,12 @@ static inline int of_get_available_child_count(const struct device_node *np)
 	_OF_DECLARE_STUB(table, name, compat, fn, fn_type)
 #endif
 
+struct platform_device;
+
 typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
 typedef int (*of_init_fn_1_ret)(struct device_node *);
 typedef void (*of_init_fn_1)(struct device_node *);
+typedef int (*of_init_fn_pdev)(struct platform_device *);
 
 #define OF_DECLARE_1(table, name, compat, fn) \
 		_OF_DECLARE(table, name, compat, fn, of_init_fn_1)
@@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
 		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
 #define OF_DECLARE_2(table, name, compat, fn) \
 		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
+#define OF_DECLARE_PDEV(table, name, compat, fn) \
+		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
 
 /**
  * struct of_changeset_entry	- Holds a changeset entry
-- 
2.43.0

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Bryan O'Donoghue]

On 25/06/2025 09:57, Daniel Lezcano wrote:
> In the context of the time keeping and the timers, some platforms have
> timers which need to be initialized very early. It is the case of the
> ARM platform which do not have the architected timers.
> 
> The macro TIMER_OF_DECLARE adds an entry in the timer init functions
> array at compile time and the function timer_probe is called from the
> timer_init() function in kernel/time.c
> 
> This array contains a t-uple with the init function and the compatible
> string.

tuple

> 
> The init function has a device node pointer parameter.
> 
> The timer_probe() function browses the of nodes and find the ones
> matching the compatible string given when using the TIMER_OF_DECLARE
> macro. It then calls the init function with the device node as a
> pointer.
> 
> But there are some platforms where there are multiple timers like the

Don't start a sentence with But.

"There are some platforms", "There are platforms" or "Some platforms"

> ARM64 with the architected timers. Those are always initialized very
> early and the other timers can be initialized later.
> 
> For this reason we find timer drivers with the platform_driver
> incarnation. Consequently their init functions are different, they
> have a platform_device pointer parameter and rely on the devm_
> function for rollbacking.
> 
> To summarize, we have:
>   - TIMER_OF_DECLARE with init function prototype:
>     int (*init)(struct device_node *np);
> 
>   - module_platform_driver (and variant) with the probe function
>     prototype:
>     int (*init)(struct platform_device *pdev);
> 
> The current situation with the timers is the following:
> 
>   - Two platforms can have the same timer hardware, hence the same
>     driver but one without alternate timers and the other with multiple
>     timers. For example, the Exynos platform has only the Exynos MCT on
>     ARM but has the architeched timers in addition on the ARM64.

architected

> 
>   - The timer drivers can be modules now which was not the case until
>     recently. TIMER_OF_DECLARE do not allow the build as a module.
> 
> It results in duplicate init functions (one with rollback and one with
> devm_) and different way to declare the driver (TIMER_OF_DECLARE and
> module_platform_driver).
> 
> This proposed change is to unify the prototyping of the init functions
> to receive a platform_device pointer as parameter. Consequently, it
> will allow a smoother and nicer module conversion and a huge cleanup
> of the init functions by removing all the rollback code from all the
> timer drivers. It introduces a TIMER_OF_DECLARE_PDEV macro.

"It introduces" => "This change introduces"

I think, it would be nice to see an accompanying patch showing how this 
change achieves that IRL.

> 
> If the macro is used a platform_device is manually allocated and
> initialized with the needed information for the probe
> function. Otherwise module_platform_driver can be use instead with the
> same probe function without the timer_probe() initialization.
> 
> I don't have an expert knowledge of the platform_device internal
> subtilitie so I'm not sure if this approach is valid. However, it has
> been tested on a Rockchip board with the "rockchip,rk3288-timer" and
> verified the macro and the devm_ rollback work correctly.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Hans de Goede <hansg@kernel.org>
> Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   drivers/clocksource/timer-probe.c | 61 ++++++++++++++++++++++++++++++-
>   include/asm-generic/vmlinux.lds.h |  2 +
>   include/linux/clocksource.h       |  3 ++
>   include/linux/of.h                |  5 +++
>   4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/timer-probe.c b/drivers/clocksource/timer-probe.c
> index b7860bc0db4b..6b2b341b8c95 100644
> --- a/drivers/clocksource/timer-probe.c
> +++ b/drivers/clocksource/timer-probe.c
> @@ -7,13 +7,18 @@
>   #include <linux/init.h>
>   #include <linux/of.h>
>   #include <linux/clocksource.h>
> +#include <linux/platform_device.h>
>   
>   extern struct of_device_id __timer_of_table[];
> +extern struct of_device_id __timer_pdev_of_table[];
>   
>   static const struct of_device_id __timer_of_table_sentinel
>   	__used __section("__timer_of_table_end");
>   
> -void __init timer_probe(void)
> +static const struct of_device_id __timer_pdev_of_table_sentinel
> +	__used __section("__timer_pdev_of_table_end");
> +
> +static int __init timer_of_probe(void)
>   {
>   	struct device_node *np;
>   	const struct of_device_id *match;
> @@ -38,6 +43,60 @@ void __init timer_probe(void)
>   		timers++;
>   	}
>   
> +	return timers;
> +}
> +
> +static int __init timer_pdev_of_probe(void)
> +{
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	const struct of_device_id *match;
> +	of_init_fn_pdev init_func;
> +	unsigned int timers = 0;
> +	int ret;

Small nit.

Reverse Christmas tree the declarations.

> +
> +	for_each_matching_node_and_match(np, __timer_pdev_of_table, &match) {
> +		if (!of_device_is_available(np))
> +			continue;
> +
> +		init_func = match->data;
> +
> +		pdev = platform_device_alloc(of_node_full_name(np), -1);
> +		if (!pdev)
> +			continue;

Shouldn't this be return -ENOMEM;

> +
> +		ret = device_add_of_node(&pdev->dev, np);
> +		if (ret) {
> +			platform_device_put(pdev);
> +			continue;

Why is this a continue ? you can get -EINVAL and -EBUSY from 
device_add_of_node() - can/should you really continue this loop after an 
-EINVAL ?

Understood for architected you want to keep going and get the system up 
at the very least I'd add a noisy message about it.

---
bod

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Daniel Lezcano]

Hi Bryan,

thanks for your review

On 25/06/2025 12:20, Bryan O'Donoghue wrote:

[ ... ]

>> +
>> +    for_each_matching_node_and_match(np, __timer_pdev_of_table, 
>> &match) {
>> +        if (!of_device_is_available(np))
>> +            continue;
>> +
>> +        init_func = match->data;
>> +
>> +        pdev = platform_device_alloc(of_node_full_name(np), -1);
>> +        if (!pdev)
>> +            continue;
> 
> Shouldn't this be return -ENOMEM;
> 
>> +
>> +        ret = device_add_of_node(&pdev->dev, np);
>> +        if (ret) {
>> +            platform_device_put(pdev);
>> +            continue;
> 
> Why is this a continue ? you can get -EINVAL and -EBUSY from 
> device_add_of_node() - can/should you really continue this loop after an 
> -EINVAL ?
> 
> Understood for architected you want to keep going and get the system up 
> at the very least I'd add a noisy message about it.

Yes, that's correct. If we bail out on failure, the system will hang. 
The platform can have more timers which will work allowing the system to 
boot.

It is expected to have the driver probe function to print a message on 
error, so adding one message seems to be redundant.

If timer_probe() fails to initialize a timer (the number of initialized 
driver is zero) then an error will be emitted.


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

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Rob Herring]

On Wed, Jun 25, 2025 at 3:57 AM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> In the context of the time keeping and the timers, some platforms have
> timers which need to be initialized very early. It is the case of the
> ARM platform which do not have the architected timers.
>
> The macro TIMER_OF_DECLARE adds an entry in the timer init functions
> array at compile time and the function timer_probe is called from the
> timer_init() function in kernel/time.c
>
> This array contains a t-uple with the init function and the compatible
> string.
>
> The init function has a device node pointer parameter.
>
> The timer_probe() function browses the of nodes and find the ones
> matching the compatible string given when using the TIMER_OF_DECLARE
> macro. It then calls the init function with the device node as a
> pointer.
>
> But there are some platforms where there are multiple timers like the
> ARM64 with the architected timers. Those are always initialized very
> early and the other timers can be initialized later.
>
> For this reason we find timer drivers with the platform_driver
> incarnation. Consequently their init functions are different, they
> have a platform_device pointer parameter and rely on the devm_
> function for rollbacking.
>
> To summarize, we have:
>  - TIMER_OF_DECLARE with init function prototype:
>    int (*init)(struct device_node *np);
>
>  - module_platform_driver (and variant) with the probe function
>    prototype:
>    int (*init)(struct platform_device *pdev);
>
> The current situation with the timers is the following:
>
>  - Two platforms can have the same timer hardware, hence the same
>    driver but one without alternate timers and the other with multiple
>    timers. For example, the Exynos platform has only the Exynos MCT on
>    ARM but has the architeched timers in addition on the ARM64.
>
>  - The timer drivers can be modules now which was not the case until
>    recently. TIMER_OF_DECLARE do not allow the build as a module.
>
> It results in duplicate init functions (one with rollback and one with
> devm_) and different way to declare the driver (TIMER_OF_DECLARE and
> module_platform_driver).
>
> This proposed change is to unify the prototyping of the init functions
> to receive a platform_device pointer as parameter. Consequently, it
> will allow a smoother and nicer module conversion and a huge cleanup
> of the init functions by removing all the rollback code from all the
> timer drivers. It introduces a TIMER_OF_DECLARE_PDEV macro.
>
> If the macro is used a platform_device is manually allocated and
> initialized with the needed information for the probe
> function. Otherwise module_platform_driver can be use instead with the
> same probe function without the timer_probe() initialization.
>
> I don't have an expert knowledge of the platform_device internal
> subtilitie so I'm not sure if this approach is valid. However, it has
> been tested on a Rockchip board with the "rockchip,rk3288-timer" and
> verified the macro and the devm_ rollback work correctly.

Have you looked at the SH "early_platform_driver"? How does this
compare? IIRC, that used to be generally available, but has been
pushed into SH since that was the only arch using it and no one likes
it.

Rob

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[William McVicker]

Hi Daniel,

On 06/25/2025, Daniel Lezcano wrote:
> In the context of the time keeping and the timers, some platforms have
> timers which need to be initialized very early. It is the case of the
> ARM platform which do not have the architected timers.
> 
> The macro TIMER_OF_DECLARE adds an entry in the timer init functions
> array at compile time and the function timer_probe is called from the
> timer_init() function in kernel/time.c
> 
> This array contains a t-uple with the init function and the compatible
> string.
> 
> The init function has a device node pointer parameter.
> 
> The timer_probe() function browses the of nodes and find the ones
> matching the compatible string given when using the TIMER_OF_DECLARE
> macro. It then calls the init function with the device node as a
> pointer.
> 
> But there are some platforms where there are multiple timers like the
> ARM64 with the architected timers. Those are always initialized very
> early and the other timers can be initialized later.
> 
> For this reason we find timer drivers with the platform_driver
> incarnation. Consequently their init functions are different, they
> have a platform_device pointer parameter and rely on the devm_
> function for rollbacking.
> 
> To summarize, we have:
>  - TIMER_OF_DECLARE with init function prototype:
>    int (*init)(struct device_node *np);
> 
>  - module_platform_driver (and variant) with the probe function
>    prototype:
>    int (*init)(struct platform_device *pdev);
> 
> The current situation with the timers is the following:
> 
>  - Two platforms can have the same timer hardware, hence the same
>    driver but one without alternate timers and the other with multiple
>    timers. For example, the Exynos platform has only the Exynos MCT on
>    ARM but has the architeched timers in addition on the ARM64.
> 
>  - The timer drivers can be modules now which was not the case until
>    recently. TIMER_OF_DECLARE do not allow the build as a module.
> 
> It results in duplicate init functions (one with rollback and one with
> devm_) and different way to declare the driver (TIMER_OF_DECLARE and
> module_platform_driver).
> 
> This proposed change is to unify the prototyping of the init functions
> to receive a platform_device pointer as parameter. Consequently, it
> will allow a smoother and nicer module conversion and a huge cleanup
> of the init functions by removing all the rollback code from all the
> timer drivers. It introduces a TIMER_OF_DECLARE_PDEV macro.
> 
> If the macro is used a platform_device is manually allocated and
> initialized with the needed information for the probe
> function. Otherwise module_platform_driver can be use instead with the
> same probe function without the timer_probe() initialization.
> 
> I don't have an expert knowledge of the platform_device internal
> subtilitie so I'm not sure if this approach is valid. However, it has
> been tested on a Rockchip board with the "rockchip,rk3288-timer" and
> verified the macro and the devm_ rollback work correctly.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Hans de Goede <hansg@kernel.org>
> Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/clocksource/timer-probe.c | 61 ++++++++++++++++++++++++++++++-
>  include/asm-generic/vmlinux.lds.h |  2 +
>  include/linux/clocksource.h       |  3 ++
>  include/linux/of.h                |  5 +++
>  4 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clocksource/timer-probe.c b/drivers/clocksource/timer-probe.c
> index b7860bc0db4b..6b2b341b8c95 100644
> --- a/drivers/clocksource/timer-probe.c
> +++ b/drivers/clocksource/timer-probe.c
> @@ -7,13 +7,18 @@
>  #include <linux/init.h>
>  #include <linux/of.h>
>  #include <linux/clocksource.h>
> +#include <linux/platform_device.h>
>  
>  extern struct of_device_id __timer_of_table[];
> +extern struct of_device_id __timer_pdev_of_table[];
>  
>  static const struct of_device_id __timer_of_table_sentinel
>  	__used __section("__timer_of_table_end");
>  
> -void __init timer_probe(void)
> +static const struct of_device_id __timer_pdev_of_table_sentinel
> +	__used __section("__timer_pdev_of_table_end");
> +
> +static int __init timer_of_probe(void)
>  {
>  	struct device_node *np;
>  	const struct of_device_id *match;
> @@ -38,6 +43,60 @@ void __init timer_probe(void)
>  		timers++;
>  	}
>  
> +	return timers;
> +}
> +
> +static int __init timer_pdev_of_probe(void)
> +{
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	const struct of_device_id *match;
> +	of_init_fn_pdev init_func;
> +	unsigned int timers = 0;
> +	int ret;
> +
> +	for_each_matching_node_and_match(np, __timer_pdev_of_table, &match) {
> +		if (!of_device_is_available(np))
> +			continue;
> +
> +		init_func = match->data;
> +
> +		pdev = platform_device_alloc(of_node_full_name(np), -1);
> +		if (!pdev)
> +			continue;
> +
> +		ret = device_add_of_node(&pdev->dev, np);
> +		if (ret) {
> +			platform_device_put(pdev);
> +			continue;
> +		}
> +
> +		dev_set_name(&pdev->dev, pdev->name);
> +
> +		ret = init_func(pdev);
> +		if (!ret) {
> +			timers++;
> +			continue;
> +		}
> +
> +		if (ret != -EPROBE_DEFER)
> +			pr_err("Failed to initialize '%pOF': %d\n", np,
> +			       ret);
> +
> +		device_remove_of_node(&pdev->dev);
> +
> +		platform_device_put(pdev);
> +	}
> +
> +	return timers;
> +}
> +
> +void __init timer_probe(void)
> +{
> +	unsigned timers = 0;
> +
> +	timers += timer_of_probe();
> +	timers += timer_pdev_of_probe();
>  	timers += acpi_probe_device_table(timer);
>  
>  	if (!timers)
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index fa5f19b8d53a..97606499c8d7 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -318,6 +318,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>  	KEEP(*(__##name##_of_table_end))
>  
>  #define TIMER_OF_TABLES()	OF_TABLE(CONFIG_TIMER_OF, timer)
> +#define TIMER_PDEV_OF_TABLES()	OF_TABLE(CONFIG_TIMER_OF, timer_pdev)
>  #define IRQCHIP_OF_MATCH_TABLE() OF_TABLE(CONFIG_IRQCHIP, irqchip)
>  #define CLK_OF_TABLES()		OF_TABLE(CONFIG_COMMON_CLK, clk)
>  #define RESERVEDMEM_OF_TABLES()	OF_TABLE(CONFIG_OF_RESERVED_MEM, reservedmem)
> @@ -714,6 +715,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>  	CLK_OF_TABLES()							\
>  	RESERVEDMEM_OF_TABLES()						\
>  	TIMER_OF_TABLES()						\
> +	TIMER_PDEV_OF_TABLES()						\
>  	CPU_METHOD_OF_TABLES()						\
>  	CPUIDLE_METHOD_OF_TABLES()					\
>  	KERNEL_DTB()							\
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 65b7c41471c3..0eeabd207040 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -289,6 +289,9 @@ extern int clocksource_i8253_init(void);
>  #define TIMER_OF_DECLARE(name, compat, fn) \
>  	OF_DECLARE_1_RET(timer, name, compat, fn)
>  
> +#define TIMER_OF_DECLARE_PDEV(name, compat, fn) \
> +	OF_DECLARE_PDEV(timer_pdev, name, compat, fn)
> +
>  #ifdef CONFIG_TIMER_PROBE
>  extern void timer_probe(void);
>  #else
> diff --git a/include/linux/of.h b/include/linux/of.h
> index a62154aeda1b..a312a6f5ecc1 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1540,9 +1540,12 @@ static inline int of_get_available_child_count(const struct device_node *np)
>  	_OF_DECLARE_STUB(table, name, compat, fn, fn_type)
>  #endif
>  
> +struct platform_device;
> +
>  typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
>  typedef int (*of_init_fn_1_ret)(struct device_node *);
>  typedef void (*of_init_fn_1)(struct device_node *);
> +typedef int (*of_init_fn_pdev)(struct platform_device *);
>  
>  #define OF_DECLARE_1(table, name, compat, fn) \
>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1)
> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
>  #define OF_DECLARE_2(table, name, compat, fn) \
>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)

To support auto-module loading you'll need to also define the
MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().

I haven't tested the patch yet, but aside from my comment above it LGTM.

Thanks,
Will

>  
>  /**
>   * struct of_changeset_entry	- Holds a changeset entry
> -- 
> 2.43.0
> 

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Arnd Bergmann]

On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
>> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
>>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
>>  #define OF_DECLARE_2(table, name, compat, fn) \
>>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
>> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
>> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
>
> To support auto-module loading you'll need to also define the
> MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
>
> I haven't tested the patch yet, but aside from my comment above it LGTM.

The patch doesn't actually have a module_platform_driver_probe()
yet either, so loading the module wouldn't actually do anything.

I feel that this RFC by itself a good step in the direction we want, 
so Daniel should go ahead with prototyping the next two steps:
adding the platform_driver registration into OF_DECLARE_PDEV,
and converting a driver so it can be used either with the _OF_DECLARE()
or the platform_driver case.

Regarding the sh_early_platform_driver code that Rob mentioned,
I think this one is already better since it doesn't duplicate
parts of the platform_driver framework and it interfaces with
device tree based probing.

     Arnd

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[William McVicker]

On 07/01/2025, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
> >>  #define OF_DECLARE_2(table, name, compat, fn) \
> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
> >
> > To support auto-module loading you'll need to also define the
> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
> >
> > I haven't tested the patch yet, but aside from my comment above it LGTM.
> 
> The patch doesn't actually have a module_platform_driver_probe()
> yet either, so loading the module wouldn't actually do anything.

Probing with TIMER_OF_DECLARE() just consists of running the match table's data
function pointer. So that is covered by Daniel's patch AFAICT. However, it's
not clear if this implementation allows you to load the kernel module after the
device boots? For example, will the Exynos MCT timer probe if I load the
exynos_mct driver after the device boots? My guess is you'd need to register
the device as a platform device with a dedicated probe function to handle that.

--Will

<snip>

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Arnd Bergmann]

On Tue, Jul 1, 2025, at 20:21, William McVicker wrote:
> On 07/01/2025, Arnd Bergmann wrote:
>> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
>> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
>> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
>> >>  #define OF_DECLARE_2(table, name, compat, fn) \
>> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
>> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
>> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
>> >
>> > To support auto-module loading you'll need to also define the
>> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
>> >
>> > I haven't tested the patch yet, but aside from my comment above it LGTM.
>> 
>> The patch doesn't actually have a module_platform_driver_probe()
>> yet either, so loading the module wouldn't actually do anything.
>
> Probing with TIMER_OF_DECLARE() just consists of running the match table's data
> function pointer. So that is covered by Daniel's patch AFAICT. However, it's
> not clear if this implementation allows you to load the kernel module after the
> device boots? For example, will the Exynos MCT timer probe if I load the
> exynos_mct driver after the device boots? My guess is you'd need to register
> the device as a platform device with a dedicated probe function to handle that.

Yes, that's what I meant: the loadable module needs a module_init()
function that registers the actual platform driver in order for the
probe function to be called. module_platform_driver_probe()
is the way I would suggest to arrange it, though that is just a
convenience helper around the registration.

The platform device at that point is created by the normal DT scan,
so there is no need to create an extra one. On the contrary, in case
we successfully call the probe function from timer_init(), we end
up with two separate 'struct platform_device' instances 

     Arnd

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[William McVicker]

On 07/01/2025, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 20:21, William McVicker wrote:
> > On 07/01/2025, Arnd Bergmann wrote:
> >> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
> >> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
> >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
> >> >>  #define OF_DECLARE_2(table, name, compat, fn) \
> >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> >> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> >> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
> >> >
> >> > To support auto-module loading you'll need to also define the
> >> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
> >> >
> >> > I haven't tested the patch yet, but aside from my comment above it LGTM.
> >> 
> >> The patch doesn't actually have a module_platform_driver_probe()
> >> yet either, so loading the module wouldn't actually do anything.
> >
> > Probing with TIMER_OF_DECLARE() just consists of running the match table's data
> > function pointer. So that is covered by Daniel's patch AFAICT. However, it's
> > not clear if this implementation allows you to load the kernel module after the
> > device boots? For example, will the Exynos MCT timer probe if I load the
> > exynos_mct driver after the device boots? My guess is you'd need to register
> > the device as a platform device with a dedicated probe function to handle that.
> 
> Yes, that's what I meant: the loadable module needs a module_init()
> function that registers the actual platform driver in order for the
> probe function to be called. module_platform_driver_probe()
> is the way I would suggest to arrange it, though that is just a
> convenience helper around the registration.
> 
> The platform device at that point is created by the normal DT scan,
> so there is no need to create an extra one. On the contrary, in case
> we successfully call the probe function from timer_init(), we end
> up with two separate 'struct platform_device' instances 
> 
>      Arnd

So then does it even make sense to have `timer_pdev_of_probe()` if it's very
unlikely that the module will even be loaded by the time `timer_probe()` runs?
Would it make sense for TIMER_OF_DECLARE_PDEV() to just have a special else case
with the module boiler plate stuff for when the driver is built as a module?
Something like:

--->o---

#if !defined(MODULE)
#define TIMER_OF_DECLARE_PDEV(...) TIMER_OF_DECLARE(...)
#else
static int timer_pdev_probe(struct platform_device *pdev)
{
	struct device *dev = &pdev->dev;
	int (*timer_init)(struct device_node *np);

	timer_init = of_device_get_match_data(dev);
	if (!timer_init)
		return -EINVAL;

	return timer_init(dev->of_node);
}

#define TIMER_OF_DECLARE_PDEV(...) \
	OF_DECLARE_PDEV(timer_pdev, name, compat, fn) \ # make this define MODULE_DEVICE_TABLE() as well
	<create struct platform_driver instance> \
	<call module_platform_driver_probe(..., timer_pdev_probe)
#endif

--->o---

--Will

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Daniel Lezcano]

On Tue, Jul 01, 2025 at 03:28:50PM -0700, Will McVicker wrote:
> On 07/01/2025, Arnd Bergmann wrote:
> > On Tue, Jul 1, 2025, at 20:21, William McVicker wrote:
> > > On 07/01/2025, Arnd Bergmann wrote:
> > >> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
> > >> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
> > >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
> > >> >>  #define OF_DECLARE_2(table, name, compat, fn) \
> > >> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> > >> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> > >> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
> > >> >
> > >> > To support auto-module loading you'll need to also define the
> > >> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
> > >> >
> > >> > I haven't tested the patch yet, but aside from my comment above it LGTM.
> > >> 
> > >> The patch doesn't actually have a module_platform_driver_probe()
> > >> yet either, so loading the module wouldn't actually do anything.
> > >
> > > Probing with TIMER_OF_DECLARE() just consists of running the match table's data
> > > function pointer. So that is covered by Daniel's patch AFAICT. However, it's
> > > not clear if this implementation allows you to load the kernel module after the
> > > device boots? For example, will the Exynos MCT timer probe if I load the
> > > exynos_mct driver after the device boots? My guess is you'd need to register
> > > the device as a platform device with a dedicated probe function to handle that.
> > 
> > Yes, that's what I meant: the loadable module needs a module_init()
> > function that registers the actual platform driver in order for the
> > probe function to be called. module_platform_driver_probe()
> > is the way I would suggest to arrange it, though that is just a
> > convenience helper around the registration.
> > 
> > The platform device at that point is created by the normal DT scan,
> > so there is no need to create an extra one. On the contrary, in case
> > we successfully call the probe function from timer_init(), we end
> > up with two separate 'struct platform_device' instances 
> > 
> >      Arnd
> 
> So then does it even make sense to have `timer_pdev_of_probe()` if it's very
> unlikely that the module will even be loaded by the time `timer_probe()` runs?
> Would it make sense for TIMER_OF_DECLARE_PDEV() to just have a special else case
> with the module boiler plate stuff for when the driver is built as a module?
> Something like:
> 
> --->o---
> 
> #if !defined(MODULE)
> #define TIMER_OF_DECLARE_PDEV(...) TIMER_OF_DECLARE(...)
> #else
> static int timer_pdev_probe(struct platform_device *pdev)
> {
> 	struct device *dev = &pdev->dev;
> 	int (*timer_init)(struct device_node *np);
> 
> 	timer_init = of_device_get_match_data(dev);
> 	if (!timer_init)
> 		return -EINVAL;
> 
> 	return timer_init(dev->of_node);
> }
> 
> #define TIMER_OF_DECLARE_PDEV(...) \
> 	OF_DECLARE_PDEV(timer_pdev, name, compat, fn) \ # make this define MODULE_DEVICE_TABLE() as well
> 	<create struct platform_driver instance> \
> 	<call module_platform_driver_probe(..., timer_pdev_probe)
> #endif
> 
> --->o---

Yes, that is exactly the goal :)

The RFC was just for the timer-probe function, I was unsure about its
validity. Let me resend with all the changes for the [no ] module
support.

--

 <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

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Daniel Lezcano]

On Tue, Jul 01, 2025 at 09:52:45AM +0200, Arnd Bergmann wrote:
> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
> >>  #define OF_DECLARE_2(table, name, compat, fn) \
> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
> >
> > To support auto-module loading you'll need to also define the
> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
> >
> > I haven't tested the patch yet, but aside from my comment above it LGTM.
> 
> The patch doesn't actually have a module_platform_driver_probe()
> yet either, so loading the module wouldn't actually do anything.
> 
> I feel that this RFC by itself a good step in the direction we want, 
> so Daniel should go ahead with prototyping the next two steps:
> adding the platform_driver registration into OF_DECLARE_PDEV,
> and converting a driver so it can be used either with the _OF_DECLARE()
> or the platform_driver case.

I'm questioning the relevance of adding the macro when the driver is
not compiled as a module.

The first step of this macro is to allow the existing init functions
to be converted to the same signature as the module probe functions in
order to share the same code and take benefit of the devm_ variants
function which will considerably reduce the code size of the drivers.

Then we have the following situations:

 1. The driver has to be loaded very early TIMER_OF_DECLARE_PDEV
 (MODULE=no) the function timer-probe() is used

 2. The driver is a module_platform_driver() and MODULE=no, then it is
 built as a builtin_platform_driver(), we do not care about having it
 loaded by timer-probe()

 3. The driver is a module_platform_driver() and MODULE=yes

If we do the change to have the TIMER_OF_DECLARE_PDEV() adding the
platform_driver registration when MODULE=yes but using timer-probe
when MODULE=no, we change the initialization and we will have issues
with timers needing resources like SCMI clocks and where the
mechanisms rely on EPROBE_DEFER.

IMO, module_platform_driver and timer_probe must be separated.

Let's assume the one possible future use case with the VF PIT. This
timer is only used on ARM but now it is also supported for the ARM64
s32g2. For the first platform we need it very early and in the second
case, we need it later because the architected timers are there.

We should endup with:

static const struct of_device_id pit_timer_of_match[] = {
	{ .compatible = "nxp,s32g2-pit", .data = &s32g2_data },
	{ .compatible = "nxp,s32g3-pit", .data = &s32g3_data },
	{ }
};
MODULE_DEVICE_TABLE(of, pit_timer_of_match);

static struct platform_driver nxp_pit_driver = {
	.driver = {
		.name = "nxp-pit",
		.of_match_table = pit_timer_of_match,
	},
	.probe = pit_timer_probe,
};
module_platform_driver(nxp_pit_driver);

TIMER_OF_DECLARE_PDEV(vf610, "fsl,vf610-pit", pit_timer_probe);

If we change the TIMER_OF_DECLARE_PDEV to a macro which relies on
timer_probe when MODULE=no, then the "nxp-pit" on the s32gX will fail
to initialize because of the SCMI clocks not ready and the routine
won't reprobe the function. This issue won't happen with
builtin_platform_driver

What about something like:

TIMER_OF_DECLARE_PLATFORM_DRIVER(__name, __driver) \
  TIMER_OF_DECLARE_PDEV(__name, __driver->probe); \
#ifdef MODULE
  module_platform_driver(__driver);
#endif

Then in the timer_probe() we browse the of_match_table compatibles and
if the probe function succeed then we do of_node_set_flag(np,
OF_POPULATED) which is supposed to prevent calling the probe function
later.

Thoughts ?

--

 <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

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Arnd Bergmann]

On Tue, Jul 8, 2025, at 11:50, Daniel Lezcano wrote:
> On Tue, Jul 01, 2025 at 09:52:45AM +0200, Arnd Bergmann wrote:
>> On Tue, Jul 1, 2025, at 01:53, William McVicker wrote:
>> >> @@ -1550,6 +1553,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
>> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
>> >>  #define OF_DECLARE_2(table, name, compat, fn) \
>> >>  		_OF_DECLARE(table, name, compat, fn, of_init_fn_2)
>> >> +#define OF_DECLARE_PDEV(table, name, compat, fn) \
>> >> +		_OF_DECLARE(table, name, compat, fn, of_init_fn_pdev)
>> >
>> > To support auto-module loading you'll need to also define the
>> > MODULE_DEVICE_TABLE() as part of TIMER_OF_DECLARE_PDEV().
>> >
>> > I haven't tested the patch yet, but aside from my comment above it LGTM.
>> 
>> The patch doesn't actually have a module_platform_driver_probe()
>> yet either, so loading the module wouldn't actually do anything.
>> 
>> I feel that this RFC by itself a good step in the direction we want, 
>> so Daniel should go ahead with prototyping the next two steps:
>> adding the platform_driver registration into OF_DECLARE_PDEV,
>> and converting a driver so it can be used either with the _OF_DECLARE()
>> or the platform_driver case.
>
> I'm questioning the relevance of adding the macro when the driver is
> not compiled as a module.
>
> The first step of this macro is to allow the existing init functions
> to be converted to the same signature as the module probe functions in
> order to share the same code and take benefit of the devm_ variants
> function which will considerably reduce the code size of the drivers.
>
> Then we have the following situations:
>
>  1. The driver has to be loaded very early TIMER_OF_DECLARE_PDEV
>  (MODULE=no) the function timer-probe() is used
>
>  2. The driver is a module_platform_driver() and MODULE=no, then it is
>  built as a builtin_platform_driver(), we do not care about having it
>  loaded by timer-probe()
>
>  3. The driver is a module_platform_driver() and MODULE=yes
>
> If we do the change to have the TIMER_OF_DECLARE_PDEV() adding the
> platform_driver registration when MODULE=yes but using timer-probe
> when MODULE=no, we change the initialization and we will have issues
> with timers needing resources like SCMI clocks and where the
> mechanisms rely on EPROBE_DEFER.
>
> IMO, module_platform_driver and timer_probe must be separated.

I assumed that the entire point of your work was to simplify
the code when you have both a early and platform_driver based
probing in a single driver.

> Let's assume the one possible future use case with the VF PIT. This
> timer is only used on ARM but now it is also supported for the ARM64
> s32g2. For the first platform we need it very early and in the second
> case, we need it later because the architected timers are there.
>
> We should endup with:
>
> static const struct of_device_id pit_timer_of_match[] = {
> 	{ .compatible = "nxp,s32g2-pit", .data = &s32g2_data },
> 	{ .compatible = "nxp,s32g3-pit", .data = &s32g3_data },
> 	{ }
> };
> MODULE_DEVICE_TABLE(of, pit_timer_of_match);
>
> static struct platform_driver nxp_pit_driver = {
> 	.driver = {
> 		.name = "nxp-pit",
> 		.of_match_table = pit_timer_of_match,
> 	},
> 	.probe = pit_timer_probe,
> };
> module_platform_driver(nxp_pit_driver);
>
> TIMER_OF_DECLARE_PDEV(vf610, "fsl,vf610-pit", pit_timer_probe);

If you think we still need a module_platform_driver(), I would
not bother adding TIMER_OF_DECLARE_PDEV() either, as the
probe functions can easily be shared as well, the way that
drivers/clocksource/renesas-ostm.c does.

The only thing that would lose is the ability to call devm_
functions because the device pointer is unavailable here.

> If we change the TIMER_OF_DECLARE_PDEV to a macro which relies on
> timer_probe when MODULE=no, then the "nxp-pit" on the s32gX will fail
> to initialize because of the SCMI clocks not ready and the routine
> won't reprobe the function. This issue won't happen with
> builtin_platform_driver

The way I had expected this to work was that TIMER_OF_DECLARE_PDEV()
always registers the platform_driver and just skips the
section magic when it's in a loadable module.

    Arnd

Re: [PATCH RFC] timer: of: Create a platform_device before the framework is initialized

[Daniel Lezcano]

On 08/07/2025 18:10, Arnd Bergmann wrote:

[ ... ]

> The way I had expected this to work was that TIMER_OF_DECLARE_PDEV()
> always registers the platform_driver and just skips the
> section magic when it's in a loadable module.

Yes it would be a nice change, but it seems to be not possible for the 
early + builtin + module combination we have AFAICS.

What about to have two macros:

/*
  * Declares module_platform_driver
  * Assigns __driver.driver.of_match_table = __of_match
  * Assigns __driver.driver.name = __name
  */
TIMER_OF_DECLARE_PLATFORM_DRIVER(__name, __driver, __of_match)

/*
  * Does the same as the above but if MODULE is not set then then we
  * end up with timer-probe() instead of builtin_platform_driver
  */
TIMER_OF_DECLARE_EARLY_PLATFORM_DRIVER(__name, __driver, __of_match)

So when someone is converting a driver into a module, it can change the 
driver to use these macros instead.


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