Message-ID: <ZVOKStmcyXtH4oZS@xhacker>
Date:   Tue, 14 Nov 2023 22:55:06 +0800
From:   Jisheng Zhang <jszhang@...nel.org>
To:     Samuel Holland <samuel.holland@...ive.com>
Cc:     Yixun Lan <dlan@...too.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Chao Wei <chao.wei@...hgo.com>,
        Chen Wang <unicorn_wang@...look.com>,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-riscv@...ts.infradead.org
Subject: Re: [PATCH 3/4] riscv: dts: sophgo: add reset dt node for cv1800b

On Mon, Nov 13, 2023 at 10:37:35AM -0500, Samuel Holland wrote:
> Hi Jisheng,

Hi Samuel,

> 
> 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@...nel.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@...3000 {
> >>> +			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).

Good idea, indeed if needed we can implement a customized driver, and I think
this can acchieve both backward and forward DT compatbility ;)

As for firmware, I guess you mean the little c906 core os firmware. Here
is my draft plan:

a sophgo custom remoteproc driver which will do something like:

load the firmware;
reset_assert(rst);
prepara cpu entry address and so on;
reset_deassert(rst);

so sticky reset still works perfectly. While I believe autoclear reset
may not work if the reset clears something we have set.

> 
> Regards,
> Samuel
> 
> >> so we should be fine with this?
> >>
> >>> +			#reset-cells = <1>;
> >>> +		};
> >>> +
> >>>  		uart0: serial@...0000 {
> >>>  			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@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

Powered by blists - more mailing lists