Re: [PATCH v5 5/6] riscv: dts: anlogic: add clocks and CRU for DR1V90

["Junhui Liu" <junhui.liu@pigmoral.tech>]

On Fri May 15, 2026 at 3:48 AM CST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `can_ext` and `wdt_ext` fixed-clock nodes are missing the mandatory `clock-frequency` property.
> --
>
> commit 2b9cd6d4a55b4608470fd48bd92b6cacba368046
> Author: Junhui Liu <junhui.liu@pigmoral.tech>
>
> riscv: dts: anlogic: add clocks and CRU for DR1V90
>
> This commit adds clocks and introduces the Clock and Reset unit node
> for the Anlogic DR1V90 SoC. It configures a main crystal oscillator
> and two optional external clocks for the CAN and WDT modules.
>
>> diff --git a/arch/riscv/boot/dts/anlogic/dr1v90.dtsi b/arch/riscv/boot/dts/anlogic/dr1v90.dtsi
>> index 9fe183f5f5c8d..574c6608aef01 100644
>> --- a/arch/riscv/boot/dts/anlogic/dr1v90.dtsi
>> +++ b/arch/riscv/boot/dts/anlogic/dr1v90.dtsi
>
> [ ... ]
>
>> @@ -40,6 +43,26 @@ cpu0_intc: interrupt-controller {
>>  		};
>>  	};
>>  
>> +	clocks {
>> +		can_ext: clock-ext-can {
>> +			compatible = "fixed-clock";
>> +			clock-output-names = "can_ext";
>> +			#clock-cells = <0>;
>> +		};
>> +
>> +		osc: clock-osc {
>> +			compatible = "fixed-clock";
>> +			clock-output-names = "osc";
>> +			#clock-cells = <0>;
>> +		};
>> +
>> +		wdt_ext: clock-ext-wdt {
>> +			compatible = "fixed-clock";
>> +			clock-output-names = "wdt_ext";
>> +			#clock-cells = <0>;
>> +		};
>
> Are the can_ext and wdt_ext fixed-clock nodes missing the mandatory
> clock-frequency property?
>
> The fixed-clock devicetree binding requires a clock-frequency property.
> Without it, the fixed-clock driver fails to read the frequency and returns
> -EIO during both of_fixed_clk_setup and platform probe, failing to register
> the clock providers.
>
> Because the cru node specifies these clocks as suppliers, the kernel's
> fw_devlink mechanism will indefinitely block the CRU driver from probing.
>
> This recursively blocks all CRU consumers, including the UART console,
> leaving the system without a working console.
>
> Would it be better to add a dummy frequency, mark these nodes with
> status = "disabled" in the dtsi, or define them only at the board level
> when they are actually present?

Yes, this is true.

I will update the binding to make both can_ext and wdt_ext optional, and
remove them from the SoC dtsi. If a specific board uses these external
clock inputs, the board-level dts can define them with their actual clock
frequencies.

>
>> +	};

-- 
Best regards,
Junhui Liu