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

From: Krzysztof Kozlowski <krzk@kernel.org>
To: 董绪洋 <dongxuyang@eswincomputing.com>,
	mturquette@baylibre.com, sboyd@kernel.org, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org,
	linux-clk@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, paul.walmsley@sifive.com,
	palmer@dabbelt.com, aou@eecs.berkeley.edu, alex@ghiti.fr,
	linux-riscv@lists.infradead.org
Cc: ningyu@eswincomputing.com, linmin@eswincomputing.com,
	huangyifeng@eswincomputing.com, pinkesh.vaghela@einfochips.com
Subject: Re: [PATCH v4 3/3] riscv: dts: eswin: Add clock driver support
Date: Tue, 19 Aug 2025 11:10:28 +0200	[thread overview]
Message-ID: <bfa23779-9861-4ae4-9ced-9f347394f033@kernel.org> (raw)
In-Reply-To: <3fa3950e.371.198c1770125.Coremail.dongxuyang@eswincomputing.com>

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