Re: [PATCH v4 3/3] riscv: dts: eswin: Add clock driver support

[Krzysztof Kozlowski <krzk@kernel.org>]

On 19/08/2025 10:34, 董绪洋 wrote:
> Hi Krzysztof,
> 
> Thank you very much for your constructive suggestions.
> 
>>>
>>> Add clock device tree support for eic7700 SoC.
>>>
>>> Signed-off-by: Yifeng Huang <huangyifeng@eswincomputing.com>
>>> Signed-off-by: Xuyang Dong <dongxuyang@eswincomputing.com>
>>> ---
>>>  arch/riscv/boot/dts/eswin/eic7700-clocks.dtsi | 2283 +++++++++++++++++
>>>  1 file changed, 2283 insertions(+)
>>>  create mode 100644 arch/riscv/boot/dts/eswin/eic7700-clocks.dtsi
>>>
>>> diff --git a/arch/riscv/boot/dts/eswin/eic7700-clocks.dtsi b/arch/riscv/boot/dts/eswin/eic7700-clocks.dtsi
>>> new file mode 100644
>>> index 000000000000..405d06f9190e
>>> --- /dev/null
>>> +++ b/arch/riscv/boot/dts/eswin/eic7700-clocks.dtsi
>>> @@ -0,0 +1,2283 @@
>>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>>> +/*
>>> + * Copyright (c) 2025, Beijing ESWIN Computing Technology Co., Ltd.
>>> + */
>>> +
>>> +/ {
>>> +	clock-controller@51828000 {
>>> +		compatible = "eswin,eic7700-clock";
>>> +		reg = <0x000000 0x51828000 0x000000 0x80000>;
>>> +		#clock-cells = <0>;
>>> +		#address-cells = <1>;
>>> +		#size-cells = <0>;
>>> +
>>> +		/* fixed clock */
>>> +		fixed_rate_clk_apll_fout2: fixed-rate-apll-fout2 {
>>
>> Such pattern was years ago NAKed.
>>
>> No, don't ever bring nodes per clock.
>>
> We have defined a large number of clock devices. 
> The comment of v3 is "Driver is also way too big for simple clock driver and I 
> am surprised to see so many redundancies.". Therefore, we modified the clock 
> driver code and moved the description of clock device from the driver to the DTS.
> 
> But, this comment is that don't ever bring nodes per clock. We’ve run into some

And? What is unclear in that comment?

> trouble and aren’t sure which approach aligns better with community guidelines. 
> Could you share your advice or suggestions on the best way forward?

Look at any other recent clock drivers.

Best regards,
Krzysztof