Re: [PATCH v2 2/8] riscv: dts: eswin: add clock generator for EIC7700 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The fixed-clock node in the SoC .dtsi is named 'clock-24m' but omits the 'clock-frequency' property, deferring it to the board-specific .dts.
--

commit 5bc9617a8cdce75f4ec3cb72e9a91cca1ea64a55
Author: Pritesh Patel <pritesh.patel@einfochips.com>

riscv: dts: eswin: add clock generator for EIC7700 SoC
    
Add clock generator node for EIC7700 SoC. HiFive Premier P550 boards have
24MHz crystal oscillator to provide the input clock.

> diff --git a/arch/riscv/boot/dts/eswin/eic7700.dtsi b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> index 430a210f01e66..af9e40d8739ad 100644
> --- a/arch/riscv/boot/dts/eswin/eic7700.dtsi
> +++ b/arch/riscv/boot/dts/eswin/eic7700.dtsi
[ ... ]
> @@ -203,6 +204,11 @@ pmu {
>  				<0x00000000 0x0000000f 0xfffffffc 0x000000ff 0x00000078>;
>  	};
>  
> +	clock_24m: clock-24m {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +

[Severity: Low]
Does this node naming and structure make sense if the frequency can vary per
board?

If the oscillator frequency can vary per board, shouldn't the node name in the
eic7700.dtsi file be generic like 'oscillator' rather than hardcoding
'clock-24m'?

Conversely, if the frequency is strictly fixed at 24MHz for all EIC7700
implementations, shouldn't clock-frequency = <24000000>; be defined directly
here in the .dtsi to prevent boilerplate duplication across board files?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706081055.1126275-1-pinkesh.vaghela@einfochips.com?part=2