[PATCH] arm64: dts: Fix nuvoton 8xx clock properties

William A. Kennington III posted 1 patch 8 months, 1 week ago
.../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
.../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
2 files changed, 14 insertions(+), 10 deletions(-)
[PATCH] arm64: dts: Fix nuvoton 8xx clock properties
Posted by William A. Kennington III 8 months, 1 week ago
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 <william@wkennington.com>
---
 .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
 .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
index ecd171b2feba..4da62308b274 100644
--- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
@@ -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>;
 			};
 		};
diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
index eeceb5b292a8..a20f95c60a62 100644
--- a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
@@ -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 {
-- 
2.49.0.604.gff1f9ca942-goog
Re: [PATCH] arm64: dts: Fix nuvoton 8xx clock properties
Posted by Krzysztof Kozlowski 8 months, 1 week ago
On 16/04/2025 01:25, William A. Kennington III wrote:
> The latest iteration of the clock driver got rid of the separate clock

I don't see the binding deprecated.

> compatible node, merging clock and reset devices.
> 
> Signed-off-by: William A. Kennington III <william@wkennington.com>
> ---
>  .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
>  .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> index ecd171b2feba..4da62308b274 100644
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> @@ -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.

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

Not explained in commit msg.


Best regards,
Krzysztof
Re: [PATCH] arm64: dts: Fix nuvoton 8xx clock properties
Posted by William Kennington 7 months, 3 weeks ago
On Tue, Apr 15, 2025 at 11:55 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 16/04/2025 01:25, William A. Kennington III wrote:
> > The latest iteration of the clock driver got rid of the separate clock
>
> I don't see the binding deprecated.
>
> > compatible node, merging clock and reset devices.
> >
> > Signed-off-by: William A. Kennington III <william@wkennington.com>
> > ---
> >  .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
> >  .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
> >  2 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > index ecd171b2feba..4da62308b274 100644
> > --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > @@ -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.

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.

>
> >                       #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>;
>
> 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.

>
>
> Best regards,
> Krzysztof
Re: [PATCH] arm64: dts: Fix nuvoton 8xx clock properties
Posted by Tomer Maimon 8 months ago
William, thanks for the patch.


On Wed, 16 Apr 2025 at 09:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 16/04/2025 01:25, William A. Kennington III wrote:
> > The latest iteration of the clock driver got rid of the separate clock
>
> I don't see the binding deprecated.
>
> > compatible node, merging clock and reset devices.
> >
> > Signed-off-by: William A. Kennington III <william@wkennington.com>
> > ---
> >  .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
> >  .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
> >  2 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > index ecd171b2feba..4da62308b274 100644
> > --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > @@ -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>;
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>;
>
> So now it lacks quite a bit of address space. This must be explained in
> commit msg.
>
> >                       #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>;
>
> Not explained in commit msg.
>
>
> Best regards,
> Krzysztof

Best regards,

Tomer
Re: [PATCH] arm64: dts: Fix nuvoton 8xx clock properties
Posted by William Kennington 7 months, 3 weeks ago
On Wed, Apr 16, 2025 at 11:44 AM Tomer Maimon <tmaimon77@gmail.com> wrote:
>
> William, thanks for the patch.
>
>
> On Wed, 16 Apr 2025 at 09:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On 16/04/2025 01:25, William A. Kennington III wrote:
> > > The latest iteration of the clock driver got rid of the separate clock
> >
> > I don't see the binding deprecated.
> >
> > > compatible node, merging clock and reset devices.
> > >
> > > Signed-off-by: William A. Kennington III <william@wkennington.com>
> > > ---
> > >  .../boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 16 ++++++----------
> > >  .../boot/dts/nuvoton/nuvoton-npcm845-evb.dts     |  8 ++++++++
> > >  2 files changed, 14 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > > index ecd171b2feba..4da62308b274 100644
> > > --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > > @@ -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>;
> 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.

> >
> > So now it lacks quite a bit of address space. This must be explained in
> > commit msg.
> >
> > >                       #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>;
> >
> > Not explained in commit msg.
> >
> >
> > Best regards,
> > Krzysztof
>
> Best regards,
>
> Tomer