Re: [PATCH 1/4] riscv: dts: spacemit: k3: add clock tree

[Yixun Lan]

Hi Samuel,

On 09:13 Sat 14 Mar     , Samuel Holland wrote:
> Hi Yixun,
> 
> On 2026-03-14 3:52 AM, Yixun Lan wrote:
> > On 20:44 Fri 13 Mar     , Samuel Holland wrote:
> >> On 2026-03-04 1:36 AM, Yixun Lan wrote:
> >>> Add clock support to SpacemiT K3 SoC, the clock tree consist of several
> >>> blocks which are APBC, APMU, DCIU, MPUM.
> >>>
> >>> Signed-off-by: Yixun Lan <dlan@xxxxxxxxxx>
> >>> ---
> >>>  arch/riscv/boot/dts/spacemit/k3.dtsi | 75 ++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 75 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> >>> index b69cf81b5d55..e3d7f3102fd5 100644
> >>> --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> >>> +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
> >>> @@ -4,6 +4,7 @@
> >>>   * Copyright (c) 2026 Guodong Xu <guodong@xxxxxxxxxxxx>
> >>>   */
> >>>  
> >>> +#include <dt-bindings/clock/spacemit,k3-clocks.h>
> >>>  #include <dt-bindings/interrupt-controller/irq.h>
> >>>  
> >>>  /dts-v1/;
> >>> @@ -398,6 +399,36 @@ core3 {
> >>>  		};
> >>>  	};
> >>>  
> >>> +	clocks {
> >>> +		vctcxo_1m: clock-1m {
> >>> +			compatible = "fixed-clock";
> >>> +			clock-frequency = <1000000>;
> >>> +			clock-output-names = "vctcxo_1m";
> >>> +			#clock-cells = <0>;
> >>> +		};
> >>> +
> >>> +		vctcxo_24m: clock-24m {
> >>> +			compatible = "fixed-clock";
> >>> +			clock-frequency = <24000000>;
> >>> +			clock-output-names = "vctcxo_24m";
> >>> +			#clock-cells = <0>;
> >>> +		};
> >>> +
> >>> +		vctcxo_3m: clock-3m {
> >>> +			compatible = "fixed-clock";
> >>> +			clock-frequency = <3000000>;
> >>> +			clock-output-names = "vctcxo_3m";
> >>> +			#clock-cells = <0>;
> >>> +		};
> >>> +
> >>> +		osc_32k: clock-32k {
> >>> +			compatible = "fixed-clock";
> >>> +			clock-frequency = <32000>;
> >>> +			clock-output-names = "osc_32k";
> >>> +			#clock-cells = <0>;
> >>> +		};
> >>
> >> Are these clocks provided by SoC or by the board? Usually there's a crystal
> >> external to the SoC that provides the root of the clock tree. If these clocks
> >> are provided by the board, they (or at least the clock-frequency property)
> >> should be in the board DT, not the SoC dtsi.
> >>
> > It's true, as a quick check, osc_32k provided by P1 PMU, while vctcxo_24m is
> > a crystal, vctcxo_1m and vctcxo_3m are also marked as external in the clock
> > tree, but I would confirm them later..
> 
> In that case, osc_32k should ideally be a reference to the P1 PMU clock
> provider, not a fixed-clock. But this may be infeasible if it creates dependency
> loops (PMU depends on I2C, I2C depends on clocks, clocks depend on PMU).
> 
Yes, in an ideal case, not only there is dependency loop, but need to
implement a clock tree for P1..

Currently, I'd leave it as fixed-clock as is, since the 32k clock is
always on from P1 since power up

Also, for vctcxo_1m and vctcxo_3m, they are clocks derived from vctcxo_24m which
unable to be gate off, so I think it's ok to leave them as fixed-clock.

> > I agree to move them out of SoC dtsi file - k3.dtsi, while due to all boards share
> > the same clock topology, what if I creating a k3-clock.dtsi and making it shared 
> > between all board dts file? to avoid massive DTS duplication
> 
> Yes, it is common practice to create a .dtsi file for things shared among
> several boards for a SoC (for example if they are all based on a reference
> platform). You may want to name it something more generic if more than just
> clocks can be shared (like k3-common.dtsi, compare jh7110-common.dtsi).
> 
k3-common.dtsi sounds good to me

> >> Also, the /clocks node is out of order.
> >>
> > I will move osc_32k before vtccxo_1m, assuming it's the problem you
> > refered to?
> 
> I mean that /clocks sorts alphabetically before /cpus. Your ordering of the
> fixed-clocks nodes themselves is fine.
> 
ok, I got

> Regards,
> Samuel
> 
> 

-- 
Yixun Lan (dlan)