diff mbox series

[linux,dev-6.6,v1,2/6] dt-bindings: clock: npcm845: Add reference 25m clock property

Message ID 20240701071048.751863-3-tmaimon77@gmail.com
State New
Headers show
Series Add NPCM8XX clock driver | expand

Commit Message

Tomer Maimon July 1, 2024, 7:10 a.m. UTC
The NPCM8XX clock driver uses a 25Mhz external clock, therefore adding
clock property.

The new required clock property does not break the NPCM8XX clock ABI
since the NPCM8XX clock driver hasn't merged yet to the Linux vanilla.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
---
 arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 9 +++++----
 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts     | 7 +++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

Comments

Andrew Jeffery July 3, 2024, 6:09 a.m. UTC | #1
On Mon, 2024-07-01 at 10:10 +0300, Tomer Maimon wrote:
> The NPCM8XX clock driver uses a 25Mhz external clock, therefore adding
> clock property.
> 
> The new required clock property does not break the NPCM8XX clock ABI
> since the NPCM8XX clock driver hasn't merged yet to the Linux vanilla.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 9 +++++----
>  arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts     | 7 +++++++

The patch subject is quite misleading - this isn't modifying the
binding at all, rather the actual devicetrees.

Has this work been sent upstream?

>  2 files changed, 12 insertions(+), 4 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..41d345448430 100644
> --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> @@ -52,6 +52,7 @@ rstc: reset-controller@f0801000 {
>  			reg = <0x0 0xf0801000 0x0 0x78>;
>  			#reset-cells = <2>;
>  			nuvoton,sysgcr = <&gcr>;
> +			clocks = <&refclk>;

Why not add `#clock-cells` here while we're at it (squash patch 3/6
into this one)?

Andrew
Tomer Maimon July 3, 2024, 8:10 a.m. UTC | #2
Hi Andrew,


On Wed, 3 Jul 2024 at 09:09, Andrew Jeffery <andrew@codeconstruct.com.au> wrote:
>
> On Mon, 2024-07-01 at 10:10 +0300, Tomer Maimon wrote:
> > The NPCM8XX clock driver uses a 25Mhz external clock, therefore adding
> > clock property.
> >
> > The new required clock property does not break the NPCM8XX clock ABI
> > since the NPCM8XX clock driver hasn't merged yet to the Linux vanilla.
> >
> > Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> > ---
> >  arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi | 9 +++++----
> >  arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts     | 7 +++++++
>
> The patch subject is quite misleading - this isn't modifying the
> binding at all, rather the actual devicetrees.
>
> Has this work been sent upstream?
It was sent months ago but is added to the clock node and not the reset node.
Now the clock reference is represented in the reset node but since the
clock driver has so many versions I am not sure this solution will be
accepted :-(
This is why I prefer to wait until the clock maintainer approves the
driver and then I could proceed with the other clock device tree
patches.

but with the OpenBMC Linux kernel is a different story since we have
customers that need to use the clock driver we need to send the
current full solution and not part like I did with the Vanilla upstream.

>
> >  2 files changed, 12 insertions(+), 4 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..41d345448430 100644
> > --- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
> > @@ -52,6 +52,7 @@ rstc: reset-controller@f0801000 {
> >                       reg = <0x0 0xf0801000 0x0 0x78>;
> >                       #reset-cells = <2>;
> >                       nuvoton,sysgcr = <&gcr>;
> > +                     clocks = <&refclk>;
>
> Why not add `#clock-cells` here while we're at it (squash patch 3/6
> into this one)?
because it is not part of the clock reference
>
> Andrew

appreicate your understanding!

Tomer
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
index ecd171b2feba..41d345448430 100644
--- a/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-common-npcm8xx.dtsi
@@ -52,6 +52,7 @@  rstc: reset-controller@f0801000 {
 			reg = <0x0 0xf0801000 0x0 0x78>;
 			#reset-cells = <2>;
 			nuvoton,sysgcr = <&gcr>;
+			clocks = <&refclk>;
 		};
 
 		clk: clock-controller@f0801000 {
@@ -81,7 +82,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 +154,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 +163,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 +172,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 a5ab2bc0f835..83c2f4e138e5 100644
--- a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
+++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-evb.dts
@@ -19,6 +19,13 @@  chosen {
 	memory {
 		reg = <0x0 0x0 0x0 0x40000000>;
 	};
+
+	refclk: refclk-25mhz {
+		compatible = "fixed-clock";
+		clock-output-names = "ref";
+		clock-frequency = <25000000>;
+		#clock-cells = <0>;
+	};
 };
 
 &serial0 {