Re: [PATCH v4 3/3] riscv: dts: canaan: Add clock initial support for K230

From: Conor Dooley <conor@kernel.org>
To: Xukai Wang <kingxukai@zohomail.com>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org,
	Samuel Holland <samuel.holland@sifive.com>,
	Troy Mitchell <TroyMitchell988@gmail.com>
Subject: Re: [PATCH v4 3/3] riscv: dts: canaan: Add clock initial support for K230
Date: Tue, 18 Feb 2025 17:00:54 +0000	[thread overview]
Message-ID: <20250218-status-friend-51fb039ab97e@spud> (raw)
In-Reply-To: <20250217-b4-k230-clk-v4-3-5a95a3458691@zohomail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2842 bytes --]

On Mon, Feb 17, 2025 at 10:45:18PM +0800, Xukai Wang wrote:
> This patch provides basic support for the K230 clock, which does not
> cover all clocks.
> 
> The clock tree of the K230 SoC consists of OSC24M,
> PLLs and sysclk.
> 
> Co-developed-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com>
> Signed-off-by: Xukai Wang <kingxukai@zohomail.com>
> ---
>  arch/riscv/boot/dts/canaan/k230.dtsi | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/canaan/k230.dtsi b/arch/riscv/boot/dts/canaan/k230.dtsi
> index 95c1a3d8fb1192e30113d96d3e96329545bc6ae7..e50ba03c2c21597e5f7d04a652db08f84101cbfb 100644
> --- a/arch/riscv/boot/dts/canaan/k230.dtsi
> +++ b/arch/riscv/boot/dts/canaan/k230.dtsi
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2024 Yangyu Chen <cyy@cyyself.name>
>   */
>  
> +#include <dt-bindings/clock/canaan,k230-clk.h>
>  #include <dt-bindings/interrupt-controller/irq.h>
>  
>  /dts-v1/;
> @@ -65,6 +66,13 @@ apb_clk: apb-clk-clock {
>  		#clock-cells = <0>;
>  	};
>  
> +	osc24m: clock-24m {
> +		compatible = "fixed-clock";
> +		clock-frequency = <24000000>;
> +		clock-output-names = "osc24m";
> +		#clock-cells = <0>;
> +	};
> +
>  	soc {
>  		compatible = "simple-bus";
>  		interrupt-parent = <&plic>;
> @@ -138,5 +146,29 @@ uart4: serial@91404000 {
>  			reg-shift = <2>;
>  			status = "disabled";
>  		};
> +
> +		sysclk: clock-controller@91102000 {
> +			compatible = "canaan,k230-clk";
> +			reg = <0x0 0x91102000 0x0 0x1000>,
> +			      <0x0 0x91100000 0x0 0x1000>;
> +			clocks = <&osc24m>;
> +			clock-output-names = "CPU0_ACLK", "CPU0_PLIC", "CPU0_NOC_DDRCP4",
> +					     "CPU0_PCLK", "PMU_PCLK", "HS_HCLK_HIGH_SRC",
> +					     "HS_HCLK_HIGH_GATE", "HS_HCLK_SRC",
> +					     "HS_SD0_HS_AHB_GAT", "HS_SD1_HS_AHB_GAT",
> +					     "HS_SSI1_HS_AHB_GA", "HS_SSI2_HS_AHB_GA",
> +					     "HS_USB0_HS_AHB_GA", "HS_USB1_HS_AHB_GA",
> +					     "HS_SSI0_AXI", "HS_SSI1", "HS_SSI2",
> +					     "HS_QSPI_AXI_SRC", "HS_SSI1_ACLK_GATE",
> +					     "HS_SSI2_ACLK_GATE", "HS_SD_CARD_SRC",
> +					     "HS_SD0_CARD_TX", "HS_SD1_CARD_TX",
> +					     "HS_SD_AXI_SRC", "HS_SD0_AXI_GATE",
> +					     "HS_SD1_AXI_GATE", "HS_SD0_BASE_GATE",
> +					     "HS_SD1_BASE_GATE", "HS_OSPI_SRC",
> +					     "HS_USB_REF_51M", "HS_SD_TIMER_SRC",
> +					     "HS_SD0_TIMER_GATE", "HS_SD1_TIMER_GATE",
> +					     "HS_USB0_REFERENCE", "HS_USB1_REFERENCE";
> +			#clock-cells = <1>;
> +		};

Apologies for not commenting on it until now, but this patch seems like
there's some hunks missing from it. I'd expect you to remove the dummy
"apb-clk-clock" from the dts and replace it with a real one sourced from
the newly added clock controller.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv