Re: [PATCH v1 22/22] riscv: dts: starfive: jhb100: Add clocks and resets nodes

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]

On Thu, Apr 02, 2026 at 03:55:23AM -0700, Changhuang Liang wrote:
> Add clocks and resets nodes for JHB100 RISC-V BMC SoC. They contain
> sys0crg/sys1crg/sys2crg/per0crg/per1crg/per2crg/per3crg.
> 
> Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com>
> ---
>  arch/riscv/boot/dts/starfive/jhb100.dtsi | 198 ++++++++++++++++++++++-
>  1 file changed, 195 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/boot/dts/starfive/jhb100.dtsi b/arch/riscv/boot/dts/starfive/jhb100.dtsi
> index 4d03470f78ab..700d00f800bc 100644
> --- a/arch/riscv/boot/dts/starfive/jhb100.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jhb100.dtsi
> @@ -4,6 +4,8 @@
>   */
>  
>  /dts-v1/;
> +#include <dt-bindings/clock/starfive,jhb100-crg.h>
> +#include <dt-bindings/reset/starfive,jhb100-crg.h>
>  
>  / {
>  	compatible = "starfive,jhb100";
> @@ -268,12 +270,96 @@ pmu {
>  			<0x00 0x22 0xFFFFFFFF 0xFFFFFF22 0x00007FF8>;	/* Event ID 34 */
>  	};
>  
> -	clk_uart: clk-uart {
> -		compatible = "fixed-clock"; /* Initial clock handler for UART */
> +	osc: osc {
> +		compatible = "fixed-clock";
>  		#clock-cells = <0>;
>  		clock-frequency = <25000000>;
>  	};

Is this really on the SoC?

>  
> +	pll0: pll0 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <2400000000>;
> +	};

What's providing all of these PLLs? Are they all fixed-frequency on-chip
PLLs without an off-chip reference? I find that somewhat unlikely.

Since devicetrees are now being imported into U-Boot, it's important to
make sure that I'm not merging fixed-clocks that later get replaced by
dedicated drivers that U-Boot won't have.

To that end, I won't apply the series this depends on without this patch
being applied at the same time.

> +
> +	pll1: pll1 {

Also, none of these follow the preferred naming scheme for
fixed-frequency clocks. Go look at the binding for how they should be,
if they are too be kept.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <1000000000>;
> +	};

Cheers,
Conor.

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