Re: [PATCH 2/2] clocksource: Add Realtek systimer as tick broadcast driver

[Krzysztof Kozlowski <krzk@kernel.org>]

On 11/11/2025 10:29, Hao-Wen Ting wrote:
> Add a tick broadcast timer driver for Realtek SoCs.
> 
> On Realtek platforms, CPUs can enter deep idle states (C-states) where
> the local timer is stopped and powered off. Without a global tick
> broadcast timer, one CPU must remain awake to wake up the others,
> preventing all CPUs from entering deep idle simultaneously.
> 
> This driver provides a tick broadcast timer which remains active
> during deep idle states. This allows all CPUs to enter power-cut
> idle states simultaneously, significantly reducing overall power
> consumption.
> 
> The timer operates at 1MHz and supports oneshot mode.
> 
> Signed-off-by: Hao-Wen Ting <haowen.ting@realtek.com>
> ---
>  MAINTAINERS                         |   5 +
>  drivers/clocksource/Kconfig         |  10 ++
>  drivers/clocksource/Makefile        |   1 +
>  drivers/clocksource/timer-realtek.c | 173 ++++++++++++++++++++++++++++
>  4 files changed, 189 insertions(+)
>  create mode 100644 drivers/clocksource/timer-realtek.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c7a116b795d5..59dfd7543c39 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -28395,3 +28395,8 @@ S:	Buried alive in reporters
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>  F:	*
>  F:	*/
> +
> +REALTEK SYSTIMER DRIVER

Why are you adding to the end of the file? Did you look at this file at
all before changing this?

> +M:	Hao-Wen Ting <haowen.ting@realtek.com>
> +S:	Maintained
> +F:	drivers/clocksource/timer-realtek.c
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index ffcd23668763..e86905378f82 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -782,4 +782,14 @@ config NXP_STM_TIMER
>            Enables the support for NXP System Timer Module found in the
>            s32g NXP platform series.
>  
> +config RTK_SYSTIMER
> +	bool "Realtek SYSTIMER support"
> +	depends on OF
Missing depends on ARCH. Please don't send drivers which do not have any
possible user.

> +	select TIMER_OF
> +	help
> +	  This enables the global tick-broadcast timer on Realtek platforms.
> +	  If your Realtek platform supports power-cut level CPU idle states,
> +	  enabling this timer allows all CPUs to enter power-cut simultaneously
> +	  to achieve lower power consumption.
> +

...

> +
> +static int __init rtk_systimer_init(struct device_node *node)
> +{
> +	int ret;
> +
> +	ret = timer_of_init(node, &_to);
> +	if (ret)
> +		return ret;
> +
> +	systimer_base = timer_of_base(&_to);
> +	clockevents_config_and_register(&_to.clkevt, SYSTIMER_RATE,
> +					SYSTIMER_MIN_DELTA, SYSTIMER_MAX_DELTA);
> +
> +	pr_info("Realtek SYSTIMER registered\n");

Drop.

This does not look like useful printk message. Drivers should be silent
on success:
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/coding-style.rst#L913
https://elixir.bootlin.com/linux/v6.15-rc7/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79

> +	return 0;
> +}
> +
> +TIMER_OF_DECLARE(rtk_systimer, "realtek,systimer", rtk_systimer_init);


Best regards,
Krzysztof