arm64: dts: Fix nuvoton 8xx clock properties

PATCH In review William A. Kennington III 3 participants 5 messages Apr 15, 2025 – Apr 25, 2025

Subsystems (2)

Device tree bindings Arm/nuvoton npcm architecture

lore

Share & tools

lore

William A. Kennington III

author

2025-04-15 23:25 UTC #0 lore raw

PATCH 1/1 arm64: dts: Fix nuvoton 8xx clock properties

The latest iteration of the clock driver got rid of the separate clock compatible node, merging clock and reset devices.

Signed-off-by William A. Kennington III <[email protected]>

---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +6 ~~-10~~

index ecd171b2feba..4da62308b274 100644
@@ -47,17 +47,13 @@ ahb {
 		interrupt-parent = <&gic>;
 		ranges;
 
-		rstc: reset-controller@f0801000 {
+		clk: rstc: reset-controller@f0801000 {
 			compatible = "nuvoton,npcm845-reset";
 			reg = <0x0 0xf0801000 0x0 0x78>;
 			#reset-cells = <2>;
 			nuvoton,sysgcr = <&gcr>;
-		};
-
-		clk: clock-controller@f0801000 {
-			compatible = "nuvoton,npcm845-clk";
+			clocks = <&refclk>;
 			#clock-cells = <1>;
-			reg = <0x0 0xf0801000 0x0 0x1000>;
 		};
 
 		apb {

@@ -81,7 +77,7 @@ timer0: timer@8000 {
 				compatible = "nuvoton,npcm845-timer";
 				interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0x8000 0x1C>;
-				clocks = <&clk NPCM8XX_CLK_REFCLK>;
+				clocks = <&refclk>;
 				clock-names = "refclk";
 			};

@@ -153,7 +149,7 @@ watchdog0: watchdog@801c {
 				interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0x801c 0x4>;
 				status = "disabled";
-				clocks = <&clk NPCM8XX_CLK_REFCLK>;
+				clocks = <&refclk>;
 				syscon = <&gcr>;
 			};

@@ -162,7 +158,7 @@ watchdog1: watchdog@901c {
 				interrupts = <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0x901c 0x4>;
 				status = "disabled";
-				clocks = <&clk NPCM8XX_CLK_REFCLK>;
+				clocks = <&refclk>;
 				syscon = <&gcr>;
 			};

@@ -171,7 +167,7 @@ watchdog2: watchdog@a01c {
 				interrupts = <GIC_SPI 49 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0xa01c 0x4>;
 				status = "disabled";
-				clocks = <&clk NPCM8XX_CLK_REFCLK>;
+				clocks = <&refclk>;
 				syscon = <&gcr>;
 			};
 		};

arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts +8 -0

index eeceb5b292a8..a20f95c60a62 100644
@@ -19,6 +19,14 @@ chosen {
 	memory@0 {
 		reg = <0x0 0x0 0x0 0x40000000>;
 	};
+
+	refclk: refclk-25mhz {
+		compatible = "fixed-clock";
+		clock-output-names = "ref";
+		clock-frequency = <25000000>;
+		#clock-cells = <0>;
+	};
+
 };
 
 &serial0 {

Signature · 1 line

2.49.0.604.gff1f9ca942-goog

Krzysztof Kozlowski

2025-04-16 06:55 UTC #1 lore raw

Quoted 1 line · William A. Kennington III

The latest iteration of the clock driver got rid of the separate clock

I don't see the binding deprecated.

Quoted 13 lines

compatible node, merging clock and reset devices.
Signed-off-by William A. Kennington III <[email protected]>
---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0 -0
index ecd171b2feba..4da62308b274 100644

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi ahb { · line 52

Quoted 7 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1

@@ -47,17 +47,13 @@ ahb {
 		interrupt-parent = <&gic>;
 		ranges;
 
-		rstc: reset-controller@f0801000 {
+		clk: rstc: reset-controller@f0801000 {
 			compatible = "nuvoton,npcm845-reset";
 			reg = <0x0 0xf0801000 0x0 0x78>;

So now it lacks quite a bit of address space. This must be explained in commit msg.

Quoted 13 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -5

 			#reset-cells = <2>;
 			nuvoton,sysgcr = <&gcr>;
-		};
-
-		clk: clock-controller@f0801000 {
-			compatible = "nuvoton,npcm845-clk";
+			clocks = <&refclk>;
 			#clock-cells = <1>;
-			reg = <0x0 0xf0801000 0x0 0x1000>;
 		};
 
 		apb {

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi timer0: timer@8000 { · line 80

Quoted 5 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1

@@ -81,7 +77,7 @@ timer0: timer@8000 {
 				compatible = "nuvoton,npcm845-timer";
 				interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
 				reg = <0x8000 0x1C>;
-				clocks = <&clk NPCM8XX_CLK_REFCLK>;
+				clocks = <&refclk>;

Not explained in commit msg.

Best regards,
Krzysztof

Tomer Maimon

maintainer

2025-04-16 18:44 UTC #2 lore raw

William, thanks for the patch.

Quoted 22 lines · Krzysztof Kozlowski

Quoted 1 line · William A. Kennington III
The latest iteration of the clock driver got rid of the separate clock
I don't see the binding deprecated.
Quoted 16 lines
compatible node, merging clock and reset devices.
Signed-off-by William A. Kennington III <[email protected]>
---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0 -0
index ecd171b2feba..4da62308b274 100644
@@ -47,17 +47,13 @@ ahb {
              interrupt-parent = <&gic>;
              ranges;
 

Quoted 4 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi ahb { · line 52
Quoted 4 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1
-             rstc: reset-controller@f0801000 {
+             clk: rstc: reset-controller@f0801000 {
                      compatible = "nuvoton,npcm845-reset";
                      reg = <0x0 0xf0801000 0x0 0x78>;

The size of the registers offset is 0xC4 (last register is at offset 0xC0) Therefore, the reg property should be modified as well to reg = <0x0 0xf0801000 0x0 0xC4>;

Quoted 26 lines

So now it lacks quite a bit of address space. This must be explained in commit msg.

Quoted 13 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -5

                      #reset-cells = <2>;
                      nuvoton,sysgcr = <&gcr>;
-             };
-
-             clk: clock-controller@f0801000 {
-                     compatible = "nuvoton,npcm845-clk";
+                     clocks = <&refclk>;
                      #clock-cells = <1>;
-                     reg = <0x0 0xf0801000 0x0 0x1000>;
              };
 
              apb {

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi timer0: timer@8000 { · line 80

Quoted 5 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1

@@ -81,7 +77,7 @@ timer0: timer@8000 {
                              compatible = "nuvoton,npcm845-timer";
                              interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
                              reg = <0x8000 0x1C>;
-                             clocks = <&clk NPCM8XX_CLK_REFCLK>;
+                             clocks = <&refclk>;

Not explained in commit msg.

Quoted 2 lines

Best regards,
Krzysztof

Best regards,

Tomer

William Kennington

author

2025-04-25 22:14 UTC #3 lore raw

Quoted 27 lines · Tomer Maimon

William, thanks for the patch.
Quoted 22 lines · Krzysztof Kozlowski
Quoted 1 line · William A. Kennington III
The latest iteration of the clock driver got rid of the separate clock
I don't see the binding deprecated.
Quoted 16 lines
compatible node, merging clock and reset devices.
Signed-off-by William A. Kennington III <[email protected]>
---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0 -0
index ecd171b2feba..4da62308b274 100644
@@ -47,17 +47,13 @@ ahb {
              interrupt-parent = <&gic>;
              ranges;
 

Quoted 7 lines

Quoted 4 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi ahb { · line 52
Quoted 4 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1
-             rstc: reset-controller@f0801000 {
+             clk: rstc: reset-controller@f0801000 {
                      compatible = "nuvoton,npcm845-reset";
                      reg = <0x0 0xf0801000 0x0 0x78>;
The size of the registers offset is 0xC4 (last register is at offset 0xC0) Therefore, the reg property should be modified as well to reg = <0x0 0xf0801000 0x0 0xC4>;

Yeah, I just looked at the DS to verify this value for the last clock register.

Quoted 32 lines

Quoted 26 lines

So now it lacks quite a bit of address space. This must be explained in commit msg.

Quoted 13 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -5

                      #reset-cells = <2>;
                      nuvoton,sysgcr = <&gcr>;
-             };
-
-             clk: clock-controller@f0801000 {
-                     compatible = "nuvoton,npcm845-clk";
+                     clocks = <&refclk>;
                      #clock-cells = <1>;
-                     reg = <0x0 0xf0801000 0x0 0x1000>;
              };
 
              apb {

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi timer0: timer@8000 { · line 80

Quoted 5 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1

@@ -81,7 +77,7 @@ timer0: timer@8000 {
                              compatible = "nuvoton,npcm845-timer";
                              interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
                              reg = <0x8000 0x1C>;
-                             clocks = <&clk NPCM8XX_CLK_REFCLK>;
+                             clocks = <&refclk>;

Not explained in commit msg.

Quoted 2 lines

Best regards,
Krzysztof

Best regards,

Tomer

William Kennington

author

2025-04-25 22:16 UTC #4 lore raw

In reply to Krzysztof Kozlowski

Quoted 22 lines · Krzysztof Kozlowski

Quoted 1 line · William A. Kennington III
The latest iteration of the clock driver got rid of the separate clock
I don't see the binding deprecated.
Quoted 16 lines
compatible node, merging clock and reset devices.
Signed-off-by William A. Kennington III <[email protected]>
---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +0 -0
index ecd171b2feba..4da62308b274 100644
@@ -47,17 +47,13 @@ ahb {
              interrupt-parent = <&gic>;
              ranges;
 

Quoted 7 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi ahb { · line 52
Quoted 4 lines
arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1
-             rstc: reset-controller@f0801000 {
+             clk: rstc: reset-controller@f0801000 {
                      compatible = "nuvoton,npcm845-reset";
                      reg = <0x0 0xf0801000 0x0 0x78>;
So now it lacks quite a bit of address space. This must be explained in commit msg.

Can do that when i make the updated series. Basically the old value was just never consumed by an actual driver and the chip reserves that entire 0x1000 size address space for clock registers. However, only 0xC4 bytes (0x78 was incorrect) of that space are used for these registers.

Quoted 12 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -5

                      #reset-cells = <2>;
                      nuvoton,sysgcr = <&gcr>;
-             };
-
-             clk: clock-controller@f0801000 {
-                     compatible = "nuvoton,npcm845-clk";
+                     clocks = <&refclk>;
                      #clock-cells = <1>;
-                     reg = <0x0 0xf0801000 0x0 0x1000>;
              };

Quoted 9 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi timer0: timer@8000 { · line 80

Quoted 7 lines

arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi +1 -1

              apb {

@@ -81,7 +77,7 @@ timer0: timer@8000 {
                              compatible = "nuvoton,npcm845-timer";
                              interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
                              reg = <0x8000 0x1C>;
-                             clocks = <&clk NPCM8XX_CLK_REFCLK>;
+                             clocks = <&refclk>;

Not explained in commit msg.

Yeah, I can do that WRT using an on board refclk instead of a value that comes from the SoC.

Quoted 4 lines

Best regards,
Krzysztof