linux-kernel - Re: [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJRtX8QFLoWnJBkepZrbneHX8qZdde=aw+zbdErVC91B=u==MA@mail.gmail.com>
Date: Tue, 16 Jan 2024 22:41:44 +0800
From: Jingbao Qiu <qiujingbao.dlmu@...il.com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc: alexandre.belloni@...tlin.com, robh+dt@...nel.org, 
	krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org, chao.wei@...hgo.com, 
	unicorn_wang@...look.com, paul.walmsley@...ive.com, palmer@...belt.com, 
	aou@...s.berkeley.edu, linux-rtc@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	dlan@...too.org, inochiama@...look.com
Subject: Re: [PATCH v6 3/3] riscv: dts: sophgo: add rtc dt node for CV1800

On Tue, Jan 16, 2024 at 3:44 PM Krzysztof Kozlowski
<krzysztof.kozlowski@...aro.org> wrote:
>
> On 15/01/2024 17:06, Jingbao Qiu wrote:
> > Add the rtc device tree node to cv1800 SoC.
> >
> > Signed-off-by: Jingbao Qiu <qiujingbao.dlmu@...il.com>
> > ---
> >  arch/riscv/boot/dts/sophgo/cv1800b.dtsi | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > index df40e87ee063..66bb4a752b91 100644
> > --- a/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > +++ b/arch/riscv/boot/dts/sophgo/cv1800b.dtsi
> > @@ -119,5 +119,17 @@ clint: timer@...00000 {
> >                       reg = <0x74000000 0x10000>;
> >                       interrupts-extended = <&cpu0_intc 3>, <&cpu0_intc 7>;
> >               };
> > +
> > +             rtc: rtc@...5000 {
> > +                     compatible = "sophgo,cv1800-rtc", "syscon";
> > +                     reg = <0x5025000 0x2000>;
> > +                     interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clocks = <&osc>;
> > +             };
> > +
> > +             por {
> > +                     compatible = "sophgo,cv1800-por";
>
> What is this? Why is it here, how did it even appear? It misses unit
> address and reg or is clearly placed in wrong place. It seems you
> entirely ignored out previous discussion.
>
> NAK
>

I'm very sorry for wasting your time. Furthermore, we would like to
thank you for your patient response.
Let me realize the rigor of Linux kernel code. I greatly admire
this.Please allow me to briefly review
our previous discussions.

CV1800 is a RISCV based SOC that includes an RTC module. The RTC
module has an OSC oscillator
and POR submodule inside.This OSC oscillator is only for RTC use, so
it does not need to be described
as a dt node. The POR submodule provides power off/on and restart
functions for CV1800. So I need
two drivers corresponding to RTC and POR respectively. RTC requires
the use of irq and clk resources
in addition to registers, while POR only requires Reg resources. The
current problem is how to describe
the relationship between RTC and POR, and how to make registers shared
by these two drivers.

In v3, I thought RTC was an MFD device because it not only had RTC
functionality but also restart and
power on/off capabilities.So my example is shown below.

syscon@...5000 {
  compatible = "sophgo,cv1800b-subsys", "syscon", "simple-mfd";
  reg = <0x05025000 0x2000>;
  rtc {
    compatible = "sophgo,cv1800b-rtc";
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk CLK_RTC_25M>;
  };
}

There were two suggestions you made at the time. Firstly, I only
described RTC and did not describe
the POR submodule. Secondly, regarding the name issue, system
controllers should not be placed
in the mfd file.Afterwards, I released the 4th version, in which I
described the POR submodule, which
is included side by side with RTC in the misc device. Then, by sharing
the register, both RTC and
POR drivers can access the registers.

misc@...5000 {
  compatible = "sophgo,cv1800-misc", "syscon", "simple-mfd";
  reg = <0x05025000 0x2000>;
  rtc  {
    compatible = "sophgo,cv1800-rtc";
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk 15>;
  };
  por  {
    compatible = "sophgo,cv1800-por";
  };
};

Your suggestion is, firstly, the por submodule does not have any
resources, so it should be deleted.
The second issue is still the name, misc is any device.
Afterwards, I released the 5th edition. In this version, I removed the
POR submodule in RTC.

rtc@...5000 {
    compatible = "sophgo,cv1800-rtc", "syscon";
    reg = <0x5025000 0x2000>;
    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
    clocks = <&clk 15>;
};

The question you raised is why syscon child nodes are used.
In this version, I will try the following methods.

rtc: rtc@...5000 {
                    compatible = "sophgo,cv1800-rtc", "syscon";
                    reg = <0x5025000 0x2000>;
                    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
                    clocks = <&osc>;
};
por {
                    compatible = "sophgo,cv1800-por";
                    sophgo,rtc-sysreg = <&rtc>;
};

My idea is that this register can be accessed through the syscon tag,
RTC driver, and reboot driver.
Then complete their functions.
I'm sorry if it was due to language differences that caused my misunderstanding.
Perhaps I can accomplish it through the following methods.
rtc: rtc@...5000 {
                    compatible = "sophgo,cv1800-rtc", "sophgo,cv1800-por";
                    reg = <0x5025000 0x2000>;
                    interrupts = <17 IRQ_TYPE_LEVEL_HIGH>;
                    clocks = <&osc>;
};
However, in reality, the POR submodule does not use IRQ and CLK.
Please do not hesitate to teach. Thanks.


Best regards,
Jingbao Qiu