Re: [PATCH 3/4] riscv: dts: sophgo: add reset dt node for cv1800b

From: Samuel Holland <samuel.holland@sifive.com>
To: Jisheng Zhang <jszhang@kernel.org>, Yixun Lan <dlan@gentoo.org>
Cc: Philipp Zabel <p.zabel@pengutronix.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Chao Wei <chao.wei@sophgo.com>,
	Chen Wang <unicorn_wang@outlook.com>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-riscv@lists.infradead.org
Subject: Re: [PATCH 3/4] riscv: dts: sophgo: add reset dt node for cv1800b
Date: Mon, 13 Nov 2023 10:37:35 -0500	[thread overview]
Message-ID: <7cccf188-c47f-4865-95ce-dcfedc1429dc@sifive.com> (raw)
In-Reply-To: <ZVI9bzAhPnHhVg8A@xhacker>

Hi Jisheng,

On 2023-11-13 9:14 AM, Jisheng Zhang wrote:
> On Mon, Nov 13, 2023 at 02:32:24PM +0000, Yixun Lan wrote:
>> On 08:55 Mon 13 Nov     , Jisheng Zhang wrote:
>>> Add the reset device tree node to cv1800b SoC.
>>>
>>> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>>> ---
>>>  arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>> index df40e87ee063..4032419486be 100644
>>> --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>> +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
>>> @@ -54,6 +54,12 @@ soc {
>>>  		dma-noncoherent;
>>>  		ranges;
>>>  
>>> +		rst: reset-controller@3003000 {
>>> +			compatible = "sophgo,cv1800b-reset";
>>> +			reg = <0x03003000 0x1000>;
>>                                           ~~~~~~~
>> 			        it should be 0x28
> 
> The reg space is 4KB, but only 0x28 are used. I think 0x1000 or 0x28 are fine
> since the ioremap granule is 4kB.
>>
>> while please also note the 0x24 == SOFT_CPUAC_RSTN, does not compatible
>> with the reset-simple driver, but as it's not implemented nor used in this driver,
> 
> But the functionality of this "autoclear" reg isn't used at all since we also
> have "sticky" reset to acchieve the same feature, I.E reset cpusys. And in the
> usage case of reseting cpusys, I believe "sticky" reset is preferred.
> 
> And except the cpusys reset which has both autoclear and sticky, other
> resets are sticky only. I'm not sure whether it's worth to write a new
> driver for almost useless feature.

As long as the device has its own binding/compatible string, it is always
possible to replace RESET_SIMPLE with a custom driver later if needed. (Or use a
more complicated driver in some other context, e.g. firmware).

Regards,
Samuel

>> so we should be fine with this?
>>
>>> +			#reset-cells = <1>;
>>> +		};
>>> +
>>>  		uart0: serial@4140000 {
>>>  			compatible = "snps,dw-apb-uart";
>>>  			reg = <0x04140000 0x100>;
>>> -- 
>>> 2.42.0
>>>
>>
>> -- 
>> Yixun Lan (dlan)
>> Gentoo Linux Developer
>> GPG Key ID AABEFD55
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv