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

[董绪洋 <dongxuyang@eswincomputing.com>]

Hi Krzysztof,

Thanks 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.

I found out that the recent clock drivers, such as sophgo/clk-sg2044.c 
and rockchip/clk-rk3562.c, the clock tree information of them was placed
in the C code. 
So, for EIC7700 SoC, should the clock tree information be placed in
clk-eic7700.c just as clk-sg2044.c? Is this understanding correct?

I would be grateful for your reply.

Regards,
Xuyang Dong