[PATCH v2 1/3] arm64: dts: socfpga: agilex5: replace clocks reference with clock-frequency for dw-apb-timer

[PATCH v2 1/3] arm64: dts: socfpga: agilex5: replace clocks reference with clock-frequency for dw-apb-timer

[Adrian Ng Ho Yin]

The Agilex5 clock manager is a platform driver initialised at
core_initcall, after time_init() where TIMER_OF_DECLARE callbacks run.
The timer cannot defer its probe, so clk_get() fails and the timer is
never brought up.

With clock-frequency, the tick rate is read directly from the DT node
without involving the clock framework, allowing the timer to initialise
regardless of when the clock manager is ready.

Replace clocks/clock-names with a static clock-frequency of 100 MHz,
which is the L4_SP clock rate configured by the bootloader.

Fixes: 2d599bc43813 ("arm64: dts: agilex5: add initial support for Intel Agilex5 SoCFPGA")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
index f54767d1526e..f5bc37fa8583 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
@@ -429,32 +429,28 @@ timer0: timer0@10c03000 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x10c03000 0x100>;
 			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkmgr AGILEX5_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer1: timer1@10c03100 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x10c03100 0x100>;
 			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkmgr AGILEX5_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer2: timer2@10d00000 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x10d00000 0x100>;
 			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkmgr AGILEX5_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer3: timer3@10d00100 {
 			compatible = "snps,dw-apb-timer";
 			reg = <0x10d00100 0x100>;
 			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clkmgr AGILEX5_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		uart0: serial@10c02000 {
-- 
2.49.GIT

Re: [PATCH v2 1/3] arm64: dts: socfpga: agilex5: replace clocks reference with clock-frequency for dw-apb-timer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Replacing 'clocks' with 'clock-frequency' to work around Linux driver initialization ordering is a Devicetree anti-pattern.
--

commit bdc3d28e7cc7c5d8502d6fe42848d2feefd3bf47
Author: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>

arm64: dts: socfpga: agilex5: replace clocks reference with clock-frequency for dw-apb-timer

This patch replaces the dynamic clock reference for the Agilex5 timers with a
hardcoded clock frequency. This is done to bypass a driver initialization
ordering issue where the clock manager probes later than the timer.

> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> index f54767d1526e1..f5bc37fa85832 100644
> --- a/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi
> @@ -429,32 +429,28 @@ timer0: timer0@10c03000 {
>  			compatible = "snps,dw-apb-timer";
>  			reg = <0x10c03000 0x100>;
>  			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
> -			clocks = <&clkmgr AGILEX5_L4_SP_CLK>;
> -			clock-names = "timer";
> +			clock-frequency = <100000000>;

[Severity: Medium]
Is it appropriate to encode an OS-specific probe-order workaround directly
into the hardware description?

Devicetree is designed to describe hardware topology rather than provide
workarounds for Linux driver initialization sequences. By replacing the clocks
reference to the Agilex5 clock manager with a hardcoded clock-frequency, the
correct physical clock association is lost.

If the bootloader changes the L4_SP clock rate in the future, wouldn't the
hardcoded 100 MHz value lead to incorrect kernel timekeeping since it no longer
queries the actual clock manager?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788457041.git.adrian.ho.yin.ng@altera.com?part=1

[PATCH v2 2/3] arm64: dts: socfpga: agilex: replace clocks reference with clock-frequency for dw-apb-timer

[Adrian Ng Ho Yin]

The Agilex clock manager is a platform driver initialised at
core_initcall, after time_init() where TIMER_OF_DECLARE callbacks run.
The timer cannot defer its probe, so clk_get() fails and the timer is
never brought up.

With clock-frequency, the tick rate is read directly from the DT node
without involving the clock framework, allowing the timer to initialise
regardless of when the clock manager is ready.

Replace clocks/clock-names with a static clock-frequency of 100 MHz,
which is the L4_SP clock rate configured by the bootloader.

Fixes: 48f36de93a50 ("arm64: dts: agilex: populate clock dts entries for Intel SoCFPGA Agilex")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 arch/arm64/boot/dts/intel/socfpga_agilex.dtsi | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
index 0dfbafde8822..4ed762c6e07f 100644
--- a/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
+++ b/arch/arm64/boot/dts/intel/socfpga_agilex.dtsi
@@ -491,32 +491,28 @@ timer0: timer0@ffc03000 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xffc03000 0x100>;
-			clocks = <&clkmgr AGILEX_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer1: timer1@ffc03100 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <GIC_SPI 114 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xffc03100 0x100>;
-			clocks = <&clkmgr AGILEX_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer2: timer2@ffd00000 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <GIC_SPI 115 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xffd00000 0x100>;
-			clocks = <&clkmgr AGILEX_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer3: timer3@ffd00100 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
 			reg = <0xffd00100 0x100>;
-			clocks = <&clkmgr AGILEX_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		uart0: serial@ffc02000 {
-- 
2.49.GIT

[PATCH v2 3/3] arm64: dts: socfpga: stratix10: replace clocks reference with clock-frequency for dw-apb-timer

[Adrian Ng Ho Yin]

The Stratix10 clock manager is a platform driver initialised at
core_initcall, after time_init() where TIMER_OF_DECLARE callbacks run.
The timer cannot defer its probe, so clk_get() fails and the timer is
never brought up.

With clock-frequency, the tick rate is read directly from the DT node
without involving the clock framework, allowing the timer to initialise
regardless of when the clock manager is ready.

Replace clocks/clock-names with a static clock-frequency of 100 MHz,
which is the L4_SP clock rate configured by the bootloader.

Fixes: d93101abe41e ("arm64: dts: stratix10: use clock bindings for the Stratix10 platform")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Ng Ho Yin <adrian.ho.yin.ng@altera.com>
---
 arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index 0d9cad0c0351..efae7b06e3fb 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -462,32 +462,28 @@ timer0: timer0@ffc03000 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <0 113 4>;
 			reg = <0xffc03000 0x100>;
-			clocks = <&clkmgr STRATIX10_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer1: timer1@ffc03100 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <0 114 4>;
 			reg = <0xffc03100 0x100>;
-			clocks = <&clkmgr STRATIX10_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer2: timer2@ffd00000 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <0 115 4>;
 			reg = <0xffd00000 0x100>;
-			clocks = <&clkmgr STRATIX10_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		timer3: timer3@ffd00100 {
 			compatible = "snps,dw-apb-timer";
 			interrupts = <0 116 4>;
 			reg = <0xffd00100 0x100>;
-			clocks = <&clkmgr STRATIX10_L4_SP_CLK>;
-			clock-names = "timer";
+			clock-frequency = <100000000>;
 		};
 
 		uart0: serial@ffc02000 {
-- 
2.49.GIT